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

Nested workspaces #5042

Open
Tracked by #41 ...
nrc opened this issue Feb 15, 2018 · 38 comments
Open
Tracked by #41 ...

Nested workspaces #5042

nrc opened this issue Feb 15, 2018 · 38 comments
Labels
A-manifest Area: Cargo.toml issues A-workspaces Area: workspaces C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` S-needs-rfc Status: Needs an RFC to make progress.

Comments

@nrc
Copy link
Member

nrc commented Feb 15, 2018

Allow multiple layers of workspaces, not just one. This would be useful for tools in the Rust repo, and I think more generally.

@nrc nrc added the A-workspaces Area: workspaces label Feb 15, 2018
@alexcrichton alexcrichton added the C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` label Feb 17, 2018
@nrc
Copy link
Member Author

nrc commented Feb 27, 2018

A lightweight way to help here might be to just ignore nested virtual manifests. If we want to be strict, we could do this only if the outer manifest includes all the workspace members in the inner manifest. This might admit some errors where there is an ignored Cargo.toml somewhere in a tree of workspaces, but I don't think that is much of a hazard, especially if the outer manifest explicitly includes the inner members.

One question is forwards compatibility, if one day we want to allow true nested workspaces, then I'm not sure how well that co-exists with this simpler proposal. I think technically it would be OK, but it might be hard to understand for users.

cc @rust-lang/cargo - thoughts? If this sounds OK, then I'd quite like to implement so that we can make Rustfmt and maybe RLS use workspaces (and even Cargo!).

@matklad
Copy link
Member

matklad commented Feb 27, 2018

This is already supported with the help of exclude key I think?

λ pwd
/home/matklad/trash/foo

~/trash/foo master*
λ tree .
.
├── bar
│   ├── Cargo.toml
│   └── src
│       └── lib.rs
├── Cargo.lock
├── Cargo.toml
└── src
    └── lib.rs

3 directories, 5 files

~/trash/foo master*
λ cat Cargo.toml
[package]
name = "foo"
version = "0.1.0"
authors = ["Aleksey Kladov <[email protected]>"]

[workspace]
exclude = [ "./bar" ]

[dependencies]
bar = { path = "./bar" }

~/trash/foo master*
λ cat bar/Cargo.toml
[package]
name = "bar"
version = "0.1.0"
authors = ["Aleksey Kladov <[email protected]>"]

[workspace]

[dependencies]

~/trash/foo master*
λ cargo build
   Compiling bar v0.1.0 (file:///home/matklad/trash/foo/bar)
   Compiling foo v0.1.0 (file:///home/matklad/trash/foo)
    Finished dev [unoptimized + debuginfo] target(s) in 0.55 secs

@nrc
Copy link
Member Author

nrc commented Feb 28, 2018

I don't think the exclude trick quite works for the case I have in mind. I have:

.
├── Cargo.toml
├── bar
│   ├── Cargo.toml
│   └── baz
│       ├── Cargo.toml
│       └── src
│           └── lib.rs
└── foo
    ├── Cargo.toml
    └── src
        └── lib.rs

Where bar/Cargo.toml is

[workspace]
members = [
    "baz",
]

and ./Cargo.toml is

workspace]
members = [
    "foo",
    "bar/baz",
]

exclude = [ "./bar" ]

When I cargo build in ., I get:

error: package `/Users/nick/hello-ws/bar/baz/Cargo.toml` is a member of the wrong workspace
expected: /Users/nick/hello-ws/Cargo.toml
actual:   /Users/nick/hello-ws/bar/Cargo.toml

And I don't think I can fix that subject to the constraint that baz may be a Git submodule and should build in its own context too.

@matklad
Copy link
Member

matklad commented Feb 28, 2018

Hm, that looks interesting!

So basically we have . workspace with ., foo and bar/baz and bar workspace with bar and bar/baz. That is, we actually have intersecting workspaces, and not just nested ones!

Why does bar/baz needs to be a member of the outer workspace though? Can it be a regular path dependency?

@nrc
Copy link
Member Author

nrc commented Mar 1, 2018

Why does bar/baz needs to be a member of the outer workspace though? Can it be a regular path dependency?

It's not a dependency of anything in the . workspace, it is just part of bar. To be precise there are multiple crates in bar where at least one is a binary that needs to be produced when building . and the others are deps of crates inside bar. So, I think at least the crates with binaries need to members of ..

The concrete example here is tools in the Rust repo when Rust is . and bar is rustfmt or the RLS.

@nrc
Copy link
Member Author

nrc commented Mar 1, 2018

I discussed with @alexcrichton on irc, it turns out that this would actually be a breaking change: in my example above, you can't currently build the outer workspace but you can build bar and bar/baz. If we adopt the semantics of ignoring bar's manifest, then we would be changing the semantics of building bar.

An alternative would be to only ignore bar's manifest if we are building in the outer workspace, not in bar or bar/baz. However, although that would be backwards compatible, it would break a key invariant of workspaces that where you build a workspace does not affect which crates are members of the workspace.

So, I think the better solution is the breaking change, since I doubt anyone actually uses nested workspaces right now since they are broken when building at the top. However, since it is breaking, it probably needs an RFC, or at least a bit more advertising and discussion than a quick 'bug fix'.

@alexcrichton
Copy link
Member

@nrc this was initially motivated by the rust-lang/rust repo, right?

I think that Cargo may have actually progressed far enough at this point to deal with git dependencies gracefully that it may work (previously we had to avoid git dependencies). I wonder if that'd be an option to avoid vendoring rustfmt's source code?

@wycats
Copy link
Contributor

wycats commented Mar 1, 2018

@nrc I'm interested to understand what experience you had that motivated this. I understand the mechanics of what you're proposing, and I'd find it helpful to understand what situation the mechanics are targeting.

@nrc
Copy link
Member Author

nrc commented Mar 1, 2018

@alexcrichton how would we run rustfmt tests if we used it as a dep?

@wycats the motivation is where you want to pull in a workspace project as a Git submodule and a workspace member of another project. The specific example here is pulling Rustfmt into the Rust repo.

@alexcrichton
Copy link
Member

@nrc bah excellent point!

@Michael-F-Bryan
Copy link

wycats the motivation is where you want to pull in a workspace project as a Git submodule and a workspace member of another project. The specific example here is pulling Rustfmt into the Rust repo.

I came here to mention this exact use case. At work I've got one project which would like to use a crate from another project. I was initially making project B a git submodule of project A and then using a path dependency to include the crate, but I encountered @alexcrichton's error message.

error: package `C:\$ProjectA\$ProjectB\some-crate\Cargo.toml` is a member of the wrong workspace
expected: C:\$ProjectA\Cargo.toml
actual:   C:\$ProjectA\$ProjectB\Cargo.toml

@kuviman
Copy link

kuviman commented Sep 21, 2018

@nrc What if, in your example, change ./Cargo.toml to simply

[workspace]
members = ["foo", "bar"]

So, that one of the workspace members is a workspace itself. This is now actually nested workspaces, not intersecting like before. So, every member of the inner workspace should be considered as a member of the outer one.

where build a workspace does not affect which crates are members of the workspace

This invariant should still hold, just some crates (like baz) would be members of several workspaces (in which case the top-level one should be used I think). Nothing is ignored, so everything that worked before would still work, there would be no breaking changes, I think.

Currently this produces this error:

error: multiple workspace roots found in the same workspace:
  /home/user/hello-ws/bar
  /home/user/hello-ws

@jrobsonchase
Copy link

Any updates on this? I like the solution proposed by @kuviman since it would reduce redundancy in nested virtual manifests.

Something I haven't seen discussed yet is how nested workspaces should behave wrt implicit members in non-virtual manifests. How would those be handled?

@PrismaPhonic
Copy link

Bumping for visibility. I would love to see this resolved. Nested workspaces seems like a common use case.

@TomzBench
Copy link

TomzBench commented Dec 8, 2019

I created a separate branch for the inner repository named "as_submodule". The inner repositories' "as_submodule" branch is identical to master branch, minus the root Cargo.toml workspace. So when somebody wants to include the project as a submodule they do git submodule add -b as_submodule. The master and development branches all act as a workspace which makes testing and stuff easier.

I don't like doing it this way, so the README points to this issue for people who judge :)

@ethindp
Copy link

ethindp commented Jan 19, 2020

This is my first "tru" involvement in the Rust development ecosystem (I'm not really sure how to contribute to Rust and am not very knowledgeable on lexers, parsers and whatnot). I would love to have this resolved. I am currently working on an operating system that is structured like so:

.
├── Cargo.lock
├── Cargo.toml
├── disk.img
├── LICENSE
├── linux-0.2.img
├── pi.vfd
├── README.md
├── src
│   ├── acpi
│   │   └── core
│   │       └── opcodes.rs
│   ├── bits.rs
│   ├── drivers
│   │   ├── bus
│   │   │   ├── mod.rs
│   │   │   └── usb
│   │   │       ├── ehci
│   │   │       │   └── mod.rs
│   │   │       ├── mod.rs
│   │   │       ├── ohci
│   │   │       │   └── mod.rs
│   │   │       ├── uhci
│   │   │       │   └── mod.rs
│   │   │       └── xhci
│   │   │           └── mod.rs
│   │   ├── fs
│   │   │   ├── ext2.rs
│   │   │   └── mod.rs
│   │   ├── hid
│   │   │   ├── keyboard
│   │   │   │   └── mod.rs
│   │   │   ├── mod.rs
│   │   │   └── mouse
│   │   │       └── mod.rs
│   │   ├── iof
│   │   │   ├── io.rs
│   │   │   └── mod.rs
│   │   ├── mod.rs
│   │   ├── net
│   │   │   └── mod.rs
│   │   ├── sound
│   │   │   ├── hda.rs
│   │   │   ├── mod.rs
│   │   │   └── pcspeaker.rs
│   │   ├── storage
│   │   │   ├── ahci
│   │   │   │   └── internal.rs
│   │   │   ├── ahci.rs
│   │   │   ├── ata
│   │   │   │   ├── dco.rs
│   │   │   │   ├── identification.rs
│   │   │   │   ├── security.rs
│   │   │   │   └── smart.rs
│   │   │   ├── ata.rs
│   │   │   ├── gpt.rs
│   │   │   └── mod.rs
│   │   ├── video
│   │   │   └── mod.rs
│   │   └── virtio
│   │       └── mod.rs
│   ├── exec
│   │   └── elf
│   │       └── types.rs
│   ├── gdt.rs
│   ├── interrupts.rs
│   ├── lib.rs
│   ├── main.rs
│   ├── memory.rs
│   ├── pci.rs
│   ├── scheduler.rs
│   ├── smbios.rs
│   ├── tasking.rs
│   ├── ui.rs
│   └── vga.rs
└── x86_64-kernel-none.json

I'd like to divide this up into workspaces: the main kernel tree as the "top-level" workspace, then nested workspaces under that (i.e. "drivers" is a workspace, "acpi" is another, ...). Currently (unless this issue has been resolved) I am unable to make this layout without creating separate repositories, which makes it harder to manage the tree as a hole as I am the only maintainer right now.

drahnr added a commit to fff-rs/juice that referenced this issue Jan 30, 2020
drahnr added a commit to fff-rs/juice that referenced this issue Jan 31, 2020
drahnr added a commit to fff-rs/juice that referenced this issue Feb 1, 2020
drahnr added a commit to fff-rs/juice that referenced this issue Feb 1, 2020
drahnr added a commit to fff-rs/juice that referenced this issue Feb 1, 2020
kylewlacy added a commit to brioche-dev/ducc that referenced this issue Feb 23, 2020
@robinmoussu
Copy link
Contributor

I got it by this issue too. I have a crate that have multiples binaries, and multiple libraries, so I'm using a workspace. I had to vendor another library that itself uses a workspace. In order to do so, I had to modify it to move its workspace from its Cargo.toml to my top-level Cargo.toml. More details on URLO.

@NER-Alliance
Copy link

Would it be possible to introduce nested workspaces "light", where they may be used under the condition that a workspace which contains a workspace must not contain a package? This would allow for most common use cases of one workspace combining multiple projects (which are workspaces with multiple crates) into one project. Though it works to open the parent folder in CLion and attaching all children to it, it would be a neat QOL change, allowing deduplication of profile configurations and allowing building / testing / etc. in one command.

@LunarLambda
Copy link

Ran into this today. Our use case is vendoring mdbook plugins as git submodules.

Cargo.toml

[workspace]
resolver = "2"
members = ["mdbook-external-links", "mdbook-linkcheck", "mdbook-toc"]

book.toml

[preprocessor.external-links]
command = "cargo run -p mdbook-external-links --locked --release --"

mdbook-linkcheck has a workspace, so adding it broke things.

I'm unsure if there's a better way that retains the ability to include plugins as submodules rather than needing them installed on the system.

@mmgeorge
Copy link

mmgeorge commented Dec 2, 2023

Also just ran into this ... was trying to add a fork of wgpu as a submodule of a project as I need to modify the naga subcrate that was recently moved to it. Would love to see support added for this! Think it will become increasingly common as more stuff moves to monorepos / workspaces.

@guilhermewerner
Copy link

I'm also having problems with this, I have a Rust/C++ project that links to a cdylib made in Rust, which is a git submodule in /vendor, and I need to add the lib as a dependency in the root Cargo.toml to compile the dll and be able to link to the C++ project.

Syndelis added a commit to Syndelis/imgui-rs that referenced this issue Dec 12, 2023
Syndelis added a commit to ufsj-dcomp/imnodes-rs that referenced this issue Dec 12, 2023
chore: remove workspace manifest
see rust-lang/cargo#5042
Syndelis added a commit to Syndelis/odeir that referenced this issue Dec 12, 2023
@NewtonChutney
Copy link

Is any RFC in progress?
(Bumping for visibility as well!)

@epage
Copy link
Contributor

epage commented Dec 27, 2023

Is any RFC in progress?

There has been exploration in the past. If @Muscraft's notes aren't captured here, it would be good to do so for anyone else who wants to take on the design work. No one is actively working on this and, usually when they do, there will be posts in the relevant issues pointing people to things like a Pre-RFC on Internals or an RFC. In other words, if there isn't already a message about progress, there unlikely is to be any.

(Bumping for visibility as well!)

FYI "bumps" are counter productive. While they send someone a notification, it is a low value notification that is more likely to annoy people than to help spur things to move forward. They make full inboxes even more full, full threads even more full, etc.

What would be helpful instead is to document use cases and how they affect designs (if yours isn't already captured). A lot of talk here is vaguely about git submodule cases without an explanation for why a package from a workspace in a submodule needs to be pulled into the users workspace. This also sounds more like wanting to have packages belong to multiple workspaces which is different than nested workspaces (strict parent/child relationships) and designs for each can go in very different directions.

In talking about use cases, it would help to consider what are needs/nice-to-haves, and don't-cares (with "why") for things like:

  • Workspace inheritance
  • Profile and patch tables (grandparent only, parent only, grandparent or parent layered on the other)
  • Lockfiles
  • Whether the grandparent workspace must always be present or can optionally be missing (e.g. gitsubmodule) and the affects on the above

@epage epage added the A-manifest Area: Cargo.toml issues label Dec 27, 2023
@NewtonChutney
Copy link

(Bumping for visibility as well!)

FYI "bumps" are counter productive. While they send someone a notification, it is a low value notification that is more likely to annoy people than to help spur things to move forward. They make full inboxes even more full, full threads even more full, etc.

I agree, will refrain from such behavior.. ᓚᘏᗢ

@PBetzler
Copy link

I stumbled over this issue and try to give you an overview over my use-case. Please be aware that I'm a beginner in terms of rust and might not have understood yet the ideal ways to set something up.

I'm having a workspace setup with multiple libs and binaries and also a fyrox game, intended to run in the browser. The fyrox game is unfortunaly a workspace on its own, because it contains automatically subprojects for the engine, etc.
I can delete the game workspace manifest and add everything to the global one, but this is an annoying step (but this itself does not rectify a change). The problem occures when I want to have a second game added to the workspace. The project names are already taken, so I can not just do the manifest modification again.

With nested worspaces I would hope to refere to the nested projects by sub_workspace::project and the projects them selves would not need to know that there are different projects in other sub-workspaces with the same name.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-manifest Area: Cargo.toml issues A-workspaces Area: workspaces C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` S-needs-rfc Status: Needs an RFC to make progress.
Projects
None yet
Development

No branches or pull requests