Skip to content

Relative cargo:rerun-if-changed paths are not resolved in dep-info files #9445

New issue

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

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

Already on GitHub? # to your account

Closed
futile opened this issue May 1, 2021 · 5 comments · Fixed by #9596
Closed

Relative cargo:rerun-if-changed paths are not resolved in dep-info files #9445

futile opened this issue May 1, 2021 · 5 comments · Fixed by #9596
Labels
C-bug Category: bug

Comments

@futile
Copy link

futile commented May 1, 2021

Problem

If I have a dependency that has a build.rs build script that outputs a relative path (e.g., cargo:rerun-if-changed=build.rs), this path will appear unchanged in dep info files of my crate.

This path can then not be resolved by external build tools.

Steps

  1. Create a new cargo project (cargo new --bin dep-info-test && cd dep-info-test)
  2. Add a dependency that outputs a cargo:rerun-if-changed=<relative path>, e.g., libc: echo 'libc = "0.2"' >> Cargo.toml
  3. Build the project and check the resulting dep-info file: cargo build && cat target/debug/dep-info-test.d:
[...]
/abs/path/to/dep-info-test/target/debug/dep-info-test: /abs/path/to/dep-info-test/src/main.rs build.rs

Possible Solution(s)
It seems that cargo's internal change-detection mechanism (something with fingerprints?) resolves relative paths output by build scripts as relative to their CARGO_MANIFEST_DIR (I think), which should probably explicitly also be done for dep-info files.

Also, in the case of crates.io- & git-dependencies, cargo doesn't write paths to the dependencies' files itself into the dep-info file, as it regards such dependencies as 'read-only'. So for non-path dependencies absolute paths that point into a dependency's CARGO_MANIFEST_DIR should probably not appear in dep-info files anymore (but should still appear for path-dependencies).

Notes

Output of cargo version:

$ cargo version
cargo 1.53.0-nightly (0ed318d18 2021-04-23)

Same behavior with current stable:

$ cargo version
cargo 1.51.0 (43b129a20 2021-03-16)

Extra discussion material

I think the dep-info files are currently misleading, as they don't capture all conditions under which cargo needs to rebuild a project. For example, environment variables registered using cargo:rerun-if-env-changed are not captured in this file. Another example are .cargo/config files, which often don't exist, but influence compilation, so they would need to be in every dep-info file. These can also appear outside the project, i.e., in every parent directory down to the root-directory.

The fact that this file is incomplete is not mentioned in the current docs, but maybe should be mentioned there.

@futile futile added the C-bug Category: bug label May 1, 2021
@ehuss
Copy link
Contributor

ehuss commented May 1, 2021

Thanks for the report! This should be fixed via #9421, which I think should be in the next nightly tomorrow.

If you would like to extend the documentation to mention things that tools should be aware of (like environment variables), a PR would be appreciated!

@futile
Copy link
Author

futile commented May 1, 2021

Ohh missed that, but great it's already fixed! I'll try tomorrow/in the next few days with the new nightly.

One issue that might still exist is that the absolute path to the build.rs is usually not inserted by cargo for git/crates.io-dependencies as these seem to be considered read-only, but from the initial post in in the PR it looks like they might be present with the PR. I'll see if that's the case after trying it out.

@futile
Copy link
Author

futile commented May 2, 2021

Just tried it out with the current nightly (cargo 1.53.0-nightly (f3e13226d 2021-04-30)), which produces the following file:

/abs/path/to/dep-info-test/target/debug/dep-info-test: /home/user/.cargo/registry/src/github.heygears.com-1ecc6299db9ec823/libc-0.2.94/build.rs /abs/path/to/dep-info-test/src/main.rs

So it's working 🎉 !

However, the absolute path to the build.rs inside cargo's local registry appears in the dep-info file now. This differs from cargo's usual behavior that doesn't add paths to its registry into dep-info files for git/crates.io-dependencies, as these are regarded read-only (I think). (For path-dependencies this aligns with cargo's current behavior.)

@futile
Copy link
Author

futile commented May 6, 2021

@ehuss ping; just wanted to ask if you want me to leave the issue open/open another issue for the small detail from my last comment, otherwise this is fixed, thanks a lot! :)

@ehuss
Copy link
Contributor

ehuss commented May 7, 2021

I'd like to keep it open to investigate what you mentioned, I haven't had time to take a look. It seems a little surprising, though I can't imagine it would have negative consequences, I'd like to understand it better.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
C-bug Category: bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants