cargo new sometimes generates invalid Cargo.toml when new crate is in a subdirectory of a workspace with Cargo 1.71

[Be-ing]

Problem

A crate in a subdirectory of a workspace opting out of being in the workspace via an empty [workspace] table or specifying its path in workspace.exclude in the workspace root's Cargo.toml fails on Windows with Cargo 1.71 whereas it worked with previous versions of Cargo.

Steps

Create a crate in a subdirectory of a workspace with the following Cargo.toml:

[package]
name = "required_libs"
version = "0.1.0"
edition = "2021"

# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

[dependencies]
[lib]
crate-type=["staticlib"]
[workspace]

Running cargo commands fail:

 'C:/Program Files/Microsoft Visual Studio/2022/Enterprise/Common7/IDE/CommonExtensions/Microsoft/CMake/CMake/bin/cmake.exe' '-E' 'env' 'CARGO_BUILD_RUSTC=C:/Users/runneradmin/.rustup/toolchains/stable-x86_64-pc-windows-msvc/bin/rustc.exe' 'C:/Users/runneradmin/.rustup/toolchains/stable-x86_64-pc-windows-msvc/bin/cargo.exe' 'rustc' '--verbose' '--color' 'never' '--target=x86_64-pc-windows-msvc' '--' '--print=native-static-libs'
error: failed to parse manifest at `C:\cxx-qt\build\corrosion\required_libs\Cargo.toml`
Caused by:
  error inheriting `version` from workspace root manifest's `workspace.package.version`
Caused by:
  `workspace.package.version` was not defined

Removing the empty [workspace] table from Cargo.toml fails with a different error. This error is also shown when specifying workspace.exclude = [ "path/to/subdir" ] in the workspace root's Cargo.toml:

'C:/Program Files/Microsoft Visual Studio/2022/Enterprise/Common7/IDE/CommonExtensions/Microsoft/CMake/CMake/bin/cmake.exe' '-E' 'env' 'CARGO_BUILD_RUSTC=C:/Users/runneradmin/.rustup/toolchains/stable-x86_64-pc-windows-msvc/bin/rustc.exe' 'C:/Users/runneradmin/.rustup/toolchains/stable-x86_64-pc-windows-msvc/bin/cargo.exe' 'rustc' '--verbose' '--color' 'never' '--target=x86_64-pc-windows-msvc' '--' '--print=native-static-libs'
error: current package believes it's in a workspace when it's not:
current:   C:\cxx-qt\build\corrosion\required_libs\Cargo.toml
workspace: C:\cxx-qt\Cargo.toml
this may be fixable by adding `build\corrosion\required_libs` to the `workspace.members` array of the manifest located at: C:\cxx-qt\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.

Possible Solution(s)

I encounter this when using Corrosion to invoke Cargo via CMake as part of building a C++ application. At CMake configure time, Corrosion autogenerates a dummy staticlib crate with the above Cargo.toml to run cargo rustc -- --print=native-static-libs, which is used to tell CMake what system libraries need to be linked whenever linking a staticlib into a C/C++ build. The generated dummy crate is put in the CMake build directory, which conventionally is inside the code repository, which also contains the workspace's virtual Cargo.toml manifest.

A workaround for this particular scenario is to create the CMake build directory outside of the code repository.

If rustc had some way to get the list of native-static-libs needed for every staticlib crate without needing to generate a dummy crate, this situation could be avoided.

Notes

Downstream Corrosion bug: corrosion-rs/corrosion#418

Somehow this bug is only occurring on Windows. Linux and macOS builds still work fine with Rust 1.71.

The root Cargo.toml for the workspace is:

[workspace]
members = [
    "crates/cxx-qt",
    "crates/cxx-qt-build",
    "crates/cxx-qt-gen",
    "crates/cxx-qt-lib",
    "crates/cxx-qt-lib-headers",
    "crates/qt-build-utils",

    "examples/cargo_without_cmake",
    "examples/demo_threading/rust",
    "examples/qml_extension_plugin/plugin/rust",
    "examples/qml_features/rust",
    "examples/qml_minimal/rust",

    "tests/basic_cxx_only/rust",
    "tests/basic_cxx_qt/rust",
    "tests/qt_types_standalone/rust",
]

[workspace.package]
edition = "2021"
license = "MIT OR Apache-2.0"
repository = "https://github.com/KDAB/cxx-qt/"
version = "0.5.3"

# Note a version needs to be specified on dependencies of packages
# we publish, otherwise crates.io complains as it doesn't know the version.
[workspace.dependencies]
cxx-qt = { path = "crates/cxx-qt" }
cxx-qt-macro = { path = "crates/cxx-qt-macro" }
cxx-qt-build = { path = "crates/cxx-qt-build" }
cxx-qt-gen = { path = "crates/cxx-qt-gen", version = "0.5.3" }
cxx-qt-lib = { path = "crates/cxx-qt-lib" }
cxx-qt-lib-headers = { path = "crates/cxx-qt-lib-headers", version = "0.5.3" }
qt-build-utils = { path = "crates/qt-build-utils", version = "0.5.3" }

# Ensure that the example comments are kept in sync
# examples/cargo_without_cmake/Cargo.toml
# examples/qml_minimal/rust/Cargo.toml
cxx = "1.0.95"
cxx-build = { version = "1.0.95", features = [ "parallel" ] }
cxx-gen = "0.7.95"
convert_case = "0.6"
proc-macro2 = "1.0"
syn = { version = "2.0", features = ["extra-traits", "full"] }
quote = "1.0"
serde = { version = "1.0", features = ["derive"] }
serde_json = "1.0"

Version

1.71

[epage]

Skimming through the 1.71 commits, nothing really stands out that would touch these areas. "workspace" appears a lot because this was when we switched to a workspace. We also touched workspace-related code but in the periphery, like cargo new.

[epage]

The error for the reproduction steps in the issue is also strange. It is trying to load workspace.package.version but the manifest listed doesn't reference it.

[Be-ing]

Perhaps the error is misleading? Maybe a problem occurred in a tangentially related part of the Cargo codebase that doesn't get caught immediately?

[Be-ing]

Skimming through the 1.71 commits, nothing really stands out that would touch these areas.

d6455a2 (#12317) touches code leading to the error, but it doesn't seem like that change would be the problem. Nevermind I was looking at the master branch.

[jschwe]

We are using cargo new --lib required_libs to let cargo generate a library crate for us. It appears that if there is a workspace in a parent directory, cargo will now (sometimes?) write version.workspace = true instead of version = "0.1.0" into the Cargo.toml.

I find this behavior a bit strange, since cargo also outputs the warning current package believes it's in a workspace when it's not:.

[Be-ing]

Well this is interesting. On GitHub Actions, cargo new --lib required_libs is generating a different Cargo.toml. The Cargo.toml I included in the first post was made from cargo new --lib required_libs locally on Linux assuming that it would be the same on CI. Here is what gets generated on CI:

[package]
name = "required_libs"
edition.workspace = true
license.workspace = true
repository.workspace = true
version.workspace = true

# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

[dependencies]

then Corrosion adds an empty [workspace] table, which explains the error. version.workspace = true is used on Linux, macOS, and Windows, but somehow, Cargo only fails on Windows???

Manually editing the locally generated Cargo.toml to version.workspace = true on Linux reproduces the error:

required_libs on  main [?] via 🦀 v1.72.0-dev 
❯ cargo build
error: failed to parse manifest at `/home/be/sw/cxx-qt/build/corrosion/required_libs/Cargo.toml`

Caused by:
  error inheriting `version` from workspace root manifest's `workspace.package.version`

Caused by:
  `workspace.package.version` was not defined

[weihanglo]

Pretty strange. I couldn't quite follow the reproducible steps. Is there a minimal reproduction without cmake involvement? Or could you provide a repository that people can clone and build and see the error?

BTW, the directory you ran a command is also a important part of reproduction. It would be great if we can have this piece of information.

I also wonder if there is any member crate depending on the generated crate? That could turn generated crate into a workspace member automatically.

[epage]

In 1.71, we added this feature. The inconsistent behavior could be a difference in CI between platforms or we could have a bug in detecting if there is a workspace present. As was said, independent reproduction steps will be important.

While we should root cause and fix this issue, I wonder if "cargo new" is intended for automation, if we intend the level of compatibility needed for automation or only or user interaction.

[Be-ing]

While we should root cause and fix this issue, I wonder if "cargo new" is intended for automation, if we intend the level of compatibility needed for automation or only or user interaction.

Yeah, I think it would be appropriate for Corrosion to supply the whole Cargo.toml file instead of running cargo new and appending a few lines.

[Be-ing]

BTW, the directory you ran a command is also a important part of reproduction. It would be great if we can have this piece of information.

cargo new is run from a subdirectory of the CMake build directory: "${CMAKE_BINARY_DIR}/corrosion". The full CMake command is:

    execute_process(
            COMMAND "${Rust_CARGO_CACHED}" new --lib required_libs
            WORKING_DIRECTORY "${CMAKE_BINARY_DIR}/corrosion"
            RESULT_VARIABLE cargo_new_result
            ERROR_QUIET
    )

so the workspace's virtual Cargo.toml would be two directories above that.

[Be-ing]

It looks like Cargo does detect and warn about this situation, but cargo new emits an invalid Cargo.toml anyway:

cxx-qt on  update_corrosion [$] is 📦 v0.5.3 via △ v3.26.4 via 🦀 v1.72.0-dev 
❯ rm -rf build

cxx-qt on  update_corrosion [$] is 📦 v0.5.3 via △ v3.26.4 via 🦀 v1.72.0-dev 
❯ mkdir -p build/corrosion

cxx-qt on  update_corrosion [$] is 📦 v0.5.3 via △ v3.26.4 via 🦀 v1.72.0-dev 
❯ cd build/corrosion/

cxx-qt/build/corrosion on  update_corrosion [$] 
❯ cargo new --lib required_libs
warning: compiling this new package may not work due to invalid workspace configuration

current package believes it's in a workspace when it's not:
current:   /home/be/sw/cxx-qt/build/corrosion/required_libs/Cargo.toml
workspace: /home/be/sw/cxx-qt/Cargo.toml

this may be fixable by adding `build/corrosion/required_libs` to the `workspace.members` array of the manifest located at: /home/be/sw/cxx-qt/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.
     Created library `required_libs` package

cxx-qt/build/corrosion on  update_corrosion [$] 
❯ cat required_libs/Cargo.toml 
[package]
name = "required_libs"
edition.workspace = true
license.workspace = true
repository.workspace = true
version.workspace = true

# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

[dependencies]

cxx-qt/build/corrosion on  update_corrosion [$] 
❯ cd required_libs/

required_libs on  main [?] is 📦 v0.5.3 via 🦀 v1.72.0-dev 
❯ cargo build
error: current package believes it's in a workspace when it's not:
current:   /home/be/sw/cxx-qt/build/corrosion/required_libs/Cargo.toml
workspace: /home/be/sw/cxx-qt/Cargo.toml

this may be fixable by adding `build/corrosion/required_libs` to the `workspace.members` array of the manifest located at: /home/be/sw/cxx-qt/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.

Perhaps Cargo should revert to generating a crate that does not try to be in the workspace (version = "0.1.0" instead of version.workspace = true) in this case while warning that the generated crate is not part of the workspace. I think this would be less bad than generating a crate that won't build (in Corrosion's case, generating a crate not part of the workspace was what was intended).

[epage]

We are considering adding the new package to the workspace members automatically and we didn't want to block one feature on the other.

Could you provide minimal reproduction steps? What I mean by that is independent of corrosion / cmake steps any of us could run.

[jschwe]

I made a minimal example here: https://github.com/jschwe/cargo_regression_repro

What corrosion wants is:

