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

(Virtual?) manifest with both package and workspace sections is accepted #3526

Closed
sanmai-NL opened this issue Jan 11, 2017 · 6 comments · Fixed by #4376
Closed

(Virtual?) manifest with both package and workspace sections is accepted #3526

sanmai-NL opened this issue Jan 11, 2017 · 6 comments · Fixed by #4376
Labels
A-documenting-cargo-itself Area: Cargo's documentation

Comments

@sanmai-NL
Copy link

sanmai-NL commented Jan 11, 2017

I created a new package using cargo new, then added the workspace section to the manifest. That did not work. When the manifest has both package and workspace sections, this is accepted by cargo and I found that confusing. Instead, cargo should report the manifest as invalid, since it apparently is.

BTW, once my error of having both sections was corrected to workspace-only, cargo reported:

is a virtual manifest, but this command requires running against an actual package in this workspace

I couldn't find a definition of ‘virtual manifest’ in the Cargo docs, however.

Version

cargo 0.17.0-nightly (40a4ce6 2017-01-06)

@sanmai-NL sanmai-NL changed the title Manifest with both package and workspace sections is accepted (Virtual?) manifest with both package and workspace sections is accepted Jan 11, 2017
@alexcrichton
Copy link
Member

Could you gist the manifest in question? This is actually intended behavior where a package can be the root of a workspace.

@kornelski
Copy link
Contributor

I've ran into the intended behavior with this package: https://github.com/imazen/imageflow

"virtual manifest" is a term I haven't heard before, so the error message is not clear to me.

I hoped that by running cargo doc in the root I'm going to get a single unified documentation for all packages (all of their modules) in the workspace, so that I can browse around and see what each package does.

I suppose cargo doc --all is the closest equivalent of what I expected.

@behnam
Copy link
Contributor

behnam commented Jul 20, 2017

Virtual manifest is defined in the RFC:

A good number of projects do not necessarily have a "root Cargo.toml" which is an appropriate root for a workspace. To accommodate these projects and allow for the output of a workspace to be configured regardless of where crates are located, Cargo will now allow for "virtual manifest" files. These manifests will currently only contains the [workspace] table and will notably be lacking a [project] or [package] top level key.

Therefore, there's no package for the workspace root, therefore no cargo package or cargo publish for the root, but still the rest of the commands should work as expected, passing in the --all option.

By default, cargo always works on one single package (vs all packages in a workspace), therefore this is all expected behavior.

I think there's nothing left open in this issue and we can close it.

@alexcrichton
Copy link
Member

Sounds good to me!

@sanmai-NL
Copy link
Author

sanmai-NL commented Jul 20, 2017

IMO concepts should be in user documentation, not only in an RFC. That issue remains.

I commented that a manifest with both package and workspace section is invalid. I do not immediately see how @behnam's comment changes this claim. That issue remains open as well then.

Please reopen the issue if this is true, @alexcrichton.

@behnam
Copy link
Contributor

behnam commented Jul 20, 2017

Valid point, @sanmai-NL. The online/current docs should contain the definition of Virtual Manifest and its behavior.

@alexcrichton, can we please reopen the issue to track the documentation matter? I can submit a PR, and take the assignment, if needed.

Also, @alexcrichton, it looks like that there's a lot of workspace-related content to go into the docs (this issue, #4304, and some other ones) What do you think about creating a separate page, like http://doc.crates.io/workspace.html, to cover all the details?

@alexcrichton alexcrichton reopened this Jul 20, 2017
@alexcrichton alexcrichton added the A-documenting-cargo-itself Area: Cargo's documentation label Jul 20, 2017
behnam added a commit to behnam/rust-cargo that referenced this issue Aug 7, 2017
Current documentation does not mention Virtual Manifests at all, which
can be confusing because they appear in error messages.

Here we add the definition, and provide a hint about `--all` option for
most cargo commands, which allow the command to work in the way most
probably expected.

Fixes <rust-lang#3526>
bors added a commit that referenced this issue Aug 8, 2017
[docs/manifest] Add Virtual Manifest section

Current documentation does not mention Virtual Manifests at all, which
can be confusing because they appear in error messages.

Here we add the definition, and provide a hint about `--all` option for
most cargo commands, which allow the command to work in the way most
probably expected.

Fixes <#3526>
@bors bors closed this as completed in #4376 Aug 8, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-documenting-cargo-itself Area: Cargo's documentation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants