feat!: Update dependencies and modernize Rust code patterns - #1
Open
taorepoara wants to merge 21 commits into
Open
feat!: Update dependencies and modernize Rust code patterns#1taorepoara wants to merge 21 commits into
taorepoara wants to merge 21 commits into
Conversation
Bumps the BUILDKIT_VERSION pinned in update.sh from v0.18 to v0.29.0 and refreshes every .proto file by re-running the script. Also adds the third-party schemas the script already references (sourcepolicy, vtprotobuf, google.rpc, google.protobuf), which were missing on disk and required by the gateway/fsutil imports. `cargo build -p buildkit-proto` succeeds. https://claude.ai/code/session_01XtZHcL6rKJDuX7tUS3okdc
Adds the new ExecOp.cdi_devices field to the production code path in ops/exec/command.rs. Test fixtures in ops/exec/mod.rs and ops/fs/mod.rs were already missing several fields added in earlier proto upgrades (Mount.tmpfs_opt / result_id / content_cache, Meta.hostname / cgroup_parent / ulimit / remove_mount_stubs_recursive / valid_exit_codes, FileActionCopy.required_paths and friends). They are now closed with `..Default::default()` so future proto schema additions don't keep breaking the same fixtures. `cargo build -p buildkit-llb` succeeds. https://claude.ai/code/session_01XtZHcL6rKJDuX7tUS3okdc
- ResolveImageConfigRequest gained resolver_type, session_id, store_id and source_policies in v0.18+ — keep existing fields and let prost defaults cover the rest via `..Default::default()`. - ReadFileRequest gained mount_index in v0.29. - frontend Result gained attestations in v0.18+. Using `..Default::default()` keeps these literals robust against future schema additions. https://claude.ai/code/session_01XtZHcL6rKJDuX7tUS3okdc
The frontend Result.result oneof variant `ref = 3` was promoted from
a plain string to a `Ref { id, def }` message. Update the solve
response match (extract `id`) and finish_with_success construction
(wrap output.0 in `Ref { id, def: None }`).
https://claude.ai/code/session_01XtZHcL6rKJDuX7tUS3okdc
The previous v0.18 upgrade swapped tonic_build::compile for prost_build::compile_protos, which dropped generation of the LlbBridgeClient gRPC client expected by buildkit-frontend. Restore tonic-build, bump it to 0.12 (the first line compatible with prost 0.13.x), add tonic 0.12 as a runtime dep so the generated client code compiles, and switch the crate to edition 2021 (TryInto in prelude is required by tonic 0.12 generated code). Also set workspace.resolver = "2" so the proto crate's edition-2021 features apply consistently across the workspace. `cargo build -p buildkit-proto` succeeds and the generated module exposes `moby::buildkit::v1::frontend::llb_bridge_client::LlbBridgeClient` again. Note: buildkit-frontend still pins tonic 0.1 + tokio 0.2 + tower 0.3 + mio 0.6, which are incompatible with tonic 0.12. Bringing buildkit-frontend forward (tonic / tokio / hyper / tower upgrade and the stdio.rs mio→AsyncFd rewrite) is a follow-up beyond the proto update scope. https://claude.ai/code/session_01XtZHcL6rKJDuX7tUS3okdc
The newer `mismatched_lifetime_syntaxes` lint (deny-by-default through `#![deny(warnings)]`) flagged three sites where an elided lifetime on an input was paired with a hidden lifetime on a return type. Tighten each signature to use `'_` consistently so the relationship is explicit. https://claude.ai/code/session_01XtZHcL6rKJDuX7tUS3okdc
After cat 1 regenerated the buildkit-proto gRPC client with tonic 0.12,
buildkit-frontend stayed pinned on tonic 0.1 / tokio 0.2 / tower 0.3,
which caused the `Channel: Service<Request<UnsyncBoxBody<Bytes,
Status>>>` trait bound to fail at every `LlbBridgeClient` call site:
the v0.1 Channel and the v0.12 generated client were incompatible.
Bumps the runtime stack to match buildkit-proto:
- tonic 0.1 -> 0.12
- tokio 0.2 -> 1
- tower 0.3 -> 0.5 (only the `util` feature is needed)
- drop mio 0.6, pin-project 0.4, libc, bytes (no longer used)
- add hyper-util 0.1 with the `tokio` feature so we can adapt our
AsyncRead/Write socket to the hyper::rt::Read/Write that
Endpoint::connect_with_connector now requires
stdio.rs is rewritten on top of `tokio::io::{stdin, stdout}` (which
became async in tokio 1.x), wrapped in `hyper_util::rt::TokioIo` at the
connector boundary; the custom mio-based `Evented` plumbing is gone.
The tokio runtime macros in the examples switch to the
`flavor = "..."` syntax. An unused `ResolveMode` re-export is removed.
`cargo build --workspace --all-targets` and `cargo test --workspace`
both pass (29 tests).
https://claude.ai/code/session_01XtZHcL6rKJDuX7tUS3okdc
The crates declare `#![deny(clippy::all)]`, and several lints have been
added to clippy since the project was last touched, so `cargo clippy`
failed with 35+ errors. Most were auto-fixable via `cargo clippy --fix`
(idiomatic `From` instead of `Into`, `#[derive(Default)]` instead of a
manual impl, redundant references / patterns, `strip_prefix` over
manual slicing, ...). The rest are mechanical:
- Drop unused lifetime parameters on impl blocks (`impl<'a> ...` where
`'a` never appears in the impl signature) in
`buildkit-llb/src/ops/{exec/command,fs/sequence,source/{git,http,
image,local}}.rs`.
- Switch `crate::...` to `$crate::...` inside the exported
`check_op!` / `check_op_property!` macros so they keep referring to
`buildkit_llb` when used from doctests / external test crates.
- Allow `clippy::result_unit_err` on the `FileOperation` trait to
preserve the public `Result<_, ()>` API.
- Allow `clippy::upper_case_acronyms` on the `LLB` test enum variant.
`cargo clippy --workspace --all-targets -- -D warnings` and
`cargo test --workspace` both pass clean.
https://claude.ai/code/session_01XtZHcL6rKJDuX7tUS3okdc
…atform()
Adds a new `crate::ops::platform` module that re-exports
`buildkit_proto::pb::Platform` together with constructor helpers for the
common targets (`linux_amd64`, `linux_arm64`, `linux_arm_v7`,
`windows_amd64`, ...) and a `platform_id` helper that produces the
canonical `<os>/<arch>[/<variant>]` string used as a key in BuildKit's
RefMap and in the `containerimage.config/<id>` metadata key.
- `Command::platform(Platform)` populates `pb::Op.platform` on exec
ops so workers can be scheduled accordingly (cross-compilation).
- `ImageSource::with_platform(Platform)` does the same on source ops
and exposes `.platform()` so the frontend bridge can pass the
constraint through to `ResolveImageConfigRequest.platform`.
Adds a `platform` arm to the internal `check_op_property!` macro and
three new serialization tests (one per builder + a unit test on
`platform_id`). `cargo test -p buildkit-llb` runs 23 tests (was 20).
https://claude.ai/code/session_01XtZHcL6rKJDuX7tUS3okdc
Picks up the platform-aware ops added on the buildkit-llb side and wires
multi-platform support through the bridge end to end:
- `Bridge::resolve_image_config` now forwards
`ImageSource::platform()` into `ResolveImageConfigRequest.platform`,
so per-platform configs are resolved correctly.
- `Bridge::solve_multi_platform[ _with_cache]` accepts a graph that
yields a `RefMap` and returns `HashMap<String, OutputRef>` keyed by
the canonical platform string. Single-ref responses degrade to a
one-entry map.
- `FrontendOutput` now has a `with_multi_platform(Vec<MultiPlatformEntry>)`
constructor alongside the existing single-output ones. Each entry
pairs a `pb::Platform` with an `OutputRef` and an optional
`ImageSpecification`; the id defaults to `platform_id(&p)` and can
be overridden via `with_id()`.
- `Bridge::finish_with_success` now takes the whole `FrontendOutput`
and dispatches: single results keep the previous wire format, while
multi-platform results are encoded as `RefResult::Refs(RefMap)`
plus a `refs.platforms` metadata blob (matching BuildKit's
`exptypes.Platforms` JSON shape - capital `ID`, dotted `os.version`
/ `os.features` keys) plus a per-platform
`containerimage.config/<id>` entry whenever `image_spec` is set.
Adds OCI types in `oci.rs` for callers that need to consume or build a
manifest list directly: `ImageIndex`, `Descriptor`, and an OCI
`Platform` that reuses the existing typed `Architecture` /
`OperatingSystem` enums and round-trips through the dotted
`os.version` / `os.features` JSON keys.
Tests: 13 in buildkit-frontend (was 9): the new ones cover the OCI
index round-trip, the `refs.platforms` JSON shape, and the duplicate /
empty error paths.
https://claude.ai/code/session_01XtZHcL6rKJDuX7tUS3okdc
The OCI image-spec has gained a handful of optional fields since v1.0
(`os.version`, `os.features`, `variant`, OCI v1.1 referrers, ...) and
Docker has long shipped its own widely-used config extensions
(`Healthcheck`, `Shell`, `ArgsEscaped`). buildkit-frontend was only
exposing the v1.0 base, so any caller round-tripping an existing image
config silently dropped these fields. This adds them.
Top-level `ImageSpecification`:
- `os_version: Option<String>` (JSON `os.version`)
- `os_features: Option<Vec<String>>` (JSON `os.features`)
- `variant: Option<String>`
`ImageConfig` now also carries the Docker extensions, all `Option<_>`
so the existing OCI-only round-trips stay byte-for-byte stable:
- `healthcheck: Option<Healthcheck>` - new struct mirroring Docker's
shape, with `Option<Duration>` fields (de)serialized as Go's
`time.Duration` JSON convention (integer count of nanoseconds).
Includes the modern `start_interval` field from newer
Docker/containerd. `Test` covers `["NONE"]`, `["CMD", ...]` and
`["CMD-SHELL", ...]` shapes.
- `shell: Option<Vec<String>>` - the default shell used by the
shell-form of `RUN` / `CMD` / `ENTRYPOINT`.
- `args_escaped: Option<bool>` - Windows-only, deprecated but still
emitted by older pipelines so we keep it for fidelity.
`ImageConfig` now also derives `Default`, so callers can pick the
fields they care about with `..Default::default()` instead of having
to spell every `None` (the examples are migrated to that style).
`Architecture` gains `Riscv64`, `Loong64` and `Wasm` to match the
modern Go/OCI list. `OperatingSystem` gains `Aix`, `Android`, `Hurd`,
`Illumos`, `Ios`, `Js` and `Zos`.
OCI v1.1 additions on the descriptor types:
- `Descriptor.artifact_type: Option<String>`
- `Descriptor.data: Option<String>` (base64 inline payload)
- `ImageIndex.subject: Option<Descriptor>` (referrers API)
- `ImageIndex.artifact_type: Option<String>`
- new `ImageManifest` struct (per-platform manifest, the type that
`ImageIndex` entries resolve to) with the same `subject` /
`artifactType` knobs.
Eight new round-trip tests cover: every Docker-extension field at
once, the dotted top-level keys with all three new optionals,
`Healthcheck::Test = ["NONE"]` minimal case, the new architecture and
OS variants, OCI v1.1 fields on `ImageIndex` and `ImageManifest`, and
the empty `ImageConfig::default()`.
`cargo test --workspace` now runs 44 tests (was 36); `cargo clippy
--workspace --all-targets -- -D warnings` is clean.
https://claude.ai/code/session_01XtZHcL6rKJDuX7tUS3okdc
Bumps the BUILDKIT_VERSION pin in update.sh from v0.29.0 to v0.30.0
and re-runs the script. The only schema delta between the two
upstream tags on the five .proto files we vendor is a single new
optional field on api/types/worker.proto:
message BuildkitVersion {
string package = 1;
string version = 2;
string revision = 3;
string dockerfileVersion = 4; // added in v0.30.0
}
gateway.proto, ops.proto, caps.proto and sourcepolicy.proto are
byte-identical to v0.29.0 in this release, so no Rust source needs
to change: the regenerated `BuildkitVersion` struct just gains a
`dockerfile_version: String` field that callers can read or ignore.
`cargo build --workspace --all-targets` and `cargo test --workspace`
(44 tests: 21 in buildkit-frontend, 23 in buildkit-llb) both pass.
https://claude.ai/code/session_01XtZHcL6rKJDuX7tUS3okdc
`impl ToString for Platform` was added in 6cbf078 ("feat: Platform serialization"). It works, but clippy's `to_string_trait_impl` lint (now part of `clippy::all`) flags it because the standard library already ships a blanket `impl<T: Display> ToString for T`. Switch to `fmt::Display` so we get `to_string()` for free and `cargo clippy --workspace --all-targets -- -D warnings` is clean. No behavior change: the formatted strings are byte-for-byte the same, so existing `platform.to_string()` callers (notably `buildkit_llb::ops::platform::platform_id`) keep working. https://claude.ai/code/session_01XtZHcL6rKJDuX7tUS3okdc
taorepoara
changed the title
buildkit-proto has been on edition 2021 for a while (since the proto regeneration commits), but the two downstream crates were still pinned to 2018, which made the workspace a bit awkward: the same patterns compiled with different rules from one crate to the next, and 2021-only conveniences (disjoint closure captures, IntoIterator for arrays by value, expanded prelude with `TryFrom`/`TryInto`/ `FromIterator`) silently degraded as soon as code crossed the edition boundary. This aligns both crates on edition 2021 to match buildkit-proto. The bump compiles clean - `cargo build --workspace --all-targets` passes, `cargo fix --workspace --edition-idioms` finds no auto-fixable idiom drift, and the existing 44 tests still pass. While here, drops two `use std::convert::TryFrom;` statements in buildkit-frontend/src/oci.rs that became redundant: the 2021 prelude imports `TryFrom` (and `TryInto`) automatically, so the `impl TryFrom<String> for ExposedPort` and the `u64::try_from(...)` call in the `opt_duration_nanos` helper module no longer need an explicit import. `cargo clippy --workspace --all-targets -- -D warnings` stays clean. https://claude.ai/code/session_01XtZHcL6rKJDuX7tUS3okdc
Aligns dev-dep pins with the rest of the workspace and brings them up to current. None of these affect the public API or runtime - they only gate the example and test compilation: - env_logger 0.6 -> 0.11 (0.6 dates from 2019) - pretty_assertions 0.6 -> 1.4 (post-1.0 stable) - regex 1.3 -> 1.11 (buildkit-llb already uses 1.11.x) - url 2.1 -> 2.5 `cargo update --dry-run --verbose` still reports five upstream majors behind (`tonic`/`tonic-build` 0.12 -> 0.14, `prost`/`prost-types` 0.13 -> 0.14, `sha2` 0.10 -> 0.11) - those are intentional API breaks comparable to the 0.1 -> 0.12 tonic migration we did earlier and warrant separate, scoped commits, not a drive-by bump. `cargo test --workspace` (44 tests) and `cargo clippy --workspace --all-targets -- -D warnings` both stay clean. https://claude.ai/code/session_01XtZHcL6rKJDuX7tUS3okdc
`pb::TmpfsOpt` is `#[derive(Copy)]` since prost only ever generates `Copy` for plain-scalar messages, so the `opt.clone()` introduced in d230692 ("feat: Implement missing commands and options") trips clippy::clone_on_copy. Just dereference the borrow. No behavior change. https://claude.ai/code/session_01XtZHcL6rKJDuX7tUS3okdc
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR modernizes the codebase by updating to newer Rust idioms, removing deprecated dependencies, and updating protobuf definitions to support new BuildKit features.
Key Changes
Dependency Updates
buildkit-protoedition from 2018 to 2021libc,mio,pin-project,byteshyper_util::rt::TokioIowrapperflavorparameterRust Code Modernization
Intotrait implementations toFrom(preferred pattern)&[T]slice references with array literals in function callsassert_eq!(x, true)toassert!(x)pattern#[allow(clippy::result_unit_err)]and#[allow(clippy::upper_case_acronyms)]attributes where appropriate'a, 'b: 'ato simpler'bwhere applicable#[derive(Default)]usage and added#[default]attribute on enum variantsif let Err(_)with.is_err()patternProtobuf Updates
google/protobuf/descriptor.proto,google/protobuf/any.proto,google/rpc/status.proto,sourcepolicy/pb/policy.proto,vtproto/ext.protoReadFileContainer,ReadDirContainer,StatFileContainerExecOpand worker recordsResolveSourceMetaRequestwith new source type fieldsBuild Configuration
I/O Implementation
StdioSocketby removing customPollEventedwrapper and using tokio's nativeStdin/StdoutdirectlyAsyncRead/AsyncWriteimplementations to use modern tokio API withReadBufTokioIofor compatibility with tonic transport layerhttps://claude.ai/code/session_01XtZHcL6rKJDuX7tUS3okdc