Do you want to try using the hadolint linter too? It also makes use of shellcheck when it is available on the system.

There's more suggestions that will be made.

Are you familiar with the difference between ARG and ENV?

There's also a pattern to use to have build args passed between stages, and includes setting the ARG's value to an ENV. During the community sprint, me and @neteler fixed a situation where the variables were actually empty and didn't do what was expected, thus the build has been doing the wrong thing for a while.

If you have the chance to take a look at the build artifacts on docker desktop (the docker builds aren't made yet on PRs here), or simply do the build on a computer with Docker Desktop, you could take a look at the resulting info (and args) that remain in the layers, and see if the correct ones are used and passed between layers.

Otherwise, if you'd want to limit the scope of the PR, apart from commented out stuff, and conditional to a full review of where each ENV+ARG is defined and used, this PR is quite fine by itself. (The AS and setting of env with a = instead of the deprecated space between is totally trivial)

I added Open Container Initiative (OCI) metadata to the image using build arguments (https://github.com/opencontainers/image-spec/blob/main/annotations.md). It currently requires users to set and pass the arguments. I will have a look at how this could be avoided / use default values if not provided by the user...

Who was the other committer of these two commits: 344e68f
2aea488?
There isn't a real profile on github for it, I never heard it before

I resolved the conflicts to integrate the changes from #5481, which is where this PR started. It was quite hard, and I ended up merging older commits of main, as if it was done regularly while in draft, starting with the very next commit after the last merge.

This does not make the PR any better, nor do I agree with/reviewed all the changes, it is just up-to-date, and containing the intent of the PR (for example, to integrate the gdal-grass cmake changes of #5106 and #5108, I moved the ARG with version just above the RUN having "make distclean", in order to keep the command chaining, but I didn't move that variable way up in the file).

Who was the other committer of these two commits: 344e68f
2aea488?

AFAIK it is the second GH profile of @ninsbl.

OK, I see that OCI lables are now added with a docker-meta GH action. So they can probably be removed from the PR (at least many)?

Another question is whether the extensive package lists should be moved to separate text files (apt.txt, requirements.txt) that could be (re-) used in multiple places? We do have a python_requirements.txt and an apt.txt in .github/workflows. Would it be feasible and beneficial to have just one set of those files to be used with binder, GH actions, docker builds?

That said, I would like to update the docker image to Ubuntu 24.04 as I want to use actinia with a more recent base image... So I am not 100% sure what is the best way forward? Simplify this PR, merge it and then create a new one for Ubuntu 24.04 support?

For the requirement list, probably no, for layer caching reasons and observability. It’s easier to inspect the image layers from the metadata when we can read what was done, not having to dive in the filesystem of the layers to see it. I think that otherwise it will look like a copy step from ./ to ./ when inspecting the image.

Maybe, if the file list with the packages are understood as coming from a git context (read about contexts and what it means in Docker), the caching can understand to invalidate it and know to rerun the step. But we would need to be careful to know its made right so that building an image won’t skip the step if the instruction didn’t change (it did, but not in the text of the Dockerfile)

If you’d like to help out for CI working with 24.04 in general (we are getting real close to 26.04), helping finding the new package names and handle the GIS packages missing in 24.04 would really help. And a test failure because of newer numpy display format to adapt.

Who was the other committer of these two commits: 344e68f
2aea488?

AFAIK it is the second GH profile of @ninsbl.

Exactly. Sorry for the confusion...

Images are tested locally and a critical vulnerability from Pillow is handled using version pinning. Remaining vulnerabilities with lower criticality are due to the old kernel (about to upgrade the distro ;-))...

There is also: https://github.com/OSGeo/grass/blob/main/docker/ubuntu_wxgui/Dockerfile ...

I do have a Dockerfile for Ubuntu 24.04 ready too. Should I include that move here, or should we first merge this with Ubuntu 22.04 and then upgrade the OS to 24.04 in a next PR?

I do have a Dockerfile for Ubuntu 24.04 ready too. Should I include that move here, or should we first merge this with Ubuntu 22.04 and then upgrade the OS to 24.04 in a next PR?

Let's not have this PR have another scope creep. There's probably some useful changes that we need in CI on other places, so at least open a draft PR for it so we know what to change elsewhere, without redoing the work.