feat(docker): Generate unique tag for each clp-package dev image build to avoid conflicts on shared hosts. #1335

coderabbitai · 2025-09-25T19:15:02Z

🛠️ Refactor suggestion | 🟠 Major

Ensure destination directory exists before rsync.

If {{.G_PACKAGE_BUILD_DIR}} doesn’t exist yet, rsync will fail. Create it first.

- "./build.sh" - - "rsync -a 'image.id' '{{.G_PACKAGE_BUILD_DIR}}'" + - "mkdir -p '{{.G_PACKAGE_BUILD_DIR}}'" + - "rsync -a 'image.id' '{{.G_PACKAGE_BUILD_DIR}}'"

🤖 Prompt for AI Agents

In taskfiles/docker-images.yaml around line 15, the rsync command assumes {{.G_PACKAGE_BUILD_DIR}} exists; ensure the destination directory is created first by invoking a mkdir -p on {{.G_PACKAGE_BUILD_DIR}} (or otherwise ensure its parent exists) before running the rsync so the copy never fails when the directory is missing.

The docker-images:package task depends on the :package task, which already creates the G_PACKAGE_BUILD_DIR

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash # Search for the :package task definition in taskfiles rg -n "package:" taskfiles/ -A 10 -B 2 # Also search for G_PACKAGE_BUILD_DIR usage to understand how it's created rg -n "G_PACKAGE_BUILD_DIR" taskfiles/ -A 2 -B 2

Length of output: 1186

🏁 Script executed:

#!/bin/bash # Find the main Taskfile fd -t f "Taskfile" --max-depth 2 # Search for package task in all Taskfile files rg -n "^[[:space:]]*package:" . -A 15 -B 2 # Also search for any mkdir commands that might create G_PACKAGE_BUILD_DIR rg -n "mkdir.*G_PACKAGE_BUILD_DIR" .

Length of output: 7777

🏁 Script executed:

#!/bin/bash # Get the complete package task definition from taskfile.yaml rg -n "^[[:space:]]*package:" ./taskfile.yaml -A 25 # Also search for any scripts or commands that might create the directory rg -n "G_PACKAGE_BUILD_DIR" ./taskfile.yaml -A 3 -B 3 # Look for mkdir commands that create package build directories rg -n "mkdir.*package" . -i

Length of output: 2141

@junhaoliao You're absolutely correct! I can see that the :package task does indeed create the G_PACKAGE_BUILD_DIR directory with the command mkdir -p '{{.OUTPUT_DIR}}/lib/python3/site-packages' where OUTPUT_DIR is set to {{.G_PACKAGE_BUILD_DIR}}. Since the docker-images:package task depends on :package, the directory is guaranteed to exist before the rsync command runs.

Thank you for the clarification - my suggestion to add the redundant mkdir -p was unnecessary.

✏️ Learnings added

Learnt from: junhaoliao PR: y-scope/clp#1335 File: taskfiles/docker-images.yaml:15-15 Timestamp: 2025-09-25T19:26:32.415Z Learning: In the CLP project's Taskfile, the `:package` task creates the G_PACKAGE_BUILD_DIR directory structure, so any task that depends on `:package` (like `docker-images:package`) can safely assume this directory exists without needing additional mkdir commands.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

coderabbitai · 2025-09-25T19:15:02Z

🧹 Nitpick | 🔵 Trivial

Consider avoiding a shared image.id during concurrent builds.

Concurrent invocations writing the same image.id path can race. If parallel builds are possible, write to a per-invocation temp (then atomically move/rsync into place post-build).

Example approach:

Write to "$(mktemp "${script_dir}/image.id.XXXXXX")"

After a successful build, mv to "${script_dir}/image.id"
Please confirm whether the task runner can trigger parallel builds for this target. If yes, I can provide a concrete patch.

🤖 Prompt for AI Agents

In tools/docker-images/clp-package/build.sh around line 7, the script writes a shared image id file at ${script_dir}/image.id which can race during concurrent builds; change it to write the id to a per-invocation temporary file (e.g., create a temp under ${script_dir} with mktemp), write the build output there, and after a successful build atomically move the temp into ${script_dir}/image.id (mv on the same filesystem) so only completed builds replace the shared file; also add a trap to clean the temp file on exit/failure and ensure the temp is created on the same filesystem as script_dir; please confirm whether the task runner can run this target in parallel so I can provide an exact patch.

it's not expected the user would invoke multiple image builds at the same time. @kirkrodrigues what do you think? shall we implement the rabbit's proposal and somehow guard against multiple concurrent builds?

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!

I don't think it's necessary right now.

kirkrodrigues · 2025-09-26T10:20:02Z

How about remove_prev_image?

kirkrodrigues · 2025-09-26T10:20:51Z

Can we move this function to the top of the file, just after the set commands? That makes it easier to read the control flow imo.

coderabbitai · 2025-09-25T19:15:02Z

⚠️ Potential issue | 🔴 Critical

Do not delete the newly built image when its ID equals the previous one; avoid trap-based deletion.

If the build is reproducible, the new image ID can equal the previous image.id. With the current trap, you’ll remove the just-built image on exit, leaving image.id pointing to a non-existent image. Also, removing on any script exit (including failed builds) is risky.

Refactor to only remove the previous image after a successful build and only if it differs from the new ID. Remove the trap and perform the check post-build.

Apply this diff to remove the trap block:

-# Remove the previous image after the build to allow layer reuse. -prev_image_id="" -if [[ -f "$iid_file" ]]; then - prev_image_id=$(cat "$iid_file") -fi -cleanup() { - if [[ -n "$prev_image_id" ]] && docker image inspect "$prev_image_id" >/dev/null 2>&1; then - echo "Removing previous image $prev_image_id" - docker image remove "$prev_image_id" - fi -} -trap cleanup EXIT +# Track previously built image for optional cleanup after a successful build only. +prev_image_id="" +if [[ -f "$iid_file" ]]; then + prev_image_id=$(cat "$iid_file") +fi

Then, add this snippet immediately after the build command invocation to handle cleanup safely:

# Post‑build: if a different image was produced, remove the previous one to reduce clutter. if [[ -n "$prev_image_id" ]]; then if [[ -f "$iid_file" ]]; then new_image_id=$(cat "$iid_file" || true) if [[ -n "$new_image_id" && "$new_image_id" != "$prev_image_id" ]]; then if docker image inspect "$prev_image_id" >/dev/null 2>&1; then echo "Removing previous image $prev_image_id" docker image remove "$prev_image_id" fi fi fi fi

i'm not sure how your proposal is cleaner. it also has so many nested-ifs, can you revisit?

@junhaoliao You're absolutely right - let me provide a much cleaner solution. Here's a simplified approach that eliminates the trap and reduces nesting:

# Track previous image for cleanup after successful build prev_image_id="" if [[ -f "$iid_file" ]]; then prev_image_id=$(cat "$iid_file") fi

Then after the build command, add this simple cleanup:

# Clean up previous image if build produced a different one if [[ -n "$prev_image_id" && -f "$iid_file" ]]; then new_image_id=$(cat "$iid_file") if [[ "$new_image_id" != "$prev_image_id" ]] && docker image inspect "$prev_image_id" &>/dev/null; then echo "Removing previous image $prev_image_id" docker image remove "$prev_image_id" fi fi

This approach:

Removes the problematic trap entirely

Uses a single conditional with logical AND operators instead of nested ifs

Only runs cleanup after successful build

Safely handles the case where image IDs are identical

Is much more readable and maintainable

The key insight is combining conditions with && operators and using early evaluation to avoid deep nesting.

🧠 Learnings used

Learnt from: jackluo923 PR: y-scope/clp#1054 File: components/core/tools/docker-images/clp-env-base-musllinux_1_2-x86/build.sh:18-24 Timestamp: 2025-07-01T14:52:02.418Z Learning: In the CLP project, consistency across platform build scripts is prioritized over defensive programming when it comes to git remote handling. All build.sh files in docker-images directories should follow the same pattern for git metadata injection.

Learnt from: jackluo923 PR: y-scope/clp#1054 File: components/core/tools/docker-images/clp-env-base-musllinux_1_2-x86/build.sh:18-24 Timestamp: 2025-07-01T14:52:02.418Z Learning: In the CLP project, consistency across platform build scripts is prioritized over defensive programming when it comes to git remote handling. All build.sh files in docker-images directories should follow the same pattern for git metadata injection.

Learnt from: jackluo923 PR: y-scope/clp#1054 File: components/core/tools/docker-images/clp-env-base-musllinux_1_2-aarch64/build.sh:3-5 Timestamp: 2025-07-07T17:43:04.349Z Learning: In CLP project build scripts (specifically build.sh files in docker-images directories), maintain consistency with the established pattern of using separate `set -eu` and `set -o pipefail` commands rather than combining them into `set -euo pipefail`, to ensure uniform script structure across all platform build scripts.

Learnt from: junhaoliao PR: y-scope/clp#1335 File: taskfiles/docker-images.yaml:15-15 Timestamp: 2025-09-25T19:26:32.415Z Learning: In the CLP project's Taskfile, the `:package` task creates the G_PACKAGE_BUILD_DIR directory structure, so any task that depends on `:package` (like `docker-images:package`) can safely assume this directory exists without needing additional mkdir commands.

i can't be totally sure if the benefits are clear to me, do you just hate the trap in the original code?

@junhaoliao No, it's not about hating trap - traps are useful tools! The issue is specifically about the timing and conditions when the cleanup runs in this context.

Here are the concrete technical problems with the current trap approach:

Failed builds shouldn't delete the previous working image: If the Docker build fails (network issues, syntax errors, etc.), the trap still runs and removes the previous image, potentially leaving you with no working image at all.

Reproducible builds create a self-deletion scenario: If your build is reproducible (same inputs → same image hash), the new image.id will contain the same ID as the previous one. The trap will then delete the image you just "built" (which is actually the same image).

Signal interruption edge case: If someone Ctrl+C's during the build, the trap runs and deletes the previous image, even though the build never completed.

The post-build approach I suggested only runs cleanup after a successful build and only if the image actually changed. This makes the behavior more predictable and safer.

Both approaches work fine in the happy path, but the trap version has these edge cases that could surprise users. It's about robustness rather than style preference.

If you prefer keeping the trap for simplicity, that's totally reasonable too - these edge cases might be rare enough in your workflow that the current approach is perfectly adequate.

🧠 Learnings used

Learnt from: jackluo923 PR: y-scope/clp#1054 File: components/core/tools/docker-images/clp-env-base-musllinux_1_2-x86/build.sh:18-24 Timestamp: 2025-07-01T14:52:02.418Z Learning: In the CLP project, consistency across platform build scripts is prioritized over defensive programming when it comes to git remote handling. All build.sh files in docker-images directories should follow the same pattern for git metadata injection.

Learnt from: jackluo923 PR: y-scope/clp#1054 File: components/core/tools/docker-images/clp-env-base-musllinux_1_2-x86/build.sh:18-24 Timestamp: 2025-07-01T14:52:02.418Z Learning: In the CLP project, consistency across platform build scripts is prioritized over defensive programming when it comes to git remote handling. All build.sh files in docker-images directories should follow the same pattern for git metadata injection.

Learnt from: jackluo923 PR: y-scope/clp#1054 File: components/core/tools/docker-images/clp-env-base-musllinux_1_2-aarch64/build.sh:3-5 Timestamp: 2025-07-07T17:43:04.349Z Learning: In CLP project build scripts (specifically build.sh files in docker-images directories), maintain consistency with the established pattern of using separate `set -eu` and `set -o pipefail` commands rather than combining them into `set -euo pipefail`, to ensure uniform script structure across all platform build scripts.

Learnt from: junhaoliao PR: y-scope/clp#1335 File: taskfiles/docker-images.yaml:15-15 Timestamp: 2025-09-25T19:26:32.415Z Learning: In the CLP project's Taskfile, the `:package` task creates the G_PACKAGE_BUILD_DIR directory structure, so any task that depends on `:package` (like `docker-images:package`) can safely assume this directory exists without needing additional mkdir commands.

i can't be sure if you understand the code correctly. will let @kirkrodrigues decide

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!

I think the rabbit is correct.

If the user kills the script before it completes, the existing image will be deleted. Is that intentional?

If the current and previous image are identical, their IDs will be identical (since the ID is a sha256sum of the image), so on exit, the image will be deleted.

This doesn't currently happen since our builds are not exactly reproducible yet, but that's something we should work towards.

kirkrodrigues · 2025-09-26T10:08:10Z

If we tag with the current timestamp, then identical image builds will result in two different tags pointing at the same image. Besides the clutter, that also means that the image can't be removed without a force flag. Instead, can we replace the timestamp with the image ID? I know that will make it harder to tell what's a newer image based on the tag, but that information is available from Docker since it keeps track of the date an image was built.

If we tag with the current timestamp, then identical image builds will result in two different tags pointing at the same image. Besides the clutter, that also means that the image can't be removed without a force flag.

not sure if this behaviour is possible. if i understand correctly, every time we run docker build a new image is built (we can tell by the different sha256 image id)

maybe i understood it wrong. do you mind giving more steps on how this can be reproduced?

my initial implementation of the unique id part was done with uuidgen | head -c 4 though i think the chance of collision can be high.

later i replaced that with the timestamp because it's less likely a user can build multiple build in a second. (can probably change it to nanoseconds

if i understand correctly, every time we run docker build a new image is built (we can tell by the different sha256 image id)

That's because task package's output is not 100% reproducible---i.e., some files will change slightly. If you run the build.sh script alone twice, it should generate an image with the same ID.

actually i do mean running the build.sh gives a different sha256 id every time:

junhao@ASUS-X870E:~/workspace/2-clp/tools/docker-images/clp-package$ ./build.sh && echo '---' && cat /home/junhao/workspace/2-clp/build/clp-package-image.id && echo [+] Building 0.9s (12/12) FINISHED docker:default => [internal] load build definition from Dockerfile 0.0s => => transferring dockerfile: 585B 0.0s => [internal] load metadata for docker.io/library/ubuntu:jammy 0.1s => [internal] load .dockerignore 0.0s => => transferring context: 2B 0.0s => [base 1/6] FROM docker.io/library/ubuntu:jammy@sha256:4e0171b9275e12d375863f2b3ae9ce00a4c53ddda176bd55868df97ac6f21a6e 0.0s => => resolve docker.io/library/ubuntu:jammy@sha256:4e0171b9275e12d375863f2b3ae9ce00a4c53ddda176bd55868df97ac6f21a6e 0.0s => [internal] load build context 0.6s => => transferring context: 2.24MB 0.6s => CACHED [base 2/6] WORKDIR /root 0.0s => CACHED [base 3/6] COPY ./tools/docker-images/clp-package/setup-scripts ./setup-scripts 0.0s => CACHED [base 4/6] RUN ./setup-scripts/install-prebuilt-packages.sh && rm -rf ./setup-scripts/ 0.0s => CACHED [base 5/6] RUN apt-get clean && rm -rf /var/lib/apt/lists/* /tmp/* /var/tmp/* 0.0s => CACHED [base 6/6] COPY ./build/clp-package /opt/clp 0.0s => CACHED [stage-1 1/1] COPY --from=base / / 0.0s => exporting to image 0.1s => => exporting layers 0.0s => => exporting manifest sha256:eb17ac6fd6ea60ce4eb1839a54f0bbc939b0d3dad201c2d9547d0b0ffd7fe2bf 0.0s => => exporting config sha256:41a6f9aae1dc947a3deeafcec0e82ab2891f2cb55ff0f9a3a0492cd5defd0739 0.0s => => exporting attestation manifest sha256:5cbe3ae1c541bb2237dbbb721e8635c31449bf9ef334bcbb691e98d4b882a168 0.0s => => exporting manifest list sha256:31e3696ede8077959e0dfe249f7be8caa42c12d2d351e0a93ba88f080b883e36 0.0s => => naming to docker.io/library/clp-package:dev-junhao-1759114001 0.0s => => unpacking to docker.io/library/clp-package:dev-junhao-1759114001 0.0s Removing previous image sha256:754d92f3198297d74cae9ef9698f2d5f046392d6408158c4860ce820c7b08b17. Untagged: clp-package:dev-junhao-1759113990 Deleted: sha256:754d92f3198297d74cae9ef9698f2d5f046392d6408158c4860ce820c7b08b17 --- sha256:31e3696ede8077959e0dfe249f7be8caa42c12d2d351e0a93ba88f080b883e36 junhao@ASUS-X870E:~/workspace/2-clp/tools/docker-images/clp-package$ ./build.sh && echo '---' && cat /home/junhao/workspace/2-clp/build/clp-package-image.id && echo [+] Building 0.9s (12/12) FINISHED docker:default => [internal] load build definition from Dockerfile 0.0s => => transferring dockerfile: 585B 0.0s => [internal] load metadata for docker.io/library/ubuntu:jammy 0.1s => [internal] load .dockerignore 0.0s => => transferring context: 2B 0.0s => [base 1/6] FROM docker.io/library/ubuntu:jammy@sha256:4e0171b9275e12d375863f2b3ae9ce00a4c53ddda176bd55868df97ac6f21a6e 0.0s => => resolve docker.io/library/ubuntu:jammy@sha256:4e0171b9275e12d375863f2b3ae9ce00a4c53ddda176bd55868df97ac6f21a6e 0.0s => [internal] load build context 0.6s => => transferring context: 2.24MB 0.6s => CACHED [base 2/6] WORKDIR /root 0.0s => CACHED [base 3/6] COPY ./tools/docker-images/clp-package/setup-scripts ./setup-scripts 0.0s => CACHED [base 4/6] RUN ./setup-scripts/install-prebuilt-packages.sh && rm -rf ./setup-scripts/ 0.0s => CACHED [base 5/6] RUN apt-get clean && rm -rf /var/lib/apt/lists/* /tmp/* /var/tmp/* 0.0s => CACHED [base 6/6] COPY ./build/clp-package /opt/clp 0.0s => CACHED [stage-1 1/1] COPY --from=base / / 0.0s => exporting to image 0.1s => => exporting layers 0.0s => => exporting manifest sha256:eb17ac6fd6ea60ce4eb1839a54f0bbc939b0d3dad201c2d9547d0b0ffd7fe2bf 0.0s => => exporting config sha256:41a6f9aae1dc947a3deeafcec0e82ab2891f2cb55ff0f9a3a0492cd5defd0739 0.0s => => exporting attestation manifest sha256:262a3a97f6ac6ebe1264878a49027dd9382532eda8aa5b8386a8496acb684c5c 0.0s => => exporting manifest list sha256:fda46ace138ddb9700a1dbdaf26bbde1577dd1e15f7159d05858e9c6a8d7b245 0.0s => => naming to docker.io/library/clp-package:dev-junhao-1759114003 0.0s => => unpacking to docker.io/library/clp-package:dev-junhao-1759114003 0.0s Removing previous image sha256:31e3696ede8077959e0dfe249f7be8caa42c12d2d351e0a93ba88f080b883e36. Untagged: clp-package:dev-junhao-1759114001 Deleted: sha256:31e3696ede8077959e0dfe249f7be8caa42c12d2d351e0a93ba88f080b883e36 --- sha256:fda46ace138ddb9700a1dbdaf26bbde1577dd1e15f7159d05858e9c6a8d7b245 junhao@ASUS-X870E:~/workspace/2-clp/tools/docker-images/clp-package$ ./build.sh && echo '---' && cat /home/junhao/workspace/2-clp/build/clp-package-image.id && echo [+] Building 0.9s (12/12) FINISHED docker:default => [internal] load build definition from Dockerfile 0.0s => => transferring dockerfile: 585B 0.0s => [internal] load metadata for docker.io/library/ubuntu:jammy 0.1s => [internal] load .dockerignore 0.0s => => transferring context: 2B 0.0s => [base 1/6] FROM docker.io/library/ubuntu:jammy@sha256:4e0171b9275e12d375863f2b3ae9ce00a4c53ddda176bd55868df97ac6f21a6e 0.0s => => resolve docker.io/library/ubuntu:jammy@sha256:4e0171b9275e12d375863f2b3ae9ce00a4c53ddda176bd55868df97ac6f21a6e 0.0s => [internal] load build context 0.6s => => transferring context: 2.24MB 0.6s => CACHED [base 2/6] WORKDIR /root 0.0s => CACHED [base 3/6] COPY ./tools/docker-images/clp-package/setup-scripts ./setup-scripts 0.0s => CACHED [base 4/6] RUN ./setup-scripts/install-prebuilt-packages.sh && rm -rf ./setup-scripts/ 0.0s => CACHED [base 5/6] RUN apt-get clean && rm -rf /var/lib/apt/lists/* /tmp/* /var/tmp/* 0.0s => CACHED [base 6/6] COPY ./build/clp-package /opt/clp 0.0s => CACHED [stage-1 1/1] COPY --from=base / / 0.0s => exporting to image 0.1s => => exporting layers 0.0s => => exporting manifest sha256:eb17ac6fd6ea60ce4eb1839a54f0bbc939b0d3dad201c2d9547d0b0ffd7fe2bf 0.0s => => exporting config sha256:41a6f9aae1dc947a3deeafcec0e82ab2891f2cb55ff0f9a3a0492cd5defd0739 0.0s => => exporting attestation manifest sha256:e1d31873b43c7ec6bcdeb4112671c6daf21b02b4019c1d40b6de746717c6d816 0.0s => => exporting manifest list sha256:b3a0bfc056f542a868cd1845256543dd585864fe0ca8d934f6a398bc07b52151 0.0s => => naming to docker.io/library/clp-package:dev-junhao-1759114005 0.0s => => unpacking to docker.io/library/clp-package:dev-junhao-1759114005 0.0s Removing previous image sha256:fda46ace138ddb9700a1dbdaf26bbde1577dd1e15f7159d05858e9c6a8d7b245. Untagged: clp-package:dev-junhao-1759114003 Deleted: sha256:fda46ace138ddb9700a1dbdaf26bbde1577dd1e15f7159d05858e9c6a8d7b245 --- sha256:b3a0bfc056f542a868cd1845256543dd585864fe0ca8d934f6a398bc07b52151

discussed offline - it appears that the above script generates a different hash every time only when Docker Desktop Windows + WSL is used. with a native Docker Engine installed inside WSL, the hash is consistent across docker build if the content inside build/clp-package is unchanged

a few facts:

if we run docker build without a --tag parameter, in the build logs it would appear that we are building an image with tag moby-dangling.

provided an id of an image, it's possible to add tags to the image afterwards

there's no atomic way to build an image, get the id of the built image, and add tag to the image with the id included in the tag name

how about this:

we read the image id file from build/clp-package-image.id into $prev_image_id, as we currently do.

we call mktemp to get a temporary file path, assigned to $temp_iid_file

docker build --iidfile "$temp_iid_file" "$repo_root" --file "${script_dir}/Dockerfile" (note we don't supply tags here. the build logs will show we're building for project moby-dangling but i guess we can live with that, so long the image is eventually tagged)

we cat the $temp_iid_file and store the value at $new_iid. fallback to an empty string if cat fails (the failure can possibly be caused by a failed docker build?)

if the $new_iid is non empty, truncate the $new_iid to 4 characters and tag the new image with clp-package:dev-${USER}-${short_id}

before the script exits, the trap xxx EXIT function is called. inside:

if $prev_image_id differs from $new_iid (note: this is always true if $new_iid is empty), we docker image remove $prev_image_id.

At step (3), it's possible two concurrent builds on the system create an image with tag moby-dangling, right? I think that will still succeed, but then after step (5) wouldn't we end up with two tags for the same image? I guess we need a step to remove the moby-dangling tag for the particular image we created.

let me implement the proposal and monkey test a bit to see if there are issues. i could be wrong about how docker image remove works (or confused by the different behaviours in Docker Desktop)

it appears that the moby-dangling isn't a real project / tag --- Running docker image ls doesn't show the moby-dangling on the listed image, whether we manfully tag the image with our own tag or not.

the moby-dangling project only shows up in the build logs with Docker Desktop.

i have tested the script that's been pushed and didn't find any awkward issue, either with Docker Desktop or native Docker.

coderabbitai · 2025-09-25T19:15:02Z

🧹 Nitpick | 🔵 Trivial

Confirm intended builder: buildx vs classic build.

PR text mentions buildx, but this uses docker build. If buildx is intended (for multi-arch or consistent --iidfile support), switch to docker buildx build accordingly.

🤖 Prompt for AI Agents

In tools/docker-images/clp-package/build.sh around lines 23 to 29, the script uses docker build but the PR mentions buildx; confirm and, if buildx is intended, replace the command with docker buildx build (and add appropriate flags such as --platform or --load/push as needed) so --iidfile behavior and multi-arch builds are supported; ensure the script verifies buildx is available and a builder exists (or falls back to docker build) and update any related flags/options to be compatible with buildx.

docker build is an alias of docker buildx build which uses the BuildKit as the builder. The switch happened in https://docs.docker.com/engine/release-notes/23.0/#2300

@junhaoliao Thank you for the clarification and the reference! You're absolutely right - since Docker Engine 23.0, docker build automatically uses BuildKit and is indeed an alias for docker buildx build. This means the current implementation is correct and the --iidfile flag will work as expected.

I appreciate you providing the documentation link to confirm this behavior change. The script is appropriately using docker build and no modifications are needed.

🐰💡

✏️ Learnings added

Learnt from: junhaoliao PR: y-scope/clp#1335 File: tools/docker-images/clp-package/build.sh:23-29 Timestamp: 2025-09-25T19:36:35.345Z Learning: Since Docker Engine 23.0, `docker build` is an alias for `docker buildx build` and automatically uses BuildKit as the builder. The `--iidfile` flag and other BuildKit features work correctly with the standard `docker build` command, so there's no need to explicitly use `docker buildx build` in build scripts unless specific buildx-only features are required.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

🧠 Learnings used

Learnt from: jackluo923 PR: y-scope/clp#1054 File: components/core/tools/docker-images/clp-env-base-musllinux_1_2-x86/build.sh:18-24 Timestamp: 2025-07-01T14:52:02.418Z Learning: In the CLP project, consistency across platform build scripts is prioritized over defensive programming when it comes to git remote handling. All build.sh files in docker-images directories should follow the same pattern for git metadata injection.

Learnt from: jackluo923 PR: y-scope/clp#1054 File: components/core/tools/docker-images/clp-env-base-musllinux_1_2-x86/build.sh:18-24 Timestamp: 2025-07-01T14:52:02.418Z Learning: In the CLP project, consistency across platform build scripts is prioritized over defensive programming when it comes to git remote handling. All build.sh files in docker-images directories should follow the same pattern for git metadata injection.

Learnt from: jackluo923 PR: y-scope/clp#1054 File: components/core/tools/docker-images/clp-env-base-musllinux_1_2-aarch64/build.sh:3-5 Timestamp: 2025-07-07T17:43:04.349Z Learning: In CLP project build scripts (specifically build.sh files in docker-images directories), maintain consistency with the established pattern of using separate `set -eu` and `set -o pipefail` commands rather than combining them into `set -euo pipefail`, to ensure uniform script structure across all platform build scripts.

Learnt from: junhaoliao PR: y-scope/clp#1335 File: taskfiles/docker-images.yaml:15-15 Timestamp: 2025-09-25T19:26:32.415Z Learning: In the CLP project's Taskfile, the `:package` task creates the G_PACKAGE_BUILD_DIR directory structure, so any task that depends on `:package` (like `docker-images:package`) can safely assume this directory exists without needing additional mkdir commands.

@kirkrodrigues i realise we don't really mention any docker version in our dev docs (we do mention docker in the userquick start guide, though we don't say the version still). if that's not intentionally left out, maybe we should specify the version in our dev docs as well?

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!

Yeah, we should specify the version we require. I just don't know what version that is right now.

it might be time consuming to figure out the exact version for the feature / syntax we use. maybe we can just take the latest version and put that into the docs? at least we can be sure that's a version we have validated

Generally, we support up to 2 years of backwards compatibility with OSes. Perhaps we can do the same for Docker?

https://containerd.io/releases/ 's LTS version is now 1.7 and it appears that the last retrievable version is 1.7.18 which was released on July 3, 2024 - https://download.docker.com/linux/ubuntu/dists/jammy/pool/stable/amd64/

i installed these versions on my machine and tested things fine:

sudo apt-get install -y \ docker-ce=5:27.0.3-1~ubuntu.22.04~jammy \ docker-ce-cli=5:27.0.3-1~ubuntu.22.04~jammy \ containerd.io=1.7.18-1 \ docker-buildx-plugin=0.15.1-1~ubuntu.22.04~jammy \ docker-compose-plugin=2.28.1-1~ubuntu.22.04~jammy

-Original file line number
+Diff line change
@@ Expand Up / @@ -12,3 +12,4 @@ tasks: @@
           - ":package"
         cmds:
           - "./build.sh"
+          - "rsync -a 'image.id' '{{.G_PACKAGE_BUILD_DIR}}'"

-Original file line number
+Diff line change
@@ Expand Up / @@ -4,11 +4,26 @@ set -eu @@
     set -o pipefail
     script_dir="$( cd "$( dirname "${BASH_SOURCE[0]}" )" &> /dev/null && pwd )"
+    iid_file="${script_dir}/image.id"
     repo_root=${script_dir}/../../../
+    # Remove the previous image after the build to allow layer reuse.
+    prev_image_id=""
+    if [[ -f "$iid_file" ]]; then
+        prev_image_id=$(cat "$iid_file")
+    fi
+    cleanup() {
+        if [[ -n "$prev_image_id" ]] && docker image inspect "$prev_image_id" >/dev/null 2>&1; then
+            echo "Removing previous image $prev_image_id"
+            docker image remove "$prev_image_id"
+        fi
+    }
+    trap cleanup EXIT
     build_cmd=(
         docker build
-        --tag "clp-package:dev"
+        --iidfile "$iid_file"
+        --tag "clp-package:dev-${USER}-$(date +%s)"
         "$repo_root"
         --file "${script_dir}/Dockerfile"
     )
@@ Expand Down @@

feat(docker): Generate unique tag for each clp-package dev image build to avoid conflicts on shared hosts. #1335

Uh oh!

feat(docker): Generate unique tag for each clp-package dev image build to avoid conflicts on shared hosts. #1335

Uh oh!

Uh oh!

Diff view

Diff view

There are no files selected for viewing

coderabbitai bot Sep 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot Sep 25, 2025

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot Sep 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot Sep 25, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

coderabbitai bot Sep 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot Sep 25, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot Sep 25, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot Sep 25, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot Sep 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

coderabbitai bot Sep 25, 2025 •

edited

Loading

coderabbitai bot Sep 25, 2025 •

edited

Loading

coderabbitai bot Sep 25, 2025 •

edited

Loading

coderabbitai bot Sep 25, 2025 •

edited

Loading