Skip to content

fix: use spec id instead of name to match package#13335

Merged
bors merged 3 commits into
rust-lang:masterfrom
0xPoe:rustin-patch-same-name
Jan 30, 2024
Merged

fix: use spec id instead of name to match package#13335
bors merged 3 commits into
rust-lang:masterfrom
0xPoe:rustin-patch-same-name

Conversation

@0xPoe

@0xPoe 0xPoe commented Jan 22, 2024

Copy link
Copy Markdown
Member

What does this PR try to resolve?

close #13310

Users tried cargo run -p foo@0.5.0.

The bug came from here:

  1. We store the spec as the name in this function: hi-rustin/cargo@aa64447/src/cargo/ops/cargo_compile/packages.rs#L201
  2. But we use it here to match only the name of the package: hi-rustin/cargo@aa64447/src/cargo/ops/cargo_compile/packages.rs#L128

So foo@0.5.0 didn't match with foo. It said there is no foo@0.5.0 in the workspace.

In order to resolve this issue, we need to utilize the package specification ID directly. If we only extract the name and drop the version specification, it could potentially lead to another bug. For instance, if you have a package foo@0.1.1 in your workspace and you intend to use foo@0.2.2, extracting only the name would result in the selection of foo@0.1.1` instead.

So in this PR I parsed the package spec ID and used it to match the package's spec ID.

How should we test and review this PR?

Check out the unit test.

Additional information

None

Signed-off-by: hi-rustin <rustin.liu@gmail.com>
@rustbot

rustbot commented Jan 22, 2024

Copy link
Copy Markdown
Collaborator

r? @weihanglo

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

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 22, 2024
Comment thread tests/testsuite/run.rs Outdated
Comment thread src/cargo/ops/cargo_compile/packages.rs
@0xPoe 0xPoe changed the title WIP: test: add a case for running binary with same name as dependency fix: use spec id instead of name to match package Jan 29, 2024
@0xPoe 0xPoe marked this pull request as ready for review January 29, 2024 11:57
@0xPoe 0xPoe requested a review from weihanglo January 29, 2024 11:57

@0xPoe 0xPoe left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

🔢 Self-check

Comment thread src/cargo/ops/cargo_compile/packages.rs

@weihanglo weihanglo left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Other parts look great. Thanks for the fix!

Comment thread src/cargo/ops/cargo_compile/packages.rs Outdated
Comment thread src/cargo/ops/cargo_compile/packages.rs Outdated
Comment thread tests/testsuite/run.rs
0xPoe added 2 commits January 30, 2024 20:55
Signed-off-by: hi-rustin <rustin.liu@gmail.com>
Signed-off-by: hi-rustin <rustin.liu@gmail.com>
@0xPoe 0xPoe force-pushed the rustin-patch-same-name branch from 3c3fe21 to 7a13864 Compare January 30, 2024 12:56
@0xPoe 0xPoe requested a review from weihanglo January 30, 2024 12:57
@weihanglo

Copy link
Copy Markdown
Member

Thanks!

@bors r+

@bors

bors commented Jan 30, 2024

Copy link
Copy Markdown
Contributor

📌 Commit 7a13864 has been approved by weihanglo

It is now in the queue for this repository.

@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 30, 2024
@bors

bors commented Jan 30, 2024

Copy link
Copy Markdown
Contributor

⌛ Testing commit 7a13864 with merge 8ed4cb1...

@bors

bors commented Jan 30, 2024

Copy link
Copy Markdown
Contributor

☀️ Test successful - checks-actions
Approved by: weihanglo
Pushing 8ed4cb1 to master...

@bors bors merged commit 8ed4cb1 into rust-lang:master Jan 30, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 5, 2024
Update cargo

24 commits in 7bb7b539558dc88bea44cee4168b6269bf8177b0..cdf84b69d0416c57ac9dc3459af80dfb4883d27a
2024-01-20 00:15:32 +0000 to 2024-02-02 19:39:16 +0000
- Apply `-Zpanic-abort-tests` to doctests too (rust-lang/cargo#13388)
- Don't print rustdoc command lines on failure by default (rust-lang/cargo#13387)
- Ensure `nonzero_exit_code` test isn't affected by developers `RUST_BACKTRACE` setting (rust-lang/cargo#13385)
- Add `docsrs` cfg as a well known `--check-cfg` (rust-lang/cargo#13383)
- fix(new): Print a note, rather than a comment, for more information (rust-lang/cargo#13371)
- Change tests to support changes to suggestion (rust-lang/cargo#13382)
- chore(ci): enable m1 runner (rust-lang/cargo#13377)
- fix(toml): Improve map/sequence error message (rust-lang/cargo#13376)
- fix(diagnostic): Don't panic on empty spans (rust-lang/cargo#13375)
- doc: Hide `cargo-fetch` description in offline man page (rust-lang/cargo#13364)
- feat: stabilize lockfile v4 (rust-lang/cargo#12852)
- fix(new): Print a 'Creating', rather than 'Created' status (rust-lang/cargo#13367)
- fix: use spec id instead of name to match package (rust-lang/cargo#13335)
- refactor(shell): Use new fancy anstyle API (rust-lang/cargo#13368)
- feat(cargo-update): `--precise` to allow yanked versions (rust-lang/cargo#13333)
- refactor: remove unnecessary Option in `Freshness::Dirty` (rust-lang/cargo#13361)
- doc: Replace version with `latest` for jobserver link (rust-lang/cargo#13366)
- test: data layout fix for `x86_64-unknown-none-gnu` (rust-lang/cargo#13362)
- docs(ref): Try to improve reg auth docs (rust-lang/cargo#13351)
- fix typo of rustbuild, instead of rustuild (rust-lang/cargo#13357)
- fix(config): Deprecate non-extension files (rust-lang/cargo#13349)
- fix(cli): Improve errors related to cargo script (rust-lang/cargo#13346)
- fix list option description starting with uppercase (rust-lang/cargo#13344)
- Fix typo in test (rust-lang/cargo#13342)
@ehuss ehuss added this to the 1.78.0 milestone Mar 2, 2024
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 run error: package(s) ... not found in workspace

6 participants