Move code style checks in CI to be part of the main build job.

[hlinnaka]

To keep the coverage on macOS, we now run the same build-neon job on macOS too, in a limited fashion.

Only run the codestyle checks in debug mode. Seems pointless to repeat them in release mode, and furthermore, it's nice to continue with the testing even when there are cosmetic issues. (I'm not sure if github will actually continue with the release-mode tests if the debug-mode build fails, but we'll find out soon)

[hlinnaka]

This is another iteration of https://github.com/neondatabase/neon/pull/2096. I'll respond to @SomeoneToIgnore 's comments here

[hlinnaka]

Would header building be able to check things like https://github.com/neondatabase/postgres/pull/138?

If it's not, let's do the whole build since there's no other linting for such erros. Otherwise, we have to change the Cache postgres build and add -01 or whatever prefix to invalidate the old, bigger one.

With this PR, we make a full Postgres build on macOS too.

[hlinnaka]

One reason to put style checks into a separate workflow was the desire to run the e2e tests despite style errors.

Hmm, I see. I'm only running the formatting checks as part of the debug build now, so the release build could still go ahead. At least in principle; I'm not sure if github will continue with the subsequent jobs that depend on the build-neon job, if the build-neon job fails in debug build but passes in release build. It would be nice if it did, and even nicer if the python tests would start running as soon as the build-neon (release) job finishes, even if build-neon (debug) is still running.

Also, if we keep it this way, the target/ directory will grow even bigger, since cargo clippy results are now stored there. I think they are already in that cache, by the way, since the Rust cache key is not changes and this PR's action was run.

True. I don't think cargo clippy adds very much bloat to the cache though. I think we can live with that.

[SomeoneToIgnore]

With this PR, we make a full Postgres build on macOS too.

That's great, also nice that we share the same "stricter compiler" flags. Yet, we also run our Python tests on mac, if I understand the changes right? I think there's a risk that some releases will catch up mac stuff for longer than needed.

formatting checks as part of the debug build now, so the release build could still go ahead

In GHActions and our matrix runs, we build debug and release configurations in parallel, waiting for both configurations to finish before taking another workflow step. Also, debug builds are usually slower, so I would add the style checks on release builds instead 🙂 Yet for cargo fmt and python checks, it does not matter much, their run is very fast.

I also see that mac pg and rust builds will be run in the same "batch", and they are slower than Linux builds, same for tests, so not sure it's acceptable.

---

I liked somebody's idea to split off mac build (I think, that's how I also tried to make it initially): unfortunately, there's no statistics in GHActions I'm aware of, but many main CI codestyle runs contain long mac builds:

We could run the mac builds in a separate workflow, and run the rest of the style checks in the "main" workflow:

• run python style checks as we do in this PR
• run cargo fmt and cargo clippy on release builds release target dirs are presumably containing lesser binaries, so adding more clippy could be ok.

I think, we could try do mv target target_bak before running clippy and see if that's better than mixing its artifacts in: one of the extra concerns could be rare incremental "compilation" panics. Usually, either of the commands, cargo build or cargo clippy but not both, were affected by this and with the "mixed" approach we'll have to invalidate the build cache for both tools at the same time.

[SomeoneToIgnore]

Superseded by https://github.com/neondatabase/neon/pull/3049