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

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

Closed
phil-opp opened this issue Oct 4, 2016 · 10 comments
Closed

Comments

@phil-opp
Copy link
Contributor

phil-opp commented Oct 4, 2016

The workspace documentation says the following:

The package.workspace manifest key (described above) is used in member crates to point at a workspace's root crate. If this key is omitted then it is inferred to be the first crate whose manifest contains [workspace] upwards in the filesystem.

(emphasis mine)

This implies that all crates below the workspace root must be part of the workspace, even if there are no dependencies on them.

For example, imagine the following workspace layout.

…
├── Cargo.toml
├── src
├── sub1
│   ├── Cargo.toml
│   └── src
└── sub2
    ├── Cargo.toml
    └── src

The root Cargo.toml defines a workpace with member sub1:

[package]
#

[workspace]
members = ["sub1"]

# no dependencies

Now we can build the root project and sub1 without problems. But when we try to build sub2, the following error occurs:

> cargo build
error: current package believes it's in a workspace when it's not:
current:   /home/…/test/sub2/Cargo.toml
workspace: /home/…/test/Cargo.toml

this may be fixable by adding `sub2` to the `workspace.members` array of the manifest located at: /home/…/test/Cargo.toml

So sub2 blindly assumes that it's part of the workspace, even though it's not part of workspace.members.

Real World Example: Imagine working with git worktree. This command allows you to temporary check out a branch in a subfolder. For example, you might want to do a quick bugfix on master while working on a feature branch (let's say that you want to introduce cargo workspaces in your project).

So you check out the master branch in a bugfix subfolder. The you cd into it and try to build it. But it fails with the same error as above because your crate now thinks that it's part of the root workspace.

@alexcrichton
Copy link
Member

This is indeed correct! Unfortunately I'm not sure there's a lot we can do about this... We can perhaps add workspace = false in addition to workspace = "..." to allow an explicit opt-out of a workspace, but I figure that if you have to modify Cargo.toml you may as well hook it up to the workspace.

cc @rust-lang/tools

@4e554c4c
Copy link

4e554c4c commented Oct 4, 2016

@alexcrichton I think that the explicit opt-out should be in the "root" crate, instead of the Cargo.toml of a dependency. Perhaps have a workspace key for each dependency that defaults to true? This would make testing crates locally, as seems to be the main issue, require no change in the crate itself which would simplify things greatly.

@alexcrichton
Copy link
Member

@4e554c4c hm yeah that sounds reasonable, some form of configuration in the workspace root indicating "please don't include this area"

@phil-opp
Copy link
Contributor Author

phil-opp commented Oct 8, 2016

To clarify: This problem even occurs when there is no dependency on sub2 at all. The root crate doesn't even know about sub2. But sub2 thinks that it's part of the workspace because there is a [workspace] section in a crate upwards the file system.

An opt-out in the root crate wouldn't really solve this problem, since sub2 is not even mentioned in the Cargo.toml of the root crate. A better solution would be: Not only look for a workspace root upwards in the file system, but for a workspace root that contains the current crate.

Alternatively, the proposed workspace = false would make sense to me. Then at least we could fix it without modifying other (completely unrelated) crates.

@alexcrichton
Copy link
Member

@phil-opp isn't the workspace root here though aware of the filesystem layout? Enough to know to reject the directory with sub2?

@alexcrichton
Copy link
Member

A better solution would be: Not only look for a workspace root upwards in the file system, but for a workspace root that contains the current crate.

Unfortunately this can wreak havoc with lock files. One of the crucial properties of a workspace is that all members agree on the set of crates in a workspace. If disagreement were allowed then this could cause Cargo.lock files to oscillate depending on which crate you compile as Cargo wouldn't always know what to resolve.

@phil-opp
Copy link
Contributor Author

isn't the workspace root here though aware of the filesystem layout? Enough to know to reject the directory with sub2?

I don't know how it's implemented. I thought that the root crate only scans its members and (transitive) path dependencies. The sub2 crate is neither.

One of the crucial properties of a workspace is that all members agree on the set of crates in a workspace.

All members agree that sub2 isn't part of the workspace.

The problem is that we can't compile sub2:

error: current package believes it's in a workspace when it's not:
…

This error message is strange. The sub2 crate “believes it's in a workspace”, but at the same time it knows that “it's not”. Couldn't we just compile the crate without throwing this error (since we're sure that the crate is not part of the workspace)?

@alexcrichton
Copy link
Member

Ah so the problem here is that sub2 does indeed think it's in a workspace. The logic for finding your workspace is to either look at package.workspace or walk up the filesystem looking for [workspace]. Because sub2 lacks a package.workspace it's walking up the filesystem and finding another crate. In that sense, not all members agree because sub2 thinks it's a member.

The change I'm thinking of here is that the workspace itself says "this path is excluded" so if the "walk up" logic walks along that path then it just aborts and says there's no workspace.

@alexcrichton
Copy link
Member

I've got an idea for a motification to workspaces which I believe fixes this. If you're interested in this issue and that doesn't work for you though please let me know!

@alexcrichton
Copy link
Member

I believe with the exclude key this is no longer an issue, so closing.

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

No branches or pull requests

3 participants