Skip to content

Refactor Linux CI to use pre-built Docker image from GHCR - #21972

Open
kadykov wants to merge 4 commits into
darktable-org:masterfrom
kadykov:ci-docker-build-environment-v2
Open

Refactor Linux CI to use pre-built Docker image from GHCR#21972
kadykov wants to merge 4 commits into
darktable-org:masterfrom
kadykov:ci-docker-build-environment-v2

Conversation

@kadykov

@kadykov kadykov commented Aug 23, 2026

Copy link
Copy Markdown
Contributor

Follows up on #19597 (devcontainer configuration).

What this changes

Replaces the per-run apt-get install steps in the Linux CI jobs with a pre-built Docker image hosted on GHCR. The image is built from .devcontainer/Dockerfile, which was introduced in the prerequisite PR and already mirrors the CI environment exactly.

Files changed

File Change
.github/workflows/build-docker.yml New build→test→push workflow: builds a candidate image from the Dockerfile, runs all four Linux CI matrix configurations against it via .github/scripts/test-image.sh, and pushes to GHCR only if every build succeeds
.github/scripts/test-image.sh New helper script that runs all Linux CI matrix configurations sequentially using docker run + .ci/ci-script.sh. Also callable locally for manual image validation
.github/workflows/ci.yml Linux jobs: use ghcr.io/darktable-org/darktable-build:latest; remove compiler and dependency install steps; permissions scoped to contents: read only
.devcontainer/devcontainer.json Switch from "build": (local Dockerfile build) to "image": (pull pre-built image — faster for devcontainer users)
.devcontainer/README.md Update CI environment section to document the build→test→push workflow and correct image update cadence
.ci/Dockerfile Removed — 2016-era Jenkins artefact, superseded by .devcontainer/Dockerfile
.gitignore Add install/ (created by test-image.sh during local runs)

Why

Single source of truth

.devcontainer/Dockerfile is the only place that defines the Linux build environment. ci.yml and the devcontainer both consume it. No more risk of drift between them.

Only tested images are published

The build-docker.yml workflow implements a build→test→push sequence. It verifies every Linux CI matrix configuration against the candidate image before updating :latest. An image that cannot build darktable is never published.

Faster CI feedback

The dependency install steps currently add several minutes to every Linux job. Pre-built images eliminate that overhead.

Easier contributor onboarding

devcontainer.json now uses "image": so opening the devcontainer is a docker pull rather than a full local build.

What does NOT change

  • Windows and macOS CI jobs — unchanged; containers are not applicable there
  • nightly.yml — the AppImage build deliberately targets ubuntu-22.04 for glibc compatibility and builds libs from source; it stays as-is
  • The CI matrix structure (compiler variants, build types, eco flags) — unchanged; compiler choice continues to be CC/CXX env vars

Image update cadence

:latest is updated only when .devcontainer/Dockerfile changes on master (triggering build-docker.yml automatically) or when a maintainer triggers a manual workflow_dispatch (e.g. to pick up upstream Ubuntu security patches). Between those events the image is frozen, which gives full control and reproducibility over the CI environment.

Image tagging strategy

Images are pushed as:

  • ghcr.io/darktable-org/darktable-build:latest — rolling, used by CI and devcontainer
  • ghcr.io/darktable-org/darktable-build:YYYY-MM-DD-SHORTSHA — pinned, for auditing

Notes

This PR replaces #21909 (same changes, cleaner git history — the earlier PR accumulated merge commits during development; please see that thread for full review context)

test-image.sh can also be used locally to validate a Dockerfile change before submitting a PR:

docker build -t darktable-build:candidate -f .devcontainer/Dockerfile .
.github/scripts/test-image.sh darktable-build:candidate "$PWD"

Copilot AI lite review requested due to automatic review settings August 23, 2026 17:52

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR refactors the Linux CI pipeline to run inside a pre-built Docker image published on GHCR (built from the devcontainer Dockerfile), removing per-run dependency installation and aligning CI + devcontainer on a single build-environment definition.

Changes:

  • Add a dedicated build→test→push workflow to build and validate a candidate image before publishing to GHCR.
  • Update Linux CI jobs to use ghcr.io/darktable-org/darktable-build:latest and drop in-job package installation.
  • Switch the devcontainer configuration and docs to consume the published image; remove the legacy .ci/Dockerfile; ignore locally-created install/ output.

Reviewed changes

