Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

nix-prefetch-git: Fix on macOS #364757

Closed
wants to merge 1 commit into from

Conversation

9999years
Copy link
Contributor

Due to a CppNix bug which causes nix-hash to error on any file in a symlinked directory, nix-prefetch-git doesn't work on macOS, where /tmp and /var are both symlinks.

Due to versions of CppNix older than 2.24 being removed from Nixpkgs despite known unfixed regressions, I can neither locate which version of CppNix introduced this bug or mitigate it by using an older version of CppNix.

Therefore, I have replaced CppNix with Lix in nix-prefetch-git (and the other nix-prefetch-scripts). Lix is based off of CppNix 2.18 and includes bugfixes backported from later versions of CppNix, so it does not suffer from this issue. Lix is compatible with CppNix 2.18, so this won't cause any regressions in behavior in the nix-prefetch-scripts.

Before:

$ ./result/bin/nix-prefetch-git https://github.com/iand675/hs-sqlcommenter.git
Initialized empty Git repository in /private/var/folders/z5/fclwwdms3r1gq4k4p3pkvvc00000gn/T/git-checkout-tmp-3WITGvxC/hs-sqlcommenter/.git/
remote: Enumerating objects: 16, done.
remote: Counting objects: 100% (16/16), done.
remote: Compressing objects: 100% (12/12), done.
remote: Total 16 (delta 0), reused 16 (delta 0), pack-reused 0 (from 0)
Unpacking objects: 100% (16/16), 14.14 KiB | 1.18 MiB/s, done.
From https://github.com/iand675/hs-sqlcommenter
 * branch            HEAD       -> FETCH_HEAD
Switched to a new branch 'fetchgit'
removing `.git'...
error: path '/var' is a symlink

After:

$ ./result/bin/nix-prefetch-git https://github.com/iand675/hs-sqlcommenter.git
Initialized empty Git repository in /private/var/folders/z5/fclwwdms3r1gq4k4p3pkvvc00000gn/T/git-checkout-tmp-mvc00tOe/hs-sqlcommenter/.git/
remote: Enumerating objects: 16, done.
remote: Counting objects: 100% (16/16), done.
remote: Compressing objects: 100% (12/12), done.
remote: Total 16 (delta 0), reused 16 (delta 0), pack-reused 0 (from 0)
Unpacking objects: 100% (16/16), 14.14 KiB | 1.18 MiB/s, done.
From https://github.com/iand675/hs-sqlcommenter
 * branch            HEAD       -> FETCH_HEAD
Switched to a new branch 'fetchgit'
removing `.git'...

git revision is 7e946654a6fb08f25ebc75b90bb6bbc60c1392bf
path is /nix/store/mkiv8ghgsyxf44ykk1n510yscvrhrgqi-hs-sqlcommenter
git human-readable version is -- none --
Commit date is 2024-07-11 19:10:11 +0200
hash is 0ijxcp0x0xismp8q457bgvj65s43s4zakk64v7h74wyknhv87hgi
{
  "url": "https://github.com/iand675/hs-sqlcommenter.git",
  "rev": "7e946654a6fb08f25ebc75b90bb6bbc60c1392bf",
  "date": "2024-07-11T19:10:11+02:00",
  "path": "/nix/store/mkiv8ghgsyxf44ykk1n510yscvrhrgqi-hs-sqlcommenter",
  "sha256": "0ijxcp0x0xismp8q457bgvj65s43s4zakk64v7h74wyknhv87hgi",
  "hash": "sha256-8cGDNrTTc3Lg2cTMqT7Rg+hi5H7rFILRrTp20MFlXUY=",
  "fetchLFS": false,
  "fetchSubmodules": false,
  "deepClone": false,
  "leaveDotGit": false
}

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 25.05 Release Notes (or backporting 24.11 and 25.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

Due to a CppNix bug which causes `nix-hash` to error on any file in a
symlinked directory, `nix-prefetch-git` doesn't work on macOS [1], where
`/tmp` and `/var` are both symlinks.

Due to versions of CppNix older than 2.24 being removed from Nixpkgs
despite known unfixed regressions [2], I can neither locate which
version of CppNix introduced this bug or mitigate it by using an older
version of CppNix.

Therefore, I have replaced CppNix with Lix [3] in `nix-prefetch-git`
(and the other `nix-prefetch-scripts`). Lix is based off of CppNix 2.18
and includes bugfixes backported from later versions of CppNix, so it
does not suffer from this issue. Lix is compatible with CppNix 2.18, so
this won't cause any regressions in behavior in the
`nix-prefetch-scripts`.

Before:

```
$ ./result/bin/nix-prefetch-git https://github.com/iand675/hs-sqlcommenter.git
Initialized empty Git repository in /private/var/folders/z5/fclwwdms3r1gq4k4p3pkvvc00000gn/T/git-checkout-tmp-3WITGvxC/hs-sqlcommenter/.git/
remote: Enumerating objects: 16, done.
remote: Counting objects: 100% (16/16), done.
remote: Compressing objects: 100% (12/12), done.
remote: Total 16 (delta 0), reused 16 (delta 0), pack-reused 0 (from 0)
Unpacking objects: 100% (16/16), 14.14 KiB | 1.18 MiB/s, done.
From https://github.com/iand675/hs-sqlcommenter
 * branch            HEAD       -> FETCH_HEAD
Switched to a new branch 'fetchgit'
removing `.git'...
error: path '/var' is a symlink
```

After:

```
$ ./result/bin/nix-prefetch-git https://github.com/iand675/hs-sqlcommenter.git
Initialized empty Git repository in /private/var/folders/z5/fclwwdms3r1gq4k4p3pkvvc00000gn/T/git-checkout-tmp-mvc00tOe/hs-sqlcommenter/.git/
remote: Enumerating objects: 16, done.
remote: Counting objects: 100% (16/16), done.
remote: Compressing objects: 100% (12/12), done.
remote: Total 16 (delta 0), reused 16 (delta 0), pack-reused 0 (from 0)
Unpacking objects: 100% (16/16), 14.14 KiB | 1.18 MiB/s, done.
From https://github.com/iand675/hs-sqlcommenter
 * branch            HEAD       -> FETCH_HEAD
Switched to a new branch 'fetchgit'
removing `.git'...

git revision is 7e946654a6fb08f25ebc75b90bb6bbc60c1392bf
path is /nix/store/mkiv8ghgsyxf44ykk1n510yscvrhrgqi-hs-sqlcommenter
git human-readable version is -- none --
Commit date is 2024-07-11 19:10:11 +0200
hash is 0ijxcp0x0xismp8q457bgvj65s43s4zakk64v7h74wyknhv87hgi
{
  "url": "https://github.com/iand675/hs-sqlcommenter.git",
  "rev": "7e946654a6fb08f25ebc75b90bb6bbc60c1392bf",
  "date": "2024-07-11T19:10:11+02:00",
  "path": "/nix/store/mkiv8ghgsyxf44ykk1n510yscvrhrgqi-hs-sqlcommenter",
  "sha256": "0ijxcp0x0xismp8q457bgvj65s43s4zakk64v7h74wyknhv87hgi",
  "hash": "sha256-8cGDNrTTc3Lg2cTMqT7Rg+hi5H7rFILRrTp20MFlXUY=",
  "fetchLFS": false,
  "fetchSubmodules": false,
  "deepClone": false,
  "leaveDotGit": false
}
```

[1]: NixOS/nix#11756 (comment)
[2]: NixOS#359215 (comment)
[3]: https://lix.systems/
@paparodeo
Copy link
Contributor

see also: #358685

@winterqt winterqt requested a review from emilazy December 13, 2024 03:58
@SuperSandro2000
Copy link
Member

IMO as a short term people can write this into their overlays but that change is really unexpected for anyone using nix-prefetch-git as it suddenly could behave slightly different than the package build which would be very hard to debug because no on is going to assume that it is using a forked package.

IMO we cannot merge this like that.

@9999years
Copy link
Contributor Author

[Lix] suddenly could behave slightly different than the package build

@SuperSandro2000 That's just not true — Lix is 100% drop-in compatible with CppNix. If the differences between Lix and CppNix 2.18 are too big for the Nixpkgs' maintainers' appetite, please re-add the non-broken versions of CppNix back into Nixpkgs so that we can continue to use cabal2nix.

It is not reasonable to expect every macOS user to add overlays to fix an extremely widely used script.

@9999years
Copy link
Contributor Author

I'm closing this now that #358685 is merged, but I'm really disappointed in the way the CppNix team has handled this issue. This wouldn't have been nearly as frustrating or difficult to work around if old CppNix versions hadn't been removed from Nixpkgs with no justification given.

@9999years 9999years closed this Dec 13, 2024
@9999years 9999years deleted the fix-nix-prefetch-scripts branch December 13, 2024 16:40
@Mic92
Copy link
Member

Mic92 commented Dec 13, 2024

The justification is that it's too much work to backport fixes to 5+ different versions.
We rather want to spent the time to make sure we can fix those issues instead and keep the gap between versions upgrades smaller.
Also what issue you are referring to regarding cabal2nix?

@9999years
Copy link
Contributor Author

@Mic92 cabal2nix calls nix-prefetch-git, which clones a Git repository to $TMP and then calls nix-hash on it, which fails on macOS on all Nix versions available in Nixpkgs.

@9999years
Copy link
Contributor Author

The justification is that it's too much work to backport fixes to 5+ different versions.

Sure, but at least keep 2.18 around, the version that actually works!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants