Building a crate that contains dangling symlinks fails

[rolfschr]

This cp fails if the tree contains symlinks that cannot be derefenced (-L).

A real world use case can be found at #361181.

This allows to work around the issue:

diff --git a/pkgs/build-support/rust/import-cargo-lock.nix b/pkgs/build-support/rust/import-cargo-lock.nix
index e88931d0f383..5613248b4959 100644
--- a/pkgs/build-support/rust/import-cargo-lock.nix
+++ b/pkgs/build-support/rust/import-cargo-lock.nix
@@ -149,6 +149,10 @@ let
               inherit (gitParts) url;
               rev = gitParts.sha; # The commit SHA is always available.
               sha256 = gitShaOutputHash.${gitParts.sha};
+              postFetch = ''
+                # Find and remove dangling symlinks.
+                find $out -xtype l -print -delete
+              '';
             }
           else if allowBuiltinFetchGit then
             builtins.fetchGit {

[rolfschr]

@TomaSajt @JohnRTitor @mmlb

You have recently touched import-cargo-lock.nix. Could you have a look into this?

[rolfschr]

@zowoq @winterqt @figsoda

Maybe you code owners could have a look into this?

[JohnRTitor]

Hi, could you open a PR?

And is this reproducible on fetchCargoVendor?

[rolfschr]

Hi, could you open a PR?

#379049

And is this reproducible on fetchCargoVendor?

I don't know. I checked the source and saw at least this line. I did a quick test with shutl.copytree with an input that does have a dangling symlink and indeed it failed. However, when providing ignore_dangling_symlinks=True (docs), the copy operation runs fine. So, if this causes a problem, the following could be done.

diff --git a/pkgs/build-support/rust/fetch-cargo-vendor-util.py b/pkgs/build-support/rust/fetch-cargo-vendor-util.py
index bb70a559fe96..7b94dc5054f1 100644
--- a/pkgs/build-support/rust/fetch-cargo-vendor-util.py
+++ b/pkgs/build-support/rust/fetch-cargo-vendor-util.py
@@ -194,7 +194,7 @@ def copy_and_patch_git_crate_subtree(git_tree: Path, crate_name: str, crate_out_
     crate_tree = crate_manifest_path.parent

     eprint(f"Copying to {crate_out_dir}")
-    shutil.copytree(crate_tree, crate_out_dir)
+    shutil.copytree(crate_tree, crate_out_dir, ignore_dangling_symlinks=True)
     crate_out_dir.chmod(0o755)

     with open(crate_manifest_path, "r") as f:

Note that I am blindly guessing here. I have no test case for fetchCargoVendor and this is beyond my expertise. I'm happy to include the aforementioned patch in the PR but I am not able to test this properly.

[TomaSajt]

It is failing with fetchCargoVendor too.

Here's a POC:

-  postPatch = ''
-    cp ${./Cargo.lock} Cargo.lock
-  '';
-
-  cargoLock = {
-    lockFile = ./Cargo.lock;
-    outputHashes = {
-      "libmdbx-0.1.4" = "sha256-ONp4uPkVCN84MObjXorCZuSjnM6uFSMXK1vdJiX074o=";
-      "lmdb-rkv-0.14.0" = "sha256-sxmguwqqcyOlfXOZogVz1OLxfJPo+Q0+UjkROkbbOCk=";
-      "quick-protobuf-0.8.1" = "sha256-dgePLYCeoEZz5DGaLifhf3gEIPaL7XB0QT9wRKY8LJg=";
-      "xdelta3-0.1.5" = "sha256-3ZZ2SDaOT8IOymgJaBCh9GNU5wpYgZnb51kN5sMsFLk=";
-    };
-  };
+  useFetchCargoVendor = true;
+  cargoHash = "sha256-Opkz3EVKw0M4LeWMsn1NlSw/Fg7cWVqnDJRRTPYYlLo=";

It would be useful to have the dangling symlink fix in fetchCargoVendor too.

[alyssais]

Huh, that cp doesn't look right to me in general? A crate could totally depend on a symlink in its tree actually being a symlink. I don't think it's right that we dereference them at all?

[TomaSajt]

btw, not sure if we should touch the fetchgit-based FOD, since it might cause the FOD hash to change in rare cases. (Though, those probably would have failed before, thus I don't think it would be a problem now) (But if we decided against this fix later, we'd have some hash breakages).

[TomaSajt]

Looked at what happens with cargo vendor itself: it resolves symlinks when vendoring and also seems to remove dangling symlinks. I'll try to find it in the upstream code.

[TomaSajt]

I came up with this patch:

-        cp -prvL "$tree/" $out
-        chmod u+w $out
+        tempDir=$(mktemp -d)
+        cp -prvd "$tree/" "$tempDir/tree"
+        tree="$tempDir/tree"
+
+        chmod -R u+w "$tree"
+        find "$tree" -xtype l -delete
+        cp -prvL "$tree" $out

there might be simplifications, but this works.
it's better since it doesn't modify the fetchgit FOD itself.

We could also just invoke python directly and do shutil.copy_tree instead

[rolfschr]

Thank you all for looking into this. I feel like rather closing #379049 because someone else is better suited. Please let me know what to do with this PR.

(BTW: I linked in the PR description the commit that caused cp -L).

[rolfschr]

Huh, that cp doesn't look right to me in general? A crate could totally depend on a symlink in its tree actually being a symlink. I don't think it's right that we dereference them at all?

A reasoning has been mentioned in the commit message at the time.

[rolfschr]

There is hasn't been any further feedback. From what I understand, the following options are on the table:

1. Keep/merge my PR as is

2. Adapt my PR and include my proposal from here to fix the Python code, too. My understanding is that this comment suggests this (does it?).

3. Do not adapt fetchgit as suggested in my PR but rather fix the Bash code (as suggested above, reasoning also above)

4. Same as 3) but include 2), too

5. Last sentence in this comment, suggest to get rid (part of?) of the BASH code and invoke Python instead (although, from the comment, it is not clear to me how to handle the original issue with dangling symlinks).
   (I wouldn't know how to do this.)

6. Same as 5 but include 2), too.

Not sure I gathered everything correctly and I don't want to complicate things too much. My gut feeling is that nobody wants to go forward here so I hoped to give some help for someone else to take a decision.