Copilot reviewed 6 out of 7 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
.github/workflows/build-docker.yml Introduces a build/test/publish pipeline for the CI image.
.github/scripts/test-image.sh Adds a helper script to run the Linux CI matrix against a candidate image.
.github/workflows/ci.yml Moves Linux CI jobs to a GHCR-hosted container image and removes apt install steps.
.devcontainer/devcontainer.json Switches devcontainer from local build to pulling the published image.
.devcontainer/README.md Updates documentation to reflect the GHCR-based CI image workflow and usage.
.ci/Dockerfile Removes an obsolete legacy Dockerfile.
.gitignore Ignores install/ artifacts created by local image testing.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread .github/workflows/build-docker.yml Outdated
Comment thread .github/scripts/test-image.sh Outdated
Comment thread .github/workflows/ci.yml
@kadykov

kadykov commented Aug 23, 2026

Copy link
Copy Markdown
Contributor Author

Notes and caveats for reviewers

A few things worth knowing before this lands:

Splitting environment and code changes

With this approach, a PR that adds a new build dependency to the Dockerfile and code that uses it must be structured as two sequential PRs: first a Dockerfile-only PR (which, when merged, triggers build-docker.yml to build, test, and push the new :latest image), then the code PR on top. This is a deliberate trade-off for the "only tested images are published" guarantee. In practice Dockerfile changes are rare, so the extra step should be infrequent.

Image freshness — no automatic base-image updates

:latest is only updated when the Dockerfile changes on master or when a maintainer triggers workflow_dispatch. Security patches that land in the upstream ubuntu:26.04 image are not picked up automatically. Two options for addressing this in a follow-up PR:

  • Scheduled rebuild: add a schedule: trigger to build-docker.yml (e.g. weekly). Because the push step is gated by all CI matrix checks passing, a bad upstream update would block the push rather than silently breaking CI.
  • Dependabot + pinned digest: switch FROM ubuntu:26.04 to FROM ubuntu:26.04@sha256:<digest> and configure Dependabot for the Docker ecosystem. Dependabot opens a PR when the digest changes; build-docker.yml then validates the new base image automatically before merge (with the PR trigger we've now added). This gives the most control.

Either approach is a small follow-up; I'm happy to add a schedule: trigger to this PR if reviewers prefer, or leave it as a tracked future improvement.

What has been tested

  • ci.yml Linux jobs using the GHCR image: ✅ confirmed working (CI ran successfully on this PR)
  • build-docker.yml on pull_request: being verified via draft PR DO NOT MERGE: verify build-docker.yml PR trigger #21978 — a trivial Dockerfile comment triggers the full build+test cycle without pushing to GHCR
  • build-docker.yml on master push: will be confirmed only once this merges

@wpferguson

Copy link
Copy Markdown
Member

I have some questions...

  • How do we handle dependencies that rely on other dependencies?
  • How do we handle dependencies that build correctly but are broken?
  • How do we handle the situation where the first PR gets merged, the image is updated, and another PR runs CI before the second PR gets merged?
  • How do we handle devs whose workload doubles because of this?
  • How do we handle devs that don't want to mess with the dockerfile at all, but are now forced to

@kadykov

kadykov commented Aug 25, 2026

Copy link
Copy Markdown
Contributor Author

@wpferguson — thanks for taking the time to ask these. They're fair concerns and deserve a thorough answer rather than a quick dismissal.

On the two questions about workload and not wanting to touch the Dockerfile

Let me start here because these are the most important, and because understanding the why changes how the other questions look.

The honest answer is: yes, for the small subset of PRs that add a new build dependency, you now need two PRs instead of one. We didn't design it that way out of preference — it's a constraint imposed by GitHub's security model.

The obvious alternative is: on every PR that modifies the Dockerfile, build a fresh image, push it to GHCR, and use it in the matrix jobs — no split needed. We implemented exactly this in an earlier draft in 87fc0c8. It failed immediately with:

ERROR: failed to build: failed to solve: failed to push ghcr.io/darktable-org/darktable-build:sha-fe0703749d83737e2a7b2236c1071ccf608869cf: denied: installation not allowed to Write organization package

GitHub deliberately prevents pushing to organisation-level container packages during pull_request events, even for collaborators in the same org. This is an intentional security boundary: it stops a contributor from submitting a PR that poisons the shared image that everyone else's CI depends on. There is no configuration option to bypass it without removing that protection entirely.

So the real choice is between three options:

Environment stability Split PRs for new deps? Security
Current master (apt-get install per job) Poor — any apt package update can silently break CI No
This PR (pre-built GHCR image) Excellent — env only changes via reviewed PRs Yes
Allow everyone to push org packages Excellent No Anyone can overwrite :latest

The current approach (installing everything fresh on every CI run) is more fragile than it looks. Package updates in the Ubuntu repos could occasionally break builds in ways that are confusing and hard to debug because nobody changed any darktable code. This PR eliminates that class of failure entirely.

As for the Dockerfile itself: it contains nothing new or Docker-specific. It's the same flat apt-get install list that currently lives in ci.yml, moved to a separate file. A contributor who needs to add a new library edits one line, not a Dockerfile concept. And developers who build natively and never touch CI don't interact with it at all.

On dependencies that rely on other dependencies

apt resolves transitive dependencies automatically, the same as today. The Dockerfile's apt-get install command uses the same package manager, the same Ubuntu repos, and the same dependency resolution as the current per-run install steps in ci.yml. The only difference is timing: once at image build time rather than on every CI run.

If you're thinking of git submodules (e.g., rawspeed): those aren't in the Docker image and are handled identically to today.

On dependencies that build correctly but are broken

From your earlier message in the devcontainer thread I think you're describing a real and frustrating problem: code that compiles and passes CI but has runtime bugs because it wasn't tested by the author on actual images. That's a genuine concern — but it's entirely separate from how the build environment is provisioned. The same test suite runs before and after this PR; we're not adding or removing any tests. If there are specific integration or runtime checks you'd like to add, that would be a great PR of its own.

On the concurrent-PR scenario

I want to make sure I understand the scenario correctly. If the concern is: "what if an image update merges while another PR is open, and breaks that PR's CI?" — that's a real possibility, but it's a smaller problem than today. Currently, the environment can silently change between any two CI runs of the same PR (a new apt package version lands in Ubuntu repos) with no record of what changed and no reviewed approval. With this PR, the environment changes only when a Dockerfile change is explicitly reviewed and merged, and the full test suite must pass before the new image is published.

What's still open

The one legitimate limitation — split PRs for new dependencies — we can partially address. Two options we're happy to add in a follow-up if the project wants them:

  1. Scheduled weekly rebuild: build-docker.yml gains a schedule: trigger so security-patched Ubuntu packages are picked up regularly. The push is gated on all CI checks passing, so a bad upstream update is caught before it affects anyone.
  2. Dependabot + pinned base image: Switch FROM ubuntu:26.04 to a digest-pinned tag and configure Dependabot to open a PR when the digest changes. Each Dependabot PR triggers build-docker.yml's full test cycle before merge, giving full auditability and control over when the base image updates.

Either of these could be a small, focused follow-up once this lands. Or if you prefer, we can do it in this PR.

@anoderay

anoderay commented Aug 25, 2026

Copy link
Copy Markdown
Collaborator

@kadykov I'm sorry but I have to say this: If I voiced my concerns here like @wpferguson did, I'd prefer an obviously human answer, not an AI one... Because of how obviously AI your answer is, it feels at least to me more like a quick dismissal rather than an proper answer...

@kadykov

kadykov commented Aug 25, 2026

Copy link
Copy Markdown
Contributor Author

@anoderay, I understand your concern that you don't like the style of the AI messages. Especially for this particular message, I will not use AI. Technically, I still use AI because I use TTS, but I don't use LLM polishing in this particular case.

So my current workflow is like this. I have an AI agent that has access to all the source code. Then, using TTS, I dictate my thoughts on the problem. And usually it is quite a lengthy document. For instance, here is the gist that I have prepared in order to answer to the last message. Then I have an AI agent that has access to the all source code, including my recent modifications of the code base. And here is what I actually got from the LLM AI agent.

As you see, both of these documents are quite far from what I have actually sent as an answer to @wpferguson. And personally, I would much prefer the LLM-polished answers than the raw output after TTS.

The truth is that I am not a native English speaker, and I am not a specialist in all the fields. Indeed, I can write a nice, well-structured answer by myself, but it would take much more longer. That's why I prefer to draft the messages with TTS, and then polish and rewrite them with LLM. If you have any advices about in what style should I use for LLM rewriting, then please tell me, and I will use it for my future messages.

And speaking about my previous message, I have read the LLM-polished version, and I agree with all the points. So, if you have any suggestion how we can improve this pull request, please tell me. But in my opinion, freezing the environment in the GitHub container registry would benefit to the stability of CI.

@wpferguson

wpferguson commented Aug 25, 2026

Copy link
Copy Markdown
Member

On dependencies that rely on other dependencies

I wasn't talking about OS stuff, I was talking about darktable stuff. For instance the lua-scripts, an external module, rely on the Lua external module. When we upgrade to Lua 5.5, it will break some of the existing lua-scripts.

So, IIUC I would have to

  • Do a PR to add Lua 5.5 to the dockerfile and remove Lua 5.4. Once that dockerfile is pushed every CI run using that dockerfile will fail because darktable will refuse to build because of the missing Lua 5.4 dependency. NOTE: There are other dependencies that work this way too.
  • I do another PR so that Lua will now use 5.5 instead of 5.4. Now CI will work unless a test involves the Lua scripts and then it might fail.
  • I do another PR to update Windows and MacOS to 5.5
  • I do another PR to update the lua-scripts to be compatible with Lua 5.5

Whereas today I could do this in one PR with 5 commits and all the operating systems covered and nothing broken.

- Remove .ci/Dockerfile (2016-era Jenkins artefact, superseded)
- .devcontainer/devcontainer.json: switch to pre-built GHCR image
- .github/workflows/build-docker.yml: implement build→test→push workflow
  that builds from .devcontainer/Dockerfile, runs all Linux CI matrix
  configurations via .github/scripts/test-image.sh, and only pushes to
  GHCR if every build succeeds
- .github/workflows/ci.yml: Linux jobs pull ghcr.io/darktable-org/
  darktable-build:latest; permissions scoped to contents:read only
- .github/scripts/test-image.sh: new helper script that runs all four
  Linux CI matrix configurations (GNU16 Release, LLVM22 Release,
  GNU16 Debug, GNU16 Release+tests) against a candidate Docker image;
  also callable locally for manual validation
- .devcontainer/README.md: update CI environment section to describe
  the build→test→push workflow and correct the GHCR image update cadence
- .gitignore: add install/ (created by test-image.sh during local runs)
…docker-build workflow, authenticate in GHCR for pulling images in CI workflow
…patch

Add a pull_request trigger on .devcontainer/Dockerfile changes so that a
Dockerfile-modifying PR gets a full build-test cycle before merge.

The "Tag and push" step is guarded by `github.event_name != 'pull_request'`
so nothing is ever published to GHCR during a PR run.
@kadykov
kadykov force-pushed the ci-docker-build-environment-v2 branch from 66f6694 to d1aa1e9 Compare August 25, 2026 16:49
@kadykov

kadykov commented Aug 25, 2026

Copy link
Copy Markdown
Contributor Author

@wpferguson First, you don't need a separate pull request for modifying Windows or macOS dependencies because we change only how we store Linux dependencies. But overall, now I think I understand your concern.

So you are worrying about updating dependencies that we cannot submit as a separate pull request because it will break the CI for others, like transition from Lua 5.4 to 5.5. In this case, you don't have other choices rather than submit all the changes in one pull request as you would do it now. Nothing changes on the macOS side and Windows side, but you should expect that all Linux CI checks will be red, because they still use the old image from the GitHub container registry with the old version of Lua.

However, because you have modified the Docker file, then the build-docker.yaml workflow will be triggered. And this workflow does exactly the same checks what we have in Linux CI, using your modified Docker file and therefore, it will use the updated version of the Lua, and this check should pass.

So overall, you will have four failing Linux CI checks and one additional "Build, test and push CI Docker image", like in #21978 that replaces them. And when this workflow is green, you can merge the PR, ignoring that the other four Linux CI checks are still red. And like that, you will update the image on the GitHub container registry and the code at the same time.

However, if you just need to add a package to the environment, or you want to do a harmless change to the environment that will not break anything, then you should just first create a separate pull request with only environment changes. In this case, build-docker.yaml workflow will verify if your environment changes would break anything or not. And only after your PR lands, create another PR with the code changes.

@jenshannoschwalm

Copy link
Copy Markdown
Collaborator

Why are these AI 'supported' texts always blown up to 10times what's required?

@kadykov

kadykov commented Aug 26, 2026

Copy link
Copy Markdown
Contributor Author

@jenshannoschwalm I believe it is because we train LLMs this way. The longer the output, the more quality response we have at the end. That's why we encourage LLMs to produce some thinking before giving the definitive answer. But it is just my opinion.

And speaking about this pull request, do you have any suggestions, or thoughts that are not related to the AI usage?

@jenshannoschwalm

Copy link
Copy Markdown
Collaborator
  1. Not sure yet if this pr "helps", i would have to try it out in real :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants