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

Fix #2595 by skipping reflexive replacements #2879

Merged
merged 4 commits into from
Jul 14, 2016

Conversation

Ericson2314
Copy link
Contributor

@Ericson2314 Ericson2314 commented Jul 14, 2016

If you know of something better than source_id == source_id, I'd gladly change it.

@rust-highfive
Copy link

r? @alexcrichton

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

@alexcrichton
Copy link
Member

Could you explain a bit at a high level what happened here? I'm trying to decipher this from the commit itself but "Replace bug" isn't very helpful nor is the PR description...

@Ericson2314
Copy link
Contributor Author

@alexcrichton Oh sorry! I thought I already edited it to take the top line of the last commit not the branch name.

@Ericson2314 Ericson2314 changed the title Replace bug Fix #2595 by skipping reflexive replacements Jul 14, 2016
@Ericson2314
Copy link
Contributor Author

Ok so before this, if a dependency matches the replacement, it will be replaced with itself (at least according to the lockfile). Replaced packages don't have their dependencies listed, so while the reflexive replacement itself works, Cargo assume the package has zero deps.

In the example, local replaces near with itself, and near depends on far. Right now that test fails when near tries to extern far.

@alexcrichton
Copy link
Member

Ok, thanks! That sounds like it'd definitely do the trick.

@Ericson2314
Copy link
Contributor Author

NP! Again sorry about the awful PR title.

* {}\n * {}\n\nboth specifications match: {}",
matched_spec.unwrap(), spec, summary.package_id());
}
// get around lack of non-lexical lifetimes
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's ok to remove comments like this

Copy link
Contributor Author

@Ericson2314 Ericson2314 Jul 14, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should I force--push? (I wrote that, as opposed to just moved it around, to be clear.)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it bounces sure but otherwise it's fine

@alexcrichton
Copy link
Member

@bors: r+ 3f655e5

Looks good to me, thanks @Ericson2314!

@bors
Copy link
Contributor

bors commented Jul 14, 2016

⌛ Testing commit 3f655e5 with merge 970535d...

bors added a commit that referenced this pull request Jul 14, 2016
Fix #2595 by skipping reflexive replacements

If you know of something better than `source_id == source_id`, I'd gladly change it.
@bors
Copy link
Contributor

bors commented Jul 14, 2016

☀️ Test successful - cargo-cross-linux, cargo-linux-32, cargo-linux-64, cargo-mac-32, cargo-mac-64, cargo-win-gnu-32, cargo-win-gnu-64, cargo-win-msvc-32, cargo-win-msvc-64
Approved by: alexcrichton
Pushing 970535d to master...

@bors bors merged commit 3f655e5 into rust-lang:master Jul 14, 2016
@Ericson2314 Ericson2314 deleted the replace-bug branch July 15, 2016 05:00
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