diff --git a/.claude/skills/release-proposal-pr-review-bumps/SKILL.md b/.claude/skills/release-proposal-pr-review-bumps/SKILL.md new file mode 100644 index 0000000000..fefd347dfc --- /dev/null +++ b/.claude/skills/release-proposal-pr-review-bumps/SKILL.md @@ -0,0 +1,180 @@ +--- +name: release-proposal-pr-review-bumps +description: Review a libdatadog (or similar Rust workspace) release-proposal PR that bumps multiple crate versions, verifying each crate's semver bump (major/minor/patch) is correct based on its ACTUAL per-crate public-API delta — not just the conventional-commit `!` markers. Use when asked to "review a release PR", "check version bumps", "verify the semver bumps", or review a "chore(release): proposal..." PR. +--- + +# Reviewing release-proposal version bumps + +A release-proposal PR (usually authored by the release bot, e.g. `chore(release): proposal for ...`) bumps several crates at once. Each crate's bump is derived from the commits since its last release. The reviewer's job has **two layers**: + +1. **Per-crate:** confirm each crate's bump matches the **real public-API change in that specific crate** (the bulk of this skill). +2. **Workspace-level:** confirm the bumps are *consistent across the dependency graph* — specifically that every major bump has cascaded through its reverse-dependency closure (see "The major-version cascade" below). The release bot routinely gets this wrong: it bumps/releases only crates that have their own commits, while silently rewriting path-dependency requirements everywhere else. That produces under-bumped dependents that force a new major of a shared crate onto consumers without a version bump — the single most damaging defect in these PRs. **Always run this check; it is easy to miss because the under-bumped crate's own diff looks innocent.** + +## The core principle + +A commit marked breaking (`!` in its conventional-commit title, e.g. `feat(data-pipeline)!: ...`) is often a **multi-crate sweep**. The breaking change usually lands in only ONE crate; the same commit may touch other crates with purely additive or internal edits. So: + +- **Never** infer a crate's bump from the `!` marker alone. +- For each crate, look at **only that crate's slice** of each commit and classify the highest-severity change to *its own* public API. + +Bump rules (per crate, based on the highest-severity change): +- **major** — a breaking public-API change: removed/renamed/signature-changed `pub` item; changed `pub` struct field type; changed/removed enum variant; removed trait method; dropped public trait impl (e.g. a `derive` removed in default builds). +- **minor** — only additive: new `pub` items, nothing removed/changed. (Promoting a `pub(crate)`/private item to `pub`, or renaming a non-`pub` item, counts as additive — it was never externally visible.) +- **patch** — internal only: private code, `#[cfg(test)]`/`mod tests`, benches, `[dev-dependencies]`, comments, bug fixes with no public-API change. + +## Watch for these subtle cases + +1. **Sub-major bump carrying a `!` commit** (minor/patch crate that includes a breaking-marked commit) — the highest-priority thing to verify. Confirm the breaking part is NOT in this crate. +2. **Transitive breakage** — a crate that re-exports, or uses in a `pub` signature, a type from a dependency that changed. If the changed type leaks into the crate's public API, the crate breaks too. If it's only used internally / behind a trait with stable signatures, it does not. + - Check: does the crate `pub use` the changed type? Does any `pub fn`/struct field expose it directly (vs. being generic over a trait whose method signatures are unchanged)? +3. **Feature-gated breaks** — a breaking change behind a non-default Cargo feature is weaker justification for a major under strict default-feature semver. Note it, but a default-surface break elsewhere still independently justifies major. +4. **Forced-major dependency bumps are often breaking** (do NOT reflexively treat as patch). When crate A goes **major**, every dependent's `Cargo.toml` requirement on A is rewritten to A's new major (`^1` → `^2`), forcing A's new major onto the dependent's consumers. Whether that obliges the dependent to *also* go major depends on whether A is **safe to duplicate** — run the two-part test in "The major-version cascade". Short version: if A is a public dependency of the dependent (exposed type, or a foreign-trait-impl on a public type) **or** A is unsafe to duplicate (singleton/global state, single-artifact link) and consumers use `^` ranges, the dependent must go **major** too. (The older guidance "dep bump = patch" is wrong for these.) Minor/patch dependency bumps of A (same major) never cascade — `^1.2` already unifies with `1.3.0`. +5. **Initial releases** (e.g. `1.0.0`, CHANGELOG newly added, previously `publish = false`/unpublished) — nothing to semver-diff against; just confirm the version is sane and the crate was genuinely unpublished. +6. **Test/bench-only commits** — patch is the safe, conservative choice even when arguably no bump was needed. + +## The major-version cascade (workspace-level check) + +Two semver-incompatible majors of the same crate can be resolved into a single dependency tree by Cargo. Whether that is harmless or a "boom" depends entirely on whether the dependency is **safe to duplicate**. Whether the dependent needs a forced-major bump (the cascade) hinges on the same question. So before forcing majors up the graph, run the test below — don't blanket-cascade. + +### The "safe to duplicate" test — BOTH checks must pass + +A crate is safe to duplicate (two majors can coexist harmlessly) **only if both** of these hold. Failing *either* one makes duplication harmful and forces the cascade. Checking only the first is the classic mistake. + +**(a) No process-global / singleton state.** Grep the crate's `src` for anything that must be unique per process: + ```bash + grep -rnE 'static |lazy_static|once_cell|OnceLock|OnceCell|Lazy|thread_local|#\[no_mangle\]|#\[export_name|#\[ctor|atexit|pthread_atfork|signal\(' /src --include=*.rs + ``` + Hits on real globals, FFI exported symbols, ctors, or fork/signal/atexit handlers = duplication is unsafe (two copies fight over the same process resource or clash at link time, especially inside the single FFI/C `builder` artifact). Comments and instance-scoped registration (e.g. `AtomicWaker::register`, "worker registered *on a SharedRuntime instance*") do NOT count — confirm the constructor is an instance method (`Foo::new()`), not a global accessor (`fn global() -> &'static Foo`). + +**(b) No shared types/traits crossing a crate boundary.** Even a globally-stateless crate is unsafe to duplicate if its types or **traits** are part of the *integration contract* between two other crates — because v1's type/trait is a different type from v2's, so a value/impl from a v1-built crate won't satisfy a v2 bound. Check the dependents, not just the dependency: + - Does dependent X **expose the dep's type in its public API** (re-export, `pub fn` arg/return, `pub` field)? e.g. `pub fn set_shared_runtime(_: Arc)`. + - Does dependent X **implement a trait from the dep on one of X's public types**? A foreign-trait impl on a public type *is* public API. e.g. `impl libdd_shared_runtime::Worker for TelemetryWorker` — even though `SharedRuntime` never appears in telemetry's signatures, consumers rely on `TelemetryWorker: Worker`. + - Does some *other* crate Y then **consume that across the boundary**? e.g. data-pipeline calls `shared_runtime.spawn_worker(telemetry_worker)` (production, `trace_exporter/builder.rs`), which requires `TelemetryWorker: ::Worker`. If telemetry is on `shared-runtime ^1` and data-pipeline on `^2`, the trait impl targets the wrong major → **hard compile error**, not just redundant copies. + ```bash + # type in dependent's public API: + grep -rnE 'pub use .*|pub fn .*|pub .*: &?(mut )?|-> .*' /src --include=*.rs | grep -v cfg.test + # foreign-trait impls of the dep's traits on the dependent's types: + grep -rnE 'impl .* for ' /src --include=*.rs | grep -v cfg.test + # cross-crate consumption of that contract (e.g. spawn/register taking the impl): + grep -rnE '\(' /src --include=*.rs | grep -v cfg.test + ``` + +Worked example (`libdd-shared-runtime`): passes (a) — instance-based `SharedRuntime::new()`, zero globals — but **fails (b)**: `TelemetryWorker` implements its `Worker` trait and data-pipeline spawns that worker on a `SharedRuntime` in production. So telemetry, data-pipeline, and any `^`-range consumer (e.g. dd-trace-rs) must all agree on one shared-runtime major → the cascade is a genuine correctness requirement here, not conservative over-bumping. + +### Classifying the dependent's bump once duplication is unsafe + +- If the dep is a **public dependency** of the dependent (fails (b): exposed type *or* foreign-trait-impl-on-public-type) → the dependent's own public contract changed with the dep's major → **major is correct, not weird**. This is the real public-dependency case. +- If the dep is **private** to the dependent (passes (b): used only internally, never crossing a boundary) but **fails (a)** (singleton/global, or single-artifact link) → the dependent's API is technically unchanged, so strict SemVer would allow patch — but a patch/minor is auto-picked by `^`-range consumers during `cargo update`, silently dragging in the incompatible major. So bump **major anyway** to force a deliberate, loud upgrade. (Major doesn't *prevent* the diamond; it makes it opt-in instead of a silent surprise.) +- If the dep passes **both** (a) and (b) → genuinely safe to duplicate → **no cascade**; a patch re-release (for manifest coherence) or nothing is fine. + +So a major bump of a non-duplicable shared crate **cascades as a major bump through its reverse-dependency closure of publishable crates**, in topological order. Each crate in the closure that gains a new-major requirement (directly or transitively, e.g. `crashtracker → telemetry → shared-runtime`) must itself go major, which in turn forces *its* dependents major, and so on. + +Rules of thumb: +- **Cascade triggers ONLY on major dependency bumps.** Minor/patch dep bumps (same major) never cascade. +- **`publish = false` crates don't need a version bump** (no registry artifact / no `^`-range consumers), but their path-dep requirements must still be internally consistent. +- A crate **already in the release list can still be under-bumped** by this rule — e.g. a crate correctly classified `patch` on its *own* API but which directly depends on a major-bumped shared crate must be upgraded to `major`. Check in-list crates against the cascade too, not just the omitted ones. +- Confirm how downstream actually pins libdatadog. If every consumer pins the *whole workspace at one exact version*, the duplicate-major hazard can't arise and the cascade is moot — but for any crate consumed independently with `^` ranges, it is real. + +## Workflow + +1. **Fetch the PR** with `gh pr view --json title,body,headRefName,baseRefName,files,commits`. The body lists each crate, its next version, the bump type, and the attributed commits. (Base ref may be another `release/...` branch in a stacked release — review only the crates in the body.) +2. **Resolve commits locally.** For each PR number in the body: `git log --oneline --all --grep="(#)" -1`. Confirm all are present. +3. **Map each commit's crate footprint** so you know which commits are multi-crate sweeps: + ``` + git show --stat --format= | grep -oE '^ [a-zA-Z0-9_./-]+' | sed 's,/.*,,' | sort | uniq -c | sort -rn + ``` +4. **Fan out one subagent per crate** (run them concurrently — multiple Agent calls in one message). Give each subagent: the crate name, proposed next version + bump, the attributed commit hashes (flagging which are `!`-marked multi-crate sweeps), and the method below. Prioritize the sub-major-with-`!` cases. +5. **Each subagent's method:** + - Inspect ONLY the crate's slice: `git show -- /` (or `/src/` to skip tests). + - Verify public reachability: is the changed item reachable from the crate root (`pub mod` chain in `lib.rs`, `pub use` re-exports)? `#[cfg(test)]`/`mod tests`/`benches/` and private items don't count. + - Classify highest severity (major/minor/patch) with **evidence**: file path, item name, before/after signature. + - Check transitive breakage via re-exports and `pub` signatures (point 2 above) and `Cargo.toml` dep changes. + - Return a verdict: is the proposed bump correct, too low, or too high — with cited evidence. +6. **Run the major-version cascade check** (workspace-level — do this whenever ANY crate in the proposal gets a *major* bump). For each major-bumped crate, compute its reverse-dependency closure among publishable workspace crates and confirm every crate in it is also bumped **major**. A helper to build the closure and surface under-bumped crates against the proposal head/base refs: + ```bash + python3 - "$HEAD_REF" "$BASE_REF" <<'PY' + import subprocess, re, sys, os + head, base = sys.argv[1], sys.argv[2] + MAJOR_BUMPED = {"libdd-shared-runtime","libdd-trace-utils","libdd-data-pipeline"} # set to the crates getting a MAJOR bump in this proposal + def manifest(ref,d): + try: return subprocess.check_output(["git","show",f"{ref}:{d}/Cargo.toml"],stderr=subprocess.DEVNULL).decode() + except Exception: return "" + def parse(ref,d): + t=manifest(ref,d) + if not t: return None + ver=re.search(r'(?m)^version\s*=\s*"([^"]+)"',t) + publish = not re.search(r'(?m)^publish\s*=\s*false',t) + cut=len(t) + for mk in ("[dev-dependencies]","[build-dependencies]"): # normal deps only + i=t.find(mk); cut=min(cut,i) if i!=-1 else cut + deps=set(re.findall(r'(?m)^(libdd-[a-z0-9-]+)\s*=',t[:cut])) + return {"ver":ver.group(1) if ver else "?","publish":publish,"deps":deps} + dirs=[d for d in os.listdir(".") if os.path.isfile(os.path.join(d,"Cargo.toml"))] + info={d:parse(head,d) for d in dirs}; info={k:v for k,v in info.items() if v} + # transitive reverse-dependency closure + targets=set(MAJOR_BUMPED); changed=True + while changed: + changed=False + for d,m in info.items(): + if d not in targets and (m["deps"] & targets): + targets.add(d); changed=True + print(f"{'crate':28} {'pub':4} {'base_ver':10} {'head_ver':10} bumped? deps-on-major") + for d in sorted(targets - MAJOR_BUMPED): + m=info[d]; b=parse(base,d) + bumped = "MAJOR" if (b and b['ver'].split('.')[0]!=m['ver'].split('.')[0]) else "** NOT-MAJOR **" + direct=sorted(m["deps"] & MAJOR_BUMPED) + print(f"{d:28} {'PUB' if m['publish'] else '-':4} {(b['ver'] if b else '?'):10} {m['ver']:10} {bumped:16} {direct}") + PY + ``` + Any `PUB` crate flagged `** NOT-MAJOR **` is a defect: it either needs adding to the release as a major bump, or (if already in the list) its bump needs raising to major. Remember the intra-closure requirement edges must also move to the new majors (e.g. `crashtracker → telemetry ^N`). +7. **Synthesize** a verdict table (crate | proposed | correct? | why) plus the cascade findings and any non-blocking notes (changelog accuracy, feature-gated breaks). + +## What the automated level cannot see + +`scripts/semver-level.sh` (and the `pr-title-semver-check` job built on it) runs +`cargo-semver-checks` plus a `cargo-public-api` diff. Treat its answer as a **floor, not a +verdict**: a `patch` result is only trustworthy for changes that touch none of the +categories below. Each is pinned by a test in `scripts/tests/semver-level/` +(`detection_matrix.bats`, grep `KNOWN MISS`), verified against cargo-semver-checks 0.47.0 +and cargo-public-api 0.52.0 — so if one starts being detected, that suite is what tells you. + +Manually check these whenever the automated level is `patch` or `minor`: + +1. **`#[repr(C)]` field reordering.** `repr_c_plain_struct_fields_reordered` is a + **warning-level** lint: cargo-semver-checks prints `Summary no semver update required` + and exits 0, so the script never sees it. This is the one that matters most here — it is + a silent ABI break for every FFI consumer compiled against the old header, and it reports + as `patch`. The rest of the repr family (`repr_c_removed`, `repr_align_changed`, + `repr_packed_added`, `enum_repr_int_changed`) fails properly. **Check any diff that + touches field order in a `#[repr(C)]` type.** +2. **Public dependency major bumps behind unchanged signatures.** `pub fn f(u: hyper::Uri)` + renders identically whether `hyper` is 0.14 or 1.0; only the resolved dependency version + moved. Overlaps with subtle case 2 above, and is the mechanism behind the cascade check. +3. **Non-host targets.** The script passes no `--target`, so only the host triple is + analysed. Windows- and macOS-only `#[cfg]` API is never compared — relevant for + crashtracker and common. +4. **Feature-gated API.** `cargo semver-checks` runs `--all-features` but `cargo public-api` + runs default features only, and the public-api pass is the *only* thing that catches + parameter and return type changes. So a signature change behind a non-default feature is + caught by neither. (Compounds subtle case 3.) +5. **Crate renames.** A renamed crate is absent from the baseline, so it is classified as a + *new* crate and reported `minor`. Renaming a published crate breaks every consumer. +6. **Declarative macro bodies.** Only removal of a `#[macro_export] macro_rules!` is linted; + narrowing or dropping an arm is invisible to both tools. +7. **Trait-impl and inference breakage.** Adding `impl Trait for T`, or an inherent method + that shadows a trait method downstream, reads as a plain addition. +8. **Behaviour.** New panics, changed error semantics, altered serde/wire representation — + no signature tool can see these. +9. **The generated C API.** Both tools read rustdoc JSON, so nothing validates `builder`'s + generated headers or pkg-config output. Per `AGENTS.md` the C FFI offers no ABI + compatibility guarantee, so this is by design — but do not mistake a green semver check + for FFI safety. + +Two properties of the tooling that also affect how you reproduce a level locally: the script +needs a **clean working tree** (`cargo public-api diff` does a real `git checkout`), and it +needs `RUSTUP_TOOLCHAIN` overridden because `rust-toolchain.toml` pins an MSRV older than +cargo-semver-checks requires. See `scripts/tests/semver-level/README.md`. + +## Output + +A concise table of per-crate verdicts with file:symbol evidence, then the **cascade verdict** (every major bump propagated through its publishable reverse-dependency closure? list any under-bumped/omitted crates and the major version each needs), then a short list of non-blocking observations. State plainly whether every bump is correct, or which need changing and to what. diff --git a/docs/release-proposals.md b/docs/release-proposals.md new file mode 100644 index 0000000000..0a4139d5c0 --- /dev/null +++ b/docs/release-proposals.md @@ -0,0 +1,234 @@ +# Release proposals (publishing crates to crates.io) + +Practical guide to the flow that publishes `libdd-*` (and other workspace) crates to +crates.io. + +> **Not this flow:** the FFI **artifact** release (`vX.Y.Z` tarballs + headers) is a +> different pipeline — `scripts/create-release.sh` and libddprof-build's +> `draft_github_release.sh`. It uses `release/vX.Y.Z` branches (one path segment), which +> do not collide with the `release//` branches used here. + +## At a glance + +``` + ┌─ GitHub Actions ────────────────────────────────────────────────┐ + │ 1. workflow_dispatch: "Release - Open a release proposal PR" │ + │ release-proposal-dispatch.yml │ + │ ├─ validate crates, membership, no ongoing proposal │ + │ ├─ create ephemeral branch release// │ + │ ├─ create proposal branch release-proposal// │ + │ ├─ per crate: semver-level.sh → cargo release version │ + │ ├─ force major on direct libdd-* major bumps │ + │ ├─ git-cliff CHANGELOGs │ + │ └─ draft PR: release-proposal/... → release/... │ + │ 2. release-proposal-test.yml (on that push / PR) │ + │ cargo package + compile dd-trace-rs against the packages │ + └─────────────────────────────────────────────────────────────────┘ + │ squash-merge the proposal PR + ▼ (push to release/**) + ┌─ GitLab (libddprof-build, via the ddbuild mirror) ──────────────┐ + │ 3. publish_cargo_crates (MANUAL job, DRY_RUN=true by default) │ + │ ├─ crates-to-package.sh → publication-order.sh → tags │ + │ ├─ publish-crates.sh: test, cargo publish, crates.io owners │ + │ ├─ create annotated GitHub tags -v │ + │ └─ create_pr_to_merge_release_branch.sh: release/... → main │ + └─────────────────────────────────────────────────────────────────┘ + │ merge that PR + ▼ + main has the bumps + CHANGELOGs +``` + +## 1. Open the proposal + +Actions → **Release - Open a release proposal PR** → Run workflow. + +| Input | Notes | +|---|---| +| `crates` | Comma-separated. Each crate is released together with its workspace `libdd-*` dependencies (`scripts/publication-order.sh`). Only publishable crates (`publish != false`) are accepted. | +| `main_start_ref` | Empty = tip of `origin/main`. A SHA/branch/tag is allowed **only if reachable from `origin/main`** or from the matching `origin/hotfix//N.x.x`. `refs/pull/*` is rejected. | +| `bypass_standard_checks` | Testing only: skips the ongoing-proposal guard and the team-membership check, uses `release-testing/` + `release-proposal-testing/` prefixes, pushes plainly (no verified commits), and stops skipping crates whose tag is not the latest. | + +Guards that will stop you: + +- **Ongoing proposal** — any existing `origin/release-proposal/*` or `origin/release/*/*` + branch aborts the run. One release at a time. +- **Membership** — the actor must be in `Datadog/apm-common-components-core`. +- **Untrusted `cargo-release` config** — the tree must not mention + `pre-release-hook` / `pre-release-replacements` anywhere in `Cargo.toml` / `release.toml`. +- Release scripts are always taken from the **workflow revision** (`github.sha`), not from + `main_start_ref`. + +What the job produces: two branches, one bump commit + one CHANGELOG commit per crate +(pushed via `DataDog/commit-headless` so they are verified), and a **draft** PR titled +`chore(release): proposal for `, based on the ephemeral `release/...` branch. +The `release-dispatch-data` artifact (1 day retention) holds the intermediate JSON +(`commits-by-crate.json`, `api-changes*.json`) — start debugging there. + +### Which crates actually get released + +`scripts/commits-since-release.sh` lists commits since `-v` **that touch +the crate's directory**, dropping merge commits, `chore(release)` subjects, and anything +authored by `dd-octo-sts[bot]`. + +- Commits found → the crate is bumped. +- No commits and a tag exists → deferred; released **only** if a direct `libdd-*` + dependency goes major in this proposal. +- No tag at all → initial release, forced to `major`, and the run **fails unless the + manifest version is exactly `0.1.0`**. +- The crate's resolved tag is not the latest SemVer tag for that crate → skipped + (that release is already on `main`), unless it is a hotfix or `bypass_standard_checks`. + +## 2. How the bump level is decided + +`scripts/semver-level.sh refs/tags/` computes `major | minor | patch` +from two tools and takes the **higher** of the two: + +1. `cargo semver-checks -p --all-features --baseline-rev ` + → `major` on "requires new major", `minor` on "requires new minor", `minor` if the + crate is absent from the baseline (new crate). +2. `cargo public-api --package diff ..HEAD` (skipped if 1. already said + major, or the crate is new) + → removed items = `major`; changed items = `major` **if** a difference survives + normalization (diff markers, `#[...]` attributes and `const`/`async`/`unsafe` are + stripped); added items = `minor`. + +No signal at all ⇒ `patch`. The level is then fed to `cargo release version -p -x `. + +Then `scripts/major-bumps-level.sh` re-reads each crate's **direct** `libdd-*` +dependency requirements at `prev_tag` vs. the proposal tree and forces `major` where a +requirement's major digit increased — this is how "protobuf 3→4" propagates to its +dependents, and how a no-commit crate can still end up in the release. + +### Limitations you must review by hand + +`semver-level.sh` looks only at the Rust public API surface. It does **not** know about: + +- **Conventional-commit intent.** `feat!:` / `BREAKING CHANGE:` markers are ignored + entirely. A breaking change that does not alter a signature lands as `patch`. +- **Behavioural breakage.** Same signature, different semantics (defaults, error + behaviour, panics, wire format, protobuf/proto file changes) ⇒ `patch`. +- **Feature-gated API.** Everything runs with `--all-features`, so API that only exists + under a non-default feature combination (e.g. `libdd-http-client`'s mutually exclusive + `reqwest-backend` / `hyper-backend`) is analysed in exactly one configuration. +- **`cargo-semver-checks` false negatives** — notably parameter type changes on + non-generic functions (`function_parameter_type_changed` is unimplemented). The + `cargo-public-api` pass exists to cover that; it needs **`cargo-public-api >= 0.52.0`** + (older versions include parameter names, so a harmless *rename* is promoted to major). + +### ⚠️ Review every `minor` and `patch` bump by hand + +Every limitation above fails in the same direction: it **under**-estimates the level. So +the bumps that need scrutiny are the low ones. + +- **`patch` / `minor` — dangerous.** A missed breaking change published as a patch or + minor silently breaks consumers on `cargo update`. Read the commits listed for that + crate in the PR body and ask whether any of them changes behaviour, an FFI layout, a + wire format, or an API under a feature the analysis did not exercise. If so, raise the + level on the proposal branch before merging. +- **`major` — safe to accept.** An over-estimated major only costs a version number; + consumers must opt in, so nothing breaks. Never argue a `major` down to save a digit. +- **Don't use the `!` marker as your verdict.** It is per-PR, while a PR usually touches + several crates: a `feat!:` in the list says *something* in that PR breaks, not that it + breaks for every crate the PR modified, and not which one. Treat a `!` under a crate as + a prompt to read that crate's slice of the diff — and remember the converse, that a + commit with no `!` can still be breaking for one of the crates it touches. + +⇒ Sanity-check each bump in the PR body against its listed commits, spending the effort on +the `patch` and `minor` rows. The `/release-proposal-pr-review-bumps` skill does exactly +this review. + +## 3. Review the proposal PR + +`release-proposal-test.yml` runs on every push to `release-proposal/**` and on PRs based +on `release/**`: + +1. `scripts/crates-to-package.sh` (base = PR base / merge-base with `main`) lists + publishable crates whose **own** version changed, then `cargo +1.92.0 package` them. + Cargo ≥ 1.92 is required because sibling versions are not on crates.io yet. +2. Resolves the newest patch of the 3 most recent `datadog-opentelemetry-v*` release + lines in `DataDog/dd-trace-rs`, and builds each of them with + `--config patch.crates-io..path=...` pointing at the unpacked `.crate` files. + The log also prints duplicated `libdd-*` versions in the tree — check it. + +Note the PR is a **draft**: mark it ready before merging. Its `skip-*` labels disable the +metadata/changelog/PR-title checks that do not apply to release commits. + +## 4. Publish (GitLab) + +Squash-merge the proposal PR into the ephemeral `release//` branch. +That push is mirrored to `gitlab.ddbuild.io/DataDog/libdatadog`, whose `.gitlab-ci.yml` +triggers libddprof-build with `LIBDATADOG_IS_RELEASE_BRANCH=true` (branch matches +`^release/` or `^hotfix/`). + +`publish_cargo_crates` is created only when **both** hold +(`.rules_run_on_module_release`): + +- `LIBDATADOG_IS_RELEASE_BRANCH == "true"`, and +- `LIBDATADOG_COMMIT_TITLE =~ /chore\(release\): proposal/` — i.e. the squash commit must + keep the PR title. **Do not** merge with a merge commit. + +It is a **manual** job with `DRY_RUN: "true"`. Run it as-is first (it validates versions, +runs the tests and `cargo publish --dry-run`), then re-run it with `DRY_RUN=false` to +publish for real. In order, per crate (`publish-crates.sh`, in publication order): + +1. tag version must equal the manifest version; +2. skip if that version is already on crates.io; +3. `cargo nextest --no-default-features` (warn only) and `--all-features` (**blocking**), + excluding `tracing_integration_tests::`; +4. `cargo publish --all-features`, then add the `github:datadog:libdatadog-owners` owner. + +Only **after every crate in the batch succeeds** are the annotated GitHub tags +`-v` created on the release-branch commit. Finally +`create_pr_to_merge_release_branch.sh` opens a draft PR `release/... → main` (skipped for +hotfixes) — merge it so the bumps and CHANGELOGs land on `main`, and delete the ephemeral +branch (a leftover `release/*/*` blocks the next proposal). + +## Hotfixes + +> ⚠️ **Untested path.** Every stage below is implemented — the dispatch workflow, the +> GitLab publish rule and the cleanup steps all special-case hotfixes — but the flow has +> never been exercised end to end on a real hotfix. + +Pass `main_start_ref = hotfix//.x.x` (the branch must exist on origin) and +**exactly one crate**. Differences: + +- the hotfix branch *is* the ephemeral branch — nothing new is created and it is never + deleted by the cleanup steps; +- crates whose tag is not the latest are **not** skipped; +- no merge-back PR to `main` is opened. + +Since the proposal PR targets `hotfix/**` and not `release/**`, only the `push`-triggered +half of `release-proposal-test.yml` runs for it. + +## Cancelling / retrying a proposal + +The dispatch job cleans up both branches on failure. If a run half-succeeded or the +proposal is wrong: close the PR and delete **both** `release-proposal/<...>` and +`release/<...>` on origin, then dispatch again. The ongoing-proposal guard will keep +failing until both are gone. + +If publication failed **after** some crates were published, those versions are on +crates.io but no GitHub tags exist. Do not re-publish them — start a new proposal for the +remaining crates from the same commit the release branch was cut from (the failure output +prints that merge-base). + +## Troubleshooting + +| Symptom | Cause / fix | +|---|---| +| `Error: A release proposal is ongoing` | A leftover `release-proposal/*` or `release/*/*` branch on origin. Delete it (or wait for the merge-back PR). | +| `Error: unknown or unpublishable crate(s)` | Typo, or the crate has `publish = false`. The error prints the valid list. | +| `Error: resolved commit ... is not reachable from origin/main` | `main_start_ref` points outside `main` / the hotfix branch. Use a commit on a trusted branch. | +| `Error: is not a 0.1.0 release` | No `-v*` tag exists, so the run treats it as an initial release. Set the manifest version to `0.1.0` or create the missing tag. | +| `No changes to push. Cancelling the workflow.` | Nothing to release: no commits touched the selected crates' directories since their tags (remember: path-filtered, and `chore(release)` / bot commits are dropped). | +| `Semver level:` followed by a `jq` parse error, or `cargo release version -x` with an empty level | `semver-level.sh` output is parsed as JSON; anything it printed on stderr (it is captured with `2>&1`) breaks the parse. Read the raw step log for the real error, usually a `cargo semver-checks` / `cargo public-api` build failure. | +| `Unexpected exit code from cargo-semver-checks` / `Unexpected error from cargo-public-api` | The crate does not build at the baseline tag or at HEAD with `--all-features`. Reproduce locally: `./scripts/semver-level.sh -v refs/tags/-v`. | +| Bump level looks too low in the PR body | Expected for behavioural/ABI/`0.x`-dependency breakage — see the limitations above. A too-low `patch`/`minor` is the failure that matters; edit the version and CHANGELOG on the proposal branch before merging, or close and re-dispatch. A too-high `major` is harmless — leave it. | +| `cargo package` fails in `release-proposal-test.yml` | Usually a sibling crate version not yet on crates.io; the job pins `cargo +1.92.0` for this. Also check for `Cargo.lock` drift and missing files in `include`. | +| dd-trace-rs compile job fails | A real breaking change reaching a consumer — that is the point of the job. Check the "Duplicated dependencies" section for two `libdd-*` majors in one tree. | +| `publish_cargo_crates` is absent from the GitLab pipeline | The merge commit title does not start with `chore(release): proposal` (merge commit instead of squash), or the branch is not `release/**` / `hotfix/**`, or the pipeline was not triggered from the mirror (`CI_PIPELINE_SOURCE != "pipeline"`). | +| `Skipping cargo package: no crates had a version bump` in GitLab | `crates-to-package.sh` compares `LIBDATADOG_COMMIT_BEFORE_SHA..LIBDATADOG_COMMIT_SHA`; a push that carries no version change (e.g. a follow-up commit on the release branch) produces nothing. Re-run the job on the push that contains the bumps. | +| `Version mismatch! tag vs Cargo.toml` | A manual edit desynced the manifest from the computed tag. Fix the manifest on the release branch. | +| `Version X of is already published` | Skipped, not an error — normal on a re-run. | +| Publication succeeded but no tags | Some crate in the batch failed; tags are created only after the whole batch succeeds. See "Cancelling / retrying" above. | +| Everything published but `main` lacks the bumps | Merge the `release/... → main` draft PR (it is not created for hotfixes — port those manually). |