Workspaces: Add a way to disable transitive inclusion of path dependencies #3192

phil-opp · 2016-10-12T11:37:41Z

Right now, a workspace root automatically includes all of its path dependencies as workspace members. It would be nice if we could disable this somehow.

This is a follow-up to #3156.

Use case:

Assume that we have a local library crate that we don't want to publish (yet). We include it in multiple executable crates through path = "../our_library" dependencies:

├── our_library
│   ├── Cargo.toml
│   └── src
├── executable_1
│   ├── Cargo.toml
│   └── src
└── executable_2
    ├── Cargo.toml
    └── src

Now we want to make the executables workspace roots. This leads to the following errors:

error: workspace member /home/…/our_library/Cargo.toml is not hierarchically below the workspace root /home/…/executable_{1,2}/Cargo.toml

So each executable requires that we make our_library a workspace member of its own workspace. But a crate can't be part of multiple workspaces.

Right now, there is no way to resolve this problem, since there is no way to disable the automatic inclusion of path dependencies.

The text was updated successfully, but these errors were encountered:

alexcrichton · 2016-10-13T18:27:35Z

Thanks for the report!

Boscop · 2016-10-30T00:45:50Z

Upvote!
I run into this problem frequently.
The easiest solution would be to only make those imported crates that are in a subfolder of the workspace members of that workspace.
Especially if the workspace members are explicitly specified, there should be no confusion!
Anyone would intuitively assume that a workspace can only include crates in the subfolders.

whitequark · 2017-01-12T05:27:12Z

This makes using cargo workspaces a nightmare when I need to develop a crate outside of the workspace. Currently, for any trivial change, I have to:

do a commit in the external crate
get its git hash
update Cargo.toml of the crate inside the workspace
rebuild
not forget to remove the dummy commits before pushing

closes rust-lang#3192

SergioBenitez · 2017-01-15T04:26:08Z

Chiming in as I've run into this as well.

I believe the correct thing to do here is to assume that any path dependency that is not hierarchically below the workspace is not part of that workspace. In other words, do not implicitly include path dependencies that are not hierarchically below the workspace as members of that workspace. If a user explicitly adds such a member to the workspace, then this error can be given.

Edit: Ah, looks like #3443 implements exactly this.

@Boscop

Path deps outside workspace are not members closes #3192 This implements @Boscop suggestion: path dependencies pointing outside the workspace are never members. Not sure that it handled #3192 fully: you can't exclude a path dependency within workspace. Not sure this is super useful though...

alexcrichton · 2017-01-18T23:19:35Z

Ah I didn't think this was entirely closed out with #3443, so I'm going to reopen. (manual annotations are still not implemented)

Find workspace via `workspace_root` link in containing member This PR proposes to change the logic for determining workspace members. Here are the algorithms we used previously for this: # [RFC](https://github.com/rust-lang/rfcs/blob/master/text/1525-cargo-workspace.md) flavor If `[members]` key present, it compliantly defines the set of members. Otherwise, members are all (transitive) path dependencies. The problem with this approach is that it violates convention over configuration principle: almost always you want a path dependency to be a member, even if there are some explicit members. Listing **all** path deps is just to much work. # Original implementation So, the actual algorithm **unconditionally** included path dependencies as memebers. This is also problematic, because certain workspace configurations become impossible. In particular, you can't have a path dependency which is not a member of the workspace. This issue was reported in #3192. # Current implementation Current implementation (was merged couple of days ago) includes path dependency into the workspace only if is inside the workspace directory. This solves the problem in #3192 perfectly. However, some configuration are still impossible: you can't have a non-member path dependency inside a workspace directory. But the thinking is that if you don't want this path-dep to be a member, just don't put it inside the workspace directory. There is another problem with current imlementation. Suppose you have an explicit member which lives alongside the workspace. Suppose this member has a path-dep which lives inside the member's folder. Under current implementation, this path-dep won't be a member of the workspace. It seems logical that it should be though (but we haven't received any actual bug reports yet)! # Implementation in this PR So, with this PR, the logic is as follows: members are explicit members + all path dependencies which reside under any of the explicit members.

alexcrichton · 2017-03-07T15:53:22Z

Ok, so here's a thought of how to fix this. Add a new key to Cargo.toml:

[workspace]
exclude = [
    "path1",
    "path/to/dir2",
    # ...
]

The idea is that as a workspace root you can dictate paths which are not included in the workspace (hierarchically). That means that if you traverse upwards through one of those dirs into the [workspace] you're not considered part of that workspace.

Thoughts?

Boscop · 2017-03-07T16:43:47Z

I like that, if all non-excluded crates in subfolders are automatically members of the workspace.

This commit adds a new key to the `Cargo.toml` manifest, `workspace.exclude`. This new key is a list of strings which is an array of directories that are excluded from the workspace explicitly. This is intended for use cases such as vendoring where path dependencies into a vendored directory don't want to pull in the workspace dependencies. There's a number of use cases mentioned on rust-lang#3192 which I believe should all be covered with this strategy. At a bare minimum it should suffice to `exclude` every directory and then just explicitly whitelist crates through `members` through inclusion, and that should give precise control over the structure of a workspace. Closes rust-lang#3192

alexcrichton · 2017-03-16T21:53:10Z

I've opened #3837 as a sample implementation of my idea to close this issue. Feedback would be much appreciated!

Add a workspace.exclude key This commit adds a new key to the `Cargo.toml` manifest, `workspace.exclude`. This new key is a list of strings which is an array of directories that are excluded from the workspace explicitly. This is intended for use cases such as vendoring where path dependencies into a vendored directory don't want to pull in the workspace dependencies. There's a number of use cases mentioned on #3192 which I believe should all be covered with this strategy. At a bare minimum it should suffice to `exclude` every directory and then just explicitly whitelist crates through `members` through inclusion, and that should give precise control over the structure of a workspace. Closes #3192

phil-opp mentioned this issue Oct 12, 2016

All crates specified with a relative path regarded as workspace members #3156

Closed

phil-opp mentioned this issue Dec 20, 2016

Remove cargo workspace phil-opp/blog_os#264

Closed

matklad mentioned this issue Dec 22, 2016

Path deps outside workspace are not members #3443

Merged

alexcrichton mentioned this issue Jan 12, 2017

cargo build on workspaces does not interact well with vendored dependencies #3539

Closed

matklad added a commit to matklad/cargo that referenced this issue Jan 14, 2017

Path deps outside workspace are not members

07c667f

closes rust-lang#3192

bors closed this as completed in #3443 Jan 18, 2017

alexcrichton reopened this Jan 18, 2017

matklad mentioned this issue Jan 19, 2017

Find workspace via workspace_root link in containing member #3562

Merged

alexcrichton mentioned this issue Feb 2, 2017

diesel migration generate create_users does not work anymore diesel-rs/diesel#602

Closed

jonysy mentioned this issue Feb 7, 2017

Consider moving the mirage and illusions repositories to the mīrō repository jonysy/miro#23

Closed

alexcrichton mentioned this issue Feb 10, 2017

cargo update behaves different with and without workspaces #3675

Closed

alexcrichton mentioned this issue Feb 17, 2017

workspaces should treat source-replacement'd crates identically to crates.io ones #3728

Closed

This was referenced Mar 7, 2017

Don't put Cargo into the rustc workspace rust-lang/rust#40297

Merged

Give Cargo it's own workspace #3801

Closed

alexcrichton mentioned this issue Mar 7, 2017

Workspaces: All crates below the root folder must be members of the workspace #3159

Closed

0xcpu mentioned this issue Mar 7, 2017

Add script for travis CI test, Fixes #69 rust-fuzz/cargo-fuzz#79

Closed

0xcpu mentioned this issue Mar 8, 2017

workspace member Cargo.toml is not hierarchically below the workspace root fuzz/Cargo.toml rust-fuzz/cargo-fuzz#80

Closed

alexcrichton mentioned this issue Mar 16, 2017

Add a workspace.exclude key #3837

Merged

bors closed this as completed in #3837 Mar 23, 2017

metajack mentioned this issue Jan 8, 2020

[fuzzing] lock Libra fuzzer deps diem/diem#2172

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Workspaces: Add a way to disable transitive inclusion of path dependencies #3192

Workspaces: Add a way to disable transitive inclusion of path dependencies #3192

phil-opp commented Oct 12, 2016 •

edited

Loading

alexcrichton commented Oct 13, 2016

Boscop commented Oct 30, 2016 •

edited

Loading

whitequark commented Jan 12, 2017 •

edited

Loading

SergioBenitez commented Jan 15, 2017 •

edited

Loading

alexcrichton commented Jan 18, 2017

alexcrichton commented Mar 7, 2017

Boscop commented Mar 7, 2017

alexcrichton commented Mar 16, 2017

Workspaces: Add a way to disable transitive inclusion of path dependencies #3192

Workspaces: Add a way to disable transitive inclusion of path dependencies #3192

Comments

phil-opp commented Oct 12, 2016 • edited Loading

alexcrichton commented Oct 13, 2016

Boscop commented Oct 30, 2016 • edited Loading

whitequark commented Jan 12, 2017 • edited Loading

SergioBenitez commented Jan 15, 2017 • edited Loading

alexcrichton commented Jan 18, 2017

alexcrichton commented Mar 7, 2017

Boscop commented Mar 7, 2017

alexcrichton commented Mar 16, 2017

phil-opp commented Oct 12, 2016 •

edited

Loading

Boscop commented Oct 30, 2016 •

edited

Loading

whitequark commented Jan 12, 2017 •

edited

Loading

SergioBenitez commented Jan 15, 2017 •

edited

Loading