-
Notifications
You must be signed in to change notification settings - Fork 1
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
Streamline the autoformat workflow to enable version pinning for all platforms #571
Conversation
c7c5294
to
6af35ab
Compare
3edd640
to
aa0a12f
Compare
Not happy with how much unneeded complexity is introduced by having three different sources for clang-format. The binary name is also quite inconsistent and renaming it as part of the workflow might work, but doesn't help with local development. Ideally, would be able to download just Stylua isn't a problem, thankfully, as their releases are much less chonky and can easily be fetched on the fly. The action doesn't work in MSYS (GitHub limitation?), which is slightly annoying and could be revisited. For now, focus on the LLVM setup... A few potential ideas:
I think the PPA is the best option for Linux and it does provide recent versions. Homebrew/MSYS are quite slow/large. Can't use the Linux releases anyway due to GLIBC issues; so there may not be a good way to get consistency and speed here? |
bbe2d85
to
22cc24e
Compare
After much iteration, this is the simplest solution I could come up with that fulfills all the requirements: * A single clang-format version is used for all platforms * The version can be pinned in one central location * Whichever version is currently used will be tracked by git * Doesn't consume resources excessively thanks to caching * No reliance on third parties, other than the LLVM organization * Uses the same script for both CI runs and local development * No excessive branching/other accidental complexity * Bonus: Approach can also be used to install clang-tidy (later) Downloading a huge release tarball just for the tiny formatter feels like overkill, but it's reasonably fast in the CI.
22cc24e
to
3665816
Compare
After much iteration, this is the simplest solution I could come up with that fulfills all the requirements:
Downloading a huge release tarball just for the tiny formatter feels like overkill, but it's reasonably fast in the CI. |
TODO:
Document the process for installing and running formatters(Tracked separately)The autoformat workflow currently doesn't pin the version of
clang-format
which is supposed to be used. That's problematic because both MSYS2 and Homebrew install the "latest" version, but their definition of "latest" can differ quite a bit.With this patch, the version is pinned for all three platforms and can manually be updated to track MSYS. It's still possibe for the versions to diverge, when MSYS deploys a new major release and the workflow hasn't been updated. In that case, there might be breaking changes which should lead the workflow to fail (can update version to re-sync). Otherwise it "just works".
As a side effect, the autoformat script will now fail unless the clang-format version is set via(Stored inEVO_AUTOFORMAT_VERSION
. This is slightly annoying because it makes running the script more cumbersome, but it can be exported automatically. If that's too inconvenient I'll have to think of a better solution later.autoformat.env
now)I don't particularly like the increase in complexity, but formatters randomly messing up the code isn't helpful either.