Reuse build setup using a dedicated github action

[jneira]

• The cache handling and several workarounds were duplicated in 5 workflows, making changes cumbersome and error prone
• This pr leverages composite actions to extract out the common steps of setting up the build:
  • https://docs.github.com/en/actions/creating-actions/creating-a-composite-action
• I've tried this in my repo and the workflows looks good for now: https://github.com/jneira/haskell-language-server/actions?query=branch%3Areuse-ci+

[Anton-Latukha]

Is this allowed 8)?

I didn't knew yaml composite exists.

Would review in near time.

[Anton-Latukha]

Checking escaping in-build:
https://github.com/haskell/haskell-language-server/runs/4707587679?check_suite_focus=true#step:3:60:

removed 'cabal.project'
'cabal-ghc901.project' -> 'cabal.project'

Great, works as intended.

[Anton-Latukha]

For me, in the GitHub Action design is useful to note several things.

Great.

😎 🍾 👏 😃 👏 👏

[Anton-Latukha]

Looked at the number of builds. What I saw looks normal ...

merging ...

[Anton-Latukha]

Build was/is expecting:

Restarted:

But maybe the backend requirement names not aligned?

[Anton-Latukha]

It may be due to 3.6 cabal setting removal from the matrix & so from the name:

All other workflows were not requiring this because they either have post_job & so merge checks depend on post_job and other workflows do not have matrix. (looking at this, this seems like a nice thing to abstract, flags probably needs to gain the post_job & CircleCI need also (because right now any GHC version change requires to readjust CircleCI merge checks) either to return 1 status or have a post_job that returns merge status product).

[Anton-Latukha]

This is actually what I arrived to with DHall, DHall solution essentially required to form a number of modules to reimport. But there is a whole hussle to convert it perfectly into GitHub yaml design caveats ( - '[ "ele.**", file.md, ... ]' & stuff).

This way of solving under current & foreseeable future conditions is much neater.

[michaelpj]

Deduplication looks amazing, just a few comments as I happened to be reading this file for the first time.

[michaelpj]

Hm, is there maybe a way to set -e globally so we can just fail if anything fails?

[jneira]

afaik the shell has -e by default but we want the opposite thing: it should not fail so cache will fallback to only index state

[michaelpj]

But why? We can fail to produce the freeze file? Probably could do with a note explaining that if it's expected...

[jneira]

good question, we had some issues producing it for bleeding-edge branches like ghc-9.2 but i think it should work now. However i am not sure if we have to be stricter and block ci. Maybe we could try the strict way and make it optional if we see there are more problems hard to fix quickly

[michaelpj]

SGTM.

Also, further dumb question: why do we need the freeze file? Given that we pin the index state, won't cabal produce a deterministic build plan anyway?

[jneira]

i ve made it strict and trying the ghc-9.2 branch here: jneira#59

[jneira]

It seems it is working: https://github.com/jneira/haskell-language-server/runs/4714665789?check_suite_focus=true

[jneira]

SGTM.

Also, further dumb question: why do we need the freeze file? Given that we pin the index state, won't cabal produce a deterministic build plan anyway?

pr's can change dependencies removing, adding or updating bounds in any of the many .cabal files. This way we can have a dedicated cache taking in account those changes. However we could hash all .cabal files. But any change in any cabalfile would miss the cache.

Otoh you can change things in the cabal.project (like constraints, flags, ghc option) which might affect dependencies. cabal freeze should record the relevant changes affecting dependencies. That way cosmetic or other changes not affecting dep resolution in cabal.project does not make miss the cache.

The unique thing we are not tracking is cabal global config, but it does not change frequently (although it did recently for windows duw to github vm inages change)

[Anton-Latukha]

Freeze file gets into account all projects & infers 1 package version set to rule them all.

Here freeze served the caching key detection purposes - so I was thinking in that adenda & framing, so to have cache fault-tolerant. If any of optional steps fail - the cache does not gets saved (but further I would show that that is a minor thing to having other perks).

There can be mutually exclusive dependencies, especially between plugins. Currently, most of them are enabled by default, so their package constraints should be solvable. But with the growing number of plugins - keeping all plugins package constraints up to date & mutually resolvable would be a challenge.

freeze in itself is a test, that all regular build targets in all projects can infer 1 set of versions of dependencies, so that is a dependency requirement test in itself & it probably should be made & placed so accordingly. Maybe now thinking on having CI to check with freeze for merge would seem viable.

And the additional thought - it seems a good strategy to check contributor PRs for consistency in update/dependency management. It would enforce also to have some backward compatibility of versions between projects, wich is both a good thing and may be a security concern, but security concern is when mainteiners can come into play. So for example, having the consistency means restricting from old versions in one place, would mean it can be at once removed in all other projects.

So indeed, having a freeze requirement seems as a feature.

[Anton-Latukha]

@michaelpj

Also, further dumb question: why do we need the freeze file? Given that we pin the index state, won't cabal produce a deterministic build plan anyway?

It is not a dumb question. It is a Cabal innerworkings, my understanding from doing my part is:

index-state is a the Hackage deps API catalog state/identifier (even more, Hackage has discrete moments when it has snapshot state available, so cabal takes index-state set - & retrieves info on the nearest Hackage snapshot in the past to it, rolls back into the past to the existing Hackage snapshot state). Then we notice that Hackage ships a lot of package versions at every moment, so index-state determines only the Hackage state & we can not say that it picks versions, it is just a Hackage time machine to the past. The catalog is a tree structure package/{set-of-all-versions} (alike in ~/.cabal/packages/, & inside index*.tar* metadata that gets downloaded form Hackage itself). So under one index-state two build targets are free to choose two mutually exclusive versions.
freeze requires all (afaik build targets, I need to look) package descriptions to be solvable as a whole.

[jneira]

All other workflows were not requiring this because they either have post_job & so merge checks depend on post_job and other workflows do not have matrix. (looking at this, this seems like a nice thing to abstract, flags probably needs to gain the post_job & CircleCI need also (because right now any GHC version change requires to readjust CircleCI merge checks) either to return 1 status or have a post_job that returns merge status product).

For now, i've changed the branch protection rules