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

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

Closed
tavianator opened this issue Sep 7, 2021 · 8 comments · Fixed by #10191
Labels
A-build-scripts Area: build.rs scripts A-rebuild-detection Area: rebuild detection and fingerprinting C-bug Category: bug E-medium Experience: Medium

Comments

@tavianator
Copy link

tavianator commented Sep 7, 2021

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)
@tavianator tavianator added the C-bug Category: bug label Sep 7, 2021
@ehuss
Copy link
Contributor

ehuss commented Sep 11, 2021

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).

@ehuss ehuss added A-build-scripts Area: build.rs scripts A-rebuild-detection Area: rebuild detection and fingerprinting E-easy Experience: Easy labels Sep 11, 2021
@chrisboyce
Copy link

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

This comment has been minimized.

@dswij
Copy link
Member

dswij commented Sep 15, 2021

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

@rustbot release-assignment

@ehuss
Copy link
Contributor

ehuss commented Sep 17, 2021

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
Copy link
Member

dswij commented Sep 18, 2021

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 ehuss added E-medium Experience: Medium and removed E-easy Experience: Easy labels Nov 29, 2021
@ehuss
Copy link
Contributor

ehuss commented Nov 29, 2021

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

@Raishav
Copy link

Raishav commented Jul 30, 2024

@tavianator - facing the same issue on version 1.80.0, is there a fix found for the same issue?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-build-scripts Area: build.rs scripts A-rebuild-detection Area: rebuild detection and fingerprinting C-bug Category: bug E-medium Experience: Medium
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants