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

Fix renaming directory project using build scripts with cross-compiling. #6328

Merged
merged 1 commit into from
Nov 18, 2018

Conversation

ehuss
Copy link
Contributor

@ehuss ehuss commented Nov 18, 2018

The rename-protection introduced in #4818 checks for paths with a prefix of /…/target/debug, but this does not work for paths used during cross-compiling.

This change checks for paths with the full OUT_DIR prefix, and the build/PKG/root-output file now includes that full OUT_DIR instead of /…/target/debug.

This also includes a few other changes:

  • Support fixing KIND=PATH style paths.
  • Support fixing paths in metadata (like cargo:root=… or cargo:include=… which are common).
  • Adds a "version" value to the metadata hash to ensure that cargo doesn't get confused with the new root-output file format.

Fixes #6177

@rust-highfive
Copy link

r? @alexcrichton

(rust_highfive has picked a reviewer for you, use r? to override)

@alexcrichton
Copy link
Member

@bors: r+

Awesome, thanks for this!

@bors
Copy link
Contributor

bors commented Nov 18, 2018

📌 Commit d1045c9 has been approved by alexcrichton

@bors
Copy link
Contributor

bors commented Nov 18, 2018

⌛ Testing commit d1045c9 with merge 53e436d...

bors added a commit that referenced this pull request Nov 18, 2018
Fix renaming directory project using build scripts with cross-compiling.

The rename-protection introduced in #4818 checks for paths with a prefix of `/…/target/debug`, but this does not work for paths used during cross-compiling.

This change checks for paths with the full `OUT_DIR` prefix, and the `build/PKG/root-output` file now includes that full `OUT_DIR` instead of `/…/target/debug`.

This also includes a few other changes:
- Support fixing KIND=PATH style paths.
- Support fixing paths in metadata (like `cargo:root=…` or `cargo:include=…` which are common).
- Adds a "version" value to the metadata hash to ensure that cargo doesn't get confused with the new `root-output` file format.

Fixes #6177
@bors
Copy link
Contributor

bors commented Nov 18, 2018

☀️ Test successful - status-appveyor, status-travis
Approved by: alexcrichton
Pushing 53e436d to master...

@ExpHP
Copy link
Contributor

ExpHP commented Nov 18, 2018

