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 package/target selection #8752

Merged
merged 22 commits into from
Oct 20, 2020

Conversation

weihanglo
Copy link
Member

@weihanglo weihanglo commented Oct 4, 2020

Resolves #8582

What

Commands that supports glob patterns on package/target selection:

  • build
  • test
  • bench
  • doc
  • check
  • fix (run check underneath the hood)
  • tree (no target selection functionalit as same as before)

Commands that supports glob patterns on target selection only:

  • rustdoc (can only build one package via -p/--package option)
  • rustc (can only build one package via -p/--package option)

Command that does not support any glob patterns for package/target selection:

  • run (you can only run at most one executable)

How

By using existing glob crate to

  1. check whether a target filter is a glob pattern, and then
  2. compare if the target matches the filter.

Also, I loosed some shell-style restriction in cargo-test-support (by adding simple quote arguments support).

Breaking Changes

Suppose no.

  • No public API introduced.
  • All valid glob pattern chars (*, ?, [, ]) characters are restricted by Cargo due to not containing inXID_Continue character set in UAX#31 and are also restricted

Performance Regression

Definitely gets some.

However, assumed no one would pass lots of package/target selection with glob patterns, so the extra overhead for ordinary use cases are glob pattern existence checks on each target filter. The current implementation is somewhat naive, so if you have any better algorithm, please help me improve it.

Documentation

I have done my best effort to write the documentation, but as a non-native English speaker I need helps to polish these sentences. Besides, I am not quite sure should we mention glob pattern in CLI help text, which is a bit lengthy at the moment....

@rust-highfive
Copy link

r? @ehuss

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

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 4, 2020
@weihanglo weihanglo changed the title Feat/glob pattern Support glob patterns for pacakge/target selection Oct 4, 2020
@weihanglo weihanglo changed the title Support glob patterns for pacakge/target selection Support glob patterns for package/target selection Oct 4, 2020
Copy link
Contributor

@ehuss ehuss left a comment

Choose a reason for hiding this comment

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

Wow, this looks great! Thanks for being so thorough with tests and docs and everything!

src/bin/cargo/commands/run.rs Outdated Show resolved Hide resolved
tests/testsuite/build.rs Outdated Show resolved Hide resolved
tests/testsuite/build.rs Outdated Show resolved Hide resolved
tests/testsuite/build.rs Outdated Show resolved Hide resolved
tests/testsuite/build.rs Show resolved Hide resolved
src/cargo/ops/cargo_compile.rs Outdated Show resolved Hide resolved
src/cargo/ops/cargo_compile.rs Outdated Show resolved Hide resolved
tests/testsuite/build.rs Outdated Show resolved Hide resolved
src/cargo/ops/cargo_compile.rs Outdated Show resolved Hide resolved
src/cargo/ops/cargo_compile.rs Outdated Show resolved Hide resolved
@weihanglo
Copy link
Member Author

Updated. Thanks for your comprehensive review, @ehuss 😄

@weihanglo weihanglo requested a review from ehuss October 10, 2020 02:39
@ehuss ehuss added the T-cargo Team: Cargo label Oct 14, 2020
@ehuss
Copy link
Contributor

ehuss commented Oct 14, 2020

@rfcbot fcp merge

@rust-lang/cargo This is an insta-stable change to the way the package and target selection flags work on the command-line, to support unix-style glob matching. I personally think it is fine to skip unstable, since I think it is unlikely that there will be much feedback on a feature like this, and I think it is unlikely the design would change. However, feel free to disagree!

Specifically, the -p --package --exclude --bin --example --test --bench flags support * ? and [] glob matching.

One of the concerns with this design is that glob characters can interact poorly with shell expansion. However, by using quotes this can be easily avoided, and I think the documentation does a good job of pointing this out.

@rfcbot
Copy link
Collaborator

rfcbot commented Oct 14, 2020

Team member @ehuss has proposed to merge this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@rfcbot rfcbot added proposed-final-comment-period An FCP proposal has started, but not yet signed off. disposition-merge FCP with intent to merge labels Oct 14, 2020
@bors
Copy link
Contributor

bors commented Oct 17, 2020

☔ The latest upstream changes (presumably #8793) made this pull request unmergeable. Please resolve the merge conflicts.

Note that reviewers usually do not review pull requests until merge conflicts are resolved! Once you resolve the conflicts, you should change the labels applied by bors to indicate that your PR is ready for review. Post this as a comment to change the labels:

@rustbot modify labels: +S-waiting-on-review -S-waiting-on-author

@weihanglo
Copy link
Member Author

Merge conflicts resolved. Just some markdown link updates.

@rfcbot rfcbot added the final-comment-period FCP — a period for last comments before action is taken label Oct 18, 2020
@rfcbot
Copy link
Collaborator

rfcbot commented Oct 18, 2020

🔔 This is now entering its final comment period, as per the review above. 🔔

@rfcbot rfcbot removed the proposed-final-comment-period An FCP proposal has started, but not yet signed off. label Oct 18, 2020
@weihanglo
Copy link
Member Author

@ehuss

Off topic, but I am quite curious. If another merge conflicts occurs, would the FCP needs a restart?

@ehuss
Copy link
Contributor

ehuss commented Oct 20, 2020

Nah, the fcp is just whether or not to approve, merge conflicts don't reset the timer.

I think this is good to go, so I'll go ahead and approve.
@bors r+

@bors
Copy link
Contributor

bors commented Oct 20, 2020

📌 Commit d613cd6 has been approved by ehuss

@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 Oct 20, 2020
@bors
Copy link
Contributor

bors commented Oct 20, 2020

⌛ Testing commit d613cd6 with merge dd83ae5...

@bors
Copy link
Contributor

bors commented Oct 20, 2020

☀️ Test successful - checks-actions
Approved by: ehuss
Pushing dd83ae5 to master...

@bors bors merged commit dd83ae5 into rust-lang:master Oct 20, 2020
@weihanglo weihanglo deleted the feat/glob-pattern branch October 21, 2020 00:43
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Oct 22, 2020
Update cargo

3 commits in 79b397d72c557eb6444a2ba0dc00a211a226a35a..dd83ae55c871d94f060524656abab62ec40b4c40
2020-10-15 14:41:21 +0000 to 2020-10-20 19:31:26 +0000
- Support glob patterns for package/target selection (rust-lang/cargo#8752)
- Update env_logger requirement from 0.7.0 to 0.8.1 (rust-lang/cargo#8795)
- Fix man page links inside `option` blocks. (rust-lang/cargo#8793)
JohnTitor added a commit to JohnTitor/rust that referenced this pull request Oct 23, 2020
Update cargo

3 commits in 79b397d72c557eb6444a2ba0dc00a211a226a35a..dd83ae55c871d94f060524656abab62ec40b4c40
2020-10-15 14:41:21 +0000 to 2020-10-20 19:31:26 +0000
- Support glob patterns for package/target selection (rust-lang/cargo#8752)
- Update env_logger requirement from 0.7.0 to 0.8.1 (rust-lang/cargo#8795)
- Fix man page links inside `option` blocks. (rust-lang/cargo#8793)
@rfcbot rfcbot added finished-final-comment-period FCP complete to-announce and removed final-comment-period FCP — a period for last comments before action is taken labels Oct 28, 2020
netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this pull request Feb 14, 2021
Pkgsrc changes:
 * Adjust patches, convert tabs to spaces so that tests pass.
 * Remove patches which are no longer needed (upstream changed)
 * Minor adjustments for SunOS, e.g. disable stack probes.
 * Adjust cargo checksum patching accordingly.
 * Remove commented-out use of PATCHELF on NetBSD, which doesn't work anyway...

Upstream changes:

Version 1.49.0 (2020-12-31)
============================

Language
-----------------------

- [Unions can now implement `Drop`, and you can now have a field in a union
  with `ManuallyDrop<T>`.][77547]
- [You can now cast uninhabited enums to integers.][76199]
- [You can now bind by reference and by move in patterns.][76119] This
  allows you to selectively borrow individual components of a type. E.g.
  ```rust
  #[derive(Debug)]
  struct Person {
      name: String,
      age: u8,
  }

  let person = Person {
      name: String::from("Alice"),
      age: 20,
  };

  // `name` is moved out of person, but `age` is referenced.
  let Person { name, ref age } = person;
  println!("{} {}", name, age);
  ```

Compiler
-----------------------

- [Added tier 1\* support for `aarch64-unknown-linux-gnu`.][78228]
- [Added tier 2 support for `aarch64-apple-darwin`.][75991]
- [Added tier 2 support for `aarch64-pc-windows-msvc`.][75914]
- [Added tier 3 support for `mipsel-unknown-none`.][78676]
- [Raised the minimum supported LLVM version to LLVM 9.][78848]
- [Output from threads spawned in tests is now captured.][78227]
- [Change os and vendor values to "none" and "unknown" for some targets][78951]

\* Refer to Rust's [platform support page][forge-platform-support] for more
information on Rust's tiered platform support.

Libraries
-----------------------

- [`RangeInclusive` now checks for exhaustion when calling `contains`
  and indexing.][78109]
- [`ToString::to_string` now no longer shrinks the internal buffer
  in the default implementation.][77997]
- [`ops::{Index, IndexMut}` are now implemented for fixed sized
  arrays of any length.][74989]

Stabilized APIs
---------------

- [`slice::select_nth_unstable`]
- [`slice::select_nth_unstable_by`]
- [`slice::select_nth_unstable_by_key`]

The following previously stable methods are now `const`.

- [`Poll::is_ready`]
- [`Poll::is_pending`]

Cargo
-----------------------
- [Building a crate with `cargo-package` should now be independently
  reproducible.][cargo/8864]
- [`cargo-tree` now marks proc-macro crates.][cargo/8765]
- [Added `CARGO_PRIMARY_PACKAGE` build-time environment
  variable.]  [cargo/8758] This variable will be set if the crate
  being built is one the user selected to build, either with `-p`
  or through defaults.
- [You can now use glob patterns when specifying packages &
  targets.][cargo/8752]


Compatibility Notes
-------------------
- [Demoted `i686-unknown-freebsd` from host tier 2 to target tier
  2 support.][78746]
- [Macros that end with a semi-colon are now treated as statements
  even if they expand to nothing.][78376]
- [Rustc will now check for the validity of some built-in attributes
  on enum variants.][77015] Previously such invalid or unused
  attributes could be ignored.
- Leading whitespace is stripped more uniformly in documentation
  comments, which may change behavior. You read [this post about
  the changes][rustdoc-ws-post] for more details.
- [Trait bounds are no longer inferred for associated types.][79904]

Internal Only
-------------
These changes provide no direct user facing benefits, but represent
significant improvements to the internals and overall performance
of rustc and related tools.

- [rustc's internal crates are now compiled using the `initial-exec` Thread
  Local Storage model.][78201]
- [Calculate visibilities once in resolve.][78077]
- [Added `system` to the `llvm-libunwind` bootstrap config option.][77703]
- [Added `--color` for configuring terminal color support to bootstrap.][79004]


[75991]: rust-lang/rust#75991
[78951]: rust-lang/rust#78951
[78848]: rust-lang/rust#78848
[78746]: rust-lang/rust#78746
[78376]: rust-lang/rust#78376
[78228]: rust-lang/rust#78228
[78227]: rust-lang/rust#78227
[78201]: rust-lang/rust#78201
[78109]: rust-lang/rust#78109
[78077]: rust-lang/rust#78077
[77997]: rust-lang/rust#77997
[77703]: rust-lang/rust#77703
[77547]: rust-lang/rust#77547
[77015]: rust-lang/rust#77015
[76199]: rust-lang/rust#76199
[76119]: rust-lang/rust#76119
[75914]: rust-lang/rust#75914
[74989]: rust-lang/rust#74989
[79004]: rust-lang/rust#79004
[78676]: rust-lang/rust#78676
[79904]: rust-lang/rust#79904
[cargo/8864]: rust-lang/cargo#8864
[cargo/8765]: rust-lang/cargo#8765
[cargo/8758]: rust-lang/cargo#8758
[cargo/8752]: rust-lang/cargo#8752
[`slice::select_nth_unstable`]: https://doc.rust-lang.org/nightly/std/primitive.slice.html#method.select_nth_unstable
[`slice::select_nth_unstable_by`]: https://doc.rust-lang.org/nightly/std/primitive.slice.html#method.select_nth_unstable_by
[`slice::select_nth_unstable_by_key`]: https://doc.rust-lang.org/nightly/std/primitive.slice.html#method.select_nth_unstable_by_key
[`hint::spin_loop`]: https://doc.rust-lang.org/stable/std/hint/fn.spin_loop.html
[`Poll::is_ready`]: https://doc.rust-lang.org/stable/std/task/enum.Poll.html#method.is_ready
[`Poll::is_pending`]: https://doc.rust-lang.org/stable/std/task/enum.Poll.html#method.is_pending
[rustdoc-ws-post]: https://blog.guillaume-gomez.fr/articles/2020-11-11+New+doc+comment+handling+in+rustdoc
@ehuss ehuss added this to the 1.49.0 milestone Feb 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
disposition-merge FCP with intent to merge finished-final-comment-period FCP complete S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-cargo Team: Cargo to-announce
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support glob patterns for crate arguments to --package and --exclude flags
5 participants