Skip to content
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

Merged
merged 1 commit into from
Oct 19, 2024

Conversation

rdw-software
Copy link
Member

@rdw-software rdw-software commented Oct 16, 2024

TODO:

  • Cleanup and rebase
  • Simplify the workflow
  • Eliminate need to define env variable for clang-format
  • 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 EVO_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. (Stored in autoformat.env now)

I don't particularly like the increase in complexity, but formatters randomly messing up the code isn't helpful either.

@rdw-software rdw-software force-pushed the various-autoformat-improvements branch 3 times, most recently from c7c5294 to 6af35ab Compare October 18, 2024 17:15
@rdw-software rdw-software marked this pull request as ready for review October 18, 2024 17:17
@rdw-software rdw-software marked this pull request as draft October 18, 2024 17:21
@rdw-software rdw-software force-pushed the various-autoformat-improvements branch 2 times, most recently from 3edd640 to aa0a12f Compare October 19, 2024 09:11
@rdw-software
Copy link
Member Author

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 clang-format from GitHub releases. Alas, it is part of the bloated 1.5 GB tarball.

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:

  • Use alternative sources like chocolatey for Windows (still depends on third parties, may not have the right version)
  • Build clang-format from source and use the latest HEAD/some specified version (still takes ages and wastes bandwidth)
  • Fetch LLVM releases from GitHub, create a release for clang-format via GitHub Actions (in another repo), fetch that
  • Just give up and again have one single source of truth - the official LLVM APT repo might have the fastest install time?
  • Fetch it once and store in GitHub's cache, then use that for a given version (can save version in env file and hash it)
  • Accept having multiple versions and create alternative formatter configs for each (doesn't seem like a good idea)
  • Build or fetch manually a compatible version, host it externally/via artifacts, and use it (don't like the security headaches)

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?

@rdw-software rdw-software force-pushed the various-autoformat-improvements branch 4 times, most recently from bbe2d85 to 22cc24e Compare October 19, 2024 15:01
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.
@rdw-software rdw-software force-pushed the various-autoformat-improvements branch from 22cc24e to 3665816 Compare October 19, 2024 15:02
@rdw-software
Copy link
Member Author

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.

@rdw-software rdw-software marked this pull request as ready for review October 19, 2024 15:03
@rdw-software rdw-software changed the title Various autoformat improvements Streamline the autoformat workflow to enable version pinning for all platforms Oct 19, 2024
@rdw-software rdw-software merged commit a6ba58c into main Oct 19, 2024
16 checks passed
@rdw-software rdw-software deleted the various-autoformat-improvements branch October 20, 2024 06:54
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.

1 participant