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

workplace.default-members and workspace.exclude don't always work well together. #8460

Closed
thomcc opened this issue Jul 7, 2020 · 0 comments · Fixed by #8485
Closed

workplace.default-members and workspace.exclude don't always work well together. #8460

thomcc opened this issue Jul 7, 2020 · 0 comments · Fixed by #8485
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`

Comments

@thomcc
Copy link
Member

thomcc commented Jul 7, 2020

One of the listed use cases for workspace.exclude in the documentation is for when you are "using a glob pattern and you want to remove a directory".

This is very handy, and lets workspace.members stay pretty clean if your project structure is mostly consistent. Unfortunately it isn't similarly supported by workspace.default-members, which must be a strict subset of workspace.members.

To see what I mean, here's an example I hit recently (this is based on the project layout of https://github.com/mozilla/application-services -- but simplified).

Essentially, I think something like the following code should work:

[workspace]
members = [
    "components/*",
    "components/support/*",
    "tools/i-compile-very-slowly-and-am-rarely-needed",
]

# Exclude a couple things that the globs in `workspace.members` catch
# overeagerly
exclude = [
    # This is not a crate, and just contains support and utility code.
    "components/support",
    # Android support code (this is in Kotlin, not Rust)
    "components/support/android",
]

# Everything except `tools/i-compile-very-slowly-and-am-rarely-needed`
default-members = [
    "components/*",
    "components/support/*",
]

However, if you try this, you get errors like "<project>/components/support/android is listed in workspace’s default-members but is not a member", because "When specified, default-members must expand to a subset of members."

Possible solution

I think that rule makes sense, and can help catch bugs, so I'm not saying it should be removed or weakened. I think my issue can be fixed by just tweaking the way that rule is implemented rather than removing it:

Instead of checking that workspace.default-members is a subset of (workspace.members - workspace.exclude) (which is what cargo does now), cargo should check that default-members is a subset of members.

Conceptually, I think this is really just a change of when the check happens -- e.g. IMO it should happen prior to applying exclude to members, as opposed to afterwards.

(Of course, I think you will also need to apply workspace.exclude to workspace.default-members to produce the computed set of default members, but that's easy and if the above holds, it shouldn't remove anything from it except what was removed from workspace.members)

Anyway, thanks for your time.


P.S. Sorry that this doesn't fit an issue template. I wrote it out in a text editor first to make sure it made sense, but now it's hard to adapt and I don't really have time to rewrite it. Let me know if you need any clarification.

Also, since it's kinda halfway between bug and feature request, the template didn't really made tons of sense, I think.

I'm also sorry that the issue title is not very descriptive.

@thomcc thomcc added the C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` label Jul 7, 2020
@ehuss ehuss added the A-workspaces Area: workspaces label Jul 7, 2020
@bors bors closed this as completed in 632b1bb Jul 23, 2020
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`
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants