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

Revert "Enable new behavior of --feature" #5430

Merged
merged 1 commit into from
Apr 28, 2018

Conversation

matklad
Copy link
Member

@matklad matklad commented Apr 28, 2018

This reverts commit 038eec5.

As discussed at #5364, the new behavior unfortunately causes real-life breakage, so we have to revert it.

This is kinda sad, this is a part of the larger issue with feature selection, which, at the moment, has a behavior which I would classify (loosely speaking) as unsound:

  • cargo build -p foo and cargo build -p foo -p bar might produce different artifacts for foo (repro)
  • cargo build -p foo might produce different artifacts, depending on cwd (repro)

The new feature behavior specifically addressed the second point.

It is unclear what we could do with this... One option, instead of flatly erroring out, as the revreted commit does, is to print a warning, but change the set of activated features. It will still be a breaking change, but it at least has a chance of working by accident.

r? @alexcrichton

@alexcrichton
Copy link
Member

@bors: r+

@bors
Copy link
Contributor

bors commented Apr 28, 2018

📌 Commit d369f97 has been approved by alexcrichton

@bors
Copy link
Contributor

bors commented Apr 28, 2018

⌛ Testing commit d369f97 with merge 0ad8f62159c12a63c729db33236b556b97d7dfa6...

@bors
Copy link
Contributor

bors commented Apr 28, 2018

💔 Test failed - status-travis

@alexcrichton
Copy link
Member

@bors: retry

@bors
Copy link
Contributor

bors commented Apr 28, 2018

⌛ Testing commit d369f97 with merge 5b125ef99c621ada116dfa7c386d98a1611e9427...

@bors
Copy link
Contributor

bors commented Apr 28, 2018

💔 Test failed - status-appveyor

@alexcrichton
Copy link
Member

@bors: retry

@bors
Copy link
Contributor

bors commented Apr 28, 2018

⌛ Testing commit d369f97 with merge 122fd5b...

bors added a commit that referenced this pull request Apr 28, 2018
Revert "Enable new behavior of `--feature`"

This reverts commit 038eec5.

As discussed at #5364, the new behavior unfortunately causes real-life breakage, so we have to revert it.

This is kinda sad, this is a part of the larger issue with feature selection, which, at the moment, has a behavior which I would classify (loosely speaking) as unsound:

* `cargo build -p foo` and `cargo build -p foo -p bar` might produce different artifacts for `foo` ([repro](https://github.com/matklad/workspace-vs-feaures))
* `cargo build -p foo` might produce different artifacts, depending on cwd ([repro](https://github.com/matklad/features-cwd))

The new feature behavior specifically addressed the second point.

It is unclear what we could do with this... One option, instead of flatly erroring out, as the revreted commit does, is to print a warning, but change the set of activated features. It will still be a breaking change, but it at least has  a chance of working by accident.

r? @alexcrichton
@bors
Copy link
Contributor

bors commented Apr 28, 2018

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

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