-
Notifications
You must be signed in to change notification settings - Fork 83
feat(docker): Add container image containing the CLP package, for Docker Compose integration (resolves #1164). #1166
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 13 commits
8ea4529
4d402c1
adef439
9aa74ff
542da81
ce43d17
2c2ee50
75f88d9
414e1fb
0e9138a
912b678
dcc8a70
46ffa55
9db7ca9
d609987
1b3959d
1f13954
aceb39f
c49d8b4
099afc0
73ca1a0
a4317ea
806f2f8
1887626
20dcfb1
ddab8d1
92861b9
b39644e
c17a782
8ff64f2
1dfe706
47d0517
628ddae
23f1ef2
63a7ec3
61da837
227d6dd
e98bb8e
e6f1af5
860e5ef
4180739
6cfabd7
f1c566f
09539ae
b49daba
be2560e
44a2dc5
2ebbfe0
1254827
62cf06a
2f29228
c413a13
bb36035
ea4ce51
3c81014
e5a66cf
ff21c8e
cbea39b
a857701
7dfbcc5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,8 +1,14 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| name: "clp-execution-image-build" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| description: "Builds a container image that contains the dependencies necessary | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| to run the CLP package." | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| name: "clp-image-build" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| description: "Builds a CLP container image." | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
junhaoliao marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| inputs: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| image_type: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| description: "Type of image to build" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| required: true | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| type: "choice" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| options: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| - "execution" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| - "package" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
kirkrodrigues marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| image_registry: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| default: "ghcr.io" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| description: "Container image registry" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -50,15 +56,29 @@ runs: | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| uses: "docker/metadata-action@902fa8ec7d6ecbf8d84d538b9b233a880e428804" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| with: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| images: "${{inputs.image_registry}}/${{steps.sanitization.outputs.REPOSITORY}}\ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /clp-execution-x86-${{inputs.platform_id}}-${{inputs.platform_version_id}}" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /clp-${{inputs.image_type}}-x86-${{inputs.platform_id}}-${{inputs.platform_version_id}}" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| images: "${{inputs.image_registry}}/${{steps.sanitization.outputs.REPOSITORY}}\ | |
| /clp-execution-x86-${{inputs.platform_id}}-${{inputs.platform_version_id}}" | |
| /clp-${{inputs.image_type}}-x86-${{inputs.platform_id}}-${{inputs.platform_version_id}}" | |
| images: "${{inputs.image_registry}}/${{steps.sanitization.outputs.REPOSITORY}}\ | |
| /clp-${{inputs.image_type}}-${{inputs.platform_id}}-${{inputs.platform_version_id}}" |
🤖 Prompt for AI Agents
In .github/actions/clp-image-build/action.yaml around lines 58 to 60, the image
tag hardcodes "x86" which causes incorrect tags for ARM images. Replace the
fixed "x86" string with a dynamic variable that reflects the actual
architecture, such as using an input or environment variable like
`${{inputs.arch}}` or `${{env.ARCH_NAME}}` to correctly tag images for different
architectures.
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about dockerfile_path to avoid confusion with "$PATH"?
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Harden path resolution: quote expansions, fail fast, validate image_type, and ensure Dockerfile exists
Unquoted variables and missing existence checks can lead to brittle failures. Also, path can be confused with $PATH.
Apply:
- - name: "Determine Dockerfile Path"
+ - name: "Determine Dockerfile Path"
id: "dockerfile"
shell: "bash"
run: |
- base_path="./tools/docker-images"
- platform="${{inputs.platform_id}}-${{inputs.platform_version_id}}"
-
- if [[ "${{inputs.image_type}}" == "execution" ]]; then
- path="$base_path/clp-execution-base-$platform/Dockerfile"
- else
- path="$base_path/clp-package-$platform/Dockerfile"
- fi
-
- echo "DOCKERFILE=$path" >> "$GITHUB_OUTPUT"
+ set -euo pipefail
+ base_path="./tools/docker-images"
+ platform="${{ inputs.platform_id }}-${{ inputs.platform_version_id }}"
+
+ case "${{ inputs.image_type }}" in
+ execution)
+ dockerfile_path="${base_path}/clp-execution-base-${platform}/Dockerfile"
+ ;;
+ package)
+ dockerfile_path="${base_path}/clp-package-${platform}/Dockerfile"
+ ;;
+ *)
+ echo "Unsupported image_type: '${{ inputs.image_type }}' (expected: execution|package)" >&2
+ exit 1
+ ;;
+ esac
+
+ if [[ ! -f "${dockerfile_path}" ]]; then
+ echo "Dockerfile not found at: ${dockerfile_path}" >&2
+ exit 1
+ fi
+
+ printf 'DOCKERFILE=%s\n' "${dockerfile_path}" >> "$GITHUB_OUTPUT"This addresses quoting, validation, and verifiable failure modes.
📝 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.
| - name: "Determine Dockerfile Path" | |
| id: "dockerfile" | |
| shell: "bash" | |
| run: | | |
| base_path="./tools/docker-images" | |
| platform="${{inputs.platform_id}}-${{inputs.platform_version_id}}" | |
| if [[ "${{inputs.image_type}}" == "execution" ]]; then | |
| path="$base_path/clp-execution-base-$platform/Dockerfile" | |
| else | |
| path="$base_path/clp-package-$platform/Dockerfile" | |
| fi | |
| echo "DOCKERFILE=$path" >> "$GITHUB_OUTPUT" | |
| - name: "Determine Dockerfile Path" | |
| id: "dockerfile" | |
| shell: "bash" | |
| run: | | |
| set -euo pipefail | |
| base_path="./tools/docker-images" | |
| platform="${{ inputs.platform_id }}-${{ inputs.platform_version_id }}" | |
| case "${{ inputs.image_type }}" in | |
| execution) | |
| dockerfile_path="${base_path}/clp-execution-base-${platform}/Dockerfile" | |
| ;; | |
| package) | |
| dockerfile_path="${base_path}/clp-package-${platform}/Dockerfile" | |
| ;; | |
| *) | |
| echo "Unsupported image_type: '${{ inputs.image_type }}' (expected: execution|package)" >&2 | |
| exit 1 | |
| ;; | |
| esac | |
| if [[ ! -f "${dockerfile_path}" ]]; then | |
| echo "Dockerfile not found at: ${dockerfile_path}" >&2 | |
| exit 1 | |
| fi | |
| printf 'DOCKERFILE=%s\n' "${dockerfile_path}" >> "$GITHUB_OUTPUT" |
🤖 Prompt for AI Agents
In .github/actions/clp-image-build/action.yaml around lines 61 to 75, the script
uses unquoted expansions, a generic variable name `path`, and lacks
validation/existence checks; update it to: enable fail-fast (set -e), use a
non-conflicting variable name like `dockerfile_path`, quote all variable
expansions when constructing paths, validate that inputs.image_type is one of
the expected values (e.g., "execution" or "package") and exit with a clear error
for invalid values, check that the computed Dockerfile exists and fail with an
informative message if not, and then write the quoted path to GITHUB_OUTPUT.
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,103 @@ | ||||||||||||||||||||||||||
| name: "clp-package-image-build" | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| on: | ||||||||||||||||||||||||||
| push: | ||||||||||||||||||||||||||
| workflow_dispatch: | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
| env: | ||||||||||||||||||||||||||
| DEPS_IMAGE_NAME_PREFIX: "clp-core-dependencies-x86-" | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| jobs: | ||||||||||||||||||||||||||
| filter-relevant-changes: | ||||||||||||||||||||||||||
| name: "filter-relevant-changes" | ||||||||||||||||||||||||||
| runs-on: "ubuntu-24.04" | ||||||||||||||||||||||||||
| outputs: | ||||||||||||||||||||||||||
| ubuntu_jammy_image_changed: "${{steps.filter.outputs.ubuntu_jammy_image}}" | ||||||||||||||||||||||||||
| steps: | ||||||||||||||||||||||||||
| - uses: "actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683" | ||||||||||||||||||||||||||
| with: | ||||||||||||||||||||||||||
| submodules: "recursive" | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| - name: "Work around actions/runner-images/issues/6775" | ||||||||||||||||||||||||||
| run: "chown $(id -u):$(id -g) -R ." | ||||||||||||||||||||||||||
| shell: "bash" | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
| - name: "Filter relevant changes" | ||||||||||||||||||||||||||
| uses: "dorny/paths-filter@de90cc6fb38fc0963ad72b210f1f284cd68cea36" | ||||||||||||||||||||||||||
| id: "filter" | ||||||||||||||||||||||||||
| with: | ||||||||||||||||||||||||||
| # Consider changes between the current commit and `main` | ||||||||||||||||||||||||||
| # NOTE: If a pull request changes one of the images, then we need to (1) build the image | ||||||||||||||||||||||||||
| # (based on commits in the PR) and then (2) build CLP using the changed image. If a pull | ||||||||||||||||||||||||||
| # request doesn't change an image, then we don't need to rebuild the image; instead we can | ||||||||||||||||||||||||||
| # use the published image which is based on `main`. So when determining what files have | ||||||||||||||||||||||||||
| # changed, we need to consider the delta between the current commit and `main` (rather | ||||||||||||||||||||||||||
| # than the current and previous commits) in order to detect if we need to rebuild the | ||||||||||||||||||||||||||
| # image (since it would be different from the published image). | ||||||||||||||||||||||||||
| base: "main" | ||||||||||||||||||||||||||
| filters: | | ||||||||||||||||||||||||||
| ubuntu_jammy_image: | ||||||||||||||||||||||||||
| - ".github/actions/**" | ||||||||||||||||||||||||||
| - ".github/workflows/clp-core-build.yaml" | ||||||||||||||||||||||||||
| - "components/core/tools/scripts/lib_install/*.sh" | ||||||||||||||||||||||||||
| - "components/core/tools/docker-images/clp-env-base-ubuntu-jammy/**" | ||||||||||||||||||||||||||
| - "components/core/tools/scripts/lib_install/ubuntu-jammy/**" | ||||||||||||||||||||||||||
coderabbitai[bot] marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||||||||||||||||||||||
| ubuntu-jammy-deps-image: | ||||||||||||||||||||||||||
| name: "ubuntu-jammy-deps-image" | ||||||||||||||||||||||||||
| needs: "filter-relevant-changes" | ||||||||||||||||||||||||||
| runs-on: "ubuntu-24.04" | ||||||||||||||||||||||||||
| steps: | ||||||||||||||||||||||||||
| - uses: "actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683" | ||||||||||||||||||||||||||
| with: | ||||||||||||||||||||||||||
| submodules: "recursive" | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| - name: "Work around actions/runner-images/issues/6775" | ||||||||||||||||||||||||||
| run: "chown $(id -u):$(id -g) -R ." | ||||||||||||||||||||||||||
| shell: "bash" | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
| - name: "Work around actions/runner-images/issues/6775" | |
| run: "chown $(id -u):$(id -g) -R ." | |
| shell: "bash" | |
| - name: "Work around actions/runner-images/issues/6775" | |
| run: "chown \"$(id -u)\":\"$(id -g)\" -R ." | |
| shell: "bash" |
🧰 Tools
🪛 actionlint (1.7.7)
57-57: shellcheck reported issue in this script: SC2046:warning:1:7: Quote this to prevent word splitting
(shellcheck)
57-57: shellcheck reported issue in this script: SC2046:warning:1:16: Quote this to prevent word splitting
(shellcheck)
🤖 Prompt for AI Agents
.github/workflows/clp-package-image-build.yaml around lines 56 to 59: the run
step uses unquoted command substitutions which can break if they contain spaces
or unexpected characters; update the run command to quote the substitutions
(e.g., chown "$(id -u):$(id -g)" -R .) so the shell treats the uid:gid as a
single argument and behaves consistently.
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this is a good idea since it might cause some issues with two different workflows trying to publish the same container, not to mention the resources it'll use.
I think a quick solution might be to move this package image build into clp-core-build workflow.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
to move this package image build into clp-core-build workflow
if we move it into clp-core-build, i believe we need to update the workflows trigger conditions. instead of an allow list, we might need to write it as
on:
pull_request:
paths: &monitored_paths
- "**"
- "!.github/ISSUE_TEMPLATE/**"
- "!.github/*"
- "!components/core/tools/scripts/lib_install/macos/**"
- "!docs/**"
push:
paths: *monitored_paths
then the other jobs in the workflow will rely on filter-relevant-changes to see if they should run, as they currently do.
is my understanding correct? are there any other paths / patterns that need to be added to the deny list (!)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's correct. That pattern should be fine.
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Job cannot reference filter-relevant-changes outputs because it isn’t in needs
ubuntu-jammy-package-image uses
needs.filter-relevant-changes.outputs.ubuntu_jammy_image_changed
but only lists ubuntu-jammy-deps-image in needs.
Workflow evaluation will fail at runtime.
-needs: "ubuntu-jammy-deps-image"
+needs:
+ - "ubuntu-jammy-deps-image"
+ - "filter-relevant-changes"📝 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.
| ubuntu-jammy-package-image: | |
| name: "ubuntu-jammy-package-image" | |
| needs: "ubuntu-jammy-deps-image" | |
| runs-on: "ubuntu-24.04" | |
| env: | |
| ubuntu-jammy-package-image: | |
| name: "ubuntu-jammy-package-image" | |
| needs: | |
| - "ubuntu-jammy-deps-image" | |
| - "filter-relevant-changes" | |
| runs-on: "ubuntu-24.04" | |
| env: |
🤖 Prompt for AI Agents
In .github/workflows/clp-package-image-build.yaml around lines 71 to 75, the job
ubuntu-jammy-package-image references outputs from filter-relevant-changes in
its needs but does not list filter-relevant-changes in its needs array. To fix
this, add filter-relevant-changes to the needs list for
ubuntu-jammy-package-image so it can access the required outputs and avoid
runtime evaluation errors.
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we move this into the scope of the step that requires it?
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick (assertive)
Guardrails look good; minor readability nit
The use_published_image condition is correct once needs is fixed. Optional: extract the OR’d predicate into a reusable job-level env for readability.
🧰 Tools
🪛 actionlint (1.7.7)
91-91: property "filter-relevant-changes" is not defined in object type {ubuntu-jammy-deps-image: {outputs: {}; result: string}}
(expression)
🤖 Prompt for AI Agents
.github/workflows/clp-package-image-build.yaml around lines 91 to 96: the OR’d
predicate used inline for use_published_image hurts readability; extract the
full boolean expression into a job-level env (e.g. USE_PUBLISHED_IMAGE) that
assigns the same expression, then reference that env in use_published_image so
the condition is clearer and reusable—ensure you preserve the original
logic/parentheses and update any dependent references accordingly.
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
|---|---|---|
|
|
@@ -69,5 +69,16 @@ 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-image-package | ||
| ``` | ||
|
|
||
| This will create a Docker image named `clp-package-<os>-ubuntu-jammy:dev` where `<os>` is `x86` or | ||
| `arm64`. | ||
|
|
||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick (assertive) Clarify local tag vs published tag and note arm64 build constraints.
Proposed tweak: ## Building a Docker image
To build a Docker image containing the CLP package, run:
```shell
task docker-image-package-This will create a Docker image named In docs/src/dev-guide/building-package.md around lines 72 to 82, clarify that |
||
| [clp-issue-872]: https://github.com/y-scope/clp/issues/872 | ||
| [Task]: https://taskfile.dev/ | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -133,6 +133,16 @@ environment. | |
| tools/docker-images/clp-execution-base-ubuntu-jammy | ||
| ``` | ||
|
|
||
| ## clp-package-x86-ubuntu-jammy | ||
|
|
||
| An image containing the CLP package built in an Ubuntu Jammy x86_64 environment. | ||
|
|
||
| * Path: | ||
|
|
||
| ```text | ||
| tools/docker-images/clp-package-ubuntu-jammy | ||
| ``` | ||
|
|
||
|
||
| [core-deps-centos-stream-9]: https://github.com/y-scope/clp/pkgs/container/clp%2Fclp-core-dependencies-x86-centos-stream-9 | ||
| [core-deps-ubuntu-jammy]: https://github.com/y-scope/clp/pkgs/container/clp%2Fclp-core-dependencies-x86-ubuntu-jammy | ||
| [core-ubuntu-jammy]: https://github.com/y-scope/clp/pkgs/container/clp%2Fclp-core-x86-ubuntu-jammy | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -76,6 +76,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. | ||
|
|
||
|
||
| ## clp-lint | ||
|
|
||
| This workflow runs linting checks on the codebase. | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -162,6 +162,17 @@ tasks: | |||||||||||||||||||||||||||||||||||||||||||||||||
| CHECKSUM_FILE: "{{.CHECKSUM_FILE}}" | ||||||||||||||||||||||||||||||||||||||||||||||||||
| INCLUDE_PATTERNS: ["{{.OUTPUT_DIR}}"] | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| docker-image-package: | ||||||||||||||||||||||||||||||||||||||||||||||||||
kirkrodrigues marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| vars: | ||||||||||||||||||||||||||||||||||||||||||||||||||
| SRC_DIR: "{{.ROOT_DIR}}/tools/docker-images/clp-package-ubuntu-jammy" | ||||||||||||||||||||||||||||||||||||||||||||||||||
| dir: "{{.SRC_DIR}}" | ||||||||||||||||||||||||||||||||||||||||||||||||||
| deps: | ||||||||||||||||||||||||||||||||||||||||||||||||||
| - "package" | ||||||||||||||||||||||||||||||||||||||||||||||||||
| sources: | ||||||||||||||||||||||||||||||||||||||||||||||||||
| - "{{.SRC_DIR}}/**/*" | ||||||||||||||||||||||||||||||||||||||||||||||||||
| cmds: | ||||||||||||||||||||||||||||||||||||||||||||||||||
| - "./build.sh" | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| docker-image-package: | |
| vars: | |
| SRC_DIR: "{{.ROOT_DIR}}/tools/docker-images/clp-package-ubuntu-jammy" | |
| dir: "{{.SRC_DIR}}" | |
| deps: | |
| - "package" | |
| sources: | |
| - "{{.SRC_DIR}}/**/*" | |
| cmds: | |
| - "./build.sh" | |
| docker-image-package: | |
| vars: | |
| SRC_DIR: "{{.ROOT_DIR}}/tools/docker-images/clp-package-ubuntu-jammy" | |
| dir: "{{.SRC_DIR}}" | |
| deps: | |
| - "package" | |
| sources: | |
| - "{{.SRC_DIR}}/**/*" | |
| - "{{.G_PACKAGE_BUILD_DIR}}/**/*" | |
| 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 taskfile.yaml around lines 165 to 175, the docker-image-package task must be
updated so image rebuilds when package output changes and to fail early if
Docker is missing: add the built package dir (e.g. include
"{{.G_PACKAGE_BUILD_DIR}}/**" or the appropriate path under that variable) to
the task's sources list so changes there trigger rebuilds, and add a
preconditions entry that verifies Docker is on PATH (run a shell check for the
docker binary and emit a clear error message and non-zero exit if not found)
before running cmds.
| Original file line number | Diff line number | Diff line change | ||
|---|---|---|---|---|
| @@ -0,0 +1,22 @@ | ||||
| FROM ubuntu:jammy AS base | ||||
|
|
||||
| COPY ./build/clp-package /opt/clp | ||||
|
|
||||
| WORKDIR /root | ||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick (assertive) WORKDIR in base doesn’t carry into the final stage Since you re-stage with -WORKDIR /root📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||
|
|
||||
| COPY ./tools/docker-images/clp-package-ubuntu-jammy/setup-scripts ./setup-scripts | ||||
| RUN ./setup-scripts/install-prebuilt-packages.sh && \ | ||||
| rm -rf ./setup-scripts/ | ||||
junhaoliao marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||
|
|
||||
| # Remove cached files | ||||
| RUN apt-get clean \ | ||||
| && rm -rf /var/lib/apt/lists/* /tmp/* /var/tmp/* | ||||
|
|
||||
| # Flatten the image | ||||
| FROM scratch | ||||
| COPY --from=base / / | ||||
|
|
||||
coderabbitai[bot] marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||
| ENV PATH="/opt/clp/sbin:${PATH}" | ||||
| ENV PATH="/opt/clp/bin:${PATH}" | ||||
| ENV PYTHONPATH="/opt/clp/lib/python3/site-packages" | ||||
| ENV CLP_HOME="/opt/clp" | ||||
|
||||
kirkrodrigues marked this conversation as resolved.
Show resolved
Hide resolved
|
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,38 @@ | ||||||||||||||||||||||||||||||||||||||
| #!/usr/bin/env bash | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| # Exit on any error | ||||||||||||||||||||||||||||||||||||||
| set -e | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| # Error on undefined variable | ||||||||||||||||||||||||||||||||||||||
| set -u | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
coderabbitai[bot] marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||
| script_dir="$( cd "$( dirname "${BASH_SOURCE[0]}" )" &> /dev/null && pwd )" | ||||||||||||||||||||||||||||||||||||||
| repo_root=${script_dir}/../../../ | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| arch=$(uname -m) | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| if [ "$arch" = "x86_64" ]; then | ||||||||||||||||||||||||||||||||||||||
| arch_name="x86" | ||||||||||||||||||||||||||||||||||||||
| elif [ "$arch" = "aarch64" ]; then | ||||||||||||||||||||||||||||||||||||||
| arch_name="arm64" | ||||||||||||||||||||||||||||||||||||||
| else | ||||||||||||||||||||||||||||||||||||||
| echo "Error: Unsupported architecture - $arch" | ||||||||||||||||||||||||||||||||||||||
| exit 1 | ||||||||||||||||||||||||||||||||||||||
| fi | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| build_cmd=( | ||||||||||||||||||||||||||||||||||||||
| docker build | ||||||||||||||||||||||||||||||||||||||
| --tag "clp-package-${arch_name}-ubuntu-jammy:dev" | ||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||
| "$repo_root" | ||||||||||||||||||||||||||||||||||||||
| --file "${script_dir}/Dockerfile" | ||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick (assertive) Preflight: fail fast if the package payload is missing Avoids a confusing Docker build error when build_cmd=(
docker build
--tag "clp-package-${arch_name}-ubuntu-jammy:dev"
"$repo_root"
--file "${script_dir}/Dockerfile"
)
+
+# Ensure build payload exists
+if [ ! -d "${repo_root%/}/build/clp-package" ]; then
+ echo "Error: Missing build/clp-package. Run 'task package' first." >&2
+ exit 1
+fi📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| 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)" | ||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick (assertive) Use repository root for Git labels to stay consistent with other image builders Current call runs - --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)"
+ --label "org.opencontainers.image.revision=$(git -C "$repo_root" rev-parse HEAD)"
+ --label "org.opencontainers.image.source=$(git -C "$repo_root" remote get-url origin)"🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||
| fi | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| "${build_cmd[@]}" | ||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,19 @@ | ||
| #!/usr/bin/env bash | ||
|
|
||
| # Exit on any error | ||
| set -e | ||
|
|
||
| # Error on undefined variable | ||
| set -u | ||
|
|
||
coderabbitai[bot] marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| 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 | ||
Uh oh!
There was an error while loading. Please reload this page.