`clang-format`: new Dockerfile & update version (fix alignment regression) about everest-ci HOT 7 CLOSED

I'm not sure if we really need to fork every dependency we have. Who should maintain all of that? The run-clang-format.py we could just put next to the dockerfile, as it is a build input. About the clang-format-17_linux-amd64 I'm not sure, personally I don't like to have binaries checked in. I would assume that "release" link to be stable. Furthermore the built docker image should be there forever.
It's again a fundamental question. Do we wan't to keep our build inputs local everywhere? I would be against it, because currently our docker images aren't reproducible anyway (things like apt get update or apk update destroy that) and it is just a lot of overhead to maintain these things and consequently we would need to do so also for all other repos (do we track system dependencies like boost, or libpcap etc...)? (Btw. the code formatting shouldn't be significant for reproducability).
If we really want to fork "all" external dependencies, than I would propose we should first come up with a stratagy on how to name and maintain them, because otherwise our EVerest repository will become a mess.

from everest-ci.

I would prefer not relying on external sources for run-clang-format script and clang-format itself

from everest-ci.

I agree with @andistorm here, let's try to keep the binary and script local. But nice find, like we suspected

from everest-ci.

The current clang-format-16 binary is uploaded as a release here: https://github.com/EVerest/run-clang-format/releases
So replacing that with the static one should do the trick.

from everest-ci.

In this specific case I disagree that forking wasn't a good idea, and I think some of the things you mentioned are not quite correct

1. If you have a look at https://github.com/EVerest/run-clang-format/releases the binary is purposefully not checked in, just uploaded as a "release" on github, we should do this with the static binary as well.
2. I forked the run-clang-format script in anticipation that we might want to change something in this script for our ci, the upstream repo hasn't been updated in over 3 years, so maintenance is basically on us anyway if we want to fix anything.
3. I don't want to break the current docker image that we told people to use for months now, without a good reason. And in that case we must update the binary, otherwise it will stay on the broken v16 release forever

So in this case I think there have been very valid reasons to do this in this way, but I totally agree that I should have documented it somewhere. I guess this is what happens when you hack away on these sort of things in a boiling tent on the CCCamp

from everest-ci.

We've had a meeting on Dec, 8th, to discuss this:

Topics

• Security of dependencies in CI
  • At the moment no specific checksum checks etc.
• run-clang-format was forked as it is unmaintained and had open issues
• CI to be runnable locally. Therefore GitHub action runner-hosts as Docker image.

Action Items

• everest-utils/docker to be moved to everest-ci
• reusable workflows
  • Actions in everest-ci/github-actions
  • composite actions
• GitHub Actions workflow files with few magic (as possible)
• run-clang-format.py next to Dockerfile
  • archive or remove run-clang-format repo
• clang-format binary as release artifact
  • automatically downloaded via CI from llvm-project and then cached on our side
  • clang-format version 15.0.6 because Ubuntu 22.04 LTS offers clang-format-15 as highest version

Follow-Up

Since then we had another meeting which got into detail of some of the topics and overhauling some ideas. Meeting notes might be added soon.

from everest-ci.

Fixed by downgrading clang-format to 15.0.6

from everest-ci.