All crates specified with a relative path regarded as workspace members

[4e554c4c]

Overview

According to the workspace RFC:

a crate which specifies [workspace] without a members key will transitively crawl path dependencies to fill in this key. This way all path dependencies (and recursively their own path dependencies) will inherently become the default value for workspace.members.

This makes testing crates that one is working on with a workspace project very difficult. A saner default would be to only include all paths below the project root in the workspace. One can override this with the .cargo/config file, however this is much less convinient.

The RFC also emphasises the fact that this default is used only when a members key is not defined. However cargo still uses all assumes all path dependancies as members regardless if the members key is defined or not.

Meta

This issue was inspired by phil-opp/blog_os#234, That project is compiled using xargo with a custom target. The issue occured while I was working on a subcrate, and have not created an SSCCE, although I can if it the bug proves to be elusive.

[Aatch]

Note that the documentation is explicit about the behaviour:

This can be done through the members key, and if it is omitted then members are implicitly included through all path dependencies. Note that members of the workspaces listed explicitly will also have their path dependencies included in the workspace.

(Emphasis mine)

[4e554c4c]

@Aatch OK thank you, so this is not a bug so I will close the issue. I still think that there should be saner defaults, so would you suggest I create a new issue or possibly comment on the RFC? Also what documentation did you cite?

[Aatch]

Actually, I was just pointing out that the documentation states the current behaviour. I actually disagree with it. Probably should keep this issue open.

[4e554c4c]

Oh alright. That's I'll edit my post though to show that it is not a bug

[phil-opp]

I think that there are two problems here:

1. The default for workspace.members is suboptimal. Instead of adding all path dependencies, we could only add path dependencies below the workspace root.
2. The workspace.members key doesn't work correctly: Even with members = [], it requires that all path dependencies are below the workspace root. For example:

[package]
name = "test"

[dependencies.foo]
path = "../foo"

[workspace]
members = []

This Cargo.toml yields the following error:

error: workspace member /home/…/foo/Cargo.toml is not hierarchically below the workspace root /home/…/test/Cargo.toml

But foo isn't even part of the workspace.

[alexcrichton]

Note that the implementation was tweaked from the exact wording of the RFC, all transitive path dependencies are included in a workspace unconditionally. (e.g. that's not a bug)

In that sense it looks like this is behaving as expected in that case? (albeit unfortunately)

[phil-opp]

So all path dependencies are implicitly added to members?

looks like this is behaving as expected in that case?

For me it was unexpected, since it's not documented anywhere. The cargo docs only say:

This can be done through the members key, and if it is omitted then members are implicitly included through all path dependencies.

(emphasis mine)

and

Note that members of the workspaces listed explicitly will also have their path dependencies included in the workspace.

But none of these rules explains the error in the above case (#3156 (comment)). It has an explicit members key and has no explicitly listed members.

[alexcrichton]

So all path dependencies are implicitly added to members?

Indeed!

For me it was unexpected, since it's not documented anywhere.

Ah oops! Sounds like a PR to the docs is in order.

But none of these rules explains the error in the above case (#3156 (comment)). It has an explicit members key and has no explicitly listed members.

Cargo visiting all path deps transitively would explain this though, right?

[Boscop]

I would also prefer that it's possible to import crates by path without making them members.

[phil-opp]

Thanks for the clarification, @alexcrichton! This rule indeed explains the error message in my example.

However, I still don't like the rule. It makes it harder to work with local dependencies (e.g. a library that should be published to crates.io eventually and thus shouldn't be part of any workspace). It also means that temporary overriding a dependency with a local copy (e.g. to test a new feature) doesn't “just work” anymore (.cargo/config overrides still work but many people don't know about it).

[alexcrichton]

@Boscop I think that's covered by #3159

@phil-opp I'm not sure I quite follow, why wouldn't you want local dependencies part of a workspace? Does dependency overriding not work? (it should)

[phil-opp]

why wouldn't you want local dependencies part of a workspace

For example if you want to use a local library in multiple, distinct crates.

Does dependency overriding not work?

Sorry for the bad phrasing. Dependency overriding works fine. I just meant naive overwriting directly in the dependencies section. E.g.:

[dependencies]
#bitflags = "0.7.0"
bitflags = { path = '../bitflags' }

I always did this before I learned about the “right” way.

[alexcrichton]

Ah ok, thanks for clarifying! I think both of those cases are covered by #3159 as well, right? (the ability to a workspace to say "don't include this dir")

[phil-opp]

I think they are separate issues.

• Workspaces: All crates below the root folder must be members of the workspace #3159: A crate erroneously thinks that it's part of a workspace just because there is a [workspace] section in a (completely unrelated) crate upwards the file system. (I've also left a comment there).
• This issue: The root crate automatically adds all of its path dependencies to workspace.members and there is no way to disable this.

[alexcrichton]

Ok, adding a way to disable transitive inclusion seems plausible to me! Want to open a separate, focused issue on that?

[phil-opp]

Ok, adding a way to disable transitive inclusion seems plausible to me!

Great! I opened #3192 for it.