• Get a library package where cargo build (or more precisely cargo rustc -- --print=native-static-libs works. The content of the library is not important, we just want to know which libraries need to be linked against for std to work.
• Anything outside (e.g. an outer workspace) should not be modified.

Previously, cargo new seemed like a good fit for this. The only limitation was, that when a parent directory contained a workspace, we needed to add an [workspace] to the Cargo.toml generated by cargo new.

We are considering adding the new package to the workspace members automatically

I guess if this is going to happen, we can't use cargo new anymore and need to ship our own dummy package, which is also fine.

[Be-ing]

I guess if this is going to happen, we can't use cargo new anymore and need to ship our own dummy package, which is also fine.

That could work. Alternatively, it would be nice if rustc or cargo could print the libraries required to link std without needing a dummy crate.

[weihanglo]

Thanks for the repro!

I guess I've figured out what's going on. #12069 use find_root_manifest_for_wd to discover the possible workspace manifest, however, without checking workspace.members field. If the newly added package is not included in workspace.members, then Cargo shouldn't try to make it inherit anything from the workspace.

For now, I consider this as a regression. We at least need to stop the invalid member from inheriting things. There are two solutions come up to my mind:

1. Create the package first. And then use Workspace::new to check if the new package is a member. If yes, edit the manifest for the inheritance.
2. Use WorkspaceRootConfig::members_paths or something to determine whether it is a member when creating the manifest.

Not sure which one is simpler. Probably the first one?

@hi-rustin are you willing to have a look at it?

[epage]

@weihanglo that was somewhat intentional as, unless the person is using globs, they can't add it to the workspace members yet.

I would assume the bigger question is why is this platform-specific.

[weihanglo]

My bad. I was talking about the repro from @jschwe, and thought it has the same root cause as the original issue.

The intentional behavior seems to be a bit unpleasant. Or did you mean when a user notices the error message “workspace.package.version was not defined”, they should know there is a need to manual fix?

[epage]

@jschwe my understanding was that the original bug only happened on Windows and not on other platforms but I'm not seeing that mentioned in the reproduction case. Is that still true or is the platform-specific aspect somehow a red herring?

[epage]

The intentional behavior seems to be a bit unpleasant. Or did you mean when a user notices the error message “workspace.package.version was not defined”, they should know there is a need to manual fix?

Whether it is unpleasant depends. I expect in most cases, the very next action is to add the package to the workspace.

That said, we can re-visit that approach and what we should do for automated adding to workspaces later (I was expecting us to add it to workspace members implicitly).

[jschwe]

Is that still true or is the platform-specific aspect somehow a red herring?

I believe the issue may have happened on all platforms, but was likely hidden on linux / mac since the libraries which should have been printed by rustc were either not needed due to simple test code, or also added on the link line via other libraries from the C side for more complex cases.

I expect in most cases, the very next action is to add the package to the workspace.

I agree, that is also my most common usecase (at least when manually invoking cargo new. Perhaps a flag could be added which prevents cargo new from modifying any existing files outside the new directory?

[epage]

To summarize

@Be-ing and others are using cargo new to programmatically create a package and then manually touch it up into a workspace (see also #8365) to avoid errors from not being a workspace member.

With #12069, we now check if it looks like the new package could be part of a workspace and automatically inherit, punting on whether it is a workspace member or not to #6378.

This broke @Be-ing and others because they could no longer just run cargo new and then add a [workspace] table.

Note: currently cargo new warns if the package could be a workspace member:

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

current package believes it's in a workspace when it's not:
current:   /home/epage/src/personal/clap/foo/Cargo.toml
workspace: /home/epage/src/personal/clap/Cargo.toml

this may be fixable by adding `foo` to the `workspace.members` array of the manifest located at: /home/epage/src/personal/clap/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.

Questions

• How much compatibility for programmatic use cases of we expect for cargo new
  • e.g. if we had done Feature Request: cargo new --in-workspace [directory] [name] #6378 and made making it a workspace member the default broken them just as much?
• What do we consider the right UX for Feature Request: cargo new --in-workspace [directory] [name] #6378
  • How does that affect Automatically inherit workspace fields when running cargo new/init #12069 in the short term?

I've nominated this for discussion in the cargo team meeting to hash some of these details out

[epage]

We discussed this in the cargo team meeting today.

While we didn't formally nail it down, our primary concern for programmatic usage is in having processes (like a button in an IDE) that run cargo new to create a package. There was also interest in having the placement of files within what is created to be predictable so the IDE could then immediately open src/main.rs for editing.

For programmatic usage that expects content of the files to be generated in a specific way we felt was out of scope. For a case like in this issue, the files could be generated directly or with tools like cargo-generate or cargo-hatch.

That still leaves the question of what we consider the right UX. Our primary concern was not regressing in what works. Since cargo new && cargo check will will in this case, putting in workspace inheritance doesn't regress that behavior. We would like to move away from any erroring and the general tone seemed like we were interested in automatically adding the package to the workspace members with a potential opt-out.

Since the use case (programmatic use) is out of scope and the proposed fix (don't inherit) was decided against, I'm closing this and discussion on workspace interactions will be carried forward in #6378.