From cbaec4e2f7ab4b9afd9483138f4c029b956a93ef Mon Sep 17 00:00:00 2001 From: Jon Gjengset Date: Tue, 20 Feb 2024 16:44:02 +0100 Subject: [PATCH 1/2] feature: Make Rust builds respect Cargo.lock when present Kraken should, by default, respect the `Cargo.lock` present in the current Cargo project rather than silently pull down dependencies not explicitly noted in the initial `Cargo.lock`. This is accomplished by passing `--locked` to `cargo. If the lockfile is out of date, the underlying package should be updated to reflect that change if `Cargo.lock` is checked in. To avoid breaking existing workflows that don't check in `Cargo.lock`, the default behavior is to only pass `--locked` if a `Cargo.lock` exists in the parent directory tree of the current working directory. The flag only really matters to CargoBuildTask, CargoTestTask, and CargoMetadata, but is also propagated to CargoPublishTask so that it isn't allowed to silently update the lockfile if it happens to be called first. Clippy doesn't take `--locked`, and so remains unmodified. Note also that checking in `Cargo.lock` is now recommended by the Rust project: https://blog.rust-lang.org/2023/08/29/committing-lockfiles.html --- .changelog/_unreleased.toml | 5 ++++ kraken-build/src/kraken/std/cargo/manifest.py | 11 +++++++- .../std/cargo/tasks/cargo_build_task.py | 28 ++++++++++++++++++- .../std/cargo/tasks/cargo_publish_task.py | 1 + .../kraken/std/cargo/tasks/cargo_test_task.py | 2 +- 5 files changed, 44 insertions(+), 3 deletions(-) create mode 100644 .changelog/_unreleased.toml diff --git a/.changelog/_unreleased.toml b/.changelog/_unreleased.toml new file mode 100644 index 00000000..a57b66fb --- /dev/null +++ b/.changelog/_unreleased.toml @@ -0,0 +1,5 @@ +[[entries]] +id = "67c15e74-a142-482f-9d2e-972a50161129" +type = "feature" +description = "Make Rust builds respect Cargo.lock when present" +author = "jon.gjengset@helsing.ai" diff --git a/kraken-build/src/kraken/std/cargo/manifest.py b/kraken-build/src/kraken/std/cargo/manifest.py index c5d6536e..0e5888cb 100644 --- a/kraken-build/src/kraken/std/cargo/manifest.py +++ b/kraken-build/src/kraken/std/cargo/manifest.py @@ -70,7 +70,7 @@ class CargoMetadata: target_directory: Path @classmethod - def read(cls, project_dir: Path) -> CargoMetadata: + def read(cls, project_dir: Path, locked: bool | None = None) -> CargoMetadata: cmd = [ "cargo", "metadata", @@ -79,6 +79,15 @@ def read(cls, project_dir: Path) -> CargoMetadata: "--manifest-path", str(project_dir / "Cargo.toml"), ] + if locked is None: + for parent in (project_dir / "Cargo.toml").absolute().parents: + if (parent / "Cargo.lock").exists(): + cmd.append("--locked") + break + elif locked: + # if locked is True, we should *always* pass --locked. + # the expectation is that the command will fail w/o Cargo.lock. + cmd.append("--locked") result = subprocess.run(cmd, stdout=subprocess.PIPE) if result.returncode != 0: logger.error("Stderr: %s", result.stderr) diff --git a/kraken-build/src/kraken/std/cargo/tasks/cargo_build_task.py b/kraken-build/src/kraken/std/cargo/tasks/cargo_build_task.py index fac0d015..3d019031 100644 --- a/kraken-build/src/kraken/std/cargo/tasks/cargo_build_task.py +++ b/kraken-build/src/kraken/std/cargo/tasks/cargo_build_task.py @@ -5,6 +5,7 @@ import subprocess as sp import time from dataclasses import dataclass +from pathlib import Path from kraken.core import Project, Property, Task, TaskStatus from kraken.std.cargo.manifest import ArtifactKind, CargoMetadata @@ -35,6 +36,11 @@ class CargoBuildTask(Task): #: Whether to build incrementally or not. incremental: Property[bool | None] = Property.default(None) + #: Whether to pass --locked to cargo or not. + #: + #: When set to None, --locked is passed if Cargo.lock exists. + locked: Property[bool | None] = Property.default(None) + #: Environment variables for the Cargo command. env: Property[dict[str, str]] = Property.default_factory(dict) @@ -58,11 +64,31 @@ def get_description(self) -> str | None: def get_cargo_command_additional_flags(self) -> list[str]: return shlex.split(os.environ.get("KRAKEN_CARGO_BUILD_FLAGS", "")) + def should_add_locked_flag(self) -> bool: + locked = self.locked.get() + if locked is None: + # pass --locked if we have a lock file + # since we may be in a workspace member, we need to search up! + for parent in (Path.cwd() / "Cargo.toml").parents: + if (parent / "Cargo.lock").exists(): + return True + elif locked: + # if locked is True, we should *always* pass --locked. + # the expectation is that the command will fail w/o Cargo.lock. + return True + return False + + def get_additional_args(self) -> list[str]: + args = self.additional_args.get() + if "--locked" not in args and self.should_add_locked_flag(): + args = ["--locked", *args] + return args + def get_cargo_command(self, env: dict[str, str]) -> list[str]: incremental = self.incremental.get() if incremental is not None: env["CARGO_INCREMENTAL"] = "1" if incremental else "0" - return ["cargo", "build"] + self.additional_args.get() + return ["cargo", "build"] + self.get_additional_args() def make_safe(self, args: list[str], env: dict[str, str]) -> None: pass diff --git a/kraken-build/src/kraken/std/cargo/tasks/cargo_publish_task.py b/kraken-build/src/kraken/std/cargo/tasks/cargo_publish_task.py index be57ab8a..332c7c21 100644 --- a/kraken-build/src/kraken/std/cargo/tasks/cargo_publish_task.py +++ b/kraken-build/src/kraken/std/cargo/tasks/cargo_publish_task.py @@ -35,6 +35,7 @@ def get_cargo_command(self, env: dict[str, str]) -> list[str]: raise ValueError(f'registry {registry.alias!r} missing a "publish_token"') command = ( ["cargo", "publish"] + + (["--locked"] if self.should_add_locked_flag() else []) + self.additional_args.get() + ["--registry", registry.alias, "--token", registry.publish_token] + ([] if self.verify.get() else ["--no-verify"]) diff --git a/kraken-build/src/kraken/std/cargo/tasks/cargo_test_task.py b/kraken-build/src/kraken/std/cargo/tasks/cargo_test_task.py index 8d0c4a03..1ad8daf2 100644 --- a/kraken-build/src/kraken/std/cargo/tasks/cargo_test_task.py +++ b/kraken-build/src/kraken/std/cargo/tasks/cargo_test_task.py @@ -9,4 +9,4 @@ class CargoTestTask(CargoBuildTask): def get_cargo_command(self, env: dict[str, str]) -> list[str]: super().get_cargo_command(env) - return ["cargo", "test"] + self.additional_args.get() + return ["cargo", "test"] + self.get_additional_args() From 166d58a15609fe0a3826ea4461801fe0fbe38dec Mon Sep 17 00:00:00 2001 From: Niklas Rosenstein Date: Tue, 27 Feb 2024 12:13:23 +0000 Subject: [PATCH 2/2] Update kraken-build/src/kraken/std/cargo/manifest.py --- kraken-build/src/kraken/std/cargo/manifest.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/kraken-build/src/kraken/std/cargo/manifest.py b/kraken-build/src/kraken/std/cargo/manifest.py index 0e5888cb..e1ad1879 100644 --- a/kraken-build/src/kraken/std/cargo/manifest.py +++ b/kraken-build/src/kraken/std/cargo/manifest.py @@ -80,7 +80,8 @@ def read(cls, project_dir: Path, locked: bool | None = None) -> CargoMetadata: str(project_dir / "Cargo.toml"), ] if locked is None: - for parent in (project_dir / "Cargo.toml").absolute().parents: + project_dir = project_dir.absolute() + for parent in [project_dir, *project_dir.parents]: if (parent / "Cargo.lock").exists(): cmd.append("--locked") break