Skip to content
Merged
Show file tree
Hide file tree
Changes from 51 commits
Commits
Show all changes
60 commits
Select commit Hold shift + click to select a range
8ea4529
feat(docker): Add CLP package image for Docker Compose integration.
junhaoliao Aug 5, 2025
4d402c1
Update install-prebuilt-packages.sh
junhaoliao Aug 6, 2025
adef439
lint
junhaoliao Aug 6, 2025
9aa74ff
Rename workflow from 'clp-execution-image-build' to 'clp-image-build'
junhaoliao Aug 6, 2025
542da81
correct action name
junhaoliao Aug 6, 2025
ce43d17
fix(workflow): Revert workflow name to 'clp-execution-image-build'
junhaoliao Aug 6, 2025
2c2ee50
Merge branch 'main' into package-image
junhaoliao Aug 6, 2025
75f88d9
fix(workflow): revert action path in workflow configuration
junhaoliao Aug 6, 2025
414e1fb
fix(build): add quotes around image tag in build command
junhaoliao Aug 6, 2025
0e9138a
fix(docker): combine install and cleanup commands in Dockerfile
junhaoliao Aug 6, 2025
912b678
Merge branch 'main' into package-image
junhaoliao Aug 8, 2025
dcc8a70
add docs
junhaoliao Aug 8, 2025
46ffa55
Merge branch 'main' into package-image
junhaoliao Aug 19, 2025
9db7ca9
improve error handling in install-prebuilt-packages.sh
junhaoliao Aug 19, 2025
d609987
set default user and group to 1000
junhaoliao Aug 19, 2025
1b3959d
update docker image name description
junhaoliao Aug 19, 2025
1f13954
update CLP package image dependencies
junhaoliao Aug 19, 2025
aceb39f
Merge branch 'main' into package-image
junhaoliao Aug 22, 2025
c49d8b4
remove unneeded prebuilt packages
junhaoliao Aug 24, 2025
099afc0
Merge branch 'main' into package-image
junhaoliao Aug 24, 2025
73ca1a0
Update MariaDB client package from libmariadb-dev to libmariadb3
junhaoliao Aug 24, 2025
a4317ea
docs - Apply suggestions from code review
junhaoliao Aug 25, 2025
806f2f8
Merge remote-tracking branch 'origin/main' into package-image
junhaoliao Aug 25, 2025
1887626
refactor(actions): Rename clp-image-build to clp-build-runtime-image …
junhaoliao Aug 25, 2025
20dcfb1
Remove incompatible input options from action.yaml
junhaoliao Aug 25, 2025
ddab8d1
rename path -> dockerfile_path
junhaoliao Aug 25, 2025
92861b9
Add comment to clarify npm cache environment variable usage
junhaoliao Aug 25, 2025
b39644e
refactor(install): Remove unnecessary packages and update dependencie…
junhaoliao Aug 25, 2025
c17a782
Rename container image name -> clp-package
junhaoliao Aug 25, 2025
8ff64f2
Move && to the next line - Apply suggestions from code review
junhaoliao Aug 25, 2025
1dfe706
alphabetize ENV definitions in Dockerfile - Apply suggestions from co…
junhaoliao Aug 25, 2025
47d0517
Merge remote-tracking branch 'junhao/package-image' into package-image
junhaoliao Aug 25, 2025
628ddae
move job 'ubuntu-jammy-package-image' to clp-core-build.yaml and rena…
junhaoliao Aug 25, 2025
23f1ef2
fix clp-build-runtime-image action to account for package image name …
junhaoliao Aug 25, 2025
63a7ec3
mark clp-build-runtime-image's platform_id and platform_version_id as…
junhaoliao Aug 25, 2025
61da837
move package image task into separate task file
junhaoliao Aug 25, 2025
227d6dd
CRLF -> LF
junhaoliao Aug 25, 2025
e98bb8e
use `paths-ignore` instead of inverted glob patterns in `paths`
junhaoliao Aug 25, 2025
e6f1af5
alphabetize
junhaoliao Aug 25, 2025
860e5ef
use absolute path for `.github/actions`
junhaoliao Aug 25, 2025
4180739
fix outdated action name
junhaoliao Aug 25, 2025
6cfabd7
fix outdated task command ` docker-images:package`
junhaoliao Aug 25, 2025
f1c566f
order task attrs according to dev guide
junhaoliao Aug 25, 2025
09539ae
fix order of ENV defs in Dockerfile
junhaoliao Aug 25, 2025
b49daba
test ci: to be reverted
junhaoliao Sep 2, 2025
be2560e
revert the last change
junhaoliao Sep 2, 2025
44a2dc5
Rename step name "Update Metadata" -> "Extract GitHub Metadata" - App…
junhaoliao Sep 2, 2025
2ebbfe0
rename action step ID for metadata extraction
junhaoliao Sep 2, 2025
1254827
update npm cache dir comment - Apply suggestions from code review
junhaoliao Sep 2, 2025
62cf06a
use yaml multiline string syntax for long `if` - Apply suggestions fr…
junhaoliao Sep 2, 2025
2f29228
Merge branch 'main' into package-image
junhaoliao Sep 2, 2025
c413a13
add checksum file for package task
junhaoliao Sep 2, 2025
bb36035
reorder COPY instruction in clp-package Dockerfile
junhaoliao Sep 2, 2025
ea4ce51
flatten the image at the last
junhaoliao Sep 2, 2025
3c81014
move COPY to later
junhaoliao Sep 2, 2025
e5a66cf
Merge branch 'main' into package-image
junhaoliao Sep 2, 2025
ff21c8e
Merge branch 'main' into package-image
junhaoliao Sep 3, 2025
cbea39b
docs: rename `clp-core-build` workflow and references -> `clp-artifac…
junhaoliao Sep 3, 2025
a857701
docs: update workflows to include CLP package image build
junhaoliao Sep 3, 2025
7dfbcc5
ci: rename workflow `clp-core-build` -> `clp-artifact-build`
junhaoliao Sep 3, 2025
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
name: "clp-execution-image-build"
description: "Builds a container image that contains the dependencies necessary
to run the CLP package."
name: "clp-build-runtime-image"
description: "Builds a container image to be used for running CLP."
Comment on lines +1 to +2
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick (assertive)

Clarify the action’s scope in the description.

Since this action now builds both execution and package runtime images, make that explicit in the description string for discoverability.

-description: "Builds a container image to be used for running CLP."
+description: "Builds CLP runtime container images (execution or package)."
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
name: "clp-build-runtime-image"
description: "Builds a container image to be used for running CLP."
name: "clp-build-runtime-image"
description: "Builds CLP runtime container images (execution or package)."
🤖 Prompt for AI Agents
.github/actions/clp-build-runtime-image/action.yaml lines 1-2: the action
description currently says "Builds a container image to be used for running
CLP." but no longer reflects that it builds both execution and package runtime
images; update the description string to explicitly state it builds both
execution and package runtime container images for CLP to improve
discoverability (e.g., change to mention "builds execution and package runtime
images for running CLP").


inputs:
image_type:
description: "Type of image to build"
required: true
Comment on lines +5 to +7
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Document allowed values for image_type and prefer validating them.

The new input is great, but the contract isn’t explicit. Document the enum in-place and validate at runtime (see suggested validation in the Dockerfile selection step).

-  image_type:
-    description: "Type of image to build"
+  image_type:
+    description: "Type of image to build. One of: execution, package."
     required: true
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
image_type:
description: "Type of image to build"
required: true
image_type:
description: "Type of image to build. One of: execution, package."
required: true
🤖 Prompt for AI Agents
.github/actions/clp-build-runtime-image/action.yaml lines 5-7: the action input
image_type lacks an explicit enum and runtime validation; update the action.yml
to document allowed values (e.g., "base", "runtime", "slim" — replace with
actual acceptable options) by adding an "options" or "enum" comment/description
and then add runtime validation where the Dockerfile is selected (or in the
entrypoint script) to reject unknown values with a clear error message and
non-zero exit code; ensure validation normalizes casing and provides a helpful
error listing the valid options so callers can correct the input.

image_registry:
default: "ghcr.io"
description: "Container image registry"
Expand All @@ -17,11 +19,11 @@ inputs:
required: false
platform_id:
description: "Platform ID of the container (e.g. ubuntu)"
required: true
required: false
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no longer needed for building the new image clp-package

platform_version_id:
description: "Platform VERSION_ID / VERSION_CODENAME of the container
(e.g. jammy, focal, etc.)"
required: true
required: false
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no longer needed for building the new image clp-package


runs:
using: "composite"
Expand All @@ -45,20 +47,37 @@ runs:
echo "REPOSITORY=$(echo '${{github.repository}}' | tr '[:upper:]' '[:lower:]')" \
>> "$GITHUB_OUTPUT"

- name: "Update Metadata"
id: "meta"
- name: "Compute metadata"
id: "compute-meta"
shell: "bash"
run: |
base_path="./tools/docker-images"

if [[ "${{inputs.image_type}}" == "execution" ]]; then
platform="${{inputs.platform_id}}-${{inputs.platform_version_id}}"
dockerfile_path="$base_path/clp-execution-base-$platform/Dockerfile"
image_name="clp-${{inputs.image_type}}-$platform"
else
dockerfile_path="$base_path/clp-package/Dockerfile"
image_name="clp-${{inputs.image_type}}"
fi

echo "DOCKERFILE=$dockerfile_path" >> "$GITHUB_OUTPUT"
echo "IMAGE_NAME=$image_name" >> "$GITHUB_OUTPUT"

- name: "Extract GitHub Metadata"
id: "extract-gh-meta"
uses: "docker/metadata-action@902fa8ec7d6ecbf8d84d538b9b233a880e428804"
with:
images: "${{inputs.image_registry}}/${{steps.sanitization.outputs.REPOSITORY}}\
/clp-execution-x86-${{inputs.platform_id}}-${{inputs.platform_version_id}}"
/${{steps.compute-meta.outputs.IMAGE_NAME}}"

- name: "Build and Push"
if: "github.event_name != 'pull_request' && github.ref == 'refs/heads/main'"
uses: "docker/build-push-action@471d1dc4e07e5cdedd4c2171150001c434f0b7a4"
with:
context: "./"
file: "./tools/docker-images/\
clp-execution-base-${{inputs.platform_id}}-${{inputs.platform_version_id}}/Dockerfile"
file: "${{steps.compute-meta.outputs.DOCKERFILE}}"
push: true
tags: "${{steps.meta.outputs.tags}}"
labels: "${{steps.meta.outputs.labels}}"
tags: "${{steps.extract-gh-meta.outputs.tags}}"
labels: "${{steps.extract-gh-meta.outputs.labels}}"
2 changes: 2 additions & 0 deletions .github/actions/run-on-image/action.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,11 @@ runs:
- run: "./tools/scripts/deps-download/init.sh"
shell: "bash"

# `--env npm_config_cache` overrides the default `/.npm` to avoid permission issues.
- run: >-
docker run
--user $(id -u):$(id -g)
--env npm_config_cache=/tmp/.npm
--volume "$GITHUB_WORKSPACE":/mnt/repo
--workdir /mnt/repo
${{steps.get_image_props.outputs.qualified_image_name}}
Expand Down
65 changes: 45 additions & 20 deletions .github/workflows/clp-core-build.yaml
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this is no longer just to build core, how about renaming it to something like clp-artifact-build.yaml?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i intentionally left the clp-core-macos workflow unchanged since it still only builds the core binaries. let me know if that should be updated as well

Original file line number Diff line number Diff line change
Expand Up @@ -2,27 +2,13 @@ name: "clp-core-build"

on:
pull_request:
paths:
- ".github/actions/clp-core-build-containers/action.yaml"
- ".github/actions/run-on-image/action.yaml"
- ".github/workflows/clp-core-build.yaml"
- ".gitmodules"
- "components/core/**"
- "taskfile.yaml"
- "taskfiles/**"
- "tools/scripts/deps-download/**"
- "!components/core/tools/scripts/lib_install/macos/**"
paths-ignore: &ignored_paths
- ".github/*"
- ".github/ISSUE_TEMPLATE/**"
- "components/core/tools/scripts/lib_install/macos/**"
- "docs/**"
push:
paths:
- ".github/actions/clp-core-build-containers/action.yaml"
- ".github/actions/run-on-image/action.yaml"
- ".github/workflows/clp-core-build.yaml"
- ".gitmodules"
- "components/core/**"
- "taskfile.yaml"
- "taskfiles/**"
- "tools/scripts/deps-download/**"
- "!components/core/tools/scripts/lib_install/macos/**"
paths-ignore: *ignored_paths
schedule:
Comment on lines 10 to 12
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix actionlint error: alias in paths-ignore is invalid

actionlint flags the alias on Line 11. Inline the list for push.

   push:
-    paths-ignore: *ignored_paths
+    paths-ignore:
+      - ".github/*"
+      - ".github/ISSUE_TEMPLATE/**"
+      - "components/core/tools/scripts/lib_install/macos/**"
+      - "docs/**"
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
push:
paths:
- ".github/actions/clp-core-build-containers/action.yaml"
- ".github/actions/run-on-image/action.yaml"
- ".github/workflows/clp-core-build.yaml"
- ".gitmodules"
- "components/core/**"
- "taskfile.yaml"
- "taskfiles/**"
- "tools/scripts/deps-download/**"
- "!components/core/tools/scripts/lib_install/macos/**"
paths-ignore: *ignored_paths
schedule:
push:
paths-ignore:
- ".github/*"
- ".github/ISSUE_TEMPLATE/**"
- "components/core/tools/scripts/lib_install/macos/**"
- "docs/**"
schedule:
🧰 Tools
🪛 actionlint (1.7.7)

11-11: "paths-ignore" section must be sequence node but got alias node with "" tag

(syntax-check)

🤖 Prompt for AI Agents
.github/workflows/clp-core-build.yaml around lines 10 to 12: actionlint flags
the YAML anchor/alias used for paths-ignore on line 11 as invalid; replace the
alias with the actual inline list of ignored paths (expand the referenced
*ignored_paths sequence directly into the push.paths-ignore block) so the push
job contains a concrete array of path globs instead of an alias.

# Run daily at 00:15 UTC (the 15 is to avoid periods of high load)
- cron: "15 0 * * *"
Expand Down Expand Up @@ -520,3 +506,42 @@ jobs:
.task/checksum/utils-cpp-lint-clang-tidy-*
build/lint-clang-tidy
key: "${{steps.cache-restore-lint-check-cpp-static-full.outputs.cache-primary-key}}"

package-image:
name: "package-image"
if: >-
!cancelled() && !failure() && (
needs.filter-relevant-changes.outputs.ubuntu_jammy_image_changed == 'false' ||
needs.ubuntu-jammy-deps-image.result == 'success'
)
needs:
- "filter-relevant-changes"
- "ubuntu-jammy-deps-image"
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not quite certain if this works if ubuntu-jammy-deps-image fails its if

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I'm not sure. We should probably test. I feel like that's why we used success() before, but maybe I'm wrong.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we could test more thoroughly, though what we have written should already work. see https://github.com/junhaoliao/clp/actions/runs/17405920984/workflow for a modification on an unrelated file, where package-image still runs with Filter ubuntu_jammy_image = false. I believe that's because we have this condition set up in package-image's if:

needs.filter-relevant-changes.outputs.ubuntu_jammy_image_changed == 'false' ||
needs.ubuntu-jammy-deps-image.result == 'success'

runs-on: "ubuntu-24.04"
steps:
- uses: "actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683"
with:
submodules: "recursive"

- name: "Workaround actions/runner-images/issues/6775"
shell: "bash"
run: "chown $(id -u):$(id -g) -R ."

- name: "Build the package"
uses: "./.github/actions/run-on-image"
env:
OS_NAME: "ubuntu-jammy"
with:
image_name: "${{env.DEPS_IMAGE_NAME_PREFIX}}${{env.OS_NAME}}"
use_published_image: >-
${{needs.filter-relevant-changes.outputs.ubuntu_jammy_image_changed == 'false'
|| (github.event_name != 'pull_request' && github.ref == 'refs/heads/main')}}
run_command: >-
CLP_CORE_MAX_PARALLELISM_PER_BUILD_TASK=$(getconf _NPROCESSORS_ONLN) task package

- uses: "./.github/actions/clp-build-runtime-image"
with:
image_type: "package"
image_registry: "ghcr.io"
image_registry_username: "${{github.actor}}"
image_registry_password: "${{secrets.GITHUB_TOKEN}}"
7 changes: 4 additions & 3 deletions .github/workflows/clp-execution-image-build.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,12 @@ name: "clp-execution-image-build"
on:
pull_request:
paths:
- ".github/actions/clp-execution-image-build/action.yaml"
- ".github/actions/clp-build-runtime-image/action.yaml"
- ".github/workflows/clp-execution-image-build.yaml"
- "tools/docker-images/**/*"
push:
paths:
- ".github/actions/clp-execution-image-build/action.yaml"
- ".github/actions/clp-build-runtime-image/action.yaml"
- ".github/workflows/clp-execution-image-build.yaml"
- "tools/docker-images/**/*"
schedule:
Expand Down Expand Up @@ -61,8 +61,9 @@ jobs:
shell: "bash"
run: "chown $(id -u):$(id -g) -R ."

- uses: "./.github/actions/clp-execution-image-build"
- uses: "./.github/actions/clp-build-runtime-image"
with:
image_type: "execution"
image_registry: "ghcr.io"
image_registry_username: "${{github.actor}}"
image_registry_password: "${{secrets.GITHUB_TOKEN}}"
Expand Down
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i understand the added dependencies are not strictly "core" related but i believe some others here (e.g. unzip) are in a similar situation. we might want to split the package specific dependencies in anther refactoring PR, though i personally don't see too much benefit yet. any concerns / other ideas?

Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,10 @@ DEBIAN_FRONTEND=noninteractive apt-get install --no-install-recommends -y \
openjdk-11-jdk \
pkg-config \
python3 \
python3-dev \
python3-pip \
python3-venv \
rsync \
software-properties-common \
unzip

Expand Down
10 changes: 10 additions & 0 deletions docs/src/dev-docs/building-package.md
Original file line number Diff line number Diff line change
Expand Up @@ -69,5 +69,15 @@ To clean up all build artifacts, run:
task clean
```

## Building a Docker image

To build a Docker image containing the CLP package, run:

```shell
task docker-images:package
```

This will create a Docker image named `clp-package:dev`.

Comment on lines +71 to +80
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick (assertive)

🛠️ Refactor suggestion

Clarify prerequisites, add a quick verification step, and document retag/push for Compose.

Users need to know which container engine is required, how to sanity-check the image, and how to retag/push it for Compose scenarios. Add the following edits:

 ## Building a Docker image

-To build a Docker image containing the CLP package, run:
+Prerequisites: Docker installed and running (or Podman configured with Docker compatibility).
+
+To build a Docker image containing the CLP package, run:

 ```shell
 task docker-images:package

-This will create a Docker image named clp-package:dev.
+This will create a locally tagged image named clp-package:dev.
+
+Quick sanity check:
+
+shell +docker run --rm clp-package:dev env | egrep '^(CLP_HOME|PYTHONPATH|PATH)=' +
+
+Optional: retag and push to GHCR (for Docker Compose use):
+
+shell +docker tag clp-package:dev ghcr.io/y-scope/clp/clp-package:dev +docker push ghcr.io/y-scope/clp/clp-package:dev +
+
+See also: the tooling container and GitHub workflow docs for image build/publish details.


<details>
<summary>🤖 Prompt for AI Agents</summary>

docs/src/dev-docs/building-package.md around lines 72 to 81: update the
"Building a Docker image" section to clarify prerequisites and verification
steps: change the sentence to say this will create a locally tagged image named
clp-package:dev, add a quick sanity-check command that runs the image and greps
for CLP_HOME, PYTHONPATH and PATH environment variables, and add optional
instructions to retag the image for GHCR and push it (for Docker Compose usage);
also add a brief pointer to the tooling container and GitHub workflow docs for
build/publish details.


</details>

<!-- fingerprinting:phantom:triton:chinchilla -->

<!-- This is an auto-generated comment by CodeRabbit -->

[clp-issue-872]: https://github.com/y-scope/clp/issues/872
[Task]: https://taskfile.dev/
10 changes: 10 additions & 0 deletions docs/src/dev-docs/tooling-containers.md
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,16 @@ environment.
tools/docker-images/clp-execution-base-ubuntu-jammy
```

