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.

Sign up for GitHub

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

Jump to bottom

Use the replacement source instead of the original source when generating -C metadata and -C extra-filename #14822

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

Use the replacement source instead of the original source when generating -C metadata and -C extra-filename #14822

wants to merge 1 commit into from

Conversation

P-E-Meunier
Copy link

@P-E-Meunier P-E-Meunier commented Nov 14, 2024 •
edited
Loading

What does this PR try to resolve?

This commit fixes an issue when replacing two different sources with the same local one (for example using cargo vendor), where rustc errs saying that two types "defined at the same place in the same file" are "different".

See #14821.

How should we test and review this PR?

Use the steps described in #14821.

Additional information

This is really useful in the context of private registries and vendored dependencies.

@rustbot
Copy link
Collaborator

rustbot commented Nov 14, 2024

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @epage (or someone else) some time within the next two weeks.

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

  • @rustbot author: the review is finished, PR author should check the comments and take action accordingly
  • @rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

@rustbot rustbot added A-cargo-targets Area: selection and definition of targets (lib, bins, examples, tests, benches) A-layout Area: target output directory layout, naming, and organization S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 14, 2024
@P-E-Meunier
Use the replacement source instead of the original source when genera…
ad9cd00 
…ting `-C metadata` and `-C extra-filename`

This commit fixes an issue when replacing two different sources with the same local one (for example using `cargo vendor`), where `rustc` errs saying that two types "defined at the same place in the same file" are "different".
epage
epage reviewed Nov 14, 2024
View reviewed changes
src/cargo/core/compiler/build_runner/compilation_files.rs
Copy link
Contributor

Choose a reason for hiding this comment

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

To help us track this, #14729 hasn't been accepted yet

(we generally ask for issues to be accepted before posting PRs, see https://doc.crates.io/contrib/process/index.html)

Copy link
Author

Choose a reason for hiding this comment

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

Oops, I meant #14821, which is an issue. I updated the initial report.

Copy link
Author

Choose a reason for hiding this comment

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

These two PRs are independent, one is for cargo vendor, the other one for cargo build. Any of them would already improve the situation and remove hacky workarounds in our workflow.

@epage epage marked this pull request as draft November 14, 2024 18:20
@P-E-Meunier P-E-Meunier mentioned this pull request Nov 15, 2024
Open
@bors
Copy link
Contributor

bors commented Dec 18, 2024

☔ The latest upstream changes (presumably bd1db93) made this pull request unmergeable. Please resolve the merge conflicts.

@rustbot
Copy link
Collaborator

rustbot commented Dec 20, 2024

☔ The latest upstream changes (possibly 081d7ba) made this pull request unmergeable. Please resolve the merge conflicts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@epage epage epage left review comments

At least 1 approving review is required to merge this pull request.

Assignees

@epage epage

Labels
A-cargo-targets Area: selection and definition of targets (lib, bins, examples, tests, benches) A-layout Area: target output directory layout, naming, and organization S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@P-E-Meunier @rustbot @bors @epage