-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
[stable-1.80] fix(vendor): Strip excluded build targets #14369
Conversation
|
@@ -220,7 +220,7 @@ fn sync( | |||
let pathsource = PathSource::new(src, id.source_id(), gctx); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is targeting rust-1.80.0
. Did that as a placeholder until we have rust-1.80.2
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That branch is for 1.80
and would never change as far as I can tell.
@@ -220,7 +220,7 @@ fn sync( | |||
let pathsource = PathSource::new(src, id.source_id(), gctx); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That branch is for 1.80
and would never change as far as I can tell.
FYI, this intentionallty is broken. I wanted to get this up for the conversation but I still need to work through the build breakages from other parts of cargo being different and then figure out what unblocking backports are needed. |
Team member @weihanglo has proposed to merge this. The next step is review by the rest of the tagged team members: No concerns currently listed. Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up! See this document for info about what commands tagged team members can give me. |
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
FYI I've given a heads up to the release team at https://rust-lang.zulipchat.com/#narrow/stream/241545-t-release/topic/1.2E80.2E2 |
Should we bump the version of cargo crate to 0.81.1? |
According to @pietroalbini, this doesn't meet the bar for a new point release, only piggy-backing off of an existing point release, as this is limited to This aligns with our last |
Stable backports
In order to make CI pass, the following PRs are also cherry-picked: