LukeMathWalker / cargo-chef

A cargo-subcommand to speed up Rust Docker builds using Docker layer caching.
Apache License 2.0
1.72k stars 113 forks source link

remove default-members from recipe #253

Closed SuperFluffy closed 7 months ago

SuperFluffy commented 7 months ago

This patch deletes the workspace.default-members array in the skeleton derived from Cargo.toml if it exists.

Cargo errors out if workspace.default-members contains entries that don't exist in workspace.members. But cargo chef prepare --bin modifies workspace.members as part of its normal operation, leaving the other fields untouched.

default-members is an entirely end-user-facing setting that controls which workspace members are built when running cargo {check, build, test} (among others) at the workspace root without specifying a target package. It has no effect on cargo chef cook.

The before/after behavior is described below.

Buggy behaviour this patch fixes

Create the following workspace:

$ cat Cargo.toml
[workspace]
members = ["foo", "bar"]
default-members = ["foo", "bar"]
resolver = "2"

Prepare a recipe for only foo:

❯ cargo chef prepare --bin foo && jq . recipe.json
{
  "skeleton": {
    "manifests": [
      {
        "relative_path": "Cargo.toml",
        "contents": "[workspace]\nmembers = [\"foo\"]\ndefault-members = [\"foo\", \"bar\"]\nresolver = \"2\"\n",
        "targets": []
      },
      {
        "relative_path": "bar/Cargo.toml",
        "contents": "bench = []\ntest = []\nexample = []\n\n[[bin]]\npath = \"src/main.rs\"\nname = \"bar\"\ntest = true\ndoctest = true\nbench = true\ndoc = true\np
lugin = false\nproc-macro = false\nharness = true\nedition = \"2021\"\nrequired-features = []\n\n[package]\nname = \"bar\"\nedition = \"2021\"\nversion = \"0.0.1\"\n
autobins = true\nautoexamples = true\nautotests = true\nautobenches = true\n\n[dependencies]\n",
        "targets": [
          {
            "path": "src/main.rs",
            "kind": "Bin",
            "name": "bar"
          }
        ]
      },
      {
        "relative_path": "foo/Cargo.toml",
        "contents": "bench = []\ntest = []\nexample = []\n\n[[bin]]\npath = \"src/main.rs\"\nname = \"foo\"\ntest = true\ndoctest = true\nbench = true\ndoc = true\nplugin = false\nproc-macro = false\nharness = true\nedition = \"2021\"\nrequired-features = []\n\n[package]\nname = \"foo\"\nedition = \"2021\"\nversion = \"0.0.1\"\nautobins = true\nautoexamples = true\nautotests = true\nautobenches = true\n\n[dependencies]\n",
        "targets": [
          {
            "path": "src/main.rs",
            "kind": "Bin",
            "name": "foo"
          }
        ]
      }
    ],
    "config_file": null,
    "lock_file": "version = 3\n\n[[package]]\nname = \"bar\"\nversion = \"0.0.1\"\n\n[[package]]\nname = \"foo\"\nversion = \"0.0.1\"\n"
  }
}

Cook it (this overrides the contents of the directory because we are outside of docker, but that's ok because we are just demonstrating the behaviour; the error is the same as would be seen in docker):

$ cargo chef cook --bin foo --recipe-path recipe.json
error: package `/Users/janis/dev/scratch/demo-cargo-chef-default-members/bar` is listed in workspace’s default-members but is not a member.
thread 'main' panicked at 'Exited with status code: 101', /Users/janis/.cargo/registry/src/index.crates.io-6f17d22bba15001f/cargo-chef-0.1.61/src/recipe.rs:189:27
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

The Cargo.toml overridden by the cook showing the problem (default-members contains entries not in members):

$ cat Cargo.toml
[workspace]
members = ["foo"]
default-members = ["foo", "bar"]
resolver = "2"

After this patch:

$ cargo chef prepare --bin foo && jq . recipe.json
{
  "skeleton": {
    "manifests": [
      {
        "relative_path": "Cargo.toml",
        "contents": "[workspace]\nmembers = [\"foo\"]\nresolver = \"2\"\n",
        "targets": []
      },
      {
        "relative_path": "bar/Cargo.toml",
        "contents": "bench = []\ntest = []\nexample = []\n\n[[bin]]\npath = \"src/main.rs\"\nname = \"bar\"\ntest = true\ndoctest = true\nbench = true\ndoc = true\np
lugin = false\nproc-macro = false\nharness = true\nedition = \"2021\"\nrequired-features = []\n\n[package]\nname = \"bar\"\nedition = \"2021\"\nversion = \"0.0.1\"\n
autobins = true\nautoexamples = true\nautotests = true\nautobenches = true\n\n[dependencies]\n",
        "targets": [
          {
            "path": "src/main.rs",
            "kind": "Bin",
            "name": "bar"
          }
        ]
      },
      {
        "relative_path": "foo/Cargo.toml",
        "contents": "bench = []\ntest = []\nexample = []\n\n[[bin]]\npath = \"src/main.rs\"\nname = \"foo\"\ntest = true\ndoctest = true\nbench = true\ndoc = true\nplugin = false\nproc-macro = false\nharness = true\nedition = \"2021\"\nrequired-features = []\n\n[package]\nname = \"foo\"\nedition = \"2021\"\nversion = \"0.0.1\"\nautobins = true\nautoexamples = true\nautotests = true\nautobenches = true\n\n[dependencies]\n",
        "targets": [
          {
            "path": "src/main.rs",
            "kind": "Bin",
            "name": "foo"
          }
        ]
      }
    ],
    "config_file": null,
    "lock_file": "version = 3\n\n[[package]]\nname = \"bar\"\nversion = \"0.0.1\"\n\n[[package]]\nname = \"foo\"\nversion = \"0.0.1\"\n"
  }
}

Cook and the resulting overriden local Cargo.toml:

$ cargo chef cook --bin foo --recipe-path recipe.json && cat Cargo.toml
   Compiling foo v0.0.1 (/Users/janis/dev/scratch/demo-cargo-chef-default-members/foo)
    Finished dev [unoptimized + debuginfo] target(s) in 0.08s
[workspace]
members = ["foo"]
resolver = "2"
SuperFluffy commented 7 months ago

I have updated the PR message to show the before/after behaviour

SuperFluffy commented 7 months ago

Bump and ping on this @LukeMathWalker. Can I do something to support merging this?

LukeMathWalker commented 7 months ago

Thanks for the PR!