So I imagine that, when mapping custom metadata, this probably only handles the path if it is at the very beginning of the value? (since searching for matches anywhere in the value would seem risky, even if it still is unlikely to generate false positives; and I also didn't notice anything performing a search in the source diff)

One could picture for instance custom metadata that looks something like this:

cargo:path=<OUTDIR>/subdir1:<OUTDIR>/subdir2

and it could be a pretty confusing footgun if this gets mapped to

cargo:path=<NEW_OUTDIR>/subdir1:<OLD_OUTDIR>/subdir2

At the same time, this mechanism alone is sufficiently powerful for people to write

cargo:outdir=<OUTDIR>

and join paths on their own, so perhaps it mostly boils down to just an issue of documentation.

@ehuss
Copy link
Contributor Author

ehuss commented Nov 18, 2018

It replaces all occurrences throughout the line:

let value = value.replace(
script_out_dir_when_generated.to_str().unwrap(),
script_out_dir.to_str().unwrap(),
);

The path should be relatively unique (/path/to/project/target/debug/build/$PKG-$HASH/out), so I think it should be safe. This also only applies when you have moved the target directory.

kennytm added a commit to kennytm/rust that referenced this pull request Nov 27, 2018
Update cargo

14 commits in b3d0b2e545b61d4cd08096911724b7d49d213f73..1ff5975b96b3d395bb962394596998dfb485f793
2018-11-15 19:13:04 +0000 to 2018-11-25 14:59:12 +0000
- Intern SourceId (rust-lang/cargo#6342)
- Tweak Layout to allow for non json file targets with internal "." (rust-lang/cargo#6255)
- Correct Target Directory command-line option (rust-lang/cargo#6343)
- Persistent data structures by im-rs (rust-lang/cargo#6336)
- Move command prelude into main library (rust-lang/cargo#6335)
- Distinguish custom build invocations (rust-lang/cargo#6331)
- Allow crate_type=bin examples to run (rust-lang/cargo#6330)
- Make verify-project honour unstable features (rust-lang/cargo#6326)
- Make autodiscovery disable inferred targets (rust-lang/cargo#6329)
- Add `c` alias for `check` (rust-lang/cargo#6218)
- Allow user aliases to override built-in aliases (rust-lang/cargo#6259)
- Fix renaming directory project using build scripts with cross-compiling. (rust-lang/cargo#6328)
- Fix add_plugin_deps-related tests. (rust-lang/cargo#6327)
- Add a glossary. (rust-lang/cargo#6321)
kennytm added a commit to kennytm/rust that referenced this pull request Nov 30, 2018
Update cargo

14 commits in b3d0b2e545b61d4cd08096911724b7d49d213f73..1ff5975b96b3d395bb962394596998dfb485f793
2018-11-15 19:13:04 +0000 to 2018-11-25 14:59:12 +0000
- Intern SourceId (rust-lang/cargo#6342)
- Tweak Layout to allow for non json file targets with internal "." (rust-lang/cargo#6255)
- Correct Target Directory command-line option (rust-lang/cargo#6343)
- Persistent data structures by im-rs (rust-lang/cargo#6336)
- Move command prelude into main library (rust-lang/cargo#6335)
- Distinguish custom build invocations (rust-lang/cargo#6331)
- Allow crate_type=bin examples to run (rust-lang/cargo#6330)
- Make verify-project honour unstable features (rust-lang/cargo#6326)
- Make autodiscovery disable inferred targets (rust-lang/cargo#6329)
- Add `c` alias for `check` (rust-lang/cargo#6218)
- Allow user aliases to override built-in aliases (rust-lang/cargo#6259)
- Fix renaming directory project using build scripts with cross-compiling. (rust-lang/cargo#6328)
- Fix add_plugin_deps-related tests. (rust-lang/cargo#6327)
- Add a glossary. (rust-lang/cargo#6321)
Centril added a commit to Centril/rust that referenced this pull request Dec 1, 2018
Update cargo

14 commits in b3d0b2e545b61d4cd08096911724b7d49d213f73..1ff5975b96b3d395bb962394596998dfb485f793
2018-11-15 19:13:04 +0000 to 2018-11-25 14:59:12 +0000
- Intern SourceId (rust-lang/cargo#6342)
- Tweak Layout to allow for non json file targets with internal "." (rust-lang/cargo#6255)
- Correct Target Directory command-line option (rust-lang/cargo#6343)
- Persistent data structures by im-rs (rust-lang/cargo#6336)
- Move command prelude into main library (rust-lang/cargo#6335)
- Distinguish custom build invocations (rust-lang/cargo#6331)
- Allow crate_type=bin examples to run (rust-lang/cargo#6330)
- Make verify-project honour unstable features (rust-lang/cargo#6326)
- Make autodiscovery disable inferred targets (rust-lang/cargo#6329)
- Add `c` alias for `check` (rust-lang/cargo#6218)
- Allow user aliases to override built-in aliases (rust-lang/cargo#6259)
- Fix renaming directory project using build scripts with cross-compiling. (rust-lang/cargo#6328)
- Fix add_plugin_deps-related tests. (rust-lang/cargo#6327)
- Add a glossary. (rust-lang/cargo#6321)
bors added a commit to rust-lang/rust that referenced this pull request Dec 4, 2018
Update cargo, rls

26 commits in b3d0b2e545b61d4cd08096911724b7d49d213f73..5e85ba14aaa20f8133863373404cb0af69eeef2c
2018-11-15 19:13:04 +0000 to 2018-12-02 14:37:25 +0000
- ConflictStoreTrie: Faster filtered search (rust-lang/cargo#6366)
- Remove `cmake` as a requirement (rust-lang/cargo#6368)
- progress: display "Downloading 1 crate" instead of "Downloading 1 crates" (rust-lang/cargo#6369)
- Use expect over unwrap, for panic-in-panic aborts (rust-lang/cargo#6364)
- Switch to pretty_env_logger, under --features pretty-env-logger (rust-lang/cargo#6362)
- use allow-dirty option in `cargo package` to skip vcs checks (rust-lang/cargo#6280)
- remove clones made redundant by Intern PackageId (rust-lang/cargo#6352)
- docs: correct profile usage of `cargo test --release` (rust-lang/cargo#6345)
- Improve doc for `cargo install` (rust-lang/cargo#6354)
- Intern PackageId (rust-lang/cargo#6332)
- Clean only release artifacts if --release option is set (rust-lang/cargo#6349)
- remove clones made redundant by Intern SourceId (rust-lang/cargo#6347)
- Intern SourceId (rust-lang/cargo#6342)
- Tweak Layout to allow for non json file targets with internal "." (rust-lang/cargo#6255)
- Correct Target Directory command-line option (rust-lang/cargo#6343)
- Persistent data structures by im-rs (rust-lang/cargo#6336)
- Move command prelude into main library (rust-lang/cargo#6335)
- Distinguish custom build invocations (rust-lang/cargo#6331)
- Allow crate_type=bin examples to run (rust-lang/cargo#6330)
- Make verify-project honour unstable features (rust-lang/cargo#6326)
- Make autodiscovery disable inferred targets (rust-lang/cargo#6329)
- Add `c` alias for `check` (rust-lang/cargo#6218)
- Allow user aliases to override built-in aliases (rust-lang/cargo#6259)
- Fix renaming directory project using build scripts with cross-compiling. (rust-lang/cargo#6328)
- Fix add_plugin_deps-related tests. (rust-lang/cargo#6327)
- Add a glossary. (rust-lang/cargo#6321)
@ehuss ehuss added this to the 1.32.0 milestone Feb 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Paths emitted by build script are not corrected when cross-compiling
5 participants