Less aggressively poison sources on builds #5288
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
I think we'll want to backport this to beta as well |
matklad
reviewed
Apr 3, 2018
tests/testsuite/freshness.rs
Outdated
| authors = [] | ||
|
|
||
| #[dependencies] | ||
| #registry2 = { version = "*", optional = true } |
There was a problem hiding this comment.
Oh I left that in by accident, either version of this triggers a bug and I was confirming that they both do
There was a problem hiding this comment.
r=me when tests exercise both non-transitive and optional conditions
matklad
reviewed
Apr 3, 2018
tests/testsuite/freshness.rs
Outdated
| [dev-dependencies] | ||
| registry2 = "*" | ||
| "#, | ||
| ) |
There was a problem hiding this comment.
No need for raw literal at the next line
alexcrichton
force-pushed
the
another-fix
branch
from
af80341
to
9add385
Compare
|
Updated! |
matklad
reviewed
Apr 3, 2018
| authors = [] | ||
|
|
||
| [dev-dependencies] | ||
| registry2 = "*" |
There was a problem hiding this comment.
So this checks only non-transitive case? Let’s add a separate test for optional dep as well?
Discovered in rust-lang#5257 the changes in rust-lang#5215 were slightly too aggressively poisoning sources to require updates, thinking that a manifest changed when it actually hadn't. Non-workspace-member path dependencies with optional/dev-dependencies don't show up in the lock file, so the previous logic would recognize this and think that the dependency missing from the lock file was just added and would require a registry update. The fix in this commit effectively just skips all of these dependencies in non-workspace members. This means that this will be slightly buggy if an optional dependency that's activated is added, but that's hopefully something we can tackle later. Closes rust-lang#5257
alexcrichton
force-pushed
the
another-fix
branch
from
9add385
to
0790db9
Compare
|
@bors: r=matklad |
|
📌 Commit 0790db9 has been approved by |
|
⌛ Testing commit 0790db9 with merge 428b4a56a896ee3d57584e1b5fbb37821d59f2f4... |
|
💔 Test failed - status-travis |
|
@bors: retry
…On Tue, Apr 3, 2018 at 4:38 PM, bors ***@***.***> wrote:
💔 Test failed - status-travis
<https://travis-ci.org/rust-lang/cargo/builds/361855623?utm_source=github_status&utm_medium=notification>
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#5288 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAD95ApkicOzmC6d1UQPq1_TUKKM6b_2ks5tk-u8gaJpZM4TFl0_>
.
|
|
⌛ Testing commit 0790db9 with merge 6727709d01ebef705d3ef97bbaf587d9aa65d0e9... |
|
💔 Test failed - status-travis |
|
@bors: retry
…On Tue, Apr 3, 2018 at 4:56 PM, bors ***@***.***> wrote:
💔 Test failed - status-travis
<https://travis-ci.org/rust-lang/cargo/builds/361857496?utm_source=github_status&utm_medium=notification>
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#5288 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAD95M62vTMb_qeuFouN8SfT4An-ZsuZks5tk-_0gaJpZM4TFl0_>
.
|
|
⌛ Testing commit 0790db9 with merge 4e392fc60a50e733c3ca4d4b0533829c12290731... |
|
💔 Test failed - status-travis |
|
@bors: retry
…On Tue, Apr 3, 2018 at 5:18 PM, bors ***@***.***> wrote:
💔 Test failed - status-travis
<https://travis-ci.org/rust-lang/cargo/builds/361866326?utm_source=github_status&utm_medium=notification>
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#5288 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAD95JV6vqA8TViQVanzNZYTNqOskXijks5tk_UdgaJpZM4TFl0_>
.
|
bors
added a commit
that referenced
this pull request
Apr 3, 2018
Less aggressively poison sources on builds Discovered in #5257 the changes in #5215 were slightly too aggressively poisoning sources to require updates, thinking that a manifest changed when it actually hadn't. Non-workspace-member path dependencies with optional/dev-dependencies don't show up in the lock file, so the previous logic would recognize this and think that the dependency missing from the lock file was just added and would require a registry update. The fix in this commit effectively just skips all of these dependencies in non-workspace members. This means that this will be slightly buggy if an optional dependency that's activated is added, but that's hopefully something we can tackle later. Closes #5257
|
☀️ Test successful - status-appveyor, status-travis |
matklad
added a commit
to matklad/rust
that referenced
this pull request
Apr 4, 2018
This includes at least two notable changes:
* a regression is fixed where Cargo would update index on every
operation rust-lang/cargo#5288
* a new unstable `--out-dir` option is implemented
rust-lang/cargo#5203
|
Will do the backports! |
matklad
added a commit
to matklad/rust
that referenced
this pull request
Apr 4, 2018
rust-lang/cargo#5288 fixes a regression where cargo would update registry on every operation.
|
rustc backport PR: rust-lang/rust#49640 |
kennytm
added a commit
to kennytm/rust
that referenced
this pull request
Apr 4, 2018
Update Cargo
This includes at least two notable changes:
* a regression is fixed where Cargo would update index on every
operation rust-lang/cargo#5288
* a new unstable `--out-dir` option is implemented
rust-lang/cargo#5203
bors
added a commit
to rust-lang/rust
that referenced
this pull request
Apr 5, 2018
[beta] Update cargo and backport #49612 rust-lang/cargo#5288 fixes a regression where cargo would update registry on every operation.
alexcrichton
added a commit
to alexcrichton/cargo
that referenced
this pull request
Apr 6, 2018
This commit extends the fix in rust-lang#5288 by moving the logic added farther up in the loop over package dependencies. This means that we won't recursively look at optional/dev path dependencies which aren't members of the workspace. This should fix the new issue that came up in rust-lang#5257 Closes rust-lang#5257
bors
added a commit
that referenced
this pull request
Apr 6, 2018
Fix another issue of poisoning too eagerly This commit extends the fix in #5288 by moving the logic added farther up in the loop over package dependencies. This means that we won't recursively look at optional/dev path dependencies which aren't members of the workspace. This should fix the new issue that came up in #5257 Closes #5257
alexcrichton
added a commit
to alexcrichton/cargo
that referenced
this pull request
Apr 6, 2018
This commit extends the fix in rust-lang#5288 by moving the logic added farther up in the loop over package dependencies. This means that we won't recursively look at optional/dev path dependencies which aren't members of the workspace. This should fix the new issue that came up in rust-lang#5257 Closes rust-lang#5257
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Discovered in #5257 the changes in #5215 were slightly too aggressively
poisoning sources to require updates, thinking that a manifest changed when it
actually hadn't.
Non-workspace-member path dependencies with optional/dev-dependencies
don't show up in the lock file, so the previous logic would recognize this and
think that the dependency missing from the lock file was just added and would
require a registry update.
The fix in this commit effectively just skips all of these dependencies in
non-workspace members. This means that this will be slightly buggy if an
optional dependency that's activated is added, but that's hopefully something we
can tackle later.
Closes #5257