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

Force testing the latest allowed versions #667

Open
nomeata opened this issue Jul 9, 2023 · 12 comments
Open

Force testing the latest allowed versions #667

nomeata opened this issue Jul 9, 2023 · 12 comments

Comments

@nomeata
Copy link
Contributor

nomeata commented Jul 9, 2023

Together with @andreasabel, at MuniHac we are thinking about various ways to automate package dependency bumps, and we found a problem not present in all those lock-file-based ecosystem, but in a constraint-solver ecosystem like ours.

Consider package netrc with a dependency on bytestring < 0.12 and also parsec. A “dumb” dependency bumper might notice that bytestring-0.12.0.0 has been released and create a PR that extends the upper bound to bytestring < 0.13 (or the equivalent using >^=). Now the CI (nicely provided by haskell-ci) runs, is green, and the developer merges the PR.

All good? No!

Because there is no version of parsec supporting bytestring-0.12.0.0, the cabal solver silently picked bytestring-0.11.x.y, and the change in the PR was not tested at all.

I wonder how we can change the CI setup provided by haskell-ci to avoid this, and ensure that such a PR, if green, can be merged with good conscience.

Is this problem analysis so far correct?


One approach would be the following:

  • haskell-ci generates an “upper bound” matrix job.
  • It uses the latest supported GHC (because with older GHCs we may not expect to be able to use the latest version of all libraries).
  • This job gathers the upper bound of all dependencies (e.g. dep-pkg-1.2.3.4)
  • And builds with --allow-newer=*:dep-pkg --constraint=dep-pkg>=1.2.3.4.

Ideally, the logic behind the last two steps can be delegated to cabal or put into a separate tool that can also be used locally.

We can also refine the version select to pick the smallest patch version of the largest minor version allowed by the bounds.

Would such functionality be welcome in haskell-ci?

@nomeata
Copy link
Contributor Author

nomeata commented Jul 9, 2023

Ah, I guess this has some overlap with what I wrote in #610, which was deemed out of scope. I guess the question is no longer “should haskell-ci create PRs”, but “could haskell-ci be made so that such a PR is sensibly tested”.

@nomeata
Copy link
Contributor Author

nomeata commented Jul 9, 2023

Also related, but somewhat dual: #653

@phadej
Copy link
Collaborator

phadej commented Jul 9, 2023

Ideally, the logic behind the last two steps can be haskell/cabal#8387 or put into a separate tool that can also be used locally.

When there is such tool or cabal-install has a flag, feel free to ping on this issue.

I don't see this as a priority as before making PRs myself (for version bounds) I do cabal build -c 'dep-pkg^>=1.2 locally; nor I usually update bounds until dependencies have caught up (so I don't need --allow-newer that often).


The tool should take deprecated versions into account. And also skip versions with base <0 like bounds (which are rare but happen, when the latest release turned out to be completely bogus).

@nomeata
Copy link
Contributor Author

nomeata commented Jul 9, 2023

When there is such tool or cabal-install has a flag, feel free to ping on this issue.

cabal build -c 'dep-pkg^>=1.2'

Right; for those who don’t want a Github Action that does that for them, I am experimenting with https://github.com/nomeata/haskell-bounds-bump-action, as a stop-gap until the typical CI setup is able to check dependency bump PRs.

@nomeata
Copy link
Contributor Author

nomeata commented Jul 9, 2023

When there is such tool or cabal-install has a flag, feel free to ping on this issue.

I have created such a tool, called cabal-force-upper-bound. It’s just a start, but it's something. It takes a cabal file and calculates --constraint and optionally --allow-newer flags:

$ cabal-force-upper-bound *.cabal
"--constraint=Cabal-syntax^>=3.10" "--constraint=base^>=4.18" "--constraint=bytestring^>=0.11" "--constraint=cabal-plan^>=0.7" "--constraint=containers^>=0.6" "--constraint=optparse-applicative^>=0.18" "--constraint=pretty^>=1.1" "--constraint=text^>=2.0"
$ cabal-force-upper-bound --allow-newer *.cabal
"--constraint=Cabal-syntax^>=3.10" --allow-newer=Cabal-syntax "--constraint=base^>=4.18" --allow-newer=base "--constraint=bytestring^>=0.11" --allow-newer=bytestring "--constraint=cabal-plan^>=0.7" --allow-newer=cabal-plan "--constraint=containers^>=0.6" --allow-newer=containers "--constraint=optparse-applicative^>=0.18" --allow-newer=optparse-applicative "--constraint=pretty^>=1.1" --allow-newer=pretty "--constraint=text^>=2.0" --allow-newer=text

I used it in it's own repo like this:

    - name: Special handling for upper-bounds
      if: matrix.plan == 'upper-bounds'
      run: |
        echo -n "extra_flags=" >> $GITHUB_ENV
        cabal-force-upper-bound --allow-newer *.cabal >> $GITHUB_ENV

    - run: cabal build ${{ env.extra_flags }}

I can publish static linux binaries via the github releases, like I do for cabal-plan-bounds.

It’s probably not mature enough to be included in haskell-ci yet… but once it has seen some more testing, is this something you’d welcome as an option in haskell-ci?

@phadej
Copy link
Collaborator

phadej commented Jul 9, 2023

but once it has seen some more testing, is this something you’d welcome as an option in haskell-ci?

Maybe.

One suggestion: if there is an option to output a cabal.project fragments, so one could do cabal-force-upper-bounds foo.cabal >> cabal.project.local, that would help integration with haskell-ci. In haskell-ci we put all these kind of "static" setup into cabal.project.local for various reasons, mostly for convenience (less stuff to pass as cli args) and inspectability (it's easy to dump cabal.project.local and inspectability (we cat cabal.project.local so the ambient setup is easy to see in the job output)

@nomeata
Copy link
Contributor Author

nomeata commented Jul 9, 2023

Sure, will do!

@nomeata
Copy link
Contributor Author

nomeata commented Jul 10, 2023

(JFTR: done, and moved the tool to its own repo: https://github.com/nomeata/cabal-force-upper-bound)

@phadej
Copy link
Collaborator

phadej commented Jul 10, 2023

Another related issue I run with aeson-2.2 upgrades myself.

There the solver successfully picked the aeson-2.2.0.0. and jobs succeeded, but the patch was bogus, and broke builds with olders aeson-2.1. I see that problem as even worse.

The same would apply for anything else which is not boot library, bytestring is, so is somewhat pinned and its range is tested implicitly.

I think that these issues are similar enough, and it would be great if they can be solved by the same solution.

E.g. testing against all major versions in range (e.g. ^>=2.0, ^>=2.1, ^>=2.2 for >=2.0 && <2.3) has the same subproblem: You need a GHC which support whole range, and that is trickier, and maybe not even possible.
Also testing the range for all dependencies is often waste of resources, only interesting once in awhile.

That said, cabal-force-upper-bound could take a list of includes/excludes. E.g. excluding boot libraries (or just testing the latest bytestring) would allow to test bytestring-0.12 on all GHCs. And in fact, when bytestring-0.11 was released that was important as pattern synonym compat worked on then most recent GHC, but failed with older GHCs.

Now it feels that specifying constraint-sets gives more assurance, for the small price of not being automatic.

@nomeata
Copy link
Contributor Author

nomeata commented Jul 10, 2023

What you describe (the worry about CI not testing lower or middle versions) is what I am trying to address with https://github.com/nomeata/cabal-plan-bounds - works well together with constraint sets as part of your build matrix. (I wish it wasn't all so complicated.)

Why do you say you need one GHC covering all major versions? Isn't it (reasonably) sufficient to check each supported major version on some GHC?

@phadej
Copy link
Collaborator

phadej commented Jul 10, 2023

sufficient to check each supported major version on some GHC?

Unfortunately not in general. I consider it to be a bad practice to have conditional (on GHC version, or anything else) public API, but sometimes it is.

For example: transformers-0.6 has QuantifiedConstraints superclass (forall m. Monad m => Monad (t m)) => MonadTrans m on GHC >=8.6, but not on older; so you really should test on GHC-8.4 with transformers-0.6. Also QuantifiedConstraints had bugs (in GHC-9.0, haskell/mtl#138) so you'd perfectly should test against that as well.

TL;DR, in perfect world you are right, but the world is not perfect, software has bugs and people make mistakes.

@nomeata
Copy link
Contributor Author

nomeata commented Jul 10, 2023

Oh, indeed. By that reasoning, in general, one would have to check any combination of GHC and dependency version (in a way, GHC is just another dependency). So perfect is not attainable; the question is what works well for most packages. Maybe the assumption that dependencies are independent is usually ok.

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

No branches or pull requests

2 participants