Install via pip instead of apt

[dkaszews]

Replace apt install with pip install, as it allows specifying minor and patch versions. Specifying just major version installs the latest version with that major number. Specifying exact version installs that version.

Partial versions like 21.1 are not supported, since that would require us detecting whether version is complete, annoying since couple like 13.0.1.1 are 4 segment. Doable with $ pip index versions --json clang-format | jq -r '.versions.[]' | grep -qxF "$CLANG_FORMAT_VERSION", but that requires installing jq so it's probably not worth it, especially since we did not have multiple minors per major since version 11.

Tested dockerfile locally (also run the test/known_{pass,fail}):

for CLANG_FORMAT_VERSION in 19 19.1.4 19.1.7 20 20.1.5 20.1.8 21 21.1.2 21.1.6; do
    docker build -t clang-format --build-arg UBUNTU_VERSION=24.04 \
        --build-arg CLANG_FORMAT_VERSION=$CLANG_FORMAT_VERSION clang-format-docker >/dev/null
    CLANG_FORMAT_INSTALLED_VERSION="$(docker run clang-format --version)"
    echo "$CLANG_FORMAT_VERSION => $CLANG_FORMAT_INSTALLED_VERSION"
done

19 => clang-format version 19.1.7
19.1.4 => clang-format version 19.1.4
19.1.7 => clang-format version 19.1.7
20 => clang-format version 20.1.8
20.1.5 => clang-format version 20.1.5
20.1.8 => clang-format version 20.1.8
21 => clang-format version 21.1.6
21.1.2 => clang-format version 21.1.2
21.1.6 => clang-format version 21.1.6

Resolves #199

[ptsouchlos]

You likely need to install pip and python first before being able to run pip. Not sure where you're seeing failures but this may be the cause.

[dkaszews]

Ubuntu tends to come with those preinstalled, unless docker has a different, very barebones image. I see errors running the test CI in my own repo:

Run "/home/runner/work/clang-format-action/clang-format-action/./../clang-format-action/check.sh" "21.1.6" "test/known_pass" "llvm" "capital" ""
Unable to find image 'ghcr.io/jidicula/clang-format:21.1.6' locally

This path suggests to me that we don't use the Dockerfile directly but instead from some repo, likely to cache it? So would need @jidicula to deploy those first (once tested to work), maybe under a new base name to avoid conflicts. Though having to manually deploy them makes me wonder how to handle so many versions, perhaps do the ones currently present on pip plus prefixes, i.e. 21.1.6 => 21.1.6, 21.1, 21.

[echoix]

It's because it isn't built yet, as the PR isn't merged. And for ghcr.io, you have to authenticate yourself before when downloading from the registry. In GItHub Actions, it is done automatically with the GITHUB_TOKEN

[dkaszews]

@echoix Doesn't that create sort of chicken-and-egg scenario? We can't run CI on new images before we merge the PR, but shouldn't merge the PR without checking the new scenarios? Or is it just an artifact of me running in a fork whose token does not match the image cache? In any way I'll try testing the new image locally to ensure it works before marking as ready for review.

[echoix]

For tests in CI like that, you could do it in two steps in the same job of a workflow file. First, build the image only tagging it with the commit hash or something generic, but don't push it!! Make sure the "load" option of docker/build-push-action is true for this.

Then, run the tests, using the commit hash or the tag used above.
Since the image was loaded to the local image store, it will be available.

Then, rerun the metadata action, with the full labels/tags wanted, and build the image again. Since nothing changed since the last build, the build cache will be used and the build will finish right away. On this build, you can specify to allow pushing to a registry.
That last step shouldn't be done in PRs. You don't want PRs for the general public to publish unreleased images under your image name.

[dkaszews]

You don't want PRs for the general public to publish unreleased images under your image name.

I'll just make this my takeaway, the other stuff is a bit over my head 😄 . I'll do local testing then ask for someone to take over the merging once ready.

[echoix]

You would want something similar to this example:

https://docs.docker.com/build/ci/github-actions/test-before-push/

But using two calls of https://github.com/docker/metadata-action. It will do the sanitation of tags, and help make sure the info is added. It's way simpler to use it rather than not use it.

[dkaszews]

@echoix I have tested the new dockerfile locally, everything works as expected. Would you care taking over this PR to do the image publishing part? I unfortunately don't have time to learn this, especially since as first-time contributor to this repo, I can't even run the CI without maintainer approving them every time.

[dkaszews]

Also, from what I understand, we need to rename all mentions of ghcr.io/jidicula/clang-format:"$CLANG_FORMAT_VERSION" to ghcr.io/jidicula/clang-format-pip:"$CLANG_FORMAT_VERSION", but then to pass CI we also need to manually build and deploy all those images, which sounds like something only @jidicula themselves can do.

[dkaszews]

If serious about minimizing image, might want to purge python3-pip as well.

[jidicula]

Sorry, but I won't be accepting this contribution because the pip build doesn't support pre-v6 versions of clang-format. To accommodate pip builds, I'd have to introduce a new code path, which increases complexity for supporting this project.

I'd prefer introducing a way to build clang-format from all available legacy versions of the LLVM project. To make them easier to run in GitHub Actions workflows, these would ideally be built as Docker images or statically linked binaries. Unfortunately, the toolchain for legacy versions of LLVM projects appears to be quite difficult to set up, and (with a cursory look) I haven't found a way to do this easily from within GitHub Actions.

[dkaszews]

Sorry, but I won't be accepting this contribution because the pip build doesn't support pre-v6 versions of clang-format.

That seems to only be 3 versions, do you have any telemetry on whether anyone actually uses them?

3: clang-format-3.9
4: clang-format-4.0
5: clang-format-5.0

I may be missing something, but it seems to me like people who rely on old versions can simply not upgrade to newer version of the action. Or keep the old apt path just for those 3, I don't quite get why a simple switch/case would increase complexity so much, since once the docker image is build, you don't care where it was built from.