-
Couldn't load subscription status.
- Fork 2.7k
fix(publish): Move .crate out of final artifact location
#15915
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
base: master
Are you sure you want to change the base?
Conversation
|
r? @weihanglo rustbot has assigned @weihanglo. Use |
| let dir = ws.build_dir().join("package").join("tmp-crate"); | ||
| let dst = dir.open_rw_exclusive_create(&filename, gctx, "package scratch space")?; |
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.
As this will cause a behavior change, Ive marked this as a draft while we wait for #15910 to propagate to users so they can prepare.
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.
Ready for merge?
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.
For myself, if we want an intermediate period, I feel like more than one release is probably needed.
This comment has been minimized.
This comment has been minimized.
When `target_dir == build_dir`, ensure `cargo publish` doesn't put intermediate artifacts in the final artifact location of `cargo package`. If anyone was relying on this behavior of `cargo publish`, it will break them. We could avoid this and instead consider the location change to be part of the opt-in of using `build-dir` (until we make it opt-out). Note that we expect to be able to change the layouf of content written to `build-dir` even if users aren't opting in. On the other hand, this will help identify people relying on intermediate artifacts. While there aren't any performance benefits to this, it consolidates all of the uplifting logic and avoids dealing with overlapping `target_dir` and `build_dir`. We could optimize this further by doing a `rename` and only doing a copy if that fails.
d742813 to
8cc523d
Compare
When
target_dir == build_dir, ensurecargo publishdoesn't put intermediate artifacts in the final artifact location ofcargo package.What does this PR try to resolve?
In #15910, users could identify that
.cratefiles fromcargo publishare not final artifacts by setting a custombuild-dir. This extends that to all users, ie whenbuild-dir = target-dir(the default currently), making it clear that these files are internal.This also cleans things up by consolidating all of the uplifting logic and avoids dealing with overlapping
target_dirandbuild_dir.How to test and review this PR?
Notes
We could optimize this further by doing a
renameand only doing a copy if that fails, effectively arename_or_copyas opposed to ourhardlink_or_copywe normally use for uplifting. The difference is that we don't do change tracking for.cratefiles but fully re-generate, so we don't benefit from keeping the.cratearound in the original location.