"failed to determine package fingerprint for build script" caused by a symlink pointing to an unreadable directory

[tavianator]

Problem

If a project with a build script has a symbolic link pointing to somewhere unreadable, cargo build fails.

Steps

$ git clone https://github.com/sharkdp/fd.git  # For example
$ cd fd
$ ln -s /root root  # Normal users can't read /root
$ cargo build
error: failed to determine package fingerprint for build script for fd-find v8.2.1 (/home/tavianator/code/sharkdp/fd)

Caused by:
  failed to determine the most recently modified file in /home/tavianator/code/sharkdp/fd

Caused by:
  failed to determine list of files in /home/tavianator/code/sharkdp/fd

Caused by:
  cannot read "/home/tavianator/code/sharkdp/fd/root"

Caused by:
  Permission denied (os error 13)

Possible Solution(s)

I suggest not following symbolic links when calculating the fingerprint.

I actually ran into this with a symlink to /, which caused

Caused by:
  cannot read "/home/tavianator/code/sharkdp/fd/why/dev/vboxusb"

meaning it was trying to recursively scan my whole file system.

Notes

Output of cargo version:

cargo 1.53.0 (4369396ce 2021-04-27)

[ehuss]

Thanks for the report! Cargo needs to follow symlinks, as what they point to may impact the build. However, for the purpose here, I think it would be fine to ignore errors reading the contents of a symlink destination.

One suggestion I have for your fd project is to use rerun-if-changed in the build script. This prevents cargo from scanning all files for possible changes (with one caveat described here, that the first build does a full scan, which it probably shouldn't).

[chrisboyce]

I see there's a a test, build_script_scan_eaccess, that specifically checks that the build command errors when it does not have access to a particular path. I'm not sure of all the use cases, but is there a semantic difference between an access error on a path within the project versus one that is symlinked?

[dswij]

@rustbot claim

[dswij]

Oh whoops, a mistake on my side, going to unassign myself.

@rustbot release-assignment

[ehuss]

Hm, so PathSource::list_files is used for several different things, including fingerprints, packaging, and vendoring. When packaging or vendoring, it is important to know about inaccessible files. However, for fingerprinting it probably isn't too important (if cargo can't access it, neither can a build script or rustc).

I can't offhand think of a really clean way to deal with that. My only idea is to pass a flag into list_files whether or not to ignore access errors. That seems a little awkward, though.

However, considering this more, maybe it is good to provide an error message early on, rather than later when publishing? That is, maybe this should just be closed?

Another thought is that this is essentially the same as #9528. I'm wondering if the suggestions there might be more in line?

Sorry, this may not have been as E-easy as I thought! But if you want to dig in and consider what might be a good behavior, it would be appreciated.

[dswij]

However, considering this more, maybe it is good to provide an error message early on, rather than later when publishing? That is, maybe this should just be closed?

I'm not too familiar in the fingerprinting aspect of cargo, but I can't think of a scenario where we want to calculate the fingerprint of inaccessible files and later on not use the file for publishing? If there's no such scenario, I think it's better to improve the error message. If there is indeed some edge cases, we might want to consider providing a clear way to ignore it.

Another thought is that this is essentially the same as #9528. I'm wondering if the suggestions there might be more in line?

I'd argue this is a different case. We want to limit the walker in #9528 rather than letting it running indefinitely, but here we can't even walk in the first place.

[ehuss]

Yea, improving the error message sounds good, though I'm uncertain of exactly how or what a better message would be.