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(vendor): Strip excluded build targets #14367

Merged
merged 5 commits into from
Aug 7, 2024
Merged

Conversation

epage
Copy link
Contributor

@epage epage commented Aug 7, 2024

What does this PR try to resolve?

Fixes #14348

How should we test and review this PR?

The commits are setup to be able to show, through the tests, how the cargo package and cargo vendor behavior diverge and then how cargo vendors behavior changes over time.

This is a very hacky solution, duplicating the minimum of what
prepare_for_publish does to fix this one issue and in the least
intrusive way to the vendor code.

The intention is to keep this low risk for backporting to beta and
stable.
We need to revisit this, refactoring the cargo package code so that we
can call into that for each vendored dependency.

Additional information

Since we already cover this for `cargo package` and we turn all
implicit targets into explicit targets (making implicit tests cover
explicit cases), this becomes redundant.
This is a **very** hacky solution, duplicating the minimum of what
`prepare_for_publish` does to fix this one issue and in the least
intrusive way to the vendor code.

The intention is to keep this low risk for backporting to beta and
stable.
We need to revisit this, refactoring the `cargo package` code so that we
can call into that for each vendored dependency.

Fixes rust-lang#14348
@rustbot
Copy link
Collaborator

rustbot commented Aug 7, 2024

r? @weihanglo

rustbot has assigned @weihanglo.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added A-manifest Area: Cargo.toml issues Command-vendor S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 7, 2024
src/cargo/ops/vendor.rs Show resolved Hide resolved
tests/testsuite/vendor.rs Show resolved Hide resolved
@epage
Copy link
Contributor Author

epage commented Aug 7, 2024

@bors r=weihanglo

@bors
Copy link
Contributor

bors commented Aug 7, 2024

📌 Commit 1ae77f5 has been approved by weihanglo

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 7, 2024
@epage epage added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. beta-nominated Nominated to backport to the beta branch. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Aug 7, 2024
@bors
Copy link
Contributor

bors commented Aug 7, 2024

⌛ Testing commit 1ae77f5 with merge f50c592...

@weihanglo
Copy link
Member

I don't think it is controversial for beta backport. #14366 contains two commits to make the beta channel CI happy. @epage, are you working on the backport? I have some free time now and can handle it if needed.

@epage
Copy link
Contributor Author

epage commented Aug 7, 2024

I can do the beta backport.

@bors
Copy link
Contributor

bors commented Aug 7, 2024

☀️ Test successful - checks-actions
Approved by: weihanglo
Pushing f50c592 to master...

@bors bors merged commit f50c592 into rust-lang:master Aug 7, 2024
22 checks passed
@epage epage deleted the vendor branch August 7, 2024 21:31
bors added a commit that referenced this pull request Aug 7, 2024
[beta-1.81] fix(vendor): Strip excluded build targets

Beta backports

* #14367

In order to make CI pass, the following PRs are also cherry-picked:

* Ignore `build_std::basic` test (see also #14366)
* 830db6f (#14341, see also #14366)
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 8, 2024
Update cargo

3 commits in 94977cb1fab003d45eb5bb108cb5e2fa0149672a..0d8d22f83b066503f6b2b755925197e959e58b4f
2024-08-06 21:42:10 +0000 to 2024-08-08 12:54:24 +0000
- fix: std Cargo.lock moved to `library` dir (rust-lang/cargo#14370)
- fix(vendor): Strip excluded build targets (rust-lang/cargo#14367)
- Infer registry (rust-lang/cargo#14340)

r? ghost
@rustbot rustbot added this to the 1.82.0 milestone Aug 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-manifest Area: Cargo.toml issues beta-nominated Nominated to backport to the beta branch. Command-vendor S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cargo vendor regression for git sources containing exclude = ["build.rs"]
4 participants