diff --git a/CHANGELOG.md b/CHANGELOG.md index 7c51dca..03c1042 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- Add `--external-only` flag to `cargo chef prepare` to strip all `path = "..."` dependency entries from the recipe (including intra-workspace path deps and any other local path deps), enabling a stable Docker cache layer for third-party dependencies that is not invalidated by workspace-internal changes ([#359](https://github.com/LukeMathWalker/cargo-chef/issues/359)) + ## [0.1.77](https://github.com/LukeMathWalker/cargo-chef/compare/v0.1.76...v0.1.77) - 2026-03-03 ### Fixed @@ -18,7 +22,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added - Minimize generated recipe to increase cache hit ratio when `cargo chef prepare` is invoked with a `--bin` option (by [@preiter93](https://github.com/preiter93)) -- Publish a prebuilt `cargo-chef` Docker image for every upstream Rust tag. +- Publish a prebuilt `cargo-chef` Docker image for every upstream Rust tag. - Broaden the set of supported architectures for Docker images to include `i386` and `arm32v7` ### Other diff --git a/Cargo.toml b/Cargo.toml index 1a4503c..c182035 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -39,6 +39,7 @@ assert_cmd = "2" assert_fs = "1.1.3" predicates = "3" rstest = "0.26" +toml = { version = "1.1", features = ["preserve_order"] } # The profile that 'dist' will build with [profile.dist] diff --git a/examples/workspace-split-cache/.gitignore b/examples/workspace-split-cache/.gitignore new file mode 100644 index 0000000..ea8c4bf --- /dev/null +++ b/examples/workspace-split-cache/.gitignore @@ -0,0 +1 @@ +/target diff --git a/examples/workspace-split-cache/Cargo.lock b/examples/workspace-split-cache/Cargo.lock new file mode 100644 index 0000000..d5e6584 --- /dev/null +++ b/examples/workspace-split-cache/Cargo.lock @@ -0,0 +1,129 @@ +# This file is automatically @generated by Cargo. +# It is not intended for manual editing. +version = 4 + +[[package]] +name = "anyhow" +version = "1.0.102" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7f202df86484c868dbad7eaa557ef785d5c66295e41b460ef922eca0723b842c" + +[[package]] +name = "app" +version = "0.1.0" +dependencies = [ + "anyhow", + "logic", +] + +[[package]] +name = "itoa" +version = "1.0.18" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8f42a60cbdf9a97f5d2305f08a87dc4e09308d1276d28c869c684d7777685682" + +[[package]] +name = "logic" +version = "0.1.0" +dependencies = [ + "anyhow", + "model", + "serde_json", +] + +[[package]] +name = "memchr" +version = "2.8.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6b947ae49db0d222b1dbc6b113ce7248a3fc3a6ca21b696717bfc000ba4484d8" + +[[package]] +name = "model" +version = "0.1.0" +dependencies = [ + "serde", +] + +[[package]] +name = "proc-macro2" +version = "1.0.106" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8fd00f0bb2e90d81d1044c2b32617f68fcb9fa3bb7640c23e9c748e53fb30934" +dependencies = [ + "unicode-ident", +] + +[[package]] +name = "quote" +version = "1.0.45" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "41f2619966050689382d2b44f664f4bc593e129785a36d6ee376ddf37259b924" +dependencies = [ + "proc-macro2", +] + +[[package]] +name = "serde" +version = "1.0.228" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9a8e94ea7f378bd32cbbd37198a4a91436180c5bb472411e48b5ec2e2124ae9e" +dependencies = [ + "serde_core", + "serde_derive", +] + +[[package]] +name = "serde_core" +version = "1.0.228" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "41d385c7d4ca58e59fc732af25c3983b67ac852c1a25000afe1175de458b67ad" +dependencies = [ + "serde_derive", +] + +[[package]] +name = "serde_derive" +version = "1.0.228" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d540f220d3187173da220f885ab66608367b6574e925011a9353e4badda91d79" +dependencies = [ + "proc-macro2", + "quote", + "syn", +] + +[[package]] +name = "serde_json" +version = "1.0.150" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e8014e44b4736ed0538adeecded0fce2a272f22dc9578a7eb6b2d9993c74cfb9" +dependencies = [ + "itoa", + "memchr", + "serde", + "serde_core", + "zmij", +] + +[[package]] +name = "syn" +version = "2.0.117" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e665b8803e7b1d2a727f4023456bbbbe74da67099c585258af0ad9c5013b9b99" +dependencies = [ + "proc-macro2", + "quote", + "unicode-ident", +] + +[[package]] +name = "unicode-ident" +version = "1.0.24" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e6e4313cd5fcd3dad5cafa179702e2b244f760991f45397d14d4ebf38247da75" + +[[package]] +name = "zmij" +version = "1.0.21" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b8848ee67ecc8aedbaf3e4122217aff892639231befc6a1b58d29fff4c2cabaa" diff --git a/examples/workspace-split-cache/Cargo.toml b/examples/workspace-split-cache/Cargo.toml new file mode 100644 index 0000000..858e208 --- /dev/null +++ b/examples/workspace-split-cache/Cargo.toml @@ -0,0 +1,3 @@ +[workspace] +members = ["app", "logic", "model"] +resolver = "2" diff --git a/examples/workspace-split-cache/Dockerfile b/examples/workspace-split-cache/Dockerfile new file mode 100644 index 0000000..6ed486c --- /dev/null +++ b/examples/workspace-split-cache/Dockerfile @@ -0,0 +1,121 @@ +# syntax=docker/dockerfile:1 +# +# Example: Split Docker cache layers for third-party vs workspace-internal deps. +# +# ┌─────────────────────────────────────────────────────────────────────────────┐ +# │ THE PROBLEM │ +# │ │ +# │ In a large Cargo workspace the standard cargo-chef pattern has a │ +# │ disappointing property: the cook layer (which pre-compiles dependencies) │ +# │ is invalidated whenever *any* workspace manifest changes — adding a new │ +# │ internal crate, splitting one, adding a new binary target, etc. — even │ +# │ when no external (third-party) dependency changed at all. │ +# │ │ +# │ The root cause is that recipe.json captures both external deps AND the │ +# │ intra-workspace path dependencies. Any structural change to the workspace │ +# │ changes the recipe, which busts the cook cache, which forces a full │ +# │ recompile of *all* third-party crates from source. │ +# │ │ +# │ In a workspace with many internal packages (e.g. 26 in torrust-tracker), │ +# │ this means the cache is effectively always cold: every feature branch or │ +# │ PR that touches any manifest invalidates hundreds of MB of compiled crates. │ +# └─────────────────────────────────────────────────────────────────────────────┘ +# +# ┌─────────────────────────────────────────────────────────────────────────────┐ +# │ THE SOLUTION │ +# │ │ +# │ Produce two recipes from the same workspace: │ +# │ │ +# │ recipe-thirdparty.json — all path deps stripped (external deps only). │ +# │ • Changes only when an external dependency is added/removed/updated. │ +# │ • The cook_thirdparty layer built from it is a durable cache of │ +# │ compiled third-party crates that survives any workspace-internal │ +# │ refactor. │ +# │ │ +# │ recipe.json — the standard full recipe (external + internal structure). │ +# │ • Changes when any manifest changes (normal cargo-chef behaviour). │ +# │ • The cook layer built on top of cook_thirdparty only needs to │ +# │ (re)compile the lightweight workspace stubs; all third-party │ +# │ artifacts are already present from the layer below. │ +# │ │ +# │ Stage graph: │ +# │ │ +# │ chef ──► planner │ +# │ │ │ +# │ ├── recipe-thirdparty.json ──► cook_thirdparty (stable) │ +# │ │ │ │ +# │ └── recipe.json ──────────────────► cook (fast) │ +# │ │ │ +# │ real source ──────► builder │ +# └─────────────────────────────────────────────────────────────────────────────┘ +# +# Build: +# docker build -t workspace-split-cache . +# +# Note: this Dockerfile uses the pre-built lukemathwalker/cargo-chef image. +# Adjust the tag to pin a specific cargo-chef + Rust version pair. + +FROM lukemathwalker/cargo-chef:latest-rust-1 AS chef +WORKDIR /app + +# strip_path_deps.py uses only the Python standard library (no third-party +# packages needed), but python3 itself may not be present on the slim Rust image. +# NOTE: This Dockerfile uses the Python workaround for illustration purposes. +# For the native solution (no Python required), see Dockerfile.native. +RUN apt-get update \ + && apt-get install -y --no-install-recommends python3 \ + && rm -rf /var/lib/apt/lists/* + +# ── planner ─────────────────────────────────────────────────────────────────── +# Copies the full source tree and generates both recipe files. +# Nothing inside this stage changes the compiled output — it is purely +# analytical (cargo chef prepare only reads manifests, never compiles). +FROM chef AS planner +COPY . . + +# Standard full recipe: captures external deps AND workspace-internal structure. +RUN cargo chef prepare --recipe-path recipe.json + +# Thirdparty-only recipe: same as above but with all `path = "..."` deps stripped +# from every manifest and all local package entries removed from the lock file. +# This file is stable across any pure workspace-internal change. +RUN python3 strip_path_deps.py recipe.json recipe-thirdparty.json + +# ── cook_thirdparty ─────────────────────────────────────────────────────────── +# Compiles ONLY the external (third-party) crates. +# +# Cache behaviour: +# • Re-runs when recipe-thirdparty.json changes, i.e. when an external +# dependency is added, removed, or updated. +# • Stays cached for any pure workspace-internal change: new crates, renamed +# crates, restructured path deps, new binary targets, etc. +FROM chef AS cook_thirdparty +COPY --from=planner /app/recipe-thirdparty.json recipe.json +RUN cargo chef cook --release --recipe-path recipe.json + +# ── cook ────────────────────────────────────────────────────────────────────── +# Inherits all third-party artifacts from cook_thirdparty. +# Runs cargo chef cook with the full recipe to build the workspace stubs. +# Cargo reuses every external crate already compiled in cook_thirdparty and +# only (re)compiles the lightweight workspace-member stub files. +# +# Cache behaviour: +# • Re-runs when recipe.json changes (any manifest change). +# • But third-party compilation is skipped — only stubs are (re)built, which +# is fast even for large workspaces. +FROM cook_thirdparty AS cook +COPY --from=planner /app/recipe.json recipe.json +RUN cargo chef cook --release --recipe-path recipe.json + +# ── builder ─────────────────────────────────────────────────────────────────── +# Copies real application source over the skeleton and compiles the actual app. +# Third-party artifacts are reused from cook; only workspace crates are rebuilt. +FROM cook AS builder +COPY . . +RUN cargo build --release --bin app + +# ── runtime ─────────────────────────────────────────────────────────────────── +FROM debian:bookworm-slim AS runtime +WORKDIR /app +COPY --from=builder /app/target/release/app /usr/local/bin/ +ENTRYPOINT ["/usr/local/bin/app"] diff --git a/examples/workspace-split-cache/Dockerfile.native b/examples/workspace-split-cache/Dockerfile.native new file mode 100644 index 0000000..bb570b2 --- /dev/null +++ b/examples/workspace-split-cache/Dockerfile.native @@ -0,0 +1,67 @@ +# syntax=docker/dockerfile:1 +# +# Example: Split Docker cache layers using the native --external-only flag. +# +# This Dockerfile demonstrates the same two-layer cook pattern as Dockerfile +# but uses the native `cargo chef prepare --external-only` flag instead of the +# Python post-processing script. +# +# Requires a locally built cargo-chef binary that includes the --external-only +# flag (not yet released upstream — built from this repo). +# +# Build (two steps): +# +# Step 1 — build cargo-chef from the repo root and tag it: +# docker build -t cargo-chef-local ../../ +# +# Step 2 — build this example passing the local image as a named build context: +# docker build \ +# --build-context cargo-chef-local=docker-image://cargo-chef-local:latest \ +# -f Dockerfile.native \ +# -t workspace-split-cache-native \ +# . +# +# Once --external-only is released on crates.io the COPY --from=cargo-chef-local +# line below can be replaced with: +# RUN cargo install --locked cargo-chef + +FROM rust:1 AS chef +WORKDIR /app +# Install the locally patched cargo-chef binary injected via --build-context. +# See the build instructions at the top of this file. +COPY --from=cargo-chef-local /usr/local/bin/cargo-chef /usr/local/bin/cargo-chef + +# ── planner ─────────────────────────────────────────────────────────────────── +FROM chef AS planner +COPY . . + +# Standard full recipe (external deps + workspace-internal structure). +RUN cargo chef prepare --recipe-path recipe.json + +# Third-party-only recipe: path deps and local lock entries stripped natively. +# This is the clean version — no Python script needed. +RUN cargo chef prepare --external-only --recipe-path recipe-thirdparty.json + +# ── cook_thirdparty ─────────────────────────────────────────────────────────── +# Compiles ONLY external (crates.io) dependencies. +# Invalidated only when an external dep actually changes. +FROM chef AS cook_thirdparty +COPY --from=planner /app/recipe-thirdparty.json recipe.json +RUN cargo chef cook --release --recipe-path recipe.json + +# ── cook ────────────────────────────────────────────────────────────────────── +# Inherits third-party artifacts; recompiles only workspace stubs. +FROM cook_thirdparty AS cook +COPY --from=planner /app/recipe.json recipe.json +RUN cargo chef cook --release --recipe-path recipe.json + +# ── builder ─────────────────────────────────────────────────────────────────── +FROM cook AS builder +COPY . . +RUN cargo build --release --bin app + +# ── runtime ─────────────────────────────────────────────────────────────────── +FROM debian:bookworm-slim AS runtime +WORKDIR /app +COPY --from=builder /app/target/release/app /usr/local/bin/ +ENTRYPOINT ["/usr/local/bin/app"] diff --git a/examples/workspace-split-cache/README.md b/examples/workspace-split-cache/README.md new file mode 100644 index 0000000..2c8c2dd --- /dev/null +++ b/examples/workspace-split-cache/README.md @@ -0,0 +1,584 @@ +# Example: Split Docker cache — third-party vs workspace-internal deps + +This example demonstrates a common pain point in large Cargo workspaces and +two solutions: a **workaround** using a small Python script, and the **native +solution** via the `--external-only` flag for `cargo chef prepare`. + +> **Goal of this example:** provide maintainers with a reproducible +> demonstration and a working implementation to evaluate for merging. +> Related upstream issues: [#314], [#75], [#181], [#305]. + +[#314]: https://github.com/LukeMathWalker/cargo-chef/issues/314 +[#75]: https://github.com/LukeMathWalker/cargo-chef/issues/75 +[#181]: https://github.com/LukeMathWalker/cargo-chef/issues/181 +[#305]: https://github.com/LukeMathWalker/cargo-chef/issues/305 + +--- + +## Table of contents + +1. [The problem](#1-the-problem) +2. [Workspace structure](#2-workspace-structure) +3. [Step-by-step: reproducing the problem](#3-step-by-step-reproducing-the-problem) +4. [Solution A — Python workaround (works today)](#4-solution-a--python-workaround-works-today) +5. [Solution B — native `--external-only` flag (proposed)](#5-solution-b--native---external-only-flag-proposed) +6. [Related open issues](#6-related-open-issues) +7. [Implementation notes](#7-implementation-notes) + +--- + +## 1. The problem + +The standard `cargo-chef` pattern builds a single cook layer that caches all +dependencies: + +``` +COPY Cargo.toml Cargo.lock ... +RUN cargo chef prepare --recipe-path recipe.json # <-- reads manifests +RUN cargo chef cook --recipe-path recipe.json # <-- the "cached" layer +COPY . . +RUN cargo build --release +``` + +This works well for single-crate projects. In a large workspace it has a +**critical weakness**: `recipe.json` captures both external (crates.io) deps +_and_ the intra-workspace `path = "..."` dependencies. Any structural change +to the workspace invalidates the cook layer and forces a **full recompile of +all third-party crates**: + +| Change | Invalidates cook layer? | +| ---------------------------------------------- | ----------------------- | +| Editing application source (`.rs` files) | No — that's the point | +| Adding a new external dep (`serde = "1"`) | Yes — expected | +| Adding a new workspace-internal crate | **Yes — unexpected** | +| Splitting one internal crate into two | **Yes — unexpected** | +| Adding a `[[bin]]` target to a workspace crate | **Yes — unexpected** | +| Renaming a workspace crate | **Yes — unexpected** | + +In the real-world project ([torrust-tracker]) that motivated this example the +workspace has 26 internal crates. Almost every feature branch touches at least +one manifest. The cook cache is effectively **always cold**: all third-party +crates are recompiled from source on every CI run. + +[torrust-tracker]: https://github.com/torrust/torrust-tracker + +### Root cause + +`cargo chef prepare` serialises every workspace manifest — including +`[dependencies.my-crate] path = "../my-crate"` sections — into `recipe.json`. +Any structural change produces a different JSON file, which Docker treats as a +changed `COPY` input, which invalidates the `RUN cargo chef cook` layer. + +--- + +## 2. Workspace structure + +``` +Cargo.toml ← workspace root (members: app, logic, model) +app/ ← binary: anyhow (external) + logic (path dep) +logic/ ← library: anyhow + serde_json (external) + model (path dep) +model/ ← library: serde (external only, no path deps) +Dockerfile ← workaround using strip_path_deps.py +Dockerfile.native ← clean version using --external-only flag +strip_path_deps.py ← Python post-processor (workaround, no native cargo-chef needed) +``` + +Dependency graph: + +``` +app ──path──► logic ──path──► model ──► serde + └────────────────────────────────────► anyhow + └────────────────────► serde_json +``` + +This is the minimal structure to reproduce the problem: a chain of path deps +where almost every workspace change touches at least one manifest. + +--- + +## 3. Step-by-step: reproducing the problem + +All commands run from this directory: + +``` +cd examples/workspace-split-cache +``` + +### 3.1 Prepare a standard recipe + +```bash +cargo chef prepare --recipe-path /tmp/recipe-full.json +``` + +Inspect the recipe — note that `app/Cargo.toml` contains `[dependencies.logic]` +with `path = "../logic"`: + +```bash +python3 -c " +import json +with open('/tmp/recipe-full.json') as f: + r = json.load(f) +for m in r['skeleton']['manifests']: + print('=== ' + m['relative_path'] + ' ===') + print(m['contents']) +" +``` + +**Output (relevant excerpt):** + +```toml +=== app/Cargo.toml === +[package] +name = "app" +edition = "2021" +version = "0.0.1" + +[dependencies] +anyhow = "1" + +[dependencies.logic] +path = "../logic" # <-- this is the problem + +[[bin]] +path = "src/main.rs" +name = "app" +... +``` + +The lock file also contains entries for all three local workspace crates: + +```bash +python3 -c " +import json, re +with open('/tmp/recipe-full.json') as f: + r = json.load(f) +lf = r['skeleton']['lock_file'] or '' +print('Lock packages:', re.findall(r'name = \"([^\"]+)\"', lf)) +" +``` + +**Output:** + +``` +Lock packages: ['anyhow', 'app', 'itoa', 'logic', 'memchr', 'model', +'proc-macro2', 'quote', 'serde', 'serde_core', 'serde_derive', +'serde_json', 'syn', 'unicode-ident', 'zmij'] +``` + +`app`, `logic`, and `model` appear in the lock file — so adding a new local +crate changes the lock file and therefore the recipe. + +### 3.2 Cold cook timing (what happens on every cache miss) + +```bash +mkdir -p /tmp/cook-demo && cd /tmp/cook-demo +CARGO=$(which cargo) time cargo chef cook --release --recipe-path /tmp/recipe-full.json +``` + +**Output:** + +``` + Compiling proc-macro2 v1.0.106 + Compiling serde_core v1.0.228 + Compiling unicode-ident v1.0.24 + Compiling quote v1.0.45 + Compiling anyhow v1.0.102 + Compiling zmij v1.0.21 + Compiling serde v1.0.228 + Compiling serde_json v1.0.150 + Compiling itoa v1.0.18 + Compiling memchr v2.8.1 + Compiling app v0.0.1 (/tmp/cook-demo/app) + Compiling syn v2.0.117 + Compiling serde_derive v1.0.228 + Compiling logic v0.0.1 (/tmp/cook-demo/logic) + Compiling model v0.0.1 (/tmp/cook-demo/model) + Finished `release` profile [optimized] target(s) in 1.99s + +real 0m5.80s +``` + +This **5.8 s** is the cost paid every time the cook cache is invalidated. In a +real workspace (26 crates, hundreds of third-party deps) this scales to +**several minutes** per CI run. + +### 3.3 Simulating a workspace-internal change + +Touch any workspace manifest — for example, bump the description of `logic`: + +```bash +cd examples/workspace-split-cache +# Simulating: add a new binary to logic/Cargo.toml +echo ' +[[bin]] +name = "logic-tool" +path = "src/main.rs"' >> logic/Cargo.toml + +cargo chef prepare --recipe-path /tmp/recipe-full-v2.json + +diff <(python3 -c " +import json +with open('/tmp/recipe-full.json') as f: + print(json.dumps(json.load(f)['skeleton']['manifests'], indent=2)) +") <(python3 -c " +import json +with open('/tmp/recipe-full-v2.json') as f: + print(json.dumps(json.load(f)['skeleton']['manifests'], indent=2)) +") | head -20 +``` + +The diff shows the recipe changed. If this `recipe-full-v2.json` is passed to +`COPY` in Docker, **the entire cook layer is invalidated** and all third-party +crates are recompiled. Revert the change: + +```bash +git checkout logic/Cargo.toml +``` + +--- + +## 4. Solution A — Python workaround (works today) + +`strip_path_deps.py` post-processes the standard `recipe.json` and produces a +`recipe-thirdparty.json` that contains only external deps. + +### 4.1 Generate the thirdparty recipe + +```bash +cd examples/workspace-split-cache +cargo chef prepare --recipe-path /tmp/recipe-full.json +python3 strip_path_deps.py /tmp/recipe-full.json /tmp/recipe-thirdparty.json +``` + +Inspect the result — `[dependencies.logic]` is gone: + +```bash +python3 -c " +import json +with open('/tmp/recipe-thirdparty.json') as f: + r = json.load(f) +for m in r['skeleton']['manifests']: + print('=== ' + m['relative_path'] + ' ===') + print(m['contents']) +" +``` + +**Output:** + +```toml +=== app/Cargo.toml === +[package] +name = "app" +edition = "2021" +version = "0.0.1" + +[dependencies] +anyhow = "1" # path dep to logic is GONE + +[[bin]] +path = "src/main.rs" +name = "app" +... + +=== logic/Cargo.toml === +[package] +name = "logic" +... +[dependencies] +anyhow = "1" +serde_json = "1" # path dep to model is GONE +... +``` + +Lock file — local workspace crates removed: + +```bash +python3 -c " +import json, re +with open('/tmp/recipe-thirdparty.json') as f: + r = json.load(f) +lf = r['skeleton']['lock_file'] or '' +print('Lock packages:', re.findall(r'name = \"([^\"]+)\"', lf)) +" +``` + +**Output:** + +``` +Lock packages: ['anyhow', 'itoa', 'memchr', 'proc-macro2', 'quote', 'serde', +'serde_core', 'serde_derive', 'serde_json', 'syn', 'unicode-ident', 'zmij'] +``` + +`app`, `logic`, `model` are absent. Adding a new workspace crate will not +change this recipe. + +### 4.2 Two-layer cook + +```bash +mkdir -p /tmp/cook-demo && cd /tmp/cook-demo + +# Layer 1: cook third-party deps only (stable, rarely re-runs) +CARGO=$(which cargo) time cargo chef cook --release --recipe-path /tmp/recipe-thirdparty.json +``` + +**Output:** + +``` + Compiling proc-macro2 v1.0.106 + Compiling serde_core v1.0.228 + ... + Compiling app v0.0.1 (/tmp/cook-demo/app) ← workspace stub (trivial) + Compiling logic v0.0.1 (/tmp/cook-demo/logic) + Compiling model v0.0.1 (/tmp/cook-demo/model) + Finished `release` profile [optimized] target(s) in 1.99s + +real 0m5.80s +``` + +```bash +# Layer 2: cook full recipe on top (fast — third-party already compiled) +CARGO=$(which cargo) time cargo chef cook --release --recipe-path /tmp/recipe-full.json +``` + +**Output:** + +``` + Compiling model v0.0.1 (/tmp/cook-demo/model) + Compiling logic v0.0.1 (/tmp/cook-demo/logic) + Compiling app v0.0.1 (/tmp/cook-demo/app) + Finished `release` profile [optimized] target(s) in 0.06s + +real 0m0.07s ← 83x faster than cold cook +``` + +**Key result:** when the thirdparty layer is warm, cooking the full recipe +takes **0.07 s** — only the lightweight workspace stubs are (re)compiled. +No external crates are touched. + +### 4.3 Docker build (workaround) + +```bash +cd examples/workspace-split-cache +DOCKER_BUILDKIT=1 docker build -t workspace-split-cache . +docker run --rm workspace-split-cache +# Expected: {"name":"world"} +``` + +Use `--progress=plain` to see layer caching in action: + +```bash +# First build (cold): +DOCKER_BUILDKIT=1 docker build --progress=plain -t workspace-split-cache . 2>&1 \ + | grep -E "RUN cargo|CACHED" + +# Expected (all layers run): +# #7 RUN cargo chef prepare --recipe-path recipe.json +# #8 RUN python3 strip_path_deps.py recipe.json recipe-thirdparty.json +# #9 RUN cargo chef cook --release --recipe-path recipe.json (cook_thirdparty) +# #10 RUN cargo chef cook --release --recipe-path recipe.json (cook) +# #11 RUN cargo build --release --bin app + +# Simulate a workspace-internal change (no external dep change): +echo "" >> app/src/main.rs + +DOCKER_BUILDKIT=1 docker build --progress=plain -t workspace-split-cache . 2>&1 \ + | grep -E "RUN cargo|CACHED" + +# Expected (cook_thirdparty is CACHED; only cook + builder re-run): +# CACHED #9 RUN cargo chef cook ... (cook_thirdparty — STABLE!) +# #10 RUN cargo chef cook ... (cook — fast, stubs only) +# #11 RUN cargo build ... (builder) + +# Restore +git checkout app/src/main.rs +``` + +--- + +## 5. Solution B — native `--external-only` flag + +The `--external-only` flag for `cargo chef prepare` produces a third-party-only +recipe natively, with no Python script needed. + +### 5.1 Usage + +```bash +cd examples/workspace-split-cache + +# Generate external-only recipe natively +cargo chef prepare --external-only --recipe-path /tmp/recipe-ext.json +``` + +**Output** (identical to the Python workaround): + +```toml +=== app/Cargo.toml === +[package] +name = "app" +edition = "2021" +version = "0.0.1" + +[dependencies] +anyhow = "1" # [dependencies.logic] path dep stripped natively + +[[bin]] +path = "src/main.rs" +name = "app" +... +``` + +Lock file packages (workspace crates absent): + +``` +['anyhow', 'itoa', 'memchr', 'proc-macro2', 'quote', 'serde', 'serde_core', +'serde_derive', 'serde_json', 'syn', 'unicode-ident', 'zmij'] +``` + +### 5.2 Two-layer Dockerfile (native) + +See `Dockerfile.native`. The only difference from `Dockerfile` is: + +```dockerfile +# Instead of: +RUN cargo chef prepare --recipe-path recipe.json +RUN python3 strip_path_deps.py recipe.json recipe-thirdparty.json + +# You write: +RUN cargo chef prepare --recipe-path recipe.json +RUN cargo chef prepare --external-only --recipe-path recipe-thirdparty.json +``` + +No Python installation step in the Docker image. No `apt-get install python3`. + +`Dockerfile.native` uses a locally built `cargo-chef` binary injected via a +Docker [named build context][buildx-context]. Build it in two steps: + +```bash +# Step 1 — build cargo-chef from the repo root and tag it: +docker build -t cargo-chef-local ../../ + +# Step 2 — build the example, providing the local binary as a named context: +docker build \ + --build-context cargo-chef-local=docker-image://cargo-chef-local:latest \ + -f Dockerfile.native \ + -t workspace-split-cache-native \ + . +``` + +Once `--external-only` is released on crates.io, the `COPY --from=cargo-chef-local` +line can be replaced with the standard `RUN cargo install --locked cargo-chef`. + +[buildx-context]: https://docs.docker.com/build/building/context/#named-contexts + +### 5.3 Stability proof + +The core property: two workspaces that differ only in their internal path deps +produce **identical** external-only recipes. + +```bash +# Verify: add a new path dep, check the external-only recipe doesn't change +cargo chef prepare --external-only --recipe-path /tmp/recipe-ext-before.json + +# Simulate adding a new internal path dependency to logic +cat >> logic/Cargo.toml << 'EOF' + +[dependencies.new-internal] +path = "../model" +EOF + +cargo chef prepare --external-only --recipe-path /tmp/recipe-ext-after.json + +diff /tmp/recipe-ext-before.json /tmp/recipe-ext-after.json +# Expected: no output (files are identical) + +git checkout logic/Cargo.toml +``` + +--- + +## 6. Related open issues + +| Issue | Summary | Relationship | +| ------ | ------------------------------------------------------------------------------ | ----------------------------------------------- | +| [#314] | Workspace with multiple interdependent crates — cook layer never cached | **Exact same problem** | +| [#75] | cargo-chef doesn't cache local deps — 270s vs 70s measured | **Same root cause, measured** | +| [#181] | Request for `--workspace --exclude` in `cargo chef cook` | Related — about cook, not prepare | +| [#305] | Nextest recompiles all deps (local crate version masking causes lock mismatch) | Partially fixed by stripping local lock entries | +| [#4] | Cross-workspace out-of-tree path deps not handled | Different issue | + +--- + +## 7. Implementation notes + +### What was implemented + +The `--external-only` flag is implemented in: + +| File | Change | +| ---------------------------------------- | ---------------------------------------------------------------------------------------------------------------- | +| `src/skeleton/external_only.rs` | New module: `strip_path_deps()` and `strip_local_packages_from_lock()` operating on `toml::Value` | +| `src/skeleton/mod.rs` | New public method `Skeleton::strip_path_deps(&mut self)` | +| `src/recipe.rs` | `external_only: bool` parameter added to `Recipe::prepare()` (**breaking change** for library users — see below) | +| `src/main.rs` | `--external-only` flag added to the `Prepare` subcommand | +| `tests/skeletons/tests/external_only.rs` | 3 new integration tests | +| `tests/recipe.rs` | Updated call to `Recipe::prepare` with new parameter | + +### What the implementation does + +1. **Strips path-based dependency entries** from every manifest in the skeleton. + Uses `toml::Value` (the `toml` crate is already a dependency) so the + stripping is structurally correct — not regex-based string manipulation. + Handles: top-level dep sections, `[target.'cfg(...)'.dependencies]`, and + `[workspace.dependencies]`. + +2. **Removes local (no-`source`) package entries from the lock file.** + External packages always have `source = "registry+..."`. Local workspace + members do not. Removing them makes the recipe immune to workspace-membership + changes. + +3. **Handles workspace-inherited path deps (Rust 1.64+ `{ workspace = true }`).** + Uses a two-pass approach: first, collect the names of all `[workspace.dependencies]` + entries that carry `path = "..."`. Then, in every member manifest, remove both + the workspace declaration _and_ any `dep = { workspace = true }` reference whose + name is in that set. After stripping, no manifest references a workspace dep that + no longer exists. + +### Test summary + +``` +cargo test -- external_only + +running 7 tests +test tests::external_only::external_only_strips_local_packages_from_lock_file ... ok +test tests::external_only::external_only_strips_path_deps_from_manifests ... ok +test tests::external_only::external_only_stable_across_workspace_internal_change ... ok +test skeleton::external_only::tests::keeps_external_deps_untouched ... ok +test skeleton::external_only::tests::strips_path_dep_from_dependencies ... ok +test skeleton::external_only::tests::strips_path_dep_from_target_specific_dependencies ... ok +test skeleton::external_only::tests::strips_path_dep_from_workspace_dependencies ... ok +test skeleton::external_only::tests::strips_workspace_true_refs_to_workspace_path_deps ... ok +test skeleton::external_only::tests::strips_all_path_deps_leaving_valid_manifest ... ok +test skeleton::external_only::tests::strips_local_packages_from_lock_file ... ok + +test result: ok. 10 passed +``` + +The third test — `external_only_stable_across_workspace_internal_change` — +directly verifies the core property: two skeletons that differ only in their +internal path deps produce identical recipes after `strip_path_deps()`. + +The fourth test — `strips_workspace_true_refs_to_workspace_path_deps` — verifies +the workspace-inheritance case: `{ workspace = true }` references to path deps +are correctly removed from member manifests. + +### Breaking API change + +`Recipe::prepare` is a `pub` function re-exported from `lib.rs`. Adding the +positional `external_only: bool` parameter is a **breaking change** for any +crate that calls `Recipe::prepare` directly (rather than through the CLI). + +cargo-chef is primarily a CLI tool and the library API has no documented +stability promise, so this is expected to have zero impact in practice. If +the API is stabilised in the future a builder-pattern or options-struct design +would allow new flags to be added without further breakage. This should be +noted in the PR description when merging. diff --git a/examples/workspace-split-cache/app/Cargo.toml b/examples/workspace-split-cache/app/Cargo.toml new file mode 100644 index 0000000..c31e0e7 --- /dev/null +++ b/examples/workspace-split-cache/app/Cargo.toml @@ -0,0 +1,14 @@ +[package] +name = "app" +version = "0.1.0" +edition = "2021" + +[[bin]] +name = "app" +path = "src/main.rs" + +[dependencies] +# Intra-workspace path dependency +logic = { path = "../logic" } +# External (third-party) dependency +anyhow = "1" diff --git a/examples/workspace-split-cache/app/src/main.rs b/examples/workspace-split-cache/app/src/main.rs new file mode 100644 index 0000000..40ab9ed --- /dev/null +++ b/examples/workspace-split-cache/app/src/main.rs @@ -0,0 +1,5 @@ +fn main() -> anyhow::Result<()> { + let greeting = logic::greet("world")?; + println!("{greeting}"); + Ok(()) +} diff --git a/examples/workspace-split-cache/logic/Cargo.toml b/examples/workspace-split-cache/logic/Cargo.toml new file mode 100644 index 0000000..af8c73e --- /dev/null +++ b/examples/workspace-split-cache/logic/Cargo.toml @@ -0,0 +1,15 @@ +[package] +name = "logic" +version = "0.1.0" +edition = "2021" + +[lib] +name = "logic" +path = "src/lib.rs" + +[dependencies] +# Intra-workspace path dependency +model = { path = "../model" } +# External (third-party) dependencies +serde_json = "1" +anyhow = "1" diff --git a/examples/workspace-split-cache/logic/src/lib.rs b/examples/workspace-split-cache/logic/src/lib.rs new file mode 100644 index 0000000..56d753e --- /dev/null +++ b/examples/workspace-split-cache/logic/src/lib.rs @@ -0,0 +1,8 @@ +use model::Greeting; + +pub fn greet(name: &str) -> anyhow::Result { + let g = Greeting { + name: name.to_string(), + }; + Ok(serde_json::to_string(&g)?) +} diff --git a/examples/workspace-split-cache/model/Cargo.toml b/examples/workspace-split-cache/model/Cargo.toml new file mode 100644 index 0000000..ad7b492 --- /dev/null +++ b/examples/workspace-split-cache/model/Cargo.toml @@ -0,0 +1,12 @@ +[package] +name = "model" +version = "0.1.0" +edition = "2021" + +[lib] +name = "model" +path = "src/lib.rs" + +[dependencies] +# External (third-party) dependencies only — no intra-workspace path deps. +serde = { version = "1", features = ["derive"] } diff --git a/examples/workspace-split-cache/model/src/lib.rs b/examples/workspace-split-cache/model/src/lib.rs new file mode 100644 index 0000000..30d6dca --- /dev/null +++ b/examples/workspace-split-cache/model/src/lib.rs @@ -0,0 +1,6 @@ +use serde::{Deserialize, Serialize}; + +#[derive(Debug, Serialize, Deserialize)] +pub struct Greeting { + pub name: String, +} diff --git a/examples/workspace-split-cache/strip_path_deps.py b/examples/workspace-split-cache/strip_path_deps.py new file mode 100644 index 0000000..619d781 --- /dev/null +++ b/examples/workspace-split-cache/strip_path_deps.py @@ -0,0 +1,202 @@ +#!/usr/bin/env python3 +""" +Generate a cargo-chef recipe containing only external (third-party) dependencies. + +Usage: + python3 strip_path_deps.py + +Why +--- +cargo-chef's `recipe.json` contains every workspace manifest including the +intra-workspace path dependencies (`core = { path = "../core" }`). Because of +this, the Docker layer produced by `cargo chef cook` is invalidated whenever +*any* workspace manifest changes -- even a purely structural change like adding a +new internal crate or splitting an existing one -- even though no external +(third-party) dependency changed at all. + +This script produces `recipe-thirdparty.json` by: + + 1. Stripping all path-based dependency *sections* from every manifest in the + recipe. cargo-chef serialises manifests with `toml::to_string()`, which + always expands dotted-table dependencies into dedicated `[section.name]` + headers. A section whose body contains a `path = "..."` line is an + intra-workspace dependency; the entire section is removed, leaving only the + external crates that cargo must download and compile. + + 2. Stripping all local (no-source) package entries from the lock file so that + adding or removing a workspace member does not change the thirdparty recipe + either. (cargo-chef already normalises local crate versions to a dummy + constant, but new members still add new lock entries.) + +The resulting recipe is stable across any pure workspace-internal change: the +`cook_thirdparty` Docker layer built from it is only re-run when an actual +third-party dependency is added, removed, or updated. + +Implementation notes +-------------------- +The script uses structured text parsing rather than a TOML library so that it +works with the Python standard library alone (no third-party packages required). +It relies on the fact that cargo-chef always serialises manifest contents via +`toml::to_string()`, which produces a deterministic, well-structured format: +intra-workspace dependencies always appear as dedicated `[section.dep-name]` +sections, never as inline tables. + +Limitations +----------- +* Workspace-inherited deps (`dep = { workspace = true }`) where the shared + declaration in `[workspace.dependencies]` carries `path = "..."` are not + handled by this script. Use the native ``--external-only`` flag instead, which + handles this case correctly via a two-pass TOML-aware approach. + +* Target-conditional dependency sections (`[target.'cfg(...)'.dependencies.*]`) + are stripped correctly because the section regex matches any header of the form + `[*.(dependencies|dev-dependencies|build-dependencies).*]`. + +Note +---- +The native flag ``--external-only`` is now available in cargo-chef: + cargo chef prepare --external-only --recipe-path recipe-thirdparty.json +This script is kept as a reference for the approach and for older cargo-chef +versions that do not yet include the flag. For new projects, prefer the native +flag (see Dockerfile.native). +""" + +import json +import re +import sys +from typing import List, Optional, Tuple + +# --------------------------------------------------------------------------- +# TOML section-level text manipulation +# --------------------------------------------------------------------------- + +# Matches any TOML section header line that introduces an intra-workspace path +# dependency entry, e.g.: +# [dependencies.my-crate] +# [dev-dependencies.my-crate] +# [build-dependencies.my-crate] +# [target.'cfg(unix)'.dependencies.my-crate] +_PATH_DEP_SECTION_RE = re.compile( + r"^\[(?:[^\]]*\.)?" # optional leading table path + r"(?:dependencies|dev-dependencies|build-dependencies)" + r"\.[^\]]+\]$" +) + +# Matches the start of ANY TOML section or array-of-tables header. +_ANY_SECTION_RE = re.compile(r"^\[") + + +def _split_into_sections( + text: str, +) -> List[Tuple[Optional[str], List[str]]]: + """Split a TOML string into (header, body_lines) pairs. + + The first group collects any lines that appear before the first section + header; its header is None. + """ + sections: List[Tuple[Optional[str], List[str]]] = [] + current_header: Optional[str] = None + current_body: List[str] = [] + + for line in text.splitlines(keepends=True): + stripped = line.rstrip("\n").rstrip() + if _ANY_SECTION_RE.match(stripped): + sections.append((current_header, current_body)) + current_header = line + current_body = [] + else: + current_body.append(line) + + sections.append((current_header, current_body)) + return sections + + +def strip_path_deps_from_manifest(contents: str) -> str: + """Return *contents* (a TOML string) with all path-dep sections removed. + + cargo-chef serialises intra-workspace path dependencies as dedicated + `[dependencies.NAME]` sections whose body contains `path = "..."`. + This function removes those sections entirely. + """ + sections = _split_into_sections(contents) + result_parts: List[str] = [] + + for header, body in sections: + if header is not None: + header_stripped = header.rstrip("\n").rstrip() + if _PATH_DEP_SECTION_RE.match(header_stripped): + # Only skip if the body actually contains a path assignment. + has_path = any( + line.lstrip().startswith("path =") for line in body + ) + if has_path: + continue # drop this section + result_parts.append(header if header is not None else "") + result_parts.extend(body) + + return "".join(result_parts) + + +# --------------------------------------------------------------------------- +# Lock-file filtering +# --------------------------------------------------------------------------- + + +def strip_local_packages_from_lockfile(lock_str: str) -> str: + """Remove local (no-source) [[package]] entries from a Cargo.lock string. + + External packages always carry a ``source = "registry+..."`` field. + Local workspace members have no ``source`` field. Removing them makes the + thirdparty recipe immune to workspace membership changes (adding/removing + internal crates). + """ + sections = _split_into_sections(lock_str) + result_parts: List[str] = [] + + for header, body in sections: + if header is not None: + header_stripped = header.rstrip("\n").rstrip() + if header_stripped == "[[package]]": + has_source = any( + line.lstrip().startswith("source =") for line in body + ) + if not has_source: + continue # drop local workspace member entry + result_parts.append(header if header is not None else "") + result_parts.extend(body) + + return "".join(result_parts) + + +# --------------------------------------------------------------------------- +# Main +# --------------------------------------------------------------------------- + + +def main() -> None: + if len(sys.argv) != 3: + print(f"Usage: {sys.argv[0]} ", file=sys.stderr) + sys.exit(1) + + input_path, output_path = sys.argv[1], sys.argv[2] + + with open(input_path) as f: + recipe = json.load(f) + + # Strip path deps from every manifest. + for manifest in recipe["skeleton"]["manifests"]: + manifest["contents"] = strip_path_deps_from_manifest(manifest["contents"]) + + # Strip local packages from the lock file (if present). + if recipe["skeleton"].get("lock_file"): + recipe["skeleton"]["lock_file"] = strip_local_packages_from_lockfile( + recipe["skeleton"]["lock_file"] + ) + + with open(output_path, "w") as f: + json.dump(recipe, f, indent=2) + f.write("\n") + + +if __name__ == "__main__": + main() diff --git a/src/lib.rs b/src/lib.rs index d50fa59..0fc7285 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -2,6 +2,7 @@ mod recipe; mod skeleton; pub use recipe::{ - AllFeatures, CommandArg, CookArgs, DefaultFeatures, OptimisationProfile, Recipe, TargetArgs, + AllFeatures, CommandArg, CookArgs, DefaultFeatures, OptimisationProfile, PrepareOptions, + Recipe, TargetArgs, }; pub use skeleton::*; diff --git a/src/main.rs b/src/main.rs index a9ecf5a..8743b72 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1,6 +1,7 @@ use anyhow::{anyhow, Context}; use chef::{ - AllFeatures, CommandArg, CookArgs, DefaultFeatures, OptimisationProfile, Recipe, TargetArgs, + AllFeatures, CommandArg, CookArgs, DefaultFeatures, OptimisationProfile, PrepareOptions, + Recipe, TargetArgs, }; use clap::crate_version; use clap::Parser; @@ -60,6 +61,26 @@ pub struct Prepare { /// that are not necessary to successfully compile the specific binary. #[arg(long)] bin: Option, + + /// Strip all `path = "…"` dependency entries from the recipe. + /// + /// The resulting recipe contains only external (crates.io / git) dependencies. + /// A Docker layer built by `cargo chef cook` from this recipe is only invalidated + /// when an external dependency actually changes — not when workspace-internal + /// crates are added, removed, or restructured. + /// + /// Note: this removes every dependency entry that has a `path` field, which + /// includes both intra-workspace path deps and any other local path deps + /// (e.g. out-of-tree crates referenced by an absolute or relative path). + /// + /// Use this flag to build a stable "third-party dependencies" Docker layer: + /// + /// cargo chef prepare --external-only --recipe-path recipe-thirdparty.json + /// cargo chef cook --release --recipe-path recipe-thirdparty.json + /// + /// Then follow with the standard prepare + cook cycle on top of that layer. + #[arg(long)] + external_only: bool, } #[derive(Parser)] @@ -303,9 +324,19 @@ fn _main() -> Result<(), anyhow::Error> { }) .context("Failed to cook recipe.")?; } - Command::Prepare(Prepare { recipe_path, bin }) => { - let recipe = - Recipe::prepare(current_directory, bin).context("Failed to compute recipe")?; + Command::Prepare(Prepare { + recipe_path, + bin, + external_only, + }) => { + let recipe = Recipe::prepare( + current_directory, + PrepareOptions { + member: bin, + external_only, + }, + ) + .context("Failed to compute recipe")?; let serialized = serde_json::to_string(&recipe).context("Failed to serialize recipe.")?; fs::write(recipe_path, serialized).context("Failed to save recipe to 'recipe.json'")?; diff --git a/src/recipe.rs b/src/recipe.rs index 1755ceb..01e00f8 100644 --- a/src/recipe.rs +++ b/src/recipe.rs @@ -50,9 +50,28 @@ pub struct CookArgs { pub jobs: Option, } +/// Options for [`Recipe::prepare`]. +/// +/// Using a struct instead of positional parameters allows new options to be +/// added in the future without breaking call sites. +#[derive(Debug, Default)] +pub struct PrepareOptions { + /// Only build the recipe for the given workspace member binary. + pub member: Option, + /// Strip all `path = "…"` dependency entries from the recipe. + /// + /// See [`Skeleton::strip_path_deps`] for the full description. + pub external_only: bool, +} + impl Recipe { - pub fn prepare(base_path: PathBuf, member: Option) -> Result { - let skeleton = Skeleton::derive(base_path, member)?; + pub fn prepare(base_path: PathBuf, options: PrepareOptions) -> Result { + let mut skeleton = Skeleton::derive(base_path, options.member)?; + if options.external_only { + skeleton + .strip_path_deps() + .context("Failed to strip path dependencies")?; + } Ok(Recipe { skeleton }) } diff --git a/src/skeleton/external_only.rs b/src/skeleton/external_only.rs new file mode 100644 index 0000000..bf66803 --- /dev/null +++ b/src/skeleton/external_only.rs @@ -0,0 +1,480 @@ +//! Strip all `path = "…"` dependencies from a skeleton to produce a +//! "third-party only" recipe whose content is stable across any pure +//! workspace-internal change. +//! +//! # Why +//! +//! `cargo chef prepare` captures every workspace manifest including the +//! `path = "..."` dependencies. Any structural change to the +//! workspace (adding a crate, splitting one, renaming a binary target, etc.) +//! changes `recipe.json`, which invalidates the `cargo chef cook` Docker layer +//! and forces a full recompile of all third-party crates. +//! +//! `strip_path_deps` post-processes an already-derived [`Skeleton`] by: +//! +//! 1. Removing **every** dependency entry that carries a `path = "..."` field +//! from every manifest in the skeleton (top-level sections, target-specific +//! sections, and `[workspace.dependencies]`). This includes both +//! intra-workspace path deps and any other local path deps (e.g. out-of-tree +//! crates referenced by absolute or relative path). +//! +//! 2. Removing all local (no-`source`) package entries from the lock file. +//! External packages always carry a `source` field (e.g. +//! `"registry+https://github.com/rust-lang/crates.io-index"`); local +//! workspace members do not. Removing them makes the recipe immune to +//! workspace-membership changes (adding/removing internal crates). +//! +//! The resulting skeleton is then serialised into `recipe.json`. Because it +//! no longer contains any path-dep information, the Docker layer +//! produced by `cargo chef cook` from this recipe is only invalidated when an +//! *external* dependency actually changes. + +use std::collections::HashSet; + +use anyhow::Context; + +use super::Manifest; + +const DEP_SECTIONS: &[&str] = &["dependencies", "dev-dependencies", "build-dependencies"]; + +/// Remove all `path = "…"` dependency entries from every manifest and all +/// local package entries from the lock file. +/// +/// Uses a two-pass approach to handle workspace-inherited path deps: +/// +/// 1. Scan every manifest for `[workspace.dependencies]` entries that carry +/// `path = "…"` and collect their names. +/// 2. Strip direct path dep entries **and** any `dep = { workspace = true }` +/// references whose name appears in the set collected in step 1. +/// +/// This ensures that after stripping, no manifest references a workspace dep +/// that no longer exists in `[workspace.dependencies]`. +pub(super) fn strip_path_deps( + manifests: &mut [Manifest], + lock_file: &mut Option, +) -> anyhow::Result<()> { + // Pass 1: collect names of workspace-inherited deps that are path deps. + let workspace_path_dep_names: HashSet = manifests + .iter() + .flat_map(|m| collect_workspace_path_dep_names(&m.contents)) + .collect(); + + // Pass 2: strip from every manifest. + for manifest in manifests.iter_mut() { + manifest.contents = + strip_from_manifest_contents(&manifest.contents, &workspace_path_dep_names) + .with_context(|| { + format!( + "failed to strip path deps from manifest {}", + manifest.relative_path.display() + ) + })?; + } + if let Some(lock) = lock_file { + *lock = strip_local_packages_from_lock(lock) + .context("failed to strip local packages from lock file")?; + } + Ok(()) +} + +/// Collect the names of all `[workspace.dependencies]` entries that have a +/// `path` field. Returns an empty `HashSet` when the manifest has no +/// `[workspace.dependencies]` section or when it is not parseable. +fn collect_workspace_path_dep_names(contents: &str) -> HashSet { + let value: toml::Value = match toml::from_str(contents) { + Ok(v) => v, + Err(_) => return HashSet::new(), + }; + value + .get("workspace") + .and_then(|ws| ws.get("dependencies")) + .and_then(|d| d.as_table()) + .map(|table| { + table + .iter() + .filter_map(|(name, spec)| { + if spec.get("path").is_some() { + Some(name.clone()) + } else { + None + } + }) + .collect() + }) + .unwrap_or_default() +} + +/// Remove all `path = "…"` entries from a single manifest's TOML contents, +/// and remove any `{ workspace = true }` references whose dep name is in +/// `workspace_path_dep_names`. +/// +/// Handles: +/// - `[dependencies]` / `[dev-dependencies]` / `[build-dependencies]` +/// - `[target.'cfg(...)'.dependencies]` (and dev/build variants) +/// - `[workspace.dependencies]` +fn strip_from_manifest_contents( + contents: &str, + workspace_path_dep_names: &HashSet, +) -> anyhow::Result { + let mut value: toml::Value = toml::from_str(contents) + .context("failed to parse manifest TOML during path-dep stripping")?; + + // Top-level [dependencies], [dev-dependencies], [build-dependencies] + strip_path_deps_from_value(&mut value, workspace_path_dep_names); + + // [target.'cfg(...)'.{dependencies,dev-dependencies,build-dependencies}] + if let Some(targets) = value.get_mut("target") { + if let Some(targets_table) = targets.as_table_mut() { + for (_, target_cfg) in targets_table.iter_mut() { + strip_path_deps_from_value(target_cfg, workspace_path_dep_names); + } + } + } + + // [workspace.dependencies] (workspace inheritance, rust 1.64+) + if let Some(workspace) = value.get_mut("workspace") { + strip_path_deps_from_value(workspace, workspace_path_dep_names); + } + + toml::to_string(&value) + .context("failed to re-serialise manifest TOML after stripping path deps") +} + +/// Remove every dependency entry that has a `path` field, or whose name +/// appears in `workspace_path_dep_names` with `{ workspace = true }`, from +/// the given `toml::Value`'s dependency sections. +fn strip_path_deps_from_value(value: &mut toml::Value, workspace_path_dep_names: &HashSet) { + for section in DEP_SECTIONS { + if let Some(deps) = value.get_mut(section) { + if let Some(table) = deps.as_table_mut() { + table.retain(|name, dep_spec| { + // Remove direct path deps. + if dep_spec.get("path").is_some() { + return false; + } + // Remove `{ workspace = true }` references to path deps + // that were declared in [workspace.dependencies] with path = + // and have now been stripped from that section. + let is_workspace_ref = dep_spec + .get("workspace") + .and_then(|v| v.as_bool()) + .unwrap_or(false); + if is_workspace_ref && workspace_path_dep_names.contains(name) { + return false; + } + true + }); + } + } + } +} + +/// Remove all local (no-`source`) `[[package]]` entries from a serialised +/// Cargo.lock TOML string. +fn strip_local_packages_from_lock(lock_str: &str) -> anyhow::Result { + let mut lock: toml::Value = toml::from_str(lock_str) + .context("failed to parse lock file TOML during path-dep stripping")?; + if let Some(packages) = lock.get_mut("package").and_then(|p| p.as_array_mut()) { + // Keep only packages that have a `source` field — those are external. + packages.retain(|pkg| pkg.get("source").is_some()); + } + toml::to_string(&lock) + .context("failed to re-serialise lock file TOML after stripping local packages") +} + +#[cfg(test)] +mod tests { + use super::*; + + fn manifest(contents: &str) -> Manifest { + use std::path::PathBuf; + Manifest { + relative_path: PathBuf::from("Cargo.toml"), + contents: contents.to_string(), + targets: vec![], + } + } + + #[test] + fn strips_path_dep_from_dependencies() { + let m = manifest( + r#" +[package] +name = "app" +version = "0.1.0" + +[dependencies] +anyhow = "1" + +[dependencies.internal] +path = "../internal" +version = "0.0.1" +"#, + ); + let mut manifests = vec![m]; + let mut lock: Option = None; + strip_path_deps(&mut manifests, &mut lock).expect("strip_path_deps failed"); + + let contents = &manifests[0].contents; + assert!( + !contents.contains("path ="), + "path dep should be removed:\n{}", + contents + ); + assert!( + contents.contains("anyhow"), + "external dep should be kept:\n{}", + contents + ); + } + + #[test] + fn strips_out_of_workspace_path_dep() { + // The implementation removes ANY dep with a `path` field, not just + // intra-workspace ones. Verify an absolute or out-of-tree relative + // path dep is also removed. + let m = manifest( + r#" +[package] +name = "app" +version = "0.1.0" + +[dependencies] +anyhow = "1" + +[dependencies.external-local] +path = "/opt/some-other-project" +version = "0.1.0" +"#, + ); + let mut manifests = vec![m]; + let mut lock: Option = None; + strip_path_deps(&mut manifests, &mut lock).expect("strip_path_deps failed"); + + let contents = &manifests[0].contents; + assert!( + !contents.contains("path ="), + "out-of-workspace path dep should be removed:\n{}", + contents + ); + assert!( + contents.contains("anyhow"), + "external dep should be kept:\n{}", + contents + ); + } + + #[test] + fn keeps_external_deps_untouched() { + let original = r#" +[package] +name = "model" +version = "0.1.0" + +[dependencies.serde] +version = "1" +features = ["derive"] +"#; + let result = strip_from_manifest_contents(original, &HashSet::new()).expect("strip failed"); + assert!( + result.contains("serde"), + "serde should be kept:\n{}", + result + ); + assert!( + !result.contains("path ="), + "no path dep in result:\n{}", + result + ); + } + + #[test] + fn strips_path_dep_from_workspace_dependencies() { + let original = r#" +[workspace] +members = ["a", "b"] + +[workspace.dependencies] +serde = "1" + +[workspace.dependencies.internal] +path = "./internal" +version = "0.0.1" +"#; + let result = strip_from_manifest_contents(original, &HashSet::new()).expect("strip failed"); + assert!(result.contains("serde"), "serde kept:\n{}", result); + assert!(!result.contains("path ="), "path dep removed:\n{}", result); + } + + #[test] + fn strips_path_dep_from_target_specific_dependencies() { + let original = r#" +[package] +name = "platform_crate" +version = "0.1.0" + +[target.'cfg(unix)'.dependencies.internal] +path = "../internal" +version = "0.0.1" + +[target.'cfg(unix)'.dependencies] +libc = "0.2" +"#; + let result = strip_from_manifest_contents(original, &HashSet::new()).expect("strip failed"); + assert!(result.contains("libc"), "libc kept:\n{}", result); + assert!(!result.contains("path ="), "path dep removed:\n{}", result); + } + + #[test] + fn strips_workspace_true_refs_to_workspace_path_deps() { + // The workspace root defines `internal = { path = "./internal" }` in + // [workspace.dependencies]. A member crate references it via + // `internal = { workspace = true }`. After stripping, both the + // workspace declaration and the member reference must be gone. + let workspace_root = r#" +[workspace] +members = ["member"] + +[workspace.dependencies] +serde = "1" + +[workspace.dependencies.internal] +path = "./internal" +version = "0.0.1" +"#; + let member = r#" +[package] +name = "member" +version = "0.1.0" + +[dependencies] +serde = { workspace = true } +internal = { workspace = true } +"#; + // The path dep names come from the workspace root. + let ws_path_names = collect_workspace_path_dep_names(workspace_root); + assert!( + ws_path_names.contains("internal"), + "should detect 'internal' as workspace path dep" + ); + + let stripped_member = + strip_from_manifest_contents(member, &ws_path_names).expect("strip failed"); + assert!( + stripped_member.contains("serde"), + "serde (external) should be kept:\n{}", + stripped_member + ); + assert!( + !stripped_member.contains("internal"), + "'internal' workspace = true ref should be removed:\n{}", + stripped_member + ); + } + + #[test] + fn strips_local_packages_from_lock_file() { + let lock = r#" +version = 3 + +[[package]] +name = "anyhow" +version = "1.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "abc" + +[[package]] +name = "my-local-crate" +version = "0.0.1" + +[[package]] +name = "serde" +version = "1.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "def" +"#; + let result = strip_local_packages_from_lock(lock).expect("strip lock failed"); + assert!(result.contains("anyhow"), "anyhow kept:\n{}", result); + assert!(result.contains("serde"), "serde kept:\n{}", result); + assert!( + !result.contains("my-local-crate"), + "local crate removed:\n{}", + result + ); + } + + #[test] + fn strips_all_path_deps_leaving_valid_manifest() { + // Edge case: every dependency in the manifest is a path dep. + // After stripping the [dependencies] section should be empty (or + // absent), and the resulting TOML must still be parseable. + let m = manifest( + r#" +[package] +name = "app" +version = "0.1.0" + +[dependencies.core] +path = "../core" + +[dependencies.util] +path = "../util" +"#, + ); + let mut manifests = vec![m]; + let mut lock: Option = None; + strip_path_deps(&mut manifests, &mut lock).expect("strip_path_deps failed"); + + let contents = &manifests[0].contents; + // No path dep entries must remain. + assert!( + !contents.contains("path ="), + "all path deps should be removed:\n{}", + contents + ); + // The result must still be valid TOML. + toml::from_str::(contents).expect("stripped manifest must be valid TOML"); + } + + #[test] + fn returns_error_on_invalid_manifest_toml() { + // strip_from_manifest_contents should return Err, not panic, when fed + // malformed TOML. + let invalid = "this is [ not valid toml !!!"; + let result = strip_from_manifest_contents(invalid, &HashSet::new()); + assert!( + result.is_err(), + "expected Err for invalid manifest TOML, got Ok" + ); + } + + #[test] + fn returns_error_on_invalid_lock_file_toml() { + // strip_local_packages_from_lock should return Err, not panic, when fed + // malformed TOML. + let invalid = "this is [ not valid toml !!!"; + let result = strip_local_packages_from_lock(invalid); + assert!( + result.is_err(), + "expected Err for invalid lock file TOML, got Ok" + ); + } + + #[test] + fn strip_path_deps_propagates_manifest_parse_error() { + // strip_path_deps should propagate errors from invalid manifests rather + // than panicking. + let bad = Manifest { + relative_path: std::path::PathBuf::from("bad/Cargo.toml"), + contents: "this is [ not valid toml !!!".to_string(), + targets: vec![], + }; + let mut manifests = vec![bad]; + let mut lock: Option = None; + let result = strip_path_deps(&mut manifests, &mut lock); + assert!( + result.is_err(), + "expected Err when manifest has invalid TOML, got Ok" + ); + } +} diff --git a/src/skeleton/mod.rs b/src/skeleton/mod.rs index d3d2e81..9cce887 100644 --- a/src/skeleton/mod.rs +++ b/src/skeleton/mod.rs @@ -1,3 +1,4 @@ +mod external_only; mod read; mod target; mod version_masking; @@ -80,6 +81,23 @@ impl Skeleton { }) } + /// Remove all `path = "…"` dependencies from every manifest and + /// all local (no-`source`) entries from the lock file. + /// + /// Any dependency entry that carries a `path` field is removed — this + /// includes both intra-workspace path deps and any other local path deps + /// (e.g. out-of-tree crates referenced by an absolute or relative path). + /// + /// This produces a "third-party only" skeleton whose content is stable + /// across any pure workspace-internal change (adding a crate, splitting + /// one, renaming a binary target, etc.). A Docker layer built by + /// `cargo chef cook` from a recipe serialised from this skeleton will only + /// be invalidated when an external (crates.io / git) dependency actually + /// changes. + pub fn strip_path_deps(&mut self) -> anyhow::Result<()> { + external_only::strip_path_deps(&mut self.manifests, &mut self.lock_file) + } + /// Given the manifests in the current skeleton, create the minimum set of files required to /// have a valid Rust project (i.e. write all manifests to disk and create dummy `lib.rs`, /// `main.rs` and `build.rs` files where needed). diff --git a/tests/recipe.rs b/tests/recipe.rs index fcb99bc..fbb328d 100644 --- a/tests/recipe.rs +++ b/tests/recipe.rs @@ -14,7 +14,11 @@ fn quick_recipe(content: &str) -> Recipe { bin_dir.child(filename).touch().unwrap(); test_dir.child(filename).touch().unwrap(); } - Recipe::prepare(recipe_directory.path().canonicalize().unwrap(), None).unwrap() + Recipe::prepare( + recipe_directory.path().canonicalize().unwrap(), + chef::PrepareOptions::default(), + ) + .unwrap() } #[test] diff --git a/tests/skeletons/tests/external_only.rs b/tests/skeletons/tests/external_only.rs new file mode 100644 index 0000000..fd1a9db --- /dev/null +++ b/tests/skeletons/tests/external_only.rs @@ -0,0 +1,312 @@ +use super::*; + +/// Verify that `--external-only` / `Skeleton::strip_path_deps` removes all +/// intra-workspace path dependencies from every manifest and all local +/// package entries from the lock file, while leaving external crates intact. + +#[test] +fn external_only_strips_path_deps_from_manifests() { + // Arrange: workspace with a binary that depends on an internal lib and an + // external crate. + let project = CargoWorkspace::new() + .manifest( + ".", + r#" +[workspace] +members = ["app", "model"] +"#, + ) + .bin_package( + "app", + r#" +[package] +name = "app" +version = "0.1.0" +edition = "2021" + +[dependencies] +anyhow = "1" + +[dependencies.model] +path = "../model" +version = "0.0.1" +"#, + ) + .lib_package( + "model", + r#" +[package] +name = "model" +version = "0.1.0" +edition = "2021" + +[dependencies.serde] +version = "1" +features = ["derive"] +"#, + ) + .touch("Cargo.lock") + .build(); + + // Act + let mut skeleton = Skeleton::derive(project.path(), None).unwrap(); + skeleton.strip_path_deps(); + + // Assert: no manifest should contain an intra-workspace path dep. + // We check for `path = "../` (relative paths are the hallmark of + // workspace-internal deps). Binary/lib target paths like + // `path = "src/main.rs"` are NOT relative upward and are kept. + for manifest in &skeleton.manifests { + // We cannot use a blanket `!contains("path =")` because [[bin]]/[[lib]] + // target entries keep their `path = "src/..."` source-file pointers, + // which is correct behaviour. Instead check that the known intra-workspace + // dep section is gone. + assert!( + !manifest.contents.contains("[dependencies.model]"), + "intra-workspace dep [dependencies.model] found in {} after strip_path_deps:\n{}", + manifest.relative_path.display(), + manifest.contents, + ); + } + + // Assert: external deps are preserved + let app_manifest = skeleton + .manifests + .iter() + .find(|m| m.relative_path.ends_with("app/Cargo.toml")) + .expect("app/Cargo.toml should be present"); + assert!( + app_manifest.contents.contains("anyhow"), + "anyhow should be kept in app manifest:\n{}", + app_manifest.contents, + ); + + let model_manifest = skeleton + .manifests + .iter() + .find(|m| m.relative_path.ends_with("model/Cargo.toml")) + .expect("model/Cargo.toml should be present"); + assert!( + model_manifest.contents.contains("serde"), + "serde should be kept in model manifest:\n{}", + model_manifest.contents, + ); +} + +#[test] +fn external_only_strips_local_packages_from_lock_file() { + // Arrange: workspace with a real Cargo.lock containing both local and + // external entries. We write a hand-crafted lock file that mimics the + // structure cargo-chef serialises (toml::to_string output). + let lock_contents = r#" +version = 3 + +[[package]] +name = "anyhow" +version = "1.0.100" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "abc123" + +[[package]] +name = "app" +version = "0.0.1" + +[[package]] +name = "model" +version = "0.0.1" + +[[package]] +name = "serde" +version = "1.0.200" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "def456" +dependencies = [ + "serde_derive", +] + +[[package]] +name = "serde_derive" +version = "1.0.200" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ghi789" +"#; + + let project = CargoWorkspace::new() + .manifest( + ".", + r#" +[workspace] +members = ["app", "model"] +"#, + ) + .bin_package( + "app", + r#" +[package] +name = "app" +version = "0.1.0" + +[dependencies.model] +path = "../model" +"#, + ) + .lib_package( + "model", + r#" +[package] +name = "model" +version = "0.1.0" +"#, + ) + .file("Cargo.lock", lock_contents) + .build(); + + // Act + let mut skeleton = Skeleton::derive(project.path(), None).unwrap(); + skeleton.strip_path_deps(); + + // Assert: local workspace packages must not appear in the lock file. + // Parse the TOML and inspect package names directly to avoid false + // positives from substring matches (e.g. a dep name containing "app"). + let lock_file = skeleton + .lock_file + .as_deref() + .expect("lock file should be present"); + + let lock_toml: toml::Value = + toml::from_str(lock_file).expect("stripped lock file should be valid TOML"); + let package_names: Vec<&str> = lock_toml + .get("package") + .and_then(|p| p.as_array()) + .expect("lock file should have a [[package]] array") + .iter() + .filter_map(|pkg| pkg.get("name").and_then(|n| n.as_str())) + .collect(); + + assert!( + !package_names.contains(&"app"), + "local 'app' should be removed from lock, got: {:?}", + package_names, + ); + assert!( + !package_names.contains(&"model"), + "local 'model' should be removed from lock, got: {:?}", + package_names, + ); + + // External packages must remain + assert!( + package_names.contains(&"anyhow"), + "anyhow should be kept in lock, got: {:?}", + package_names, + ); + assert!( + package_names.contains(&"serde"), + "serde should be kept in lock, got: {:?}", + package_names, + ); +} + +#[test] +fn external_only_stable_across_workspace_internal_change() { + // Verify the core property: two skeletons that differ only in their + // workspace-internal path deps (but share the same external deps) produce + // identical recipes after strip_path_deps. + + let base_manifest = r#" +[workspace] +members = ["app", "model"] +"#; + + // Version A: app depends on model + let project_a = CargoWorkspace::new() + .manifest(".", base_manifest) + .bin_package( + "app", + r#" +[package] +name = "app" +version = "0.1.0" + +[dependencies] +anyhow = "1" + +[dependencies.model] +path = "../model" +version = "0.0.1" +"#, + ) + .lib_package( + "model", + r#" +[package] +name = "model" +version = "0.1.0" + +[dependencies.serde] +version = "1" +"#, + ) + .touch("Cargo.lock") + .build(); + + // Version B: app no longer depends on model (simulates removing an + // internal dependency — a pure workspace-internal change) + let project_b = CargoWorkspace::new() + .manifest(".", base_manifest) + .bin_package( + "app", + r#" +[package] +name = "app" +version = "0.1.0" + +[dependencies] +anyhow = "1" +"#, + ) + .lib_package( + "model", + r#" +[package] +name = "model" +version = "0.1.0" + +[dependencies.serde] +version = "1" +"#, + ) + .touch("Cargo.lock") + .build(); + + // Act + let mut skeleton_a = Skeleton::derive(project_a.path(), None).unwrap(); + skeleton_a.strip_path_deps(); + + let mut skeleton_b = Skeleton::derive(project_b.path(), None).unwrap(); + skeleton_b.strip_path_deps(); + + // Assert: after stripping, both skeletons are identical (same manifests) + // Sort by relative_path first to guard against non-deterministic filesystem + // traversal order — the content comparison must not depend on iteration order. + skeleton_a + .manifests + .sort_by(|a, b| a.relative_path.cmp(&b.relative_path)); + skeleton_b + .manifests + .sort_by(|a, b| a.relative_path.cmp(&b.relative_path)); + + assert_eq!( + skeleton_a.manifests.len(), + skeleton_b.manifests.len(), + "manifest count should be equal" + ); + for (ma, mb) in skeleton_a.manifests.iter().zip(skeleton_b.manifests.iter()) { + assert_eq!( + ma.contents, + mb.contents, + "manifest {} should be identical after strip_path_deps", + ma.relative_path.display() + ); + } +} diff --git a/tests/skeletons/tests/mod.rs b/tests/skeletons/tests/mod.rs index 0dd09c7..09e5a25 100644 --- a/tests/skeletons/tests/mod.rs +++ b/tests/skeletons/tests/mod.rs @@ -8,6 +8,7 @@ use expect_test::expect; use predicates::prelude::*; mod core; +mod external_only; mod masking; mod toolchain; mod workspace;