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: use spec id instead of name to match package #13335

Merged
merged 3 commits into from
Jan 30, 2024

Conversation

Rustin170506
Copy link
Member

@Rustin170506 Rustin170506 commented Jan 22, 2024

What does this PR try to resolve?

close #13310

Users tried cargo run -p [email protected].

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 [email protected] didn't match with foo. It said there is no [email protected] 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 [email protected] in your workspace and you intend to use [email protected], extracting only the name would result in the selection of [email protected]` 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

@rustbot
Copy link
Collaborator

rustbot commented Jan 22, 2024

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
tests/testsuite/run.rs Outdated Show resolved Hide resolved
@Rustin170506 Rustin170506 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
@Rustin170506 Rustin170506 marked this pull request as ready for review January 29, 2024 11:57
Copy link
Member Author

@Rustin170506 Rustin170506 left a comment

Choose a reason for hiding this comment

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

🔢 Self-check

src/cargo/ops/cargo_compile/packages.rs Show resolved Hide resolved
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.

Other parts look great. Thanks for the fix!

src/cargo/ops/cargo_compile/packages.rs Outdated Show resolved Hide resolved
src/cargo/ops/cargo_compile/packages.rs Outdated Show resolved Hide resolved
@weihanglo
Copy link
Member

Thanks!

@bors r+

@bors
Copy link
Contributor

bors commented Jan 30, 2024

📌 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
Copy link
Contributor

bors commented Jan 30, 2024

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

@bors
Copy link
Contributor

bors commented Jan 30, 2024

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

@bors bors merged commit 8ed4cb1 into rust-lang:master Jan 30, 2024
20 checks passed
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