Add a workspace.default-members config that overrides implied --all #4743

SimonSapin · 2017-11-23T15:27:51Z

Fixes #4507.

rust-highfive · 2017-11-23T15:27:54Z

r? @matklad

(rust_highfive has picked a reviewer for you, use r? to override)

SimonSapin · 2017-11-23T15:28:05Z

r? @rust-lang/cargo

matklad

Changes looks great to me, let's see what others think!

matklad · 2017-11-23T15:36:14Z

src/cargo/core/workspace.rs

@@ -46,6 +46,9 @@ pub struct Workspace<'cfg> {
    // set above.
    members: Vec<PathBuf>,

+    // The subset of `members` that are “default”.


Let's expand this comment a bit, to explain when exactly this field is used?

I wrote some things.

matklad · 2017-11-23T15:38:03Z

src/cargo/core/workspace.rs

@@ -370,7 +413,7 @@ impl<'cfg> Workspace<'cfg> {
                      root_manifest: &Path,
                      is_path_dep: bool) -> CargoResult<()> {
        let manifest_path = paths::normalize_path(manifest_path);
-        if self.members.iter().any(|p| p == &manifest_path) {
+        if self.members.contains(&manifest_path) {


lukaslueg · 2017-11-24T12:02:04Z

src/cargo/ops/cargo_compile.rs

        -> CargoResult<Self>
    {
-        let all = all || (virtual_ws && package.is_empty());


Bikeshedding: this could be match (all, exclude.is_empty(), package.is_empty()) and (false, true, true)...

I did that at first and found it confusing when it came to writing all the patterns. Both because of boolean overload, and because false felt like "negative" and so "not present".

matklad · 2017-11-28T10:23:03Z

@withoutboats ping: could you look at this small addition/nice usability improvement to Cargo.toml format? I'd love to r+ it, but I just want another pair of eyes to look at this feature as a whole :)

alexcrichton · 2017-11-28T17:42:25Z

Could this be sure to update the documentation for workspaces as well? I'm not entirely sure what this feature is doing at a first glance and the docs would certainly help me learn!

SimonSapin · 2017-11-28T17:53:05Z

Sure. Are the sources for what will become "the cargo book" still at src/doc in this repo?

SimonSapin · 2017-11-28T18:10:46Z

Pushed a couple more commits.

SimonSapin · 2017-11-28T19:37:45Z

CI had an identical failure on all platforms, but I couldn’t reproduce :/

failures:
---- frozen_flag_preserves_old_lockfile stdout ----
	running `/home/travis/build/SimonSapin/cargo/target/debug/cargo build --locked`
thread 'frozen_flag_preserves_old_lockfile' panicked at '
Expected: execs
    but: exited with exit code: 101
--- stdout
--- stderr
    Updating registry `file:///home/travis/build/SimonSapin/cargo/target/cit/t0/registry`
error: checksum for `foo v0.1.0` changed between lock files
this could be indicative of a few possible errors:
    * the lock file is corrupt
    * a replacement source in use (e.g. a mirror) returned a different checksum
    * the source itself may be corrupt in one way or another
unable to verify that `foo v0.1.0` is the same as when the lockfile was generated
', /home/travis/.cargo/registry/src/github.com-1ecc6299db9ec823/hamcrest-0.1.1/src/core.rs:31:12
note: Run with `RUST_BACKTRACE=1` for a backtrace.
failures:
    frozen_flag_preserves_old_lockfile

matklad · 2017-11-28T19:38:35Z

Wait, this is exactly the issue that @nrc was seeing recently!

nrc · 2017-11-28T21:02:05Z

Yeah, I saw this failure on the Rust CI, (after updating RLS and Rustfmt and not touching Cargo). I could repro locally inside Rust, but not in Cargo's own repo. I got nowhere trying to investigate. I would love if we could just disable this test for now so I can land the RLS/Rustfmt update.

When no package is selected with `-p`, the default behavior for virtual workspaces is to select every member as if `--all` were used.

rust-lang#4335

alexcrichton · 2017-11-29T15:16:06Z

Sounds reasonable to me from a feature perspective!

matklad · 2017-11-29T15:19:02Z

@bors r+

The test should now be fixed in master.

Thanks @SimonSapin !

bors · 2017-11-29T15:19:03Z

📌 Commit 82d563b has been approved by matklad

bors · 2017-11-29T15:19:10Z

⌛ Testing commit 82d563b with merge 5bb478a...

Add a workspace.default-members config that overrides implied --all Fixes #4507.

bors · 2017-11-29T16:08:20Z

☀️ Test successful - status-appveyor, status-travis
Approved by: matklad
Pushing 5bb478a to master...

rust-lang/cargo@5bb478a Pick up `workspace.default-members` support: rust-lang/cargo#4743

Update Cargo to Wed Nov 29 15:19:05 2017 +0000 rust-lang/cargo@5bb478a Pick up `workspace.default-members` support: rust-lang/cargo#4743

nrc · 2017-12-03T23:46:16Z

src/doc/manifest.md

@@ -520,8 +520,19 @@ crate will be treated as a normal package, as well as a workspace. If the
 manifest*.

 When working with *virtual manifests*, package-related cargo commands, like


This seems wrong - an earlier commit makes default-members work for non-virtual manifests too, right?

Oops, I forgot about that when writing the docs. Submitted #4784, thanks.

…ild' … and 'cargo test', etc. Include Servo and its unit tests, but not Stylo because that would try to compile the style crate with incompatible feature flags: rust-lang/cargo#4463 `workspace.default-members` was added in rust-lang/cargo#4743. Older Cargo versions ignore it.

Use workspace.default-members to specify default crates for cargo build … and 'cargo test', etc. Include Servo and its unit tests, but not Stylo because that would try to compile the style crate with incompatible feature flags: rust-lang/cargo#4463 `workspace.default-members` was added in rust-lang/cargo#4743. Older Cargo versions ignore it.  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/19476)

…ild' … and 'cargo test', etc. Include Servo and its unit tests, but not Stylo because that would try to compile the style crate with incompatible feature flags: rust-lang/cargo#4463 `workspace.default-members` was added in rust-lang/cargo#4743. Older Cargo versions ignore it.

Use workspace.default-members to specify default crates for cargo build … and 'cargo test', etc. Include Servo and its unit tests, but not Stylo because that would try to compile the style crate with incompatible feature flags: rust-lang/cargo#4463 `workspace.default-members` was added in rust-lang/cargo#4743. Older Cargo versions ignore it.  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/19476)

…ild' … and 'cargo test', etc. Include Servo and its unit tests, but not Stylo because that would try to compile the style crate with incompatible feature flags: rust-lang/cargo#4463 `workspace.default-members` was added in rust-lang/cargo#4743. Older Cargo versions ignore it.

Use workspace.default-members to specify default crates for cargo build … and 'cargo test', etc. Include Servo and its unit tests, but not Stylo because that would try to compile the style crate with incompatible feature flags: rust-lang/cargo#4463 `workspace.default-members` was added in rust-lang/cargo#4743. Older Cargo versions ignore it.  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/19476)

…t crates for cargo build (from servo:default-members); r=nox … and 'cargo test', etc. Include Servo and its unit tests, but not Stylo because that would try to compile the style crate with incompatible feature flags: rust-lang/cargo#4463 `workspace.default-members` was added in rust-lang/cargo#4743. Older Cargo versions ignore it. Source-Repo: https://github.com/servo/servo Source-Revision: df68eea3f21cc3bbf24d5bbb66be42c4e3a9e427 --HG-- rename : servo/tests/unit/stylo/Cargo.toml => servo/ports/geckolib/tests/Cargo.toml rename : servo/tests/unit/stylo/build.rs => servo/ports/geckolib/tests/build.rs rename : servo/tests/unit/stylo/lib.rs => servo/ports/geckolib/tests/lib.rs rename : servo/tests/unit/stylo/servo_function_signatures.rs => servo/ports/geckolib/tests/servo_function_signatures.rs rename : servo/tests/unit/stylo/size_of.rs => servo/ports/geckolib/tests/size_of.rs rename : servo/tests/unit/stylo/specified_values.rs => servo/ports/geckolib/tests/specified_values.rs extra : subtree_source : https%3A//hg.mozilla.org/projects/converted-servo-linear extra : subtree_revision : 0939a7049dc771e9d1b4f45f6e3ade2866266fa4

…t crates for cargo build (from servo:default-members); r=nox … and 'cargo test', etc. Include Servo and its unit tests, but not Stylo because that would try to compile the style crate with incompatible feature flags: rust-lang/cargo#4463 `workspace.default-members` was added in rust-lang/cargo#4743. Older Cargo versions ignore it. Source-Repo: https://github.com/servo/servo Source-Revision: df68eea3f21cc3bbf24d5bbb66be42c4e3a9e427

…t crates for cargo build (from servo:default-members); r=nox … and 'cargo test', etc. Include Servo and its unit tests, but not Stylo because that would try to compile the style crate with incompatible feature flags: rust-lang/cargo#4463 `workspace.default-members` was added in rust-lang/cargo#4743. Older Cargo versions ignore it. Source-Repo: https://github.com/servo/servo Source-Revision: df68eea3f21cc3bbf24d5bbb66be42c4e3a9e427 UltraBlame original commit: d1606d726592e798d5cafba5cbbf0f8326d67243

rust-highfive assigned matklad Nov 23, 2017

rust-highfive unassigned matklad Nov 23, 2017

SimonSapin mentioned this pull request Nov 23, 2017

Add a 'worskpace.default-packages' config to override implied --all? #4507

Closed

matklad reviewed Nov 23, 2017

View reviewed changes

lukaslueg reviewed Nov 24, 2017

View reviewed changes

nrc mentioned this pull request Nov 28, 2017

Fix frozen_flag_preserves_old_lockfile test #4758

Closed

SimonSapin force-pushed the default-members branch from c04b304 to d76c91b Compare November 29, 2017 08:08

SimonSapin added 5 commits November 29, 2017 10:59

Add a workspace.default-members config that overrides implied --all

ba7911d

When no package is selected with `-p`, the default behavior for virtual workspaces is to select every member as if `--all` were used.

Make default-members apply to non-virtual workspaces too

8d5e73c

Expand documentation for Workspace::default_members field

499a678

Update virtual workspace docs for implied --all

1240f42

rust-lang#4335

Document workspace.default-members

82d563b

SimonSapin force-pushed the default-members branch from d76c91b to 82d563b Compare November 29, 2017 09:59

bors added a commit that referenced this pull request Nov 29, 2017

Auto merge of #4743 - SimonSapin:default-members, r=matklad

5bb478a

Add a workspace.default-members config that overrides implied --all Fixes #4507.

bors merged commit 82d563b into rust-lang:master Nov 29, 2017

matklad added the relnotes Release-note worthy label Nov 29, 2017

SimonSapin mentioned this pull request Nov 29, 2017

Update Cargo to Wed Nov 29 15:19:05 2017 +0000 rust-lang/rust#46376

Merged

SimonSapin added a commit to SimonSapin/rust that referenced this pull request Nov 30, 2017

Update Cargo to Wed Nov 29 15:19:05 2017 +0000

2544b9f

rust-lang/cargo@5bb478a Pick up `workspace.default-members` support: rust-lang/cargo#4743

nrc reviewed Dec 3, 2017

View reviewed changes

SimonSapin mentioned this pull request Dec 4, 2017

Use workspace.default-members to specify default crates for cargo build servo/servo#19476

Merged

SimonSapin deleted the default-members branch December 4, 2017 11:44

ehuss added this to the 1.24.0 milestone Feb 6, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add a workspace.default-members config that overrides implied --all #4743

Add a workspace.default-members config that overrides implied --all #4743

SimonSapin commented Nov 23, 2017

rust-highfive commented Nov 23, 2017

SimonSapin commented Nov 23, 2017

matklad left a comment

matklad Nov 23, 2017

SimonSapin Nov 23, 2017

matklad Nov 23, 2017

lukaslueg Nov 24, 2017

SimonSapin Nov 24, 2017 •

edited

Loading

matklad commented Nov 28, 2017

alexcrichton commented Nov 28, 2017

SimonSapin commented Nov 28, 2017

SimonSapin commented Nov 28, 2017

SimonSapin commented Nov 28, 2017

matklad commented Nov 28, 2017

nrc commented Nov 28, 2017

alexcrichton commented Nov 29, 2017

matklad commented Nov 29, 2017

bors commented Nov 29, 2017

bors commented Nov 29, 2017

bors commented Nov 29, 2017

nrc Dec 3, 2017

SimonSapin Dec 6, 2017

		@@ -520,8 +520,19 @@ crate will be treated as a normal package, as well as a workspace. If the
		manifest*.

		When working with virtual manifests, package-related cargo commands, like

Add a workspace.default-members config that overrides implied --all #4743

Add a workspace.default-members config that overrides implied --all #4743

Conversation

SimonSapin commented Nov 23, 2017

rust-highfive commented Nov 23, 2017

SimonSapin commented Nov 23, 2017

matklad left a comment

Choose a reason for hiding this comment

matklad Nov 23, 2017

Choose a reason for hiding this comment

SimonSapin Nov 23, 2017

Choose a reason for hiding this comment

matklad Nov 23, 2017

Choose a reason for hiding this comment

lukaslueg Nov 24, 2017

Choose a reason for hiding this comment

SimonSapin Nov 24, 2017 • edited Loading

Choose a reason for hiding this comment

matklad commented Nov 28, 2017

alexcrichton commented Nov 28, 2017

SimonSapin commented Nov 28, 2017

SimonSapin commented Nov 28, 2017

SimonSapin commented Nov 28, 2017

matklad commented Nov 28, 2017

nrc commented Nov 28, 2017

alexcrichton commented Nov 29, 2017

matklad commented Nov 29, 2017

bors commented Nov 29, 2017

bors commented Nov 29, 2017

bors commented Nov 29, 2017

nrc Dec 3, 2017

Choose a reason for hiding this comment

SimonSapin Dec 6, 2017

Choose a reason for hiding this comment

SimonSapin Nov 24, 2017 •

edited

Loading