Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

With package selection, --package <non-existent> --workspace doesn't warn or error #12978

Closed
sunshowers opened this issue Nov 14, 2023 · 9 comments · Fixed by #15071 · May be fixed by #14755
Closed

With package selection, --package <non-existent> --workspace doesn't warn or error #12978

sunshowers opened this issue Nov 14, 2023 · 9 comments · Fixed by #15071 · May be fixed by #14755
Assignees
Labels
A-cli Area: Command-line interface, option parsing, etc. C-bug Category: bug S-needs-mentor Status: Issue or feature is accepted, but needs a team member to commit to helping and reviewing.

Comments

@sunshowers
Copy link
Contributor

sunshowers commented Nov 14, 2023

Problem

Originally reported as nextest-rs/nextest#1108.

With Cargo's package selection, this errors out:

$ cargo build --package non-existent
error: package ID specification `non-existent` did not match any packages

However, this does not error out:

$ cargo build --package non-existent --workspace
<build occurs...>

I think this should at least produce a warning and maybe eventually error out.

(I tried searching for issues a bit but I couldn't figure out the right keywords.)

Steps

As above.

Possible Solution(s)

Produce a warning on --workspace and -p <non-existent>. Maybe eventually turn it into an error

Notes

No response

Version

% cargo +nightly version --verbose           
cargo 1.76.0-nightly (6790a5127 2023-11-10)
release: 1.76.0-nightly
commit-hash: 6790a5127895debec95c24aefaeb18e059270df3
commit-date: 2023-11-10
host: x86_64-unknown-linux-gnu
libgit2: 1.7.1 (sys:0.18.1 vendored)
libcurl: 8.4.0-DEV (sys:0.4.68+curl-8.4.0 vendored ssl:OpenSSL/1.1.1u)
ssl: OpenSSL 1.1.1u  30 May 2023
os: Pop!_OS 22.04 (jammy) [64-bit]
@sunshowers sunshowers added C-bug Category: bug S-triage Status: This issue is waiting on initial triage. labels Nov 14, 2023
@epage epage added A-cli Area: Command-line interface, option parsing, etc. S-needs-mentor Status: Issue or feature is accepted, but needs a team member to commit to helping and reviewing. and removed S-triage Status: This issue is waiting on initial triage. labels Nov 15, 2023
@766974616c79
Copy link
Contributor

@rustbot claim

@progressive-galib
Copy link

progressive-galib commented Dec 13, 2024

@weihanglo looks like #14755 doesnt fix it, i can pick it up

@linyihai
Copy link
Contributor

This below is a test snapshot for current behavior, Cargo will ignore the --pacakge flags due to it prefers the workspace option


    p.cargo("check --package baz --workspace")
        .with_stderr_data(str![[r#"
[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s

"#]])
        .run();

    p.cargo("check --package nonexistence --workspace")
        .with_stderr_data(str![[r#"
[CHECKING] baz v0.1.0 ([ROOT]/foo/baz)
[CHECKING] foo v0.1.0 ([ROOT]/foo)
[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s

"#]])
        .run();

IMO, If --package and --workspace both were specified, In this case, warn --package nonexistence will be ignored much better than warn --package nonexistence didn't match.

@epage
Copy link
Contributor

epage commented Jan 16, 2025

IMO, If --package and --workspace both were specified, In this case, warn --package nonexistence will be ignored much better than warn --package nonexistence didn't match.

I disagree. The warning would only be about the --package being redundant. The user expressed an intent with --package nonexistent and we should warn, or even error, if that intent was not satisfied.

For myself, I feel like non-existent packages should probably be an error, rather than a warning,. That would be a change in behavior but I think we can chalk it up to a bug. If we feel a transition period is needed, we could start as a warning.

@linyihai
Copy link
Contributor

The user expressed an intent with --package nonexistent and we should warn, or even error, if that intent was not satisfied.

Yes I agree it.

The warning would only be about the --package being redundant.

IMO, I think it would be good to also add this warning -workspace will be selected and ignored -package if they passed in the same time. Or we could add this description in somewhere (Cargo book or the --help).

@epage
Copy link
Contributor

epage commented Jan 20, 2025

Yes I agree it.

What are your thoughts on error vs warn?

IMO, I think it would be good to also add this warning -workspace will be selected and ignored -package if they passed in the same time. Or we could add this description in somewhere (Cargo book or the --help).

I'd prefer to discuss changes like that separate from this so we don't get bogged down in details from the other.

@linyihai
Copy link
Contributor

What are your thoughts on error vs warn?

My assumption that --package nonexistence --workspace this case will be very rare, it looks like a hard error is fine to me.

But given that this situation was introduced from the start, we can start with warn first.

@epage
Copy link
Contributor

epage commented Feb 4, 2025

@linyihai we talked about this in the Cargo team meeting today and felt there is not likely enough of an impact of fixing this CLI bug to have a gradual transition, preferring instead to make this an error from the beginning.

@linyihai
Copy link
Contributor

linyihai commented Feb 6, 2025

That makes sense. I had updated the PR to make this an error

github-merge-queue bot pushed a commit that referenced this issue Feb 6, 2025
### What does this PR try to resolve?

Fixes #12978

currently, if `--package `and `--workspace` passe in the same time, the
`--package` will be ignore even the package doesnt exist in the
workspace, this PR regards this behavior as an error

### How should we test and review this PR?

one commit add test, one commit fixes the issue.

### Additional information
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
A-cli Area: Command-line interface, option parsing, etc. C-bug Category: bug S-needs-mentor Status: Issue or feature is accepted, but needs a team member to commit to helping and reviewing.
Projects
None yet
5 participants