Get all members as `available targets` even though default-members was specified. #15199

linyihai · 2025-02-18T11:06:44Z

What does this PR try to resolve?

The old behavior gets members from default-members if specified in a virtual workspace, this PR gets all workspace members for Available target hints.

Fixes #14544

How should we test and review this PR?

The first commit added the test, the second commit addressed the issue.

Additional information

rustbot · 2025-02-18T11:06:50Z

r? @weihanglo

rustbot has assigned @weihanglo.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

epage · 2025-02-18T12:33:59Z

tests/testsuite/workspaces.rs

+[ERROR] "--bin" takes one argument.
+Available binaries:
+    crate1
+    crate2


Fixes #14544

If the fix only adds bins from all targets to this error theo it will lead to user frustration as they then add --bin crate2 and it fails without saying why.

The core problem in the Issue is the --bin crate2 error message. It only suggested touching this error message as an addendum and suggested it show what the needed package is. I agree with that with the caveat that we need to double check if our completion or fish's completion script uses the output from this. While this is meant for humans and can change, we still want to play nice with these.

While the focus of #14544 is now included, two of the 3 improvements still require investigating their impact on cargo's and fish's completions and one still has the issue of showing values that can't be used.

@linyihai maybe we should just split this PR so we can move forward the --bin foo case and deal with --bin separately

Let us focus on the --bin foo case case

src/cargo/util/workspace.rs

src/cargo/ops/cargo_compile/unit_generator.rs

tests/testsuite/workspaces.rs

src/cargo/ops/cargo_compile/unit_generator.rs

tests/testsuite/build.rs

tests/testsuite/workspaces.rs

epage · 2025-02-21T15:48:59Z

tests/testsuite/workspaces.rs

 [ERROR] no bin target named `crate2`.
-Available bin targets:
-    another
+Available bin targets in crate2:


For cases like this where the bin and target name are redundant, should we clarify this

Available bin targets in package crate2:

I removed the targets that it seems not nessary.

epage · 2025-02-24T18:36:08Z

src/cargo/ops/cargo_compile/unit_generator.rs

@@ -286,7 +286,7 @@ impl<'a> UnitGenerator<'a, '_> {
            if !suggestion.is_empty() {
                write!(
                    msg,
-                    "no {} target {} `{}`{}{}",
+                    "no {} target {} `{}` in default-run packages{}{}",


We're always saying in default-run packages even if the user explicitly said --package foo?

epage · 2025-02-24T18:37:31Z

src/cargo/ops/cargo_compile/unit_generator.rs

+        let pattern = build_glob(target_name)?;
+        let filter = |t: &Target| {
+            if is_glob {
+                is_expected_kind(t) && pattern.matches(t.name())
+            } else {
+                is_expected_kind(t) && t.name() == target_name
+            }
        };


A refactor for pulling out the filter creation should probably be in its own refactor commit

epage · 2025-02-24T18:39:35Z

tests/testsuite/workspaces.rs

+[HELP] a target with a similar name exists: `crate1`
+Available bin in `crate2` package:
+    crate2


Shouldn't these have separate [HELP] prefixes?

…ave the `Target`

rustbot assigned weihanglo Feb 18, 2025

rustbot added A-cli Area: Command-line interface, option parsing, etc. A-workspaces Area: workspaces S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 18, 2025

epage reviewed Feb 18, 2025

View reviewed changes

linyihai force-pushed the available-packages branch from 3c409c9 to ee13fd6 Compare February 19, 2025 11:48

rustbot added the A-cargo-targets Area: selection and definition of targets (lib, bins, examples, tests, benches) label Feb 19, 2025

linyihai force-pushed the available-packages branch from ee13fd6 to d1ed4c8 Compare February 19, 2025 12:19

epage reviewed Feb 19, 2025

View reviewed changes

src/cargo/util/workspace.rs Outdated Show resolved Hide resolved

epage reviewed Feb 19, 2025

View reviewed changes

src/cargo/ops/cargo_compile/unit_generator.rs Outdated Show resolved Hide resolved

epage reviewed Feb 19, 2025

View reviewed changes

src/cargo/ops/cargo_compile/unit_generator.rs Show resolved Hide resolved

epage reviewed Feb 19, 2025

View reviewed changes

src/cargo/ops/cargo_compile/unit_generator.rs Outdated Show resolved Hide resolved

linyihai force-pushed the available-packages branch from d1ed4c8 to 7389aff Compare February 20, 2025 12:25

epage reviewed Feb 20, 2025

View reviewed changes

tests/testsuite/workspaces.rs Outdated Show resolved Hide resolved

epage reviewed Feb 20, 2025

View reviewed changes

tests/testsuite/workspaces.rs Outdated Show resolved Hide resolved

epage reviewed Feb 20, 2025

View reviewed changes

src/cargo/ops/cargo_compile/unit_generator.rs Outdated Show resolved Hide resolved

epage reviewed Feb 20, 2025

View reviewed changes

src/cargo/ops/cargo_compile/unit_generator.rs Outdated Show resolved Hide resolved

linyihai force-pushed the available-packages branch 2 times, most recently from 3d78767 to 7735088 Compare February 21, 2025 14:37

epage reviewed Feb 21, 2025

View reviewed changes

tests/testsuite/build.rs Outdated Show resolved Hide resolved

epage reviewed Feb 21, 2025

View reviewed changes

tests/testsuite/workspaces.rs Outdated Show resolved Hide resolved

epage reviewed Feb 21, 2025

View reviewed changes

linyihai force-pushed the available-packages branch 2 times, most recently from 2ae0f25 to c3aa679 Compare February 23, 2025 04:48

epage reviewed Feb 24, 2025

View reviewed changes

linyihai and others added 2 commits February 25, 2025 20:06

test(workspace): Add test about workspace

8b9dc21

refactor: Pull out the filter logic.

3824d83

Yihai Lin and others added 3 commits February 25, 2025 20:10

feat(workspace): Search other crates if the default-members doesn't h…

8cd875c

…ave the `Target`

refactor: clean some duplicate code.

ac56062

feat: Make no target found hint more clear.

685f0fd

linyihai force-pushed the available-packages branch from c3aa679 to 685f0fd Compare February 25, 2025 12:13

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Get all members as `available targets` even though default-members was specified. #15199

Get all members as `available targets` even though default-members was specified. #15199

linyihai commented Feb 18, 2025

rustbot commented Feb 18, 2025

epage Feb 18, 2025

epage Feb 20, 2025

epage Feb 24, 2025

linyihai Feb 25, 2025

epage Feb 21, 2025

linyihai Feb 23, 2025

epage Feb 24, 2025

epage Feb 24, 2025

epage Feb 24, 2025

Get all members as available targets even though default-members was specified. #15199

Are you sure you want to change the base?

Get all members as available targets even though default-members was specified. #15199

Conversation

linyihai commented Feb 18, 2025

What does this PR try to resolve?

How should we test and review this PR?

Additional information

rustbot commented Feb 18, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Get all members as `available targets` even though default-members was specified. #15199

Get all members as `available targets` even though default-members was specified. #15199