fix(publish): Move .crate out of final artifact location (rust-lang#15915)

When `target_dir == build_dir`, ensure `cargo publish` doesn't put
intermediate artifacts in the final artifact location of `cargo
package`.

### What does this PR try to resolve?

In rust-lang#15910, users could identify that `.crate` files from `cargo publish`
are not final artifacts by setting a custom `build-dir`. This extends
that to all users, ie when `build-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_dir` and `build_dir`.

### How to test and review this PR?

### Notes

We could optimize this further by doing a `rename` and only doing a copy
if that fails, effectively a `rename_or_copy` as opposed to our
`hardlink_or_copy` we normally use for uplifting. The difference is that
we don't do change tracking for `.crate` files but fully re-generate, so
we don't benefit from keeping the `.crate` around in the original
location.

Author: weihanglo

src/cargo/ops/cargo_package/mod.rs

@@ -163,11 +163,8 @@ fn create_package(
     let filename = pkg.package_id().tarball_name();
     let build_dir = ws.build_dir();
     paths::create_dir_all_excluded_from_backups_atomic(build_dir.as_path_unlocked())?;
-    let dir = build_dir.join("package");
-    let mut dst = {
-        let tmp = format!(".{}", filename);
-        dir.open_rw_exclusive_create(&tmp, gctx, "package scratch space")?
-    };
+    let dir = build_dir.join("package").join("tmp-crate");
+    let dst = dir.open_rw_exclusive_create(&filename, gctx, "package scratch space")?;
 
     // Package up and test a temporary tarball and only move it to the final
     // location if it actually passes all our tests. Any previously existing
@@ -179,14 +176,10 @@ fn create_package(
     let uncompressed_size = tar(ws, opts, pkg, local_reg, ar_files, dst.file(), &filename)
         .context("failed to prepare local package for uploading")?;
 
-    dst.seek(SeekFrom::Start(0))?;
-    let dst_path = dst.parent().join(&filename);
-    dst.rename(&dst_path)?;
-
     let dst_metadata = dst
         .file()
         .metadata()
-        .with_context(|| format!("could not learn metadata for: `{}`", dst_path.display()))?;
+        .with_context(|| format!("could not learn metadata for: `{}`", dst.path().display()))?;
     let compressed_size = dst_metadata.len();
 
     let uncompressed = HumanBytes(uncompressed_size);
@@ -220,23 +213,17 @@ pub fn package(ws: &Workspace<'_>, opts: &PackageOpts<'_>) -> CargoResult<Vec<Fi
 
     let packaged = do_package(ws, opts, pkgs)?;
 
+    // Uplifting artifacts
     let mut result = Vec::new();
     let target_dir = ws.target_dir();
-    let build_dir = ws.build_dir();
-    if target_dir == build_dir {
-        result.extend(packaged.into_iter().map(|(_, _, src)| src));
-    } else {
-        // Uplifting artifacts
-        paths::create_dir_all_excluded_from_backups_atomic(target_dir.as_path_unlocked())?;
-        let artifact_dir = target_dir.join("package");
-        for (pkg, _, src) in packaged {
-            let filename = pkg.package_id().tarball_name();
-            let dst =
-                artifact_dir.open_rw_exclusive_create(filename, ws.gctx(), "uplifted package")?;
-            src.file().seek(SeekFrom::Start(0))?;
-            std::io::copy(&mut src.file(), &mut dst.file())?;
-            result.push(dst);
-        }
+    paths::create_dir_all_excluded_from_backups_atomic(target_dir.as_path_unlocked())?;
+    let artifact_dir = target_dir.join("package");
+    for (pkg, _, src) in packaged {
+        let filename = pkg.package_id().tarball_name();
+        let dst = artifact_dir.open_rw_exclusive_create(filename, ws.gctx(), "uplifted package")?;
+        src.file().seek(SeekFrom::Start(0))?;
+        std::io::copy(&mut src.file(), &mut dst.file())?;
+        result.push(dst);
     }
 
     Ok(result)

tests/testsuite/build_dir.rs

@@ -524,8 +524,8 @@ fn cargo_package_should_build_in_build_dir_and_output_to_target_dir() {
 [ROOT]/foo/build-dir/package/foo-0.0.1/Cargo.toml
 [ROOT]/foo/build-dir/package/foo-0.0.1/Cargo.toml.orig
 [ROOT]/foo/build-dir/package/foo-0.0.1/src/main.rs
-[ROOT]/foo/build-dir/package/foo-0.0.1.crate
 [ROOT]/foo/build-dir/CACHEDIR.TAG
+[ROOT]/foo/build-dir/package/tmp-crate/foo-0.0.1.crate
 
 "#]]);
 

tests/testsuite/build_dir_legacy.rs

@@ -493,8 +493,8 @@ fn cargo_package_should_build_in_build_dir_and_output_to_target_dir() {
 [ROOT]/foo/build-dir/package/foo-0.0.1/Cargo.toml
 [ROOT]/foo/build-dir/package/foo-0.0.1/Cargo.toml.orig
 [ROOT]/foo/build-dir/package/foo-0.0.1/src/main.rs
-[ROOT]/foo/build-dir/package/foo-0.0.1.crate
 [ROOT]/foo/build-dir/CACHEDIR.TAG
+[ROOT]/foo/build-dir/package/tmp-crate/foo-0.0.1.crate
 
 "#]]);