Skip to content

fix: display absolute path in the missing in PATH warning #16125

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 9 commits into from
Nov 6, 2025
Merged

fix: display absolute path in the missing in PATH warning #16125

merged 9 commits into from
Nov 6, 2025
+199 −16

Conversation

@0xPoe
Copy link
Member

@0xPoe 0xPoe commented Oct 16, 2025 •
edited
Loading

What does this PR try to resolve?

close #16023

Improve the warning message for realtive install location.

  1. Did a small refactoring to inline GlobalContext::get_path to prevent potential misuse in the future.
  2. Made Cargo always display the absolute install path in warnings.
  3. Added a warning indicating that install.root paths without a trailing backslash will be deprecated in the future.

How to test and review this PR?

Please check the unit test.

r?@ghost

ehuss
ehuss reviewed Oct 16, 2025
View reviewed changes
@0xPoe 0xPoe force-pushed the poe-patch-install.root branch 3 times, most recently from daf3b81 to 9e4b6b4 Compare October 27, 2025 20:06
@0xPoe 0xPoe force-pushed the poe-patch-install.root branch 4 times, most recently from b141907 to 7867550 Compare October 29, 2025 20:37
@0xPoe 0xPoe changed the title refactor: inline GlobalContext::get_path fix: display absolute path in the missing in PATH warning Oct 29, 2025
@0xPoe 0xPoe force-pushed the poe-patch-install.root branch from 8995ab9 to 34cc964 Compare November 2, 2025 21:25
@0xPoe 0xPoe marked this pull request as ready for review November 2, 2025 21:25
@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 2, 2025
0xPoe
0xPoe commented Nov 2, 2025
View reviewed changes
Copy link
Member Author

@0xPoe 0xPoe left a comment •
edited by rustbot
Loading

Choose a reason for hiding this comment

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

🔢 Self-check (PR reviewed by myself and ready for feedback)

  • Code compiles successfully

  • Unit tests added

  • All tests pass

  • Comments added where necessary

  • Logs added where necessary

  • PR title and description updated

  • Documentation PR created (or confirmed not needed)

  • PR size is reasonable

View changes since this review

@0xPoe 0xPoe force-pushed the poe-patch-install.root branch 3 times, most recently from 080a4e7 to 24904ce Compare November 2, 2025 22:14
@0xPoe 0xPoe requested a review from ehuss November 2, 2025 22:16
weihanglo
weihanglo reviewed Nov 3, 2025
View reviewed changes
@0xPoe 0xPoe force-pushed the poe-patch-install.root branch from 24904ce to fb0690b Compare November 3, 2025 21:08
@rustbot rustbot added the A-configuration Area: cargo config files and env vars label Nov 3, 2025
@0xPoe 0xPoe requested a review from weihanglo November 3, 2025 21:30
@0xPoe 0xPoe force-pushed the poe-patch-install.root branch from cb41a3f to dc89d66 Compare November 3, 2025 21:37
weihanglo
weihanglo reviewed Nov 5, 2025
View reviewed changes
Copy link
Member

@weihanglo weihanglo left a comment •
edited by rustbot
Loading

Choose a reason for hiding this comment

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

The final diff looks actually clearer than the commit history. Not sure the history was intended that way, though I feel like we might want a fixup.

Anyway, thanks for fixing this!

View changes since this review

@0xPoe 0xPoe force-pushed the poe-patch-install.root branch 2 times, most recently from 4346e43 to 9980469 Compare November 6, 2025 19:51
@0xPoe 0xPoe force-pushed the poe-patch-install.root branch 5 times, most recently from 877a68e to 01e1c7e Compare November 6, 2025 20:34
@0xPoe 0xPoe force-pushed the poe-patch-install.root branch 2 times, most recently from bcf5166 to 42ae2e4 Compare November 6, 2025 20:43
@0xPoe 0xPoe force-pushed the poe-patch-install.root branch from 42ae2e4 to 35fb192 Compare November 6, 2025 20:44
@0xPoe 0xPoe requested a review from weihanglo November 6, 2025 20:54
weihanglo
weihanglo approved these changes Nov 6, 2025
View reviewed changes
Copy link
Member

@weihanglo weihanglo left a comment •
edited by rustbot
Loading

Choose a reason for hiding this comment

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

Nice! Thanks a lot for reorganizing commits!

View changes since this review

@weihanglo weihanglo added this pull request to the merge queue Nov 6, 2025
Merged via the queue into rust-lang:master with commit 5854d39 Nov 6, 2025
25 checks passed
@rustbot rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 6, 2025
@0xPoe 0xPoe deleted the poe-patch-install.root branch November 7, 2025 08:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@weihanglo weihanglo weihanglo approved these changes

@ehuss ehuss Awaiting requested review from ehuss

Assignees

No one assigned

Labels

A-configuration Area: cargo config files and env vars Command-install

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

False positive "be sure to add .local/bin to your PATH" warning with relative install root

4 participants

@0xPoe @ehuss @weihanglo @rustbot