Skip to content

Conversation

ChrisDenton
Copy link
Member

What does this PR try to resolve?

Follow up to #9991. The issue explains why but in short canonicalising executable paths causes issues for symlinks. It seems this was missed in #9991.

How should we test and review this PR?

Note that the comment I deleted is wrong since #9991.

Running existing test should ensure this doesn't break anything.

@rustbot
Copy link
Collaborator

rustbot commented Mar 26, 2025

r? @ehuss

rustbot has assigned @ehuss.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added A-configuration Area: cargo config files and env vars S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 26, 2025
@weihanglo weihanglo requested a review from Copilot March 27, 2025 00:12
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.

Copy link
Member

@weihanglo weihanglo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems reasonable. How did you discover this though?

@weihanglo
Copy link
Member

Sorry I shouldn't click that Copilot review button… Was just curious 😆.

@ChrisDenton
Copy link
Member Author

I was investigating issues caused by the rustup shims now being symlinks by default in 1.28+ and this looked suspicious.

I'm not yet sure if this caused a real world breakage but it definitely looks like it could if $CARGO is set and cargo recursively calls into itself.

@weihanglo weihanglo added this pull request to the merge queue Mar 27, 2025
Merged via the queue into rust-lang:master with commit 84a38b9 Mar 27, 2025
21 checks passed
@ChrisDenton ChrisDenton deleted the canon branch March 27, 2025 01:41
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 6, 2025
Update cargo

17 commits in a6c604d1b8a2f2a8ff1f3ba6092f9fda42f4b7e9..0e93c5bf6a1d5ee7bc2af63d1afb16cd28793601
2025-03-26 18:11:00 +0000 to 2025-04-05 00:00:24 +0000
- chore(deps): bump openssl from 0.10.71 to 0.10.72 (rust-lang/cargo#15394)
- chore(ci): restore cargo-util semver check (rust-lang/cargo#15389)
- docs(changelog): polish changelog items (rust-lang/cargo#15379)
- chore(deps): update msrv (1 version) to v1.86 (rust-lang/cargo#15381)
- chore: add aarch64 linux runner (rust-lang/cargo#15077)
- Added `build_directory` field to cargo metadata output (rust-lang/cargo#15377)
- chore(deps): update rust crate rusqlite to 0.34.0 (rust-lang/cargo#15373)
- Prevent undeclared public network access (rust-lang/cargo#15368)
- rename the `author` field to be `authors` in book.toml (rust-lang/cargo#15362)
- move modules from kebab-case to snake_case (rust-lang/cargo#14439)
- chore: bump to 0.89.0; update changelog (rust-lang/cargo#15372)
- docs(unstable): update `-Zrustdoc-depinfo` tracking issue link (rust-lang/cargo#15371)
- fix(tree): Make output more deterministic (rust-lang/cargo#15369)
- feat: rustdoc depinfo rebuild detection via -Zrustdoc-depinfo (rust-lang/cargo#15359)
- Rename the gc config table (rust-lang/cargo#15367)
- Revert "Temporarily ignore cargo_test_doctest_xcompile_ignores" (rust-lang/cargo#15357)
- Don't canonicalize executable path in `cargo_exe` (rust-lang/cargo#15355)

r? ghost
@rustbot rustbot added this to the 1.88.0 milestone Apr 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-configuration Area: cargo config files and env vars S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants