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

Support glob patterns for crate arguments to --package and --exclude flags #8582

Closed
fitzgen opened this issue Aug 3, 2020 · 3 comments · Fixed by #8752
Closed

Support glob patterns for crate arguments to --package and --exclude flags #8582

fitzgen opened this issue Aug 3, 2020 · 3 comments · Fixed by #8752
Labels
A-cargo-targets Area: selection and definition of targets (lib, bins, examples, tests, benches) A-cli Area: Command-line interface, option parsing, etc. C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted`

Comments

@fitzgen
Copy link
Member

fitzgen commented Aug 3, 2020

Describe the problem you are trying to solve

In workspaces where there are many crates, I would like to only build/test the crates that share the "peepmatic-" name prefix. Or I would like to exclude all of the crates with the "wasmtime-" name prefix.

Describe the solution you'd like

Support glob-style * wildcards for the -p/--package and --exclude flags when building in a workspace, like

$ cargo build --exclude peepmatic-*

Notes

Would help cut down on monstrosities like this:

        cargo +nightly \
            -Zfeatures=all -Zpackage-features \
            test \
            --features test-programs/test_programs \
            --features experimental_x64 \
            --all \
            --exclude lightbeam \
            --exclude peepmatic \
            --exclude peepmatic-automata \
            --exclude peepmatic-fuzzing \
            --exclude peepmatic-macro \
            --exclude peepmatic-runtime \
            --exclude peepmatic-test

We could collapse all those excludes into just --exlude lightbeam --exclude peepmatic*.

@fitzgen fitzgen added the C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` label Aug 3, 2020
@ehuss ehuss added A-cargo-targets Area: selection and definition of targets (lib, bins, examples, tests, benches) A-cli Area: Command-line interface, option parsing, etc. labels Aug 19, 2020
@weihanglo
Copy link
Member

weihanglo commented Sep 13, 2020

I am interesting in this and wrote a proof-of-concept. However I need guidance on some design topics.

First, we know that cargo uses an onion model to determine the final compilation targets (borrows from @ehuss's comment):

workspace > package > target crate > modules > (cfg, attr, ...)

Supposed this issue is trying to deal with workspace level and we can just stick to the function Packages::to_package_id_specs where the actual opt-out works. The following are design issues that call for advice.

  • The current opt-out (--exclude) mechanism compares packages as string but not PackageIdSpec, which does not match what command line help says (--exclude <SPEC>...). Not sure shall --exclude respect the Spec or not.
  • The original approach using BTreeSet is very clever but my POC gets worse on time complexity (O(pkg * excludes)). Is there any space to improve? Is it need to resolve immediately?
  • Do we need all the glob functionalities or just a simple * wildcard? Should we write simple wildcard from scratch?
  • Bash on macOS expands glob args automatically. For example cargo build --workspace --exclude tokio-* is interpreted as cargo build --workspace --exclude tokio-test tokio-util tokio-macros, which is not we desire. By quoting around the arg we can solve it but not sure is this ergonomics enough.

Here is my proof-of-concept on opt-out process:

            Packages::OptOut(opt_out) => {
                let opt_out = opt_out
                    .iter()
                    .map(|pat| (glob::Pattern::new(pat).unwrap(), 0))
                    .collect::<Vec<_>>();
                let packages = ws
                    .members()
                    .filter(|pkg| opt_out.iter().any(|(pat, _)| pat.matches(pkg.name().as_str())))
                    .map(Package::package_id)
                    .map(PackageIdSpec::from_package_id)
                    .collect();
                    let not_match = opt_out
                        .iter()
                        .filter(|(_, count)| *count == 0)
                        .map(|(pat, _)| pat.as_str())
                        .collect::<Vec<_>>();
                if !not_match.is_empty() {
                    ws.config().shell().warn(format!(
                        "excluded package(s) {} not found in workspace `{}`",
                        not_match.join(", "),
                        ws.root().display(),
                    ))?;
                }
                packages
            }

@weihanglo
Copy link
Member

@fitzgen I've created a pull request #8752 for this issue. Is that what you expect?

@fitzgen
Copy link
Member Author

fitzgen commented Oct 8, 2020

Based on the documentation changes, yes. I am not familiar enough with the cargo codebase to comment on the implementation.

@bors bors closed this as completed in dd83ae5 Oct 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-cargo-targets Area: selection and definition of targets (lib, bins, examples, tests, benches) A-cli Area: Command-line interface, option parsing, etc. C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted`
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants