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

cargo should support glob syntax on workspace excludes #6009

Open
Tracked by #11405
bjgill opened this issue Sep 11, 2018 · 7 comments
Open
Tracked by #11405

cargo should support glob syntax on workspace excludes #6009

bjgill opened this issue Sep 11, 2018 · 7 comments
Labels
A-workspaces Area: workspaces C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` S-needs-design Status: Needs someone to work further on the design for the feature or fix. NOT YET accepted.

Comments

@bjgill
Copy link

bjgill commented Sep 11, 2018

Currently, we can include crates in a workspace with glob syntax (see #3911). However, we can't exclude crates in the same way.

See https://github.com/rust-lang/cargo/blob/master/src/cargo/core/workspace.rs#L826, where it looks as if we're just doing manifest_path.starts_with(self.root_dir.join(exclude)).

We've hit this because we have a set of autogenerated crates with a common suffix that we cannot neatly exclude without glob support.

@alexcrichton alexcrichton added A-workspaces Area: workspaces C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` labels Sep 11, 2018
@nrc
Copy link
Member

nrc commented Aug 8, 2019

I believe this has been implemented now.

@nrc nrc closed this as completed Aug 8, 2019
@ehuss
Copy link
Contributor

ehuss commented Aug 8, 2019

I believe this is still an issue. The exclude field is still using starts_with.

@ehuss ehuss reopened this Aug 8, 2019
@ehuss
Copy link
Contributor

ehuss commented Aug 8, 2019

cc #6745, which is a similar issue.

The exclude key was added in #3837. There doesn't appear to be any discussion at the time about the format to use. It was enhanced in #4297 to support overlapping.

I think it would be good to consider what format and behavior members and exclude should have. Would it make sense to have something more powerful like gitignore? I think it may be confusing if one uses globs and the other uses gitignore. Would we need a transition period since it may result in different behavior (similar to #4268)? Also consider that package include/exclude are mutually exclusive, but workspace members/exclude is not.

See also #4593 for improving members matching.

@behnam
Copy link
Contributor

behnam commented Aug 9, 2019

I was supposed to work on this while back, but didn't find the time to formalize the problem and proposed solution. Let me try to do that:

Now that the file include/exclude rules are on gitignore-style patterns, I think we all agree that having a similar feature for member packages makes the most sense.

The problem, however, is the fact that when working with the files under a package, we already have a root defined (package root is the directory where the manifest file is seating). That allows having the anchor gitignore-style rules need when writing them.

For members, however, we don't have such a well-defined root anchor, since the workspace package (like A), can be in a directory next to a member package (like B). Like this:

<project-root>
 |-- A
 |-- B
...

I was thinking to add another general rule to gitignore rule-set, which would allow getting outside of the current root (package root = dir of manifest) and point to parent and sibling directories.

This additional rule would be: "If the pattern starts with ../, then it's a relative glob, ...". (We need to figure out the exact details of the rest of the rule to keep it consistent with the rest of gitignore/glob rules.

Having that, then we can have A include (or exclude) B (or a package under it) in the example above.

Hope you find this helpful. Would love to hear what you think!

@weihanglo
Copy link
Member

weihanglo commented Nov 22, 2022

[This is yet resolved]

I am going to close this. Further discussion could happen in #11405. If you think this is wrong please leave a comment. We could reopen it.

Edit: I think it is better to leave it open.

@weihanglo weihanglo closed this as not planned Won't fix, can't repro, duplicate, stale Nov 22, 2022
@weihanglo weihanglo reopened this Nov 22, 2022
@epage epage added the S-needs-design Status: Needs someone to work further on the design for the feature or fix. NOT YET accepted. label Oct 24, 2023
zhassan-aws added a commit to model-checking/kani that referenced this issue Nov 6, 2023
Update to the latest Rust toolchain (2023-11-06).

The relevant changes are:
- rust-lang/rust#117507: this required changing
the import of `Span` from `rustc_span::source_map::Span` to
`rustc_span::Span`.
- rust-lang/rust#114208: this changed the data
field for the `OffsetOf` variant of `NullOp` from `List<FieldIdx>` to
`List<(VariantIdx, FieldIdx)>`, which required updating the relevant
code in `rvalue.rs`.
- rust-lang/rust#115626: the unchecked shift
operators have been separated from the `unchecked_math` feature, so this
required changing the feature annotation in
`tests/ui/should-panic-attribute/unexpected-failures/test.rs`
- Some rustc change (not sure which one) result in a line in
`tests/coverage/unreachable/variant/main.rs` getting optimized out. To
maintain what this test is testing, I changed the `match` to make it a
bit less-prone to optimization.
- A change in `cargo` (rust-lang/cargo#12779)
resulted in an update to Kani's workspace `Cargo.toml` when `cargo add`
is executed inside `tests/script-based-pre/build-cache-bin`. This is
apparently intended behavior, so I had to make the `exclude` in the
`Cargo.toml` more specific to make sure this doesn't happen (I tried
using a glob, but that didn't work, apparently because of
rust-lang/cargo#6009.

Resolves #2848 

By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 and MIT licenses.
@Emilgardis
Copy link
Contributor

Emilgardis commented Feb 3, 2024

Linking in #12779 for visibility since this is more of a problem for me now with cargo new adding packages to members if the path is not a direct glob excluded path.
i.e for

[workspace]
members = []
exclude = ["directory/**"]

cargo new in directory/ works as expected since #13261, but cargo new in directory/subdir/ does add the new package to members

edit: actually, I don't think it works at all :/

@workingjubilee
Copy link
Member

was just bitten by this... my primary use-case for glob exclusions is in a workspace that has no root package, but has many examples that each have to be their own package because the tooling works on the package level.

workingjubilee added a commit to pgcentralfoundation/pgrx that referenced this issue Aug 9, 2024
Test that pgrx will always build after we publish it. It should never
again take an excessive amount of time to release pgrx, because we will
always be confident we are ready to release. This does not finish making
the release "turnkey", but it does take care of every obstacle to such.

Note that there is a technicality: we are "only" testing that our
_package_ builds. We are not actually pulling a published-to-cargo
release and testing it. We are not running the `cargo publish --dry-run`
command, either, because there is no `cargo publish --workspace`.
Instead, we are packaging the workspace and rebuilding *that package*.
This unfortunately demands that we factor out all the packages in the
workspace that are not going to be published.

Other details of the refactoring are informed by these oddities:
- `package.publish = false` doesn't play well with the extensions to
`cargo package --workspace`:
rust-lang/cargo#14356
- `workspace.exclude = []` does not properly support globs:
rust-lang/cargo#6009
- We *need* to reuse the same `CARGO_TARGET_DIR` for building and
testing our many examples or we run out of storage space on the runners.
zjp-CN added a commit to os-checker/docs that referenced this issue Nov 3, 2024
*Because workspace.exclude doesn't support glob syntax.*

error: current package believes it's in a workspace when it's not:
current:   /rust/my/docs/repos/os-checker/plugin-cargo/Cargo.toml
workspace: /rust/my/docs/Cargo.toml

this may be fixable by ensuring that this crate is depended on by the workspace root: /rust/my/docs/Cargo.toml
Alternatively, to keep it out of the workspace, add the package to the `workspace.exclude` array, or add an empty `[workspace]` table to the package's manif
est.

ref: rust-lang/cargo#6009
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-workspaces Area: workspaces C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` S-needs-design Status: Needs someone to work further on the design for the feature or fix. NOT YET accepted.
Projects
None yet
Development

No branches or pull requests

9 participants