Skip to content

Commit 78a5531

Browse files
authored
fix(publish): Don't generate final artifacts (#15910)
### What does this PR try to resolve? When splitting `build-dir` out of `target-dir` (see #14125), we kept `.crate` files as final artifacts of `cargo package`. However, that also made them final artifacts of `cargo publish` when the side effect of that should be the publish operation. This changes things so that we instead package within `build-dir` and `cargo package` then uplifts these to `target-dir`. Behavior change: when running `cargo publish` without `build-dir` set, `target/package/*.crate` will be moved to `target/package/tmp-crate/*.crate`. This should be fine as its an intermediate artifact but there is a risk that this can break someone who did not realize that. Part of the reason for making the `build-dir` / `target-dir` split is to make our intermediate/final artifact intent clearer. We did intend to be a transition period before changing things, like with #15010. I'm assuming its fine that we don't have a transition period here. ### How to test and review this PR? ### Notes This was discussed at [#t-cargo > `-Zbuild-dir` and `cargo publish` / `cargo package` @ 💬](https://rust-lang.zulipchat.com/#narrow/channel/246057-t-cargo/topic/.60-Zbuild-dir.60.20and.20.60cargo.20publish.60.20.2F.20.60cargo.20package.60/near/537336819)
2 parents 05bf93c + c8a15f3 commit 78a5531

File tree

3 files changed

+56
-6
lines changed

3 files changed

+56
-6
lines changed

src/cargo/ops/cargo_package/mod.rs

Lines changed: 22 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -160,7 +160,7 @@ fn create_package(
160160
}
161161

162162
let filename = pkg.package_id().tarball_name();
163-
let dir = ws.target_dir().join("package");
163+
let dir = ws.build_dir().join("package");
164164
let mut dst = {
165165
let tmp = format!(".{}", filename);
166166
dir.open_rw_exclusive_create(&tmp, gctx, "package scratch space")?
@@ -217,10 +217,27 @@ pub fn package(ws: &Workspace<'_>, opts: &PackageOpts<'_>) -> CargoResult<Vec<Fi
217217
// So we need filter
218218
pkgs.retain(|(pkg, _feats)| specs.iter().any(|spec| spec.matches(pkg.package_id())));
219219

220-
Ok(do_package(ws, opts, pkgs)?
221-
.into_iter()
222-
.map(|x| x.2)
223-
.collect())
220+
let packaged = do_package(ws, opts, pkgs)?;
221+
222+
let mut result = Vec::new();
223+
let target_dir = ws.target_dir();
224+
let build_dir = ws.build_dir();
225+
if target_dir == build_dir {
226+
result.extend(packaged.into_iter().map(|(_, _, src)| src));
227+
} else {
228+
// Uplifting artifacts
229+
let artifact_dir = target_dir.join("package");
230+
for (pkg, _, src) in packaged {
231+
let filename = pkg.package_id().tarball_name();
232+
let dst =
233+
artifact_dir.open_rw_exclusive_create(filename, ws.gctx(), "uplifted package")?;
234+
src.file().seek(SeekFrom::Start(0))?;
235+
std::io::copy(&mut src.file(), &mut dst.file())?;
236+
result.push(dst);
237+
}
238+
}
239+
240+
Ok(result)
224241
}
225242

226243
/// Packages an entire workspace.

src/doc/src/reference/build-cache.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ the `target` directory:
6262
Directory | Description
6363
----------|------------
6464
<code style="white-space: nowrap">target/doc/</code> | Contains rustdoc documentation ([`cargo doc`]).
65-
<code style="white-space: nowrap">target/package/</code> | Contains the output of the [`cargo package`] and [`cargo publish`] commands.
65+
<code style="white-space: nowrap">target/package/</code> | Contains the output of the [`cargo package`].
6666

6767
Cargo also creates several other directories and files in the build-dir needed for the build
6868
process. The build-dir layout is considered internal to Cargo, and is subject to

tests/testsuite/build_dir.rs

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
use std::path::PathBuf;
1313

1414
use crate::prelude::*;
15+
use cargo_test_support::registry::RegistryBuilder;
1516
use cargo_test_support::{Project, prelude::*};
1617
use cargo_test_support::{paths, project, str};
1718
use std::env::consts::{DLL_PREFIX, DLL_SUFFIX, EXE_SUFFIX};
@@ -359,6 +360,38 @@ fn cargo_package_should_build_in_build_dir_and_output_to_target_dir() {
359360
assert!(package_build_dir.join("foo-0.0.1").is_dir());
360361
}
361362

363+
#[cargo_test]
364+
fn cargo_publish_should_only_touch_build_dir() {
365+
let registry = RegistryBuilder::new().http_api().http_index().build();
366+
367+
let p = project()
368+
.file("src/main.rs", r#"fn main() { println!("Hello, World!") }"#)
369+
.file(
370+
".cargo/config.toml",
371+
r#"
372+
[build]
373+
target-dir = "target-dir"
374+
build-dir = "build-dir"
375+
"#,
376+
)
377+
.build();
378+
379+
p.cargo("publish")
380+
.replace_crates_io(registry.index_url())
381+
.enable_mac_dsym()
382+
.run();
383+
384+
assert_build_dir_layout(p.root().join("build-dir"), "debug");
385+
386+
let package_artifact_dir = p.root().join("target-dir/package");
387+
assert!(!package_artifact_dir.exists());
388+
389+
let package_build_dir = p.root().join("build-dir/package");
390+
assert_exists(&package_build_dir);
391+
assert_exists(&package_build_dir.join("foo-0.0.1"));
392+
assert!(package_build_dir.join("foo-0.0.1").is_dir());
393+
}
394+
362395
#[cargo_test]
363396
fn cargo_clean_should_clean_the_target_dir_and_build_dir() {
364397
let p = project()

0 commit comments

Comments
 (0)