Skip to content

Revert "fetchgit: take postCheckout to allow gathering revision and commit metadata without leaving .git"#473619

Closed
RaitoBezarius wants to merge 1 commit intomasterfrom
revert-465497-fetchgit-postCheckout
Closed

Revert "fetchgit: take postCheckout to allow gathering revision and commit metadata without leaving .git"#473619
RaitoBezarius wants to merge 1 commit intomasterfrom
revert-465497-fetchgit-postCheckout

Conversation

@RaitoBezarius
Copy link
Member

Reverts #465497 due to incomplete backward compatibility for tag fetching which seemed intended.

@RaitoBezarius
Copy link
Member Author

@philiptaron As you merged the original PR, can you please take a look at this revert? Thank you.

@nixpkgs-ci nixpkgs-ci bot added 10.rebuild-linux: 11-100 This PR causes between 11 and 100 packages to rebuild on Linux. 10.rebuild-darwin: 11-100 This PR causes between 11 and 100 packages to rebuild on Darwin. 6.topic: fetch Fetchers (e.g. fetchgit, fetchsvn, ...) labels Dec 23, 2025
@qubitnano
Copy link
Contributor

Several packages now need fetchTags explicitly enabled since this commit

#473003

#473002

#473067

@RaitoBezarius
Copy link
Member Author

I don't think that the solution is to add fetchTags, given this is a non backward compatible change.

@me-and
Copy link
Member

me-and commented Dec 23, 2025

From a small sample, it looks like the repositories that have been affected here are ones that are using deepClone and therefore implicitly leaveDotGit. Per https://nixos.org/manual/nixpkgs/stable/#fetchgit, this should only be used for testing purposes unless the .git directory is removed in the postFetch phase, and per #8567 the .git directory is known to be non-deterministic even without any changes in the fetcher.

IMVHO, the fix here is not to back this change out: backwards-incompatible changes are sometimes necessary, particularly when they're to function that is explicitly unstable. The correct fix here is for the affected derivations to make use of the new postCheckout hook to get a stable derivation without the .git directory being included in the output.

That being said, I think there should be a release note for this change to direct people to use postCheckout rather than leaveDotGit. And possibly, as I suggested in #8567, adding an evaluation warning anywhere leaveDotGit is still used.

@ShamrockLee
Copy link
Contributor

ShamrockLee commented Dec 23, 2025

due to incomplete backward compatibility for tag fetching which seemed intended.

The incompatibility is not intended. Let me fix it.

Update: I pushed the fix to PR #473411.

@ShamrockLee
Copy link
Contributor

@RaitoBezarius I merged the fixing PR #473411. Please test again if the incompatibility persists.

@nixpkgs-ci nixpkgs-ci bot added the 2.status: merge conflict This PR has merge conflicts with the target branch label Dec 26, 2025
@philiptaron philiptaron closed this Jan 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

2.status: merge conflict This PR has merge conflicts with the target branch 6.topic: fetch Fetchers (e.g. fetchgit, fetchsvn, ...) 10.rebuild-darwin: 11-100 This PR causes between 11 and 100 packages to rebuild on Darwin. 10.rebuild-linux: 11-100 This PR causes between 11 and 100 packages to rebuild on Linux.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants