Feature Request: cargo new --in-workspace [directory] [name]

[jamescostian]

The problem

Imagine you're working in a project called foo and it's a workspace. You want to create a new member, bar, as a subdirectory of foo. If you use cargo new bar (or mkdir bar && cd bar && cargo init) you'll get warnings like this:

warning: compiling this new crate may not work due to invalid workspace configuration

current package believes it's in a workspace when it's not:
current:   [new project's Cargo.toml]
workspace: [encompassing workspace's Cargo.toml]

this may be fixable by ensuring that this crate is depended on by the workspace root: 
[encompassing workspace's Cargo.toml]

Ideally, that manual work should be automated (if the user wants it).

There's more manual work if you don't want subdirectories. For example, if foo is in code/foo and you want to make bar in code/bar, then you have to change the Cargo.toml files in both directories after using cargo new

The solution

In the case of creating bar as a subdirectory, you'd cd to the workspace root and use cargo new --in-workspace . bar and foo/Cargo.toml would be updated to include bar in its [workspace] section. In the case of creating bar outside of the root, you'd use cargo new --in-workspace ../foo bar as an example. Similar capabilities should also be given to cargo init for consistency

If the directory passed to --in-workspace is not a workspace, then there would be an error.

I've never contributed to cargo before (and am pretty new to Rust) but I don't think this is too complicated, and if the idea is accepted I'd love to try and implement it. The changes would take place in this file. Add a boolean to NewOptions, change its constructor, update this behavior to change the workspaces section of the root's Cargo.toml (and the child's package.workspace if the child isn't a subdirectory of the root), and it'll be mostly done.

Notes

This is not a breaking change. The new behavior only goes into effect when a new argument (--in-workspace) is specified.

Relevant reading: the original RFC

Alternative ideas:

• cargo new --root [directory] [name]
• cargo new --member [name] - strictly for subdirectories (only addresses foo/bar example; does not address the issue of creating code/bar when your root is code/foo)

[kornelski]

I've ran into "compiling this new crate may not work" error a few times, too.

It's a bit silly that Cargo makes a non-working crate, knows it, even knows how to fix it, but just leaves it broken.

To me it would make most sense if Cargo just automatically added the new crate to the workspace.members in this situation.

[jamescostian]

That sounds pretty reasonable, and the only way I could see that breaking anything is if someone is depending on using cargo new programmatically and from within a crate. But that's quite an edge-case, so it's pretty likely that your suggestion would work perfectly

[matklad]

There's a reverse problem as well, exemplified by this thread:

https://salsa.zulipchat.com/#narrow/stream/145099-general/topic/revamped.20API/near/155324150

If a user creates, via cargo new, a parent/Cargo.toml and a parent/child/Cargo.toml, they naively expect that they've created a workspace, while it's not actually the case. What's bad about this situation is that the user doesn't get any warnings: everything works, it just uses path dependencies instead of workspaces. cargo test works, it just does not test everything.

I think it's clear now that workspaces are the default mode for working on multi-crate projects. I suggest the following change to cargo new / cargo init behavior:

• if the path to the new crate is not a part of the workspace, add an empty [workspace] section to Cargo.toml
• if the path is part of the workspace, adjust the members key (if it exists), or add the new crate as a path-dependency (printing that this was done).

That is, cargo new should always operate with workspaces, by either creating a fresh new one, or adding a member to an existing one.

[czipperz]

What is the progress here? I would really like this feature.

[jamescostian]

I'm no longer using rust, so I'm not interested in contributing to fixing this. But if anyone is willing to take the torch on this, I think it would be beneficial to the rust community (or, at least to the parts of the rust community that like modular code)

[czipperz]

Now that I think about this more, I'm coming to the conclusion that adding a flag to new/init is the wrong approach. This is because the user gets the warning after the action has already completed and these commands cannot be re-ran (they cause an error the second time as it has already been initialized). Maybe we should have a cargo add-to-workspace command? But then it seems like it's not much better than just doing it yourself since it's a pretty easy change.

[kornelski]

There are 3 options here:

1. Add to workspace automatically
2. Don't add to workspace automatically
3. Do nothing, and ask user what to do instead (e.g. by erroring new and telling user to call cargo add-to-workspace if they mean it)

AFAIK 2 creates a create that won't work, so it's broken. Option 3 is asking whether you want a broken crate or a non-broken crate, which is absurd (or a very very edge case at best). So that leaves 1 as the only sensible option?

[czipperz]

But 1 removes backwards compatibility and makes automation hard. I do agree, if we were doing this from the ground up we should do the first.

[kornelski]

Why would any existing users rely on cargo new producing a crate that can't be used?

When do you need to automate creation of unusable crates?

[czipperz]

It's still completely usable. It's just not part of the workspace. Try it in a local repository. It works fine, just there are two Cargo.lock files and two target directories.

[czipperz]

So let's say that cargo new will automatically add the crate to the workspace, which as we all agree on is the most logical path to pursue. Should it automatically create a workspace entry if one doesn't exist? IE it adds the following lines to the Cargo.toml file:

[workspace]
members = ["subcrate"]

[kornelski]

It's still completely usable. It's just not part of the workspace.

Are you sure you're reproducing the problem correctly? Two crates can be nested if there is no workspace involved. But as soon as you create a workspace at a higher level, cargo new just becomes broken:

cargo new level1
cd level1
echo "[workspace]" >> Cargo.toml
cargo new level2
cd level2
cargo test

error: current package believes it's in a workspace when it's not:
current: /private/tmp/level1/level2/Cargo.toml
workspace: /private/tmp/level1/Cargo.toml

this may be fixable by ensuring that this crate is depended on by the workspace root: /private/tmp/level1/Cargo.toml
Alternatively, to keep it out of the workspace, add the package to the workspace.exclude array, or add an empty [workspace] table to the package's manifest.

[arn-the-long-beard]

Hello , I got the error described in this issue when I follow the book there https://doc.rust-lang.org/book/ch14-03-cargo-workspaces.html , but with 3 projects instead of one :

Imagine you have the following Cargo.toml

[workspace]

members = [
    "client",
    "server",
    "shared"
]

Then

cargo new client --lib

failed to read path/to/server 

Caused by:
  No such file or directory (os error 2)
     Created library `client` package

cargo new server

failed to read path/to/shared

Caused by:
  No such file or directory (os error 2)
     Created library `client` package

cargo new shared

-> No error

I understand the error is thrown because cargo tries to read the other crates ( They are not yet created ) so we get error . That is very logical.

But :

Is this the expected behavior while creating a workspace?
Is this related to the feature described here ?

Thank you in advance. I am a baby Rustacean, so maybe my questions are not smart :P , so you can tell me :D

[saschanaz]

A workaround: add the crate name to the workspace member list before cargo new, then it just proceeds without warning.

[danieleades]

A workaround: add the crate name to the workspace member list before cargo new, then it just proceeds without warning.

That's not really helpful, since this ticket is a feature request for not having to use that workaround.

[krtab]

So I have started looking at how to implement this feature, and in my opinion it would be quite awkward to do given the current code structure

Current code structure issue

cargo::ops::cargo_new::mk creates a package by (see code at the end of this comment):

1. Writing files from hardcoded "templates".
2. Using cargo::core::workspace::Workspace::new() to try to create a Workspace struct from these files. Workspace::new is a faillible function, which implements amongst others, the sanity check leading to this issue.
3. Checking whether 2. returned an Err and if so, turning that error into a displayed warning using cargo::display_warning_with_error.

Importantly, all errors are anyhow::Error.

The logical place to put code that add the newly created package to the pre-existing workspace would be in cargo_new::mk. Unfortunately, this currently cannot be done without either dirty string comparison or duplicating the whole logic checking for workspace/package nesting, because the error we have access to have been "type erased" (not sure it is the proper term).

Possible remedies

I am not sure how to move forward on this issue. Possible solutions that come to mind are:

1. Implement some string comparison in mk to check if the error message is the relevant one (very very ugly and fragile imo).
2. Create a new function workspace::new_sanitize that could write to the disk and sanitize the workspace instead of erroring, taking care to duplicate as little code as possible.
3. Create an enum Error and make a new function workspace::new_precise_error that would return such enum in the Err variant. Workspace::new would then only be a wrapper around it.
4. Create a WriteableWorkspace struct that can be written to disk, with &mut methods to change its attributes and and write method to finalize it for example. (this would be a massive code reorganization, I'm not seriously envisioning it as a practical solution here).

Appendix

cargo/src/cargo/ops/cargo_new.rs

Lines 708 to 847 in ad50d0d

	fn mk(config: &Config, opts: &MkOptions<'_>) -> CargoResult<()> {
	let path = opts.path;
	let name = opts.name;
	let cfg = config.get::<CargoNewConfig>("cargo-new")?;
	// Using the push method with multiple arguments ensures that the entries
	// for all mutually-incompatible VCS in terms of syntax are in sync.
	let mut ignore = IgnoreList::new();
	ignore.push("/target", "^target/", "target");
	if !opts.bin {
	ignore.push("Cargo.lock", "glob:Cargo.lock", "Cargo.lock,*/Cargo.lock");
	}
	let vcs = opts.version_control.unwrap_or_else(|| {
	let in_existing_vcs = existing_vcs_repo(path.parent().unwrap_or(path), config.cwd());
	match (cfg.version_control, in_existing_vcs) {
	(None, false) => VersionControl::Git,
	(Some(opt), false) => opt,
	(_, true) => VersionControl::NoVcs,
	}
	});
	init_vcs(path, vcs, config)?;
	write_ignore_file(path, &ignore, vcs)?;
	let mut cargotoml_path_specifier = String::new();
	// Calculate what `[lib]` and `[[bin]]`s we need to append to `Cargo.toml`.
	for i in &opts.source_files {
	if i.bin {
	if i.relative_path != "src/main.rs" {
	cargotoml_path_specifier.push_str(&format!(
	r#"
	[[bin]]
	name = "{}"
	path = {}
	"#,
	i.target_name,
	toml::Value::String(i.relative_path.clone())
	));
	}
	} else if i.relative_path != "src/lib.rs" {
	cargotoml_path_specifier.push_str(&format!(
	r#"
	[lib]
	name = "{}"
	path = {}
	"#,
	i.target_name,
	toml::Value::String(i.relative_path.clone())
	));
	}
	}
	// Create `Cargo.toml` file with necessary `[lib]` and `[[bin]]` sections, if needed.
	paths::write(
	&path.join("Cargo.toml"),
	format!(
	r#"[package]
	name = "{}"
	version = "0.1.0"
	edition = {}
	{}
	# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
	[dependencies]
	{}"#,
	name,
	match opts.edition {
	Some(edition) => toml::Value::String(edition.to_string()),
	None => toml::Value::String(Edition::LATEST_STABLE.to_string()),
	},
	match opts.registry {
	Some(registry) => format!(
	"publish = {}\n",
	toml::Value::Array(vec!(toml::Value::String(registry.to_string())))
	),
	None => "".to_string(),
	},
	cargotoml_path_specifier
	)
	.as_bytes(),
	)?;
	// Create all specified source files (with respective parent directories) if they don't exist.
	for i in &opts.source_files {
	let path_of_source_file = path.join(i.relative_path.clone());
	if let Some(src_dir) = path_of_source_file.parent() {
	paths::create_dir_all(src_dir)?;
	}
	let default_file_content: &[u8] = if i.bin {
	b"\
	fn main() {
	println!(\"Hello, world!\");
	}
	"
	} else {
	b"\
	#[cfg(test)]
	mod tests {
	#[test]
	fn it_works() {
	let result = 2 + 2;
	assert_eq!(result, 4);
	}
	}
	"
	};
	if !path_of_source_file.is_file() {
	paths::write(&path_of_source_file, default_file_content)?;
	// Format the newly created source file
	match Command::new("rustfmt").arg(&path_of_source_file).output() {
	Err(e) => log::warn!("failed to call rustfmt: {}", e),
	Ok(output) => {
	if !output.status.success() {
	log::warn!("rustfmt failed: {:?}", from_utf8(&output.stdout));
	}
	}
	};
	}
	}
	if let Err(e) = Workspace::new(&path.join("Cargo.toml"), config) {
	crate::display_warning_with_error(
	"compiling this new package may not work due to invalid \
	workspace configuration",
	&e,
	&mut config.shell(),
	);
	}
	Ok(())
	}

[kornelski]

I think it would be good to start using precise error enums in Cargo, at least in "leaf" functions. For another feature I needed to handle network errors, and I had to downcast Anyhow errors, which didn't feel elegant.

[magnusdr]

I support this feature request for the sake of having some sort of suggested default to organize workspaces. Let's look at three of the most popular game engines written in Rust:

• Bevy have a separate crates folder that all but the errors-crate resides in (this is also what the RFC1525 suggests).
• Amethyst have these in its project root folder, all prefixed with amethyst_*
• Piston chose to put this into subfolders inside src

It's just a matter of opening Cargo.toml and scroll to the workspace section to figure this out, so it's not that hard. It would however be nice if we had some "suggested structure" to people who start new projects, like this feature could provide. This way, it would be easier to spot a workspace and its members at a glance, without having to inspect Cargo.toml.

[epage]

cargo has now switched from toml-rs to toml_edit (#10086). cargo init now has access to a format-preserving toml parser / generator for adding a member to the workspace's manfiest.

Considerations from lessons learned from cargo-add

• To select where in the array to add, we should check if the array is sorted and preserve that sorting. Otherwise, we just add to the end
• For our insert location, we should determine the expected formatting and format the new member accordingly (ie whitespace)
  • We should watch for the need for reformatting other items when we insert at the beginning or the end

[epage]

We talked about this issue today in the cargo team meeting. Particularly with closing #12360, we are interested in removing the cargo new && cargo check error case due to a missing workspace.members entry

Some potential solutions

• Automatically add to members with a flag to opt-out
  • cargo new --workspace to create a workspace table (Add cargo new --workspace to create a workspace #8365) is one way to opt-out
    • Should we be encouraging workspace tables and default to only be creating the workspace and require separate --bin and --lib flags?
    • Side note: I'd like to expand --bin to optionally accept a bin name
• Provide a way to not be a workspace member with a flag to opt-in
  • Opt-out could be implemented by adding a [workspace] table (see also Add cargo new --workspace to create a workspace #8365) or an idea I've had of allowing package.workspace = false
  • The name for an opt-in flag has to be understandable if we separately implement Add cargo new --workspace to create a workspace #8365

The general feel I had from the cargo team is that we prefer to opt-in by default

• If nothing else, Automatically inherit workspace fields when running cargo new/init #12069 will be activated in these cases and removing all of that is much easier than adding it in by hand
• A concern was raised about editing uncommitted files but if you have a workspace, you likely have VCS and likely we won't be changing things in a way that can't easily be undone

On a related note, should we support a flag to explicitly say where the workspace is for when it can't be auto-discovered? Personally, I would consider being nested under the workspace where it can be auto-detected the golden path and we don't need to be going out of our way to streamline manually discovered workspaces.

A part of me wants this to be:

• Initially, we add --workspace which creates a [workspace] table and requires separate --bin and --lib flags
  • If no [package] is generated (no --bin or --lib), add resolver = "2"
  • Maybe automatically add workspace.package.edition = "2021"?
  • Without --workspace, we auto-discover a workspace and, if present, adds "self" to the workspace.members
• If we add support for package.workspace = false to turn off auto-discover without needing a [workspace] table, we could then allow --workspace=<value>
  • --workspace is a shorthand for --workspace self
  • --workspace auto (default) auto-discovers a workspace and, if present, adds "self" to the workspace.members
  • Maybe automatically add workspace.package.edition = "2021"?
  • --workspace=no[ne] which sets package.workspace = false

However, I'm concerned that is a little too magical but that is also a future problem...

[fogti]

I think it's necessary for good ergonomics that such feature won't interfere with existing wildcards in workspace.members, that is, if the crate would automatically become a member of the correct workspace, the workspace Cargo.toml shouldn't be modified.

[epage]

Sorry that I didn't make it clear but my assumption was only to add if needed.

[epage]

We talked about this in the cargo team meeting.

We are fine moving forward with cargo new implicitly adding itself to the discovered workspace if a glob won't take care of it automatically.

We are fine with deferring an opt-out. Designing the opt-out will likely require enough of the design of #8365 to make sure they don't cause each other problems. In finalizing the PR for this Issue, I would recommend creating a separate issue for us to track opt-out support.

The relevant section of code starts here. We can load the workspace with toml_edit and edit workspace.members to have the relative path to the new package. We'll need cargo new tests and cargo init tests. We should ensure we have tests that verify (1) no workspace found, (2) workspace without globs, and (3) workspace with globs. We should also stress the existing formatting of workspace.members to ensure we preserve it as well as possible.

[calavera]

I'm going to take a stab at adding the new package to the members list.

@rustbot claim