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

Add globbing to workspace.exclude list #11374

Closed
wants to merge 3 commits into from
Closed

Conversation

Zagitta
Copy link

@Zagitta Zagitta commented Nov 14, 2022

Motivation

Hello, I've added the ability to use globs in workspace.exclude to facilitate easier usage of git submodules as dependencies.

A common way of using git submodules is to have them all under a subfolder like repos or third-party and it would be nice to be able to just add new submodules without having to change the workspace.exclude list every time.

This small change allows the usage of globs in workspace.exclude just like it's allowed in workspace.members eg:

[workspace]
memebers = ["crates/*"]
exclude = ["repos/*"]

Implementation

The current implementation is not ideal as it just silently eats any globbing errors. However, in the interest of making the change as minimal as possible for the initial round of reviews, I deemed that acceptable.
If desired I can refactor the is_excluded function to return a CargoResult<bool> similar to what expand_member_path does for its glob handling.

Testing

I've added a test case to the testsuite called excluded_glob which should cover most cases but I'm open to hear if I missed any edge cases.

@rustbot
Copy link
Collaborator

rustbot commented Nov 14, 2022

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @weihanglo (or someone else) soon.

Please see the contribution instructions for more information.

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 14, 2022
@ehuss
Copy link
Contributor

ehuss commented Nov 15, 2022

There is a broader discussion going on about the format that the workspace members/exclude fields should be using, and whether or not they should possibly be using gitignore-style patterns. I think it would be good to have a plan on how it should work before making changes here. There's more context in #4593, #6009 (comment), #6745, and #11362.

@weihanglo
Copy link
Member

Thanks for the pull request!

I am going to close this. Further discussion could happen in #11405. If we reach a consensus on supporting glob in workspace.exclud this could be revived :)

@weihanglo weihanglo closed this Nov 22, 2022
@weihanglo weihanglo removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants