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

New semantics for --features flag #5353

Merged
merged 1 commit into from
Apr 13, 2018
Merged

New semantics for --features flag #5353

merged 1 commit into from
Apr 13, 2018

Conversation

matklad
Copy link
Member

@matklad matklad commented Apr 13, 2018

Historically, feature-related flags like --all-features,
--no-default-features and --features operated on the current
package. That is, cargo --package foo --feature feat would activate
feat for the package at the current working directory, and not for the
foo package. -Z package-features flag implements the more obvious
semantics for this combination of flags. This changes behavior, and that
is why we want to start with an unstable opt-in. The changes are:

  • --feature flag affects the selected package. It is an error to
    specify --feature with more than a single -p, or with -p outside
    workspace (the latter could work in theory, but would be hard to
    implement).

  • --all-features and --no-default-features affect all selected
    packages, and not the one at cwd.

  • The package in cwd is not implicitly enabled when doing feature
    selection. That is, cargo build -Z package-features -p foo could
    select less features for various packages than cargo build -p foo.

r? @alexcrichton

Historically, feature-related flags like `--all-features`,
`--no-default-features` and `--features` operated on the *current*
package. That is, `cargo --package foo --feature feat` would activate
`feat` for the package at the current working directory, and not for the
`foo` package. `-Z package-features` flag implements the more obvious
semantics for this combination of flags. This changes behavior, and that
is why we want to start with an unstable opt-in. The changes are:

* `--feature` flag affects the selected package. It is an error to
  specify `--feature` with more than a single `-p`, or with `-p` outside
  workspace (the latter could work in theory, but would be hard to
  implement).

* `--all-features` and `--no-default-features` affect all selected
  packages, and not the one at cwd.

* The package in `cwd` is not implicitly enabled when doing feature
  selection. That is, `cargo build -Z package-features -p foo` could
  select *less* features for various packages than `cargo build -p foo`.
@alexcrichton
Copy link
Member

@bors: r+

Nice!

@bors
Copy link
Contributor

bors commented Apr 13, 2018

📌 Commit 8d0b31b has been approved by alexcrichton

@bors
Copy link
Contributor

bors commented Apr 13, 2018

⌛ Testing commit 8d0b31b with merge 008c369...

bors added a commit that referenced this pull request Apr 13, 2018
New semantics for `--features` flag

Historically, feature-related flags like `--all-features`,
`--no-default-features` and `--features` operated on the *current*
package. That is, `cargo --package foo --feature feat` would activate
`feat` for the package at the current working directory, and not for the
`foo` package. `-Z package-features` flag implements the more obvious
semantics for this combination of flags. This changes behavior, and that
is why we want to start with an unstable opt-in. The changes are:

* `--feature` flag affects the selected package. It is an error to
  specify `--feature` with more than a single `-p`, or with `-p` outside
  workspace (the latter could work in theory, but would be hard to
  implement).

* `--all-features` and `--no-default-features` affect all selected
  packages, and not the one at cwd.

* The package in `cwd` is not implicitly enabled when doing feature
  selection. That is, `cargo build -Z package-features -p foo` could
  select *less* features for various packages than `cargo build -p foo`.

r? @alexcrichton
@bors
Copy link
Contributor

bors commented Apr 13, 2018

☀️ Test successful - status-appveyor, status-travis
Approved by: alexcrichton
Pushing 008c369 to master...

@bors bors merged commit 8d0b31b into rust-lang:master Apr 13, 2018
@matklad matklad deleted the features branch April 13, 2018 17:16
matklad added a commit to matklad/rust that referenced this pull request Apr 13, 2018
This includes rust-lang/cargo#5353,
which we might want to test via opt-in in the wild
kennytm added a commit to kennytm/rust that referenced this pull request Apr 14, 2018
Update Cargo

This includes rust-lang/cargo#5353, which we  want to test via opt-in in the wild.

This'll break RLS, the fix is rust-lang/rls#822
alexcrichton added a commit to alexcrichton/cargo that referenced this pull request May 21, 2018
This fixes an accidental regression introduced in rust-lang#5012 where the
`--all-features` CLI flag was only propagated to the "main crate" as opposed to
all workspace packages. This behavior has [already been deemed][pr] as
"basically not what you want", but for now it's best to avoid the regression.

Closes rust-lang#5518

[pr]: rust-lang#5353
bors added a commit that referenced this pull request May 24, 2018
Copy `--all-features` request to all workspace members

This fixes an accidental regression introduced in #5012 where the
`--all-features` CLI flag was only propagated to the "main crate" as opposed to
all workspace packages. This behavior has [already been deemed][pr] as
"basically not what you want", but for now it's best to avoid the regression.

Closes #5518

[pr]: #5353
alexcrichton added a commit to alexcrichton/cargo that referenced this pull request May 24, 2018
This fixes an accidental regression introduced in rust-lang#5012 where the
`--all-features` CLI flag was only propagated to the "main crate" as opposed to
all workspace packages. This behavior has [already been deemed][pr] as
"basically not what you want", but for now it's best to avoid the regression.

Closes rust-lang#5518

[pr]: rust-lang#5353
wking added a commit to wking/openshift-release that referenced this pull request Jan 31, 2020
…Check Cargo.lock, etc.

Require Cargo.lock to be fresh when we build.  Not sure if there's a
way to run this to only bump when required, but we generally want to
trust our dependencies to correctly SemVer and track their upstream
bugfixes, so I'm not too concerned about this failing after a
dependency releases a new patch version or whatever.

We don't need &&-chaining or anything, because this eventually gets
called with errexit.  For example, [1]:

  --> RUN ["/bin/bash","-c","set -o errexit; umask 0002; echo \"Checking Cargo.lock\" \u0026\u0026\ncargo generate-lockfile \u0026\u0026\ngit diff --exit-code \u0026\u0026\necho \"Building release binaries\" \u0026\u0026\ncargo build --locked --release\n"]

Also spit out the Rust and Cargo versions to make it easier to
understand why they recommend lockfile changes when they do recommend
changes.

The codegen-protoc business follows [2] and, with the 'git diff',
ensures the automatically-generated
cincinnati/src/plugins/interface.rs is up to date too.  The cd
business avoids [3]:

  error: --features is not allowed in the root of a virtual workspace

from:

  cargo build --features=codegen-protoc --release

and [4]:

  error: the `-Z` flag is only accepted on the nightly channel of Cargo, but this is the `stable` channel

from:

  cargo build -Z package-features --features=codegen-protoc --release -p cincinnati

because they were building in a root outside of the
cincinnati/Cargo.toml package that actually needs the feature flag
[5].

[1]: https://storage.googleapis.com/origin-ci-test/pr-logs/pull/openshift_release/6935/rehearse-6935-pull-ci-openshift-cincinnati-master-images/4/build-log.txt
[2]: https://github.com/openshift/cincinnati/tree/a03ccf39ce8633f8585272053bc595ac28fe9ed0#updating-the-plugin-interface-scheme
[3]: https://prow.svc.ci.openshift.org/view/gcs/origin-ci-test/pr-logs/pull/openshift_release/6935/rehearse-6935-pull-ci-openshift-cincinnati-master-images/8#1:build-log.txt%3A30
[4]: https://prow.svc.ci.openshift.org/view/gcs/origin-ci-test/pr-logs/pull/openshift_release/6935/rehearse-6935-pull-ci-openshift-cincinnati-master-images/10#1:build-log.txt%3A30
[5]: rust-lang/cargo#5353
@ehuss ehuss added this to the 1.27.0 milestone Feb 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants