Skip to content

git: fix cygwin build#476295

Merged
philiptaron merged 1 commit intoNixOS:masterfrom
corngood:cygwin-git
Jan 2, 2026
Merged

git: fix cygwin build#476295
philiptaron merged 1 commit intoNixOS:masterfrom
corngood:cygwin-git

Conversation

@corngood
Copy link
Copy Markdown
Contributor

@corngood corngood commented Jan 2, 2026

Things done

  • Built on platform:
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • Tested, as applicable:
  • Ran nixpkgs-review on this PR. See nixpkgs-review usage.
  • Tested basic functionality of all binary files, usually in ./result/bin/.
  • Nixpkgs Release Notes
    • Package update: when the change is major or breaking.
  • NixOS Release Notes
    • Module addition: when adding a new NixOS module.
    • Module update: when the change is significant.
  • Fits CONTRIBUTING.md, pkgs/README.md, maintainers/README.md and other READMEs.

Add a 👍 reaction to pull requests you find important.

@nixpkgs-ci nixpkgs-ci bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux. 2.status: merge-bot eligible This PR can be merged by commenting "@NixOS/nixpkgs-merge-bot merge". labels Jan 2, 2026
@me-and
Copy link
Copy Markdown
Member

me-and commented Jan 2, 2026

...I had the impression that Cygwin wasn't supported by Nixpkgs any more. And, as a maintainer for the Nixpkgs git package and the maintainer for the Git package for Cygwin, I probably ought to have known that!

I don't have a Cygwin Nix environment to test with (although maybe I should fix that). It seems likely to me that a more general fix would work better -- possibly some sort of hook that runs in stdenv to wrap ln where required -- but I've no objection to this change if it's a useful quick fix.

@corngood
Copy link
Copy Markdown
Contributor Author

corngood commented Jan 2, 2026

I don't have a Cygwin Nix environment to test with (although maybe I should fix that). It seems likely to me that a more general fix would work better -- possibly some sort of hook that runs in stdenv to wrap ln where required -- but I've no objection to this change if it's a useful quick fix.

This change is only actually required for cross-builds, because cygwin itself has its magical .exe suffix handling, so omitting the suffix still works there.

@me-and
Copy link
Copy Markdown
Member

me-and commented Jan 2, 2026

This change is only actually required for cross-builds, because cygwin itself has its magical .exe suffix handling, so omitting the suffix still works there.

Yeah, I figured as much! I was initially confused about why this was necessary given the magic .exe handling, but I could well believe the extension needs to be included when the symlink is being created in a non-Cygwin environment.

@corngood
Copy link
Copy Markdown
Contributor Author

corngood commented Jan 2, 2026

@me-and if you're interested in nixpkgs/cygwin stuff, the parent PR (#475998) has my next block of work, which is cross-building a stdenv bootstrap tarball.

It's been discussed a bit on https://matrix.to/#/#windows:nixos.org

@philiptaron
Copy link
Copy Markdown
Contributor

Change looks fine to me, but I echo Adam's feedback that a cleanup pass when #nix-does-windows is checked in seems warranted.

@philiptaron philiptaron added this pull request to the merge queue Jan 2, 2026
@nixpkgs-ci nixpkgs-ci bot added 12.approvals: 1 This PR was reviewed and approved by one person. 12.approved-by: package-maintainer This PR was reviewed and approved by a maintainer listed in any of the changed packages. labels Jan 2, 2026
Merged via the queue into NixOS:master with commit bce7b10 Jan 2, 2026
35 of 37 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

2.status: merge-bot eligible This PR can be merged by commenting "@NixOS/nixpkgs-merge-bot merge". 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux. 12.approvals: 1 This PR was reviewed and approved by one person. 12.approved-by: package-maintainer This PR was reviewed and approved by a maintainer listed in any of the changed packages.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants