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

Normalize --path when install bin outside current workspace #10335

Merged
merged 3 commits into from
Jan 27, 2022

Conversation

weihanglo
Copy link
Member

What does this PR try to resolve?

Fixes #10283

For Workspace::find_root, cargo_util::path::PathAncestors won't do
path normalization while walking back the ancestors. The responsibility
lies in the caller. Thus, cargo install should normalize its --path
argument before passing in SourceId::for_path and Workspace::new.

Config::reload_rooted_at is not affected because cargo always starts
searching and merging configs from where it is invoked.

How should we test and review this PR?

A new test install::install_relative_path_outside_current_ws should cover this case.
You can also follow the reproducible steps described in #10283.

Make up this test case to reflect the current incorrect behavior.
For `Workspace::find_root`, `cargo_util::path::PathAncestors` won't do
path normalization while walking back the ancestors. The responsibility
lies in the caller. Thus, `cargo install` should normalize its `--path`
argument before passing in `SourceId::for_path` and `Workspace::new`.

`Config::reload_rooted_at` is not affected because cargo always starts
searching and merging configs from where it is invoked.
@rust-highfive
Copy link

r? @alexcrichton

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 27, 2022
@weihanglo weihanglo changed the title Normalize --path to install bin outside current workspace Normalize --path when install bin outside current workspace Jan 27, 2022
@weihanglo
Copy link
Member Author

weihanglo commented Jan 27, 2022

Oops. Seems that clap updates its error message with optional args.

Edited: Upgrade clap: #10336

@alexcrichton
Copy link
Member

@bors: r+

Looks good to me, thanks!

@bors
Copy link
Contributor

bors commented Jan 27, 2022

📌 Commit 76301eb has been approved by alexcrichton

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 27, 2022
@bors
Copy link
Contributor

bors commented Jan 27, 2022

⌛ Testing commit 76301eb with merge 5137905...

@bors
Copy link
Contributor

bors commented Jan 27, 2022

☀️ Test successful - checks-actions
Approved by: alexcrichton
Pushing 5137905 to master...

@bors bors merged commit 5137905 into rust-lang:master Jan 27, 2022
@weihanglo weihanglo deleted the issue-10283 branch January 27, 2022 16:40
ehuss added a commit to ehuss/rust that referenced this pull request Feb 1, 2022
Update cargo

10 commits in 1c034752de0df744fcd7788fcbca158830b8bf85..25fcb135d02ea897ce894b67ae021f48107d522b
2022-01-25 22:36:53 +0000 to 2022-02-01 01:32:48 +0000
- fix(install): Keep v1 file formatting the same (rust-lang/cargo#10349)
- fix(vendor): Use tables for sample config (rust-lang/cargo#10348)
- Add bash completion for `cargo clippy` (rust-lang/cargo#10347)
- Do not ignore `--features` when `--all-features` is present (rust-lang/cargo#10337)
- test: Fix compatibilty with new toml_edit (rust-lang/cargo#10350)
- extra-link-arg-etc: support all link types (credit `@davidhewitt)` (rust-lang/cargo#10274)
- Make clippy happy (rust-lang/cargo#10340)
- Update publishing link for semver rules. (rust-lang/cargo#10338)
- Normalize --path when install bin outside current workspace (rust-lang/cargo#10335)
- Bump clap to v3.0.13 (rust-lang/cargo#10336)
@ehuss ehuss added this to the 1.60.0 milestone Feb 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

cargo install --path is affected by the current workspace
5 participants