## clp-package

An image containing the CLP package.

* Path:

```text
tools/docker-images/clp-package
```
Comment on lines +150 to +158
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick (assertive)

Match other sections: add GH Packages link and pull command

Keep the clp-package section consistent with the rest.

 ## clp-package
 
 An image containing the CLP package.
 
+* [GitHub Packages page][clp-package]
+* Pull command:
+
+  ```bash
+  docker pull ghcr.io/y-scope/clp/clp-package:main
+  ```
+
 * Path:
 
   ```text
   tools/docker-images/clp-package

+[clp-package]: https://github.com/y-scope/clp/pkgs/container/clp%2Fclp-package


<details>
<summary>🤖 Prompt for AI Agents</summary>

In docs/src/dev-docs/tooling-containers.md around lines 150 to 158, the
clp-package section is missing the GitHub Packages pull command and the
reference link used by other image sections; update the section to include a
code block with the docker pull command for ghcr.io/y-scope/clp/clp-package:main
and add the markdown reference link [clp-package]:
https://github.com/y-scope/clp/pkgs/container/clp%2Fclp-package at the end of
the section so it matches the format of the other image entries.


</details>

<!-- fingerprinting:phantom:triton:chinchilla -->

<!-- This is an auto-generated comment by CodeRabbit -->


[core-deps-centos-stream-9]: https://github.com/y-scope/clp/pkgs/container/clp%2Fclp-core-dependencies-x86-centos-stream-9
[core-deps-manylinux_2_28-x86_64]: https://github.com/y-scope/clp/pkgs/container/clp%2Fclp-core-dependencies-x86-manylinux_2_28
[core-deps-musllinux_1_2-x86_64]: https://github.com/y-scope/clp/pkgs/container/clp%2Fclp-core-dependencies-x86-musllinux_1_2
Expand Down
4 changes: 4 additions & 0 deletions docs/src/dev-docs/tooling-gh-workflows.md
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,10 @@ This workflow builds CLP-core on macOS and runs its unit tests.
This workflow builds a container image that contains the dependencies necessary to run the CLP
package.

## clp-package-image-build

This workflow builds a container image that contains the CLP package.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to update this file to reflect the latest changes, right?


## clp-lint

This workflow runs linting checks on the codebase.
1 change: 1 addition & 0 deletions taskfile.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ shopt: ["globstar"]

includes:
deps: "taskfiles/deps/main.yaml"
docker-images: "taskfiles/docker-images.yaml"
docs: "taskfiles/docs.yaml"
lint: "taskfiles/lint.yaml"
utils: "tools/yscope-dev-utils/exports/taskfiles/utils/utils.yaml"
Expand Down
13 changes: 13 additions & 0 deletions taskfiles/docker-images.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
version: "3"

tasks:
package:
vars:
SRC_DIR: "{{.ROOT_DIR}}/tools/docker-images/clp-package"
dir: "{{.SRC_DIR}}"
sources:
- "{{.SRC_DIR}}/**/*"
deps:
- ":package"
cmds:
- "./build.sh"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Rebuild image when package output changes and fail fast if Docker is missing.
Current sources only watch the image directory; images may go stale when only build/clp-package changes. Also add a Docker precondition.

 tasks:
   package:
     vars:
       SRC_DIR: "{{.ROOT_DIR}}/tools/docker-images/clp-package"
     dir: "{{.SRC_DIR}}"
     sources:
       - "{{.SRC_DIR}}/**/*"
+      - "{{.G_PACKAGE_BUILD_DIR}}/**/*"
     deps:
       - ":package"
+    preconditions:
+      - sh: "command -v docker"
+        msg: "Docker is required to build the package image. Please install Docker and try again."
     cmds:
       - "./build.sh"
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
tasks:
package:
vars:
SRC_DIR: "{{.ROOT_DIR}}/tools/docker-images/clp-package"
dir: "{{.SRC_DIR}}"
sources:
- "{{.SRC_DIR}}/**/*"
deps:
- ":package"
cmds:
- "./build.sh"
tasks:
package:
vars:
SRC_DIR: "{{.ROOT_DIR}}/tools/docker-images/clp-package"
dir: "{{.SRC_DIR}}"
sources:
- "{{.SRC_DIR}}/**/*"
- "{{.G_PACKAGE_BUILD_DIR}}/**/*"
deps:
- ":package"
preconditions:
- sh: "command -v docker"
msg: "Docker is required to build the package image. Please install Docker and try again."
cmds:
- "./build.sh"
🤖 Prompt for AI Agents
In taskfiles/docker-images.yaml around lines 3–13, the task only watches the
image source directory so rebuilds are missed when the package build output
(tools/docker-images/clp-package or its build artifacts) changes and there is no
precondition to ensure Docker is available; update the sources list to include
the package build outputs (e.g. add the build/clp-package output paths or
tooling artifacts under tools/docker-images/clp-package/**) so changes there
trigger the task, and add a fail-fast Docker precondition to the task commands
(check that the docker CLI exists and exit with a clear error if not) before
running ./build.sh.

Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,6 @@ set -u
apt-get update
DEBIAN_FRONTEND=noninteractive apt-get install --no-install-recommends -y \
ca-certificates \
checkinstall \
curl \
libcurl4 \
libmariadb-dev \
libssl-dev \
python3 \
rsync \
zstd
libmariadb3 \
python3
24 changes: 24 additions & 0 deletions tools/docker-images/clp-package/Dockerfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
FROM ubuntu:jammy AS base

COPY ./build/clp-package /opt/clp

WORKDIR /root

COPY ./tools/docker-images/clp-package/setup-scripts ./setup-scripts
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@coderabbitai it appears that we should always use COPY --link to speed up builds. if you agree, file an issue

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧩 Analysis chain

🌐 Web query:

Docker COPY --link flag benefits and best practices BuildKit performance

💡 Result:

Summary — what COPY --link does

  • COPY --link (Dockerfile frontend v1.4+) makes COPY/ADD create an independent layer containing only the copied files and then "link" that layer onto prior layers instead of copying files on top of the previous snapshot. This decouples the COPY from earlier commands and improves caching and parallelism. [1][2]

Key benefits

  • Stronger cache stability: COPY steps don’t get invalidated when earlier commands in the same stage change, so cache hits are more reliable. Useful in multi-stage builds where intermediate stages often change. [1][2]
  • Faster/parallel builds: BuildKit can finish COPY without waiting for earlier commands to complete, allowing more parallelism and shorter build times. [1][2]
  • Avoids pulling full base-image blobs during build/export: BuildKit can link layers by metadata (manifest) so you don’t always need the base image blobs locally to complete a build; this speeds CI and rebasing. [1][2][3]
  • Smaller/less redundant on-disk work: BuildKit avoids duplicating file blobs when possible (hard-link-like behavior), lowering disk I/O and temporary storage during builds. [3][6]

Practical best practices

  • Enable the correct frontend: add the Dockerfile header: # syntax=docker/dockerfile:1.4 (or later) so the COPY --link frontend is used. [1][2]
  • Use it for files/directories with stable destination semantics — avoid ambiguous destination semantics (e.g., relying on COPY to append into an existing dir vs create a file). When --link is used, the destination layer starts empty, so ambiguous targets like COPY my-file /data (expecting /data/my-file) can change behavior. Test paths explicitly. [1][3]
  • Avoid copying into or relying on symlink resolution in the destination layer; symlink resolution that existed in earlier layers may not be present for a linked layer. [3]
  • Prefer explicit destination directories (use trailing slash when you mean a directory) and avoid COPY --link . . with unclear WORKDIR/dot semantics unless you’ve validated it. [1][3]
  • Use in CI with --cache-from / remote cache and Buildx to benefit from remote cache/higher parallelism; it enables easier image rebases and better remote-cache reuse. [2][3]
  • Test interactions with --chown and platform/drivers: some environments or driver combos (e.g., docker-container driver, Docker Desktop on Windows) have had issues reported; validate your CI/worker combination. [4]

When not to use it / caveats

  • Behavioral differences vs classic COPY (destination/symlink semantics) — if your Dockerfile relies on previous-layer filesystem state, do not enable --link for that COPY. [1][3]
  • Some combinations historically had platform/driver bugs (e.g., chown resolution in certain buildx/docker-container setups); test across your build targets. [4]
  • Because it's a semantics change, don’t flip a production pipeline to --link without validating image layout and runtime behavior. [1][4]

Quick example

  • Top of Dockerfile: # syntax=docker/dockerfile:1.4
  • Typical pattern:
    FROM node:18-alpine AS builder
    WORKDIR /app
    COPY --link package*.json ./
    RUN npm ci
    COPY --link . .
    RUN npm run build
    FROM nginx:alpine
    COPY --link --from=builder /app/dist /usr/share/nginx/html
    (works best with BuildKit/Buildx and remote cache configured). [2][3]

Sources
[1] Docker blog — Image rebase and improved remote cache support in new BuildKit (March 17, 2022).
[2] Dockerfile reference (COPY --link docs).
[3] How-To‑Geek explanation and examples of COPY --link behavior and benefits.
[4] moby/buildkit GitHub issues (observed chown/driver interaction notes).


🏁 Script executed:

#!/bin/bash
# Find all Dockerfiles in the repository to include in the issue
fd -t f -e Dockerfile --full-path | head -20

Length of output: 0


🏁 Script executed:

#!/bin/bash
# Find all Dockerfiles in the repository (files named "Dockerfile")
fd -t f '^Dockerfile$' --full-path

# Also check for files with Dockerfile prefix
fd -t f '^Dockerfile' --full-path

Length of output: 0


🏁 Script executed:

#!/bin/bash
# Use find to locate Dockerfiles
find . -type f -name "Dockerfile" 2>/dev/null | head -20

# Also search for Dockerfile mentions in the repo to understand structure
rg -l "^FROM " --type-add 'docker:Dockerfile*' -t docker 2>/dev/null | head -10 || echo "rg with type filter failed, trying simple search"

# Alternative: just search for FROM statements which are typical in Dockerfiles
rg -l "^FROM " 2>/dev/null | grep -i dockerfile | head -10

Length of output: 1973


🏁 Script executed:

#!/bin/bash
# Create a GitHub issue for adopting COPY --link in Dockerfiles

gh issue create \
  --title "Adopt COPY --link in Dockerfiles for improved build performance" \
  --body "## Summary

This issue tracks the adoption of \`COPY --link\` in all Dockerfiles across the repository to improve build performance and cache stability.

## Background

The \`COPY --link\` flag (available in BuildKit with Dockerfile frontend v1.4+) creates independent layers for copied files, decoupling them from earlier commands in the build stage. This approach offers several advantages over standard \`COPY\` instructions.

**Reference**: Suggested in PR #1166, comment https://github.com/y-scope/clp/pull/1166#discussion_r2425008953 by @junhaoliao

## Benefits

1. **Stronger cache stability**: COPY steps won't be invalidated when earlier commands in the same stage change, leading to more reliable cache hits
2. **Faster parallel builds**: BuildKit can execute COPY operations without waiting for earlier commands to complete
3. **Reduced I/O and disk usage**: BuildKit avoids duplicating file blobs when possible through hard-link-like behavior
4. **Improved CI/CD performance**: Works well with remote cache and enables easier image rebasing

## Implementation Requirements

### 1. Add syntax header to all Dockerfiles
Each Dockerfile must start with:
\`\`\`dockerfile
# syntax=docker/dockerfile:1.4
\`\`\`

### 2. Update COPY instructions
Replace \`COPY\` with \`COPY --link\` where appropriate:
\`\`\`dockerfile
COPY --link ./source ./destination
\`\`\`

### 3. Dockerfiles to update

The following Dockerfiles should be evaluated and updated:
- \`tools/docker-images/clp-package/Dockerfile\`
- \`tools/docker-images/clp-execution-base-ubuntu-jammy/Dockerfile\`
- \`components/core/tools/docker-images/clp-core-ubuntu-jammy/Dockerfile\`
- \`components/core/tools/docker-images/clp-env-base-centos-stream-9/Dockerfile\`
- \`components/core/tools/docker-images/clp-env-base-manylinux_2_28-aarch64/Dockerfile\`
- \`components/core/tools/docker-images/clp-env-base-manylinux_2_28-x86_64/Dockerfile\`
- \`components/core/tools/docker-images/clp-env-base-musllinux_1_2-aarch64/Dockerfile\`
- \`components/core/tools/docker-images/clp-env-base-musllinux_1_2-x86_64/Dockerfile\`
- \`components/core/tools/docker-images/clp-env-base-ubuntu-jammy/Dockerfile\`

## Important Caveats

1. **Destination semantics**: \`COPY --link\` creates layers starting from an empty filesystem, which can change behavior when copying to ambiguous destinations. Use explicit trailing slashes for directories.

2. **Symlink resolution**: Avoid copying into or relying on symlink resolution from earlier layers, as linked layers don't inherit the previous layer's filesystem state.

3. **Testing required**: Before production deployment:
   - Validate image behavior and file layouts
   - Test across different platforms (Linux, macOS, Windows)
   - Verify CI/CD pipelines work correctly
   - Check interactions with \`--chown\` flags

4. **BuildKit requirement**: Ensure CI/CD environments use BuildKit (enabled by default in Docker 23.0+, or set \`DOCKER_BUILDKIT=1\`).

## Testing Plan

1. Update one Dockerfile as a proof-of-concept
2. Verify build times and cache behavior improvements
3. Test resulting images for correct runtime behavior
4. Roll out to remaining Dockerfiles incrementally
5. Update CI/CD workflows if needed to ensure BuildKit is enabled

## References

- [Docker BuildKit COPY --link documentation](https://docs.docker.com/engine/reference/builder/#copy---link)
- [Docker blog: Image rebase and improved remote cache support](https://www.docker.com/blog/image-rebase-and-improved-remote-cache-support-in-new-buildkit/)
- [How-To Geek: Docker COPY --link explanation](https://www.howtogeek.com/devops/how-docker-copy-link-improves-dockerfile-caching/)

## Success Criteria

- [ ] All Dockerfiles updated with syntax header and \`COPY --link\`
- [ ] Build times improved or maintained
- [ ] Cache hit rates improved
- [ ] All images validated for correct runtime behavior
- [ ] CI/CD pipelines functioning correctly" \
  --assignee junhaoliao \
  --label enhancement

Length of output: 42


@junhaoliao I completely agree! Using COPY --link offers significant benefits for build performance and cache stability, especially with the multi-stage builds in this repository.

I've created issue #1408 to track the adoption of COPY --link across all Dockerfiles in the repository. The issue includes:

  • Benefits: Improved cache stability, faster parallel builds, reduced I/O, and better CI/CD performance
  • Implementation details: Adding the # syntax=docker/dockerfile:1.4 header and updating COPY instructions
  • All 9 Dockerfiles that should be updated (including the new clp-package/Dockerfile and existing execution/environment images)
  • Important caveats: Destination semantics, symlink resolution, and testing requirements
  • Testing plan: Incremental rollout with validation at each step

The key benefits for your use case include better cache hits when earlier build stages change and improved build parallelism in CI/CD pipelines.

RUN ./setup-scripts/install-prebuilt-packages.sh \
&& rm -rf ./setup-scripts/

# Remove cached files
RUN apt-get clean \
&& rm -rf /var/lib/apt/lists/* /tmp/* /var/tmp/*

# Flatten the image
FROM scratch
COPY --from=base / /

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick (assertive)

Confirm apt hygiene and reproducibility of prebuilt package installation

Assuming setup-scripts/install-prebuilt-packages.sh does apt-get update/upgrade as needed and sets DEBIAN_FRONTEND=noninteractive. If not, please add it there to avoid interactive prompts and ensure deterministic installs.

I can draft the install-prebuilt-packages.sh to include:

  • set -eu; set -o pipefail
  • apt-get update; apt-get install -y --no-install-recommends …
  • DEBIAN_FRONTEND=noninteractive
  • cleanup: apt-get clean && rm -rf /var/lib/apt/lists/*
🧰 Tools
🪛 Checkov (3.2.334)

[LOW] 1-24: Ensure that HEALTHCHECK instructions have been added to container images

(CKV_DOCKER_2)

ENV CLP_HOME="/opt/clp"
ENV PATH="${CLP_HOME}/bin:${PATH}"
ENV PATH="${CLP_HOME}/sbin:${PATH}"
ENV PYTHONPATH="${CLP_HOME}/lib/python3/site-packages"
Comment on lines +13 to +16
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Below syntax reduces building layers while keeping VCS friendliness:

ENV KEY1=VAL1
    KEY2=VAL2

@coderabbitai create an issue for implementing the improvement

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Create a GitHub issue for the Dockerfile ENV optimization

gh issue create \
  --repo y-scope/clp \
  --title "Optimize ENV declarations in clp-package Dockerfile to reduce layers" \
  --body "## Context

As discussed in PR #1166 (https://github.com/y-scope/clp/pull/1166#discussion_r2403578550), the ENV declarations in \`tools/docker-images/clp-package/Dockerfile\` can be optimized to reduce the number of image layers while maintaining VCS friendliness.

## Current Implementation

Currently, the Dockerfile uses separate ENV statements:
\`\`\`dockerfile
ENV CLP_HOME=\"/opt/clp\"
ENV PATH=\"\${CLP_HOME}/bin:\${PATH}\"
ENV PATH=\"\${CLP_HOME}/sbin:\${PATH}\"
ENV PYTHONPATH=\"\${CLP_HOME}/lib/python3/site-packages\"
\`\`\`

## Proposed Improvement

Use multi-line ENV syntax to reduce layers:
\`\`\`dockerfile
ENV KEY1=VAL1 \\
    KEY2=VAL2
\`\`\`

## References

- PR: https://github.com/y-scope/clp/pull/1166
- Comment: https://github.com/y-scope/clp/pull/1166#discussion_r2403578550
- Requested by: @junhaoliao" \
  --assignee junhaoliao

Length of output: 42


@junhaoliao I've created issue #1378 to track this optimization. The issue includes the context from this discussion, the current implementation, and the proposed improvement with appropriate backlinks to this PR and comment.


USER 1000:1000
24 changes: 24 additions & 0 deletions tools/docker-images/clp-package/build.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
#!/usr/bin/env bash

set -eu
set -o pipefail

script_dir="$( cd "$( dirname "${BASH_SOURCE[0]}" )" &> /dev/null && pwd )"
repo_root=${script_dir}/../../../

build_cmd=(
docker build
--tag "clp-package:dev"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, in hindsight, I think we should include the architecture in the image name (to clearly indicate to users that it's not an ARM image). What do you think?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. while i see misusing an image built on a different arch could harm performance,
  2. using a unified name should make our container orchestration infra platform agnostic.

i feel the benefits that (2.) brings outweighs the harm (1.) could potentially cause. (1.) should not be an issue once we start looking into building mutli platform images

"$repo_root"
--file "${script_dir}/Dockerfile"
)

if command -v git >/dev/null && git -C "$script_dir" rev-parse --is-inside-work-tree >/dev/null ;
then
build_cmd+=(
--label "org.opencontainers.image.revision=$(git -C "$script_dir" rev-parse HEAD)"
--label "org.opencontainers.image.source=$(git -C "$script_dir" remote get-url origin)"
)
fi

"${build_cmd[@]}"
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
#!/usr/bin/env bash

set -eu
set -o pipefail

apt-get update
DEBIAN_FRONTEND=noninteractive apt-get install --no-install-recommends -y \
ca-certificates \
libcurl4 \
libmariadb3 \
python3
Comment on lines +3 to +11
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick (assertive)

Good minimal set; add apt cache cleanup to shrink the package image

Strict mode and package list look solid and consistent with prior learnings. Add cleanup to cut tens of MB from the final image layer.

 apt-get update
 DEBIAN_FRONTEND=noninteractive apt-get install --no-install-recommends -y \
   ca-certificates \
   libcurl4 \
   libmariadb3 \
   python3
+
+# Reduce layer size
+apt-get clean
+rm -rf /var/lib/apt/lists/*
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
set -eu
set -o pipefail
apt-get update
DEBIAN_FRONTEND=noninteractive apt-get install --no-install-recommends -y \
ca-certificates \
libcurl4 \
libmariadb3 \
python3
set -eu
set -o pipefail
apt-get update
DEBIAN_FRONTEND=noninteractive apt-get install --no-install-recommends -y \
ca-certificates \
libcurl4 \
libmariadb3 \
python3
# Reduce layer size
apt-get clean
rm -rf /var/lib/apt/lists/*
🤖 Prompt for AI Agents
In tools/docker-images/clp-package/setup-scripts/install-prebuilt-packages.sh
around lines 3 to 11, the script installs packages but does not clean apt cache,
which inflates the final image; after the apt-get install command add commands
to remove apt lists and clean the cache (e.g., run apt-get clean and remove
/var/lib/apt/lists/*) so the package metadata is deleted and the image size is
reduced.

Loading