From 0886e93197b5c50945ab681d238096774277758a Mon Sep 17 00:00:00 2001 From: AlexReal Date: Fri, 18 Sep 2026 11:53:27 -0500 Subject: [PATCH] fix(fallback): repack only the target that provides the requested package MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A polyglot repository can declare a target rooted at generated output that its source tag cannot contain. `pack_all` fails on the first such target, so repacking a tag archive refused the whole repository and every package in it became unresolvable from github.com. Live case: `ORESoftware/ores-interfaces` declares `[targets.public-contracts] dir = "generated/public"`, which `.gitignore` excludes, so `zed pack` fails there too. That has nothing to do with the root package `oresoftware/ores-interfaces` a consumer actually asked for. `pack::pack_target` is the existing per-target body of `pack_all`, lifted into its own function; `pack_all` now calls it per target, so publishing is unchanged and digests still match byte for byte. The fallback selects the one target providing the requested package and packs only that. Verified with the registry pointed at http://127.0.0.1:9: the full ORESoftware/ores-compose graph installs — flags-2-env, ores-clis-core, ores-interfaces, ores-locks-and-leases, ores-otel/ores-interfaces and ores-contracts from github.com, next-loggers from a local checkout. cargo test --locked --lib: 619 passed. Co-Authored-By: Claude Opus 5 --- src/pack.rs | 113 ++++++++++++++++++++++++----------------- src/source_fallback.rs | 42 ++++++++++++--- 2 files changed, 100 insertions(+), 55 deletions(-) diff --git a/src/pack.rs b/src/pack.rs index 151515cdb..52c48a5bd 100644 --- a/src/pack.rs +++ b/src/pack.rs @@ -80,57 +80,74 @@ pub fn pack_all( let mut packages = Vec::with_capacity(manifest.targets.len()); for (target, _) in manifest.target_package_names() { - let mut derived = manifest - .manifest_for_target(&target) - .with_context(|| format!("target `{target}` disappeared during packing"))?; - let section = manifest - .targets - .get(&target) - .with_context(|| format!("target `{target}` disappeared during packing"))?; - let source = project.join(§ion.dir); - if !source.is_dir() { - bail!( - "target `{target}` source root `{}` is not a directory", - section.dir - ); - } + packages.push(pack_target(project, manifest, &target, Some(&output))?); + } + Ok(packages) +} - // A root target is the repository's canonical package. Older manifests - // named it `-repository` to avoid a schema-level name clash; - // the emitted artifact must nevertheless use the exact root identity - // so `zed install org/repository-name` works as expected. - if section.dir == "." { - derived.package.name = manifest.package.name.clone(); - } else if source.join(zed_interfaces::paths::MANIFEST_FILE).exists() { - bail!( - "target `{target}` contains its own {}; declare packages only in the repository-root manifest", - zed_interfaces::paths::MANIFEST_FILE - ); - } +/// Build the one artifact `pack_all` would emit for a single declared target. +/// +/// Resolving one package must not require every sibling target to be present: +/// a target rooted at generated output is absent from a source tag, and that +/// alone should not make the repository's other packages unresolvable. +pub(crate) fn pack_target( + project: &Path, + manifest: &Manifest, + target: &str, + out_dir: Option<&Path>, +) -> Result { + let output = out_dir + .map(Path::to_path_buf) + .unwrap_or_else(|| project.join(PACK_OUT_DIR)); + let mut derived = manifest + .manifest_for_target(target) + .with_context(|| format!("target `{target}` disappeared during packing"))?; + let section = manifest + .targets + .get(target) + .with_context(|| format!("target `{target}` disappeared during packing"))?; + let source = project.join(§ion.dir); + if !source.is_dir() { + bail!( + "target `{target}` source root `{}` is not a directory", + section.dir + ); + } - let ignore_rules = crate::publish_ignore::read_rules(&source)?; - let staging = tempfile::tempdir().context("create target packing directory")?; - copy_files(&source, staging.path(), &derived, &ignore_rules)?; - copy_root_legal_files(project, staging.path())?; - fs::write( - staging.path().join(zed_interfaces::paths::MANIFEST_FILE), - derived.to_toml_string()?, - )?; - - let packed = pack_format_with_ignore_rules( - staging.path(), - &derived, - Some(&output), - ArtifactFormat::TarGz, - &ignore_rules, - )?; - packages.push(PackagedTarget { - target: Some(target), - manifest: derived, - packed, - }); + // A root target is the repository's canonical package. Older manifests + // named it `-repository` to avoid a schema-level name clash; + // the emitted artifact must nevertheless use the exact root identity + // so `zed install org/repository-name` works as expected. + if section.dir == "." { + derived.package.name = manifest.package.name.clone(); + } else if source.join(zed_interfaces::paths::MANIFEST_FILE).exists() { + bail!( + "target `{target}` contains its own {}; declare packages only in the repository-root manifest", + zed_interfaces::paths::MANIFEST_FILE + ); } - Ok(packages) + + let ignore_rules = crate::publish_ignore::read_rules(&source)?; + let staging = tempfile::tempdir().context("create target packing directory")?; + copy_files(&source, staging.path(), &derived, &ignore_rules)?; + copy_root_legal_files(project, staging.path())?; + fs::write( + staging.path().join(zed_interfaces::paths::MANIFEST_FILE), + derived.to_toml_string()?, + )?; + + let packed = pack_format_with_ignore_rules( + staging.path(), + &derived, + Some(&output), + ArtifactFormat::TarGz, + &ignore_rules, + )?; + Ok(PackagedTarget { + target: Some(target.to_string()), + manifest: derived, + packed, + }) } fn copy_files( diff --git a/src/source_fallback.rs b/src/source_fallback.rs index ce33d2d86..930178173 100644 --- a/src/source_fallback.rs +++ b/src/source_fallback.rs @@ -948,14 +948,42 @@ fn repack_tag_archive(raw: &Path, org: &str, name: &str, version: &str, dest: &P // checkout's untracked inputs, which a tag archive cannot contain. let manifest = crate::pack_inputs::harden_manifest(crate::pack_guard::harden_manifest(manifest)); - let packages = crate::pack::pack_all(&tree, &manifest, Some(&staging.path().join("out"))) - .context("repack GitHub tag archive")?; - let root = packages - .into_iter() - .find(|package| package.manifest.package.name == name) - .with_context(|| { - format!("GitHub tag archive for {org}/{name}@{version} publishes no root package") + let out = staging.path().join("out"); + // Pack only the target that provides the requested package. Packing every + // declared target would fail on a sibling rooted at generated output that + // a source tag cannot contain, making this package unresolvable for a + // reason that has nothing to do with it. + let root = if manifest.is_polyglot() { + let mut selected = None; + for (target, target_package) in manifest.target_package_names() { + let is_root = manifest + .targets + .get(&target) + .is_some_and(|section| section.dir == "."); + let provided = if is_root { + manifest.package.name.clone() + } else { + target_package + }; + if provided == name { + selected = Some(target); + break; + } + } + let target = selected.with_context(|| { + format!("GitHub tag archive for {org}/{name}@{version} publishes no such package") })?; + crate::pack::pack_target(&tree, &manifest, &target, Some(&out)) + .context("repack GitHub tag archive")? + } else { + crate::pack::pack_all(&tree, &manifest, Some(&out)) + .context("repack GitHub tag archive")? + .into_iter() + .find(|package| package.manifest.package.name == name) + .with_context(|| { + format!("GitHub tag archive for {org}/{name}@{version} publishes no root package") + })? + }; // Callers hand over a store cache path whose directory may not exist yet // (a frozen fetch uses a fresh isolated store); `download_url` creates it // for the other locators, so this one must too.