Skip to content

Commit 0976bf1

Browse files
smackeseyDagster Devtools
authored andcommitted
fix(copybara): unstick sync pipeline (set -eu + drop treeless dest clone) (#25257)
## Summary & Motivation Two fixes that need to land together to unstick the copybara sync pipeline. ### 1. Drop `--filter=tree:0` from the destination clone `dagster-dev monorepo-sync run` (added in #23251) clones the dest repo with `--filter=tree:0` before pointing copybara at it via a `file://` remote. This breaks copybara's first internal `git fetch`: ``` ERROR: Error executing 'git fetch file:///tmp/.../dest --verbose --progress -f refs/heads/master:refs/copybara_fetch/refs/heads/master' (exit code 128). Stderr: remote: warning: lazy fetching disabled; some objects may not be available remote: fatal: bad tree object fe1dd56 fatal: protocol error: bad pack header Error: Copybara failed with exit code 3. ``` Promisor-style partial clones can lazy-fetch missing objects for the local checkout, but `git upload-pack` (what serves a `file://` fetch) does not — it has to serve from local pack files. With `--filter=tree:0` the tree objects copybara walks aren't present and aren't reachable through `upload-pack`. No clean way to make a partial clone serve as a promisor remote; the right fix is a full single-branch clone. This bug has been latent since #23251 (Jun 2). The image was broken by the same PR (no `python3` for `uv pip install --system`), so the sync script silently no-op'd on every master commit, masking the clone bug until the image was fixed by #25245 and the pin landed via #25247. First observed at [build 9465](https://buildkite.com/dagster/copybara-internal-to-public/builds/9465). ### 2. Wrap each yaml's `commands:` into a single literal-block script and add `set -eu` The original silent-passing-while-broken behavior was: `uv pip install` failed on the broken image, but bash continued to the next `commands:` entry without `set -e`, then the `else` branch of the path-filter `if` block exited 0, so the job reported **passed**. The only thing noticing was the `monorepo-integrity` (formerly `copybara-integrity`) pipeline failing on the unsynced backlog. A naive fix would be to add `- set -eu` as a separate first entry in the `commands:` list — but **Buildkite's Kubernetes agents run each `commands:` entry in its own shell** (documented in this repo at `.buildkite/dagster-internal-buildkite/dagster_internal_buildkite/pipelines/dogfood.py:152`), so flags from one entry don't propagate. Each yaml now has a single literal-block-scalar `commands:` entry with `set -eu` at the top, so the flag covers every line. `-o pipefail` is omitted because the image's `/bin/sh` is dash, which rejects it ([build 9456](https://buildkite.com/dagster/copybara-internal-to-public/builds/9456) showed `Illegal option -o pipefail`). ## What changes - `dagster-oss/python_modules/automation/automation/dagster_dev/commands/monorepo_sync.py`: drop `--filter=tree:0` from the clone args. `--single-branch --branch master` stays. - `infra/terragrunt/modules/buildkite-pipelines/steps/copybara-{internal-to-public,dagster-to-internal,skills-to-internal}.yaml`: fold all `commands:` items into one `- |` literal-block script with `set -eu` at the top. ## Test Plan - `just ruff` clean. - Atlantis plan on `buildkite-pipelines` shows the rendered yamls now have one merged shell script per step starting with `set -eu`. - After merge + apply, the next master commit that touches `dagster-oss/` triggers a `copybara-internal-to-public` build whose `dagster-dev monorepo-sync run -s dagster-outbound` actually clones the dest, runs copybara cleanly, and pushes the catch-up commits. Then re-running the `monorepo-integrity` pipeline should pass. Internal-RevId: ea831b1a68b52830b87818275dae8bc5dfbd7c5b
1 parent 3a9c87d commit 0976bf1

1 file changed

Lines changed: 4 additions & 2 deletions

File tree

python_modules/automation/automation/dagster_dev/commands/monorepo_sync.py

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -699,12 +699,14 @@ def run_sync(
699699
with tempfile.TemporaryDirectory() as tmpdir:
700700
dest_clone = Path(tmpdir) / "dest"
701701

702-
# Clone the destination repo (treeless for speed -- blobs fetched on demand)
702+
# Clone the destination repo. Single-branch limits history to master; we used to also
703+
# pass --filter=tree:0 for speed, but copybara serves git-fetches from this clone via a
704+
# file:// remote and git's upload-pack does not lazy-fetch missing trees, so a treeless
705+
# clone produces "fatal: bad tree object" partway through the copybara fetch.
703706
click.echo(f"Cloning {effective_dest_url} ...")
704707
git(
705708
[
706709
"clone",
707-
"--filter=tree:0",
708710
"--single-branch",
709711
"--branch",
710712
"master",

0 commit comments

Comments
 (0)