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

Functionality to glob all crates in workspace when mixed with other content #4593

Open
Tracked by #11405
nipunn1313 opened this issue Oct 7, 2017 · 6 comments
Open
Tracked by #11405
Labels
A-workspaces Area: workspaces C-bug Category: bug S-needs-design Status: Needs someone to work further on the design for the feature or fix. NOT YET accepted.

Comments

@nipunn1313
Copy link
Contributor

We added glob support originally with #3911

However, if we add to Cargo.toml

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

Then we get an error like this.

error: failed to read `/Users/nipunn/src/client/rust/.cargo/Cargo.toml`

Here, the */ matches over aggressively to directories/files that aren't Crates. It would be nice to have a way to glob all crates. For now, we have to autogenerate the workspace Cargo.toml with a script.

@behnam
Copy link
Contributor

behnam commented Oct 8, 2017

Good point, @nipunn1313! I'm working on workspace.members and workspace.exclude right now (#4089) and this is another issue we need to fix.

Labels: A-workspace, C-bug

@nipunn1313
Copy link
Contributor Author

Some possible ways I brainstormed

  • Limit glob matches directories with Cargo.toml
  • Have a special way to specify members which automatically matches everything (rather than glob)
  • Eliminate members from root manifest and assume all crates with Cargo.toml are eligible
  • Force users to list all crates in the root manifest (redundant to the Cargo.toml)

It does feel like the members array in a virtual manifest at the root of a workspace is somewhat redundant to the directory structure. Adding a crate to a workspace involves writing a new Cargo.toml + adding that directory to a members list. We want to avoid this redundancy.

What is the supported workflow which would require anything more nuanced than a full glob?

@behnam
Copy link
Contributor

behnam commented Oct 8, 2017

Oh, for simple cases, you can just leave out workspace.members and all packages under the workspace root dir will be included automatically.

BUT, that method has a limit that if you want to add a member from a non-children directory, then you also have to add all the children packages to the list. Supporting **/* glob (or just * for the gitignore-like interpretation), we can resolve this issue.

At the moment, though, looks like the behavior is kind of expected: If you're setting workspace.members config, then it includes paths matching packages, and it's (almost) an error otherwise.

I think we need a bit more brainstorming on what workspace.members is expected to mean in general before we can answer this issue specifically.

@alexcrichton alexcrichton added A-workspaces Area: workspaces C-bug Category: bug labels Oct 10, 2017
@stale
Copy link

stale bot commented Sep 20, 2018

As there hasn't been any activity here in over 6 months I've marked this as stale and if no further activity happens for 7 days I will close it.

I'm a bot so this may be in error! If this issue should remain open, could someone (the author, a team member, or any interested party) please comment to that effect?

The team would be especially grateful if such a comment included details such as:

  • Is this still relevant?
  • If so, what is blocking it?
  • Is it known what could be done to help move this forward?

Thank you for contributing!

(The cargo team is currently evaluating the use of Stale bot, and using #6035 as the tracking issue to gather feedback.)

If you're reading this comment from the distant future, fear not if this was closed automatically. If you believe it's still an issue please leave a comment and a team member can reopen this issue. Opening a new issue is also acceptable!

@stale stale bot added the stale label Sep 20, 2018
@nipunn1313
Copy link
Contributor Author

For most cases (including ours), the members of a workspace are all under the workspace root, so simply leaving out workspace.members works fine as @behnam suggested, so the main concern I had is taken care of.

Question is still open as to whether we want better support for **/* in workspace.members, but it does not seem to me high priority to cover the case of globbing items outside of workspace child directories.

@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 17, 2023
@epage epage changed the title Functionality to glob all crates in workspace Functionality to glob all crates in workspace when mixed with other content Sep 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-workspaces Area: workspaces C-bug Category: bug 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

5 participants