Post remove ci templates

[jneira]

Adressing some issues after the new gha workflows in #7952

@andreabedini i started to work on this before your comments in #7592. Let me know if you want to continue the great work done there with your own pr. No problem at all with that

---

Please include the following checklist in your PR:

• Patches conform to the coding conventions.

[andreabedini]

Thanks for pinging me @jneira. My problem with testing using old GHCs is that it is very arbitrary what we are going to test.

• Why only on Linux?
• After compiling cabal with what version of GHC? (the one that GHCup marks as recommended? 8.10.7)

There's a proverbial combinatorial explosion of possible configurations to tests. I honestly do not know what we should tests.

One suggestion I could give now is that perhaps you can consider making validate-old-ghcs a separate workflow, so we can unify the main workflows (Linux / MacOS / Windows) into something simple.

[jneira]

Thanks for pinging me @jneira. My problem with testing using old GHCs is that it is very arbitrary what we are going to test.

* Why only on Linux?

I guess it already was only tested in linux to save ci resources and simplify the scripts. It is not comprehensive but it worked well enough until now afaik so i only keep the existing behaviour

* After compiling cabal with what version of GHC? (the one that GHCup marks as recommended? 8.10.7)

Again, i am keeping the existing behaviour, All builds > 8.0 should work so i hope the chosen main ghc is somewhat arbitrary. We could try 8.10.7 and see how is going (but i prefer start with the existing one to not mix things)

There's a proverbial combinatorial explosion of possible configurations to tests. I honestly do not know what we should tests.

Hmm the same combination as before, adding the recently removed ones < 8.0 could be a good start imo

One suggestion I could give now is that perhaps you can consider making validate-old-ghcs a separate workflow, so we can unify the main workflows (Linux / MacOS / Windows) into something simple.

I considerated it but as the original workflow made the old-ghcs job require the main one so i am keeping that here.
You can unify workflows adding macos to the matrix of the validate job. Afaics the old-ghcs does not get in the way to do it.

[jneira]

I could not resist to add

# See: https://docs.github.com/en/actions/reference/workflow-syntax-for-github-actions#concurrency.
concurrency:
  group: ${{ github.head_ref }}-${{ github.workflow }}
  cancel-in-progress: true

to all workflows, to cancel ongoing runs on new commits of the same workflow, another way to save resources

The unique caveat i can think of is it does not allow to abuse ci and check different implementations concurrently, but i think upstream ci should not be abused that way. You always can open prs in your own fork for those kind of checks (or test locally of course)

[andreabedini]

to all workflows, to cancel ongoing runs on new commits of the same workflow, another way to save resources

Fair enough. I would prefer to see all the issues at once that having to discover them one by one but I don't have a strong opinion here.

Edit: I would not say "abuse", it's the CI's job to run tests!

[jneira]

Edit: I would not say "abuse", it's the CI's job to run tests!

yeah and it continues doing it, but only for the last commit of the pr/branch if several of them are ran concurrently 😉

[andreabedini]

I just had a little thought, on GHC 8.8.4 we pass --solver-benchmarks but I belive we should not.

1. it looks like it's compiled but not executed anywhere (I cannot find it looking here, surely I must be missing it).

2. if it's a benchmark, its result might not fail/pass, and in this case I would prefer not having it as part of the CI. We could but we are trying to save on resources.

3. given the name, I assume it's benchmarking cabal-install-solver, in that case we could split it into a separate workflow and only run it when paths under cabal-install-solver/ change.

my 2c.

[phadej]

if it's a benchmark, its result might not fail/pass, and in this case I would prefer not having it as part of the CI. We could but we are trying to save on resources.

It runs solver for one package 5 times (look what validate.sh does). I.e. it definitely doesn't waste resources (compared to time spend figuring out why they don't build / run).

Don't assume, check!

[phadej]

only run it when paths under cabal-install-solver/ change.

Bad idea. it's like testing cabal-install only when code in cabal-install changes.

[grayjay]

I just had a little thought, on GHC 8.8.4 we pass --solver-benchmarks but I belive we should not.

Please don't remove the run of solver-benchmarks from CI. The benchmark's unit tests only cover limited functionality, so it is useful to verify that the whole benchmark can run for a single package. The benchmark runs the cabal executable, so it is testing more than just the solver.

it looks like it's compiled but not executed anywhere (I cannot find it looking here, surely I must be missing it).

I'm not very familiar with the current CI setup, so I don't know whether there is currently an issue preventing the benchmark from being tested.

[andreabedini]

if it's a benchmark, its result might not fail/pass, and in this case I would prefer not having it as part of the CI. We could but we are trying to save on resources.

It runs solver for one package 5 times (look what validate.sh does). I.e. it definitely doesn't waste resources (compared to time spend figuring out why they don't build / run).

Don't assume, check!

:-( I didn't assume, I did check the logs and I could not find anything related to the benchmark.

only run it when paths under cabal-install-solver/ change.

Bad idea. it's like testing cabal-install only when code in cabal-install changes.

And what is the argument against that?

@phadej I admire your work. I am just trying to keep things simple here, for everybody's benefit.

[andreabedini]

Please don't remove the run of solver-benchmarks from CI. The benchmark's unit tests only cover limited functionality, so it is useful to verify that the whole benchmark can run for a single package. The benchmark runs the cabal executable, so it is testing more than just the solver.

Nobody needs to worry, I have no intention to remove it.

[Mikolaj]

Bad idea. it's like testing cabal-install only when code in cabal-install changes.

And what is the argument against that?

The argument is that changes in Cabal library affect how cabal-install works, so cabal-install should be tested also after any Cabal changes. Not the other way around, but I think most of our tests are cabal-install tests.

[jneira]

I am continuing the work in my fork to not take too much ci resources here: jneira#3

There i've changed the job names to Validate ubuntu-latest ghc-${{ matrix.ghc }} and Validate macos-latest ghc-${{ matrix.ghc }} to make it easier the merge between both and not having to update branch protection rules again.
Unfortunately you cant use runner.os (one of Linux, macOs, Windows) in the job name field 🤷

[jneira]

Ok, i think i made the old-ghcs job work (see https://github.com/jneira/cabal/runs/5285931614) restoring in practice the old job.

• I only have converted it in a matrix removing the testing for the main ghc version already done in the validate step
• It builds 8.8.4 several times, the solution could be reuse the artifact generated in the validate job for that ghc. It supposes changes in the workflow (upload artifacts) and maybe in the validate script, to make it use such artifact

Thing i tried (reverse engineering @phadej's work on the way):

• Use ubuntu-lastest with no container and installing old ghcs using setup-haskell action (https://github.com/jneira/cabal/actions/runs/1874957564): it started to install , 7.10.3, 7.4.2, 7.2.2 and 7.0.4 but no 7.8.4 and 7.6.3 🤷
• Use ubuntu-18.04 with no container and installing old ghcs using setup-haskell action (https://github.com/jneira/cabal/runs/5272663327?check_suite_focus=true). It installed all old ghcs but the validate script throwed cabal-plan: /lib/x86_64-linux-gnu/libm.so.6: version ```GLIBC_2.29' not found (required by cabal-plan) so i deduced it needs the -dyn ghc flavours (or an older ubuntu? not sure)
• Use ubuntu-18.04 with no container and installing old ghcs (dyn flavour) using setup-haskell action (https://github.com/jneira/cabal/runs/5272663216): it seemed the dyn flavour is not available for ubuntu-18.04 so it needs the container using xenial (16.04)
• Using xenial based container (https://github.com/jneira/cabal/runs/5284347655): The actions/checkout has problems with container using old versions of git so it has to be done manually
• So it ends with the original setup working: https://github.com/jneira/cabal/actions/runs/1880540596

So we could use ubuntu-18.04 if the validate script would support not dyn ghc flavours. I guess it is not possible for old ghc versions.

@phadej i would appreciate if you could confirm those guesses from a linux beginner

[jneira]

Again, i am keeping the existing behaviour, All builds > 8.0 should work so i hope the chosen main ghc is somewhat arbitrary. We could try 8.10.7 and see how is going (but i prefer start with the existing one to not mix things)

will try with ghc-8.10.7

[jneira]

Again, i am keeping the existing behaviour, All builds > 8.0 should work so i hope the chosen main ghc is somewhat arbitrary. We could try 8.10.7 and see how is going (but i prefer start with the existing one to not mix things)

will try with ghc-8.10.7

it failed cause it has to use ghcup and haskell-setup action needs sudo in scope to install ghc with ghcup.
Otoh the xenial container we are using has not sudo in scope: https://github.com/jneira/cabal/runs/5287480736?check_suite_focus=true

🤦

[jneira]

Ok i've restored linux and macos checks and will add old-ghc ones as soon the new jobs are picked up by github ui
So the rest of prs need #8000 to be merged honouring ci

Using post-jobs make the protection rules more maintainable, you can use them as a resume of all matrix job and allows to have one check per workflow

[jneira]

I know it is good to make us eager to fix failing jobs, but having the red cross everywhere (in commit, pr list, etc) makes me 😿

[andreabedini]

LGTM. Thanks for adding all the comments! I am a bit sad we do this on Linux only since I was going to merge the platforms but alas, it will do for now.

[jneira]

LGTM. Thanks for adding all the comments! I am a bit sad we do this on Linux only since I was going to merge the platforms but alas, it will do for now.

thanks! @Mikolaj @fgaz this need one more approval to be merged and unblock the rest of prs

[Mikolaj]

Thank you!

[Mikolaj]

@Mergifyio rebase

[mergify]

rebase

✅ Branch has been successfully rebased

[fgaz]

why head_ref and not ref?

[jneira]

Good point head_ref is only defined for pull requests and workflows are triggered for direct commits too.
The consequence is that, for consecutive direct and merge commits in master (and 3.6, 3.4 etc), they will be cancelled but the last one.
So we will lose the status of those intermmediate commits.
There is a small probability of a green pr breaking master cause we are not enforcing the pr is up to date with master before merging it. Also there is a even small probability of merge another pr just after the offending one and we will not know the real cause is the intemmediate and no the final one.
Will change asap