CI: remove dependency on cabal-plan (#8891) #8893

ulysses4ever · 2023-04-03T16:21:11Z

Let's see what CI thinks...

fgaz · 2023-04-03T16:57:48Z

validate.sh

@@ -369,7 +368,6 @@ step_time_summary() {
 step_build() {
 print_header "build"
 timed $CABALNEWBUILD $TARGETS --dry-run || exit 1
-$CABALPLAN topo --builddir=$BUILDDIR || exit 1


If we still want a list of packages in the plan (though not topologically sorted) we can do it with a jq command:

jq -r '."install-plan" | map(."pkg-name" + "-" + ."pkg-version" + " " + ."component-name") | join("\n")' "$BUILDDIR/cache/plan.json"

iirc it's installed by default on github runners

TBH I don't see a big value in this list. If someone feels strongly about it, I could add this oneliner, perhaps. But I'm not a fan of this solution either: it looks rather fragile to my uniformed taste.

Ah, I guess, it was requested here: #8440 so there are users for it...

Since #8440 was opened by @andreasabel, paging him in... Andreas, maybe you have a preference between:

keeping cabal-plan; possibly, switching to binary distribution to save time and dependency management;

removing cabal-plan and print the list of dependencies to be built using the on-liner provided above by fgaz;

removing cabal-plan and keeping the list of dependencies to be built that is already produced by cabal build [--dry-run] actually; (see e.g. here)

I personally lean towards (3) as I fail to see much advantage of anything else. One difference between cabal build --dry-run and cabal-plan topo that I noticed today (and you can check it using the CI link above) is that the latter seems to print nearly twice as much as the former. I'm not sure why that is...

One difference between cabal build --dry-run and cabal-plan topo that I noticed today (and you can check it using the CI link above) is that the latter seems to print nearly twice as much as the former. I'm not sure why that is...

cabal build does not print cached stuff, it only prints what it's going to download/build

If we want the whole build plan we need 1. or 2. (or cabal freeze I guess)

Good to know, thank you!

I don't see much emotion one way or another, so I'm just leaving the current version (with your oneliner) for the review...

Where is the one-liner replacement for the cabal-plan printing? I cannot see it, I only see the deletion of the cabal-plan step.

@andreasabel oops: forgot to commit. Fixed.

ulysses4ever · 2023-04-03T17:02:30Z

Hmm, CI fails with rather mysterious solver error in the testsuite... https://github.com/haskell/cabal/actions/runs/4598996409/jobs/8123811204?pr=8893

fgaz · 2023-04-03T17:04:23Z

>>> -j2 --hide-successes --with-ghc=ghc

Probably cabal list-bin returned nothing in $(cabal list-bin some_exe) -j2 [...]

ulysses4ever · 2023-04-05T17:56:21Z

Probably cabal list-bin returned nothing in $(cabal list-bin some_exe) -j2 [...]

Well, yes, but the solver error I was referring to above happens before this. And because of this error cabal list-bin is not functional, as it requires a resolved plan...

ulysses4ever · 2023-04-05T23:04:53Z

All right, i can reproduce it under GHC 844 on Linux but not 944. CI also showed the failure only under 844/Linux and stopped in all other configuration. Still no idea why it happens. The actual build ends fine but then list-bin kicks the solver again for some mysterious reason, and that fails...

ulysses4ever · 2023-04-05T23:15:58Z

Got it: list-bin needed to have the same project file as was used for the build.

ulysses4ever · 2023-04-07T20:16:24Z

All right, please review!

andreasabel

Thanks for your work on this, @ulysses4ever!

The printing of the complete build plan was added for a reason, which can be read up upon in #8398 (comment): Folks relax an upper bound, which is not picked up by the solver due to another dependency, and then CI/build passes and people think relaxing the bound was fine.
So, the correct workflow is:

relax the bound of a dependency,
build,
check that the new version of the dependency was indeed picked up.

The inclusion of cabal-plan served to enable step 3 by inspecting logs of CI runs.
@andreabedini added this rather recently (Aug 2022) upon my request. We have to avoid the whack-a-mole-situation, that one developer adds a feature that the next developer removes, which is then missed and added again etc.

If you want to remove it, you need to explain how step 3 of the bump workflow can be facilitated. Ideally would be a check that in the latest GHC, all the maximum versions of the dependencies are indeed picked up. Atm, cabal does not offer such a check.
As pointed out by @fgaz, cabal build does not output the complete build plan, only what has to be built, which does not include existing builds that can be reused.

andreasabel · 2023-04-08T07:19:37Z

validate.sh

@@ -369,7 +368,6 @@ step_time_summary() {
 step_build() {
 print_header "build"
 timed $CABALNEWBUILD $TARGETS --dry-run || exit 1
-$CABALPLAN topo --builddir=$BUILDDIR || exit 1


Where is the one-liner replacement for the cabal-plan printing? I cannot see it, I only see the deletion of the cabal-plan step.

ulysses4ever · 2023-04-08T17:31:21Z

@andreasabel thanks for the review and I whole-heartedly agree. Let's leave the full list of dependencies then (getting it one way or another). I'm sad to say: I have no clue about the procedure for bumping dependencies you're describing. Perhaps, it'd be good to document it somewhere. Although, I'm not sure which place is the best for such information. Maybe we could have a little "CI Maintainer Guide" or something and reference it from validate.sh and validate.yml...

ulysses4ever · 2023-04-11T20:06:44Z

@fgaz @andreabedini any interest here for a review or an opinion?

.github/workflows/validate.yml

andreasabel

Thanks for the update.

fgaz

Thanks!

fgaz reviewed Apr 3, 2023

View reviewed changes

ulysses4ever force-pushed the ci-no-cabal-plan branch from 20d1d93 to 1767cda Compare April 5, 2023 23:14

ulysses4ever force-pushed the ci-no-cabal-plan branch 2 times, most recently from 3d1ca1d to 4432d40 Compare April 6, 2023 12:53

ulysses4ever marked this pull request as ready for review April 7, 2023 20:15

ulysses4ever added the attention: needs-review label Apr 7, 2023

andreasabel requested changes Apr 8, 2023

View reviewed changes

ulysses4ever force-pushed the ci-no-cabal-plan branch 2 times, most recently from 504a7a3 to bdebdbf Compare April 8, 2023 17:22

fgaz reviewed Apr 12, 2023

View reviewed changes

.github/workflows/validate.yml Show resolved Hide resolved

ulysses4ever force-pushed the ci-no-cabal-plan branch from bdebdbf to a80b696 Compare April 13, 2023 22:31

andreasabel requested a review from andreabedini April 14, 2023 07:44

andreasabel approved these changes Apr 14, 2023

View reviewed changes

ulysses4ever force-pushed the ci-no-cabal-plan branch from a80b696 to 14a1bff Compare April 14, 2023 08:37

fgaz approved these changes Apr 14, 2023

View reviewed changes

ulysses4ever force-pushed the ci-no-cabal-plan branch from 14a1bff to d2bedea Compare April 14, 2023 14:39

ulysses4ever added the merge me Tell Mergify Bot to merge label Apr 14, 2023

ffaf1 removed the attention: needs-review label Apr 15, 2023

mergify bot added the merge delay passed Applied (usually by Mergify) when PR approved and received no updates for 2 days label Apr 17, 2023

CI: remove dependency on cabal-plan (#8891)

66c4d80

ulysses4ever force-pushed the ci-no-cabal-plan branch from d2bedea to 66c4d80 Compare April 17, 2023 18:53

mergify bot merged commit dbfce81 into master Apr 17, 2023

fgaz deleted the ci-no-cabal-plan branch April 19, 2023 06:23

ulysses4ever mentioned this pull request Apr 19, 2023

CI RFC: remove dependency on cabal-plan #8891

Closed

CI: remove dependency on cabal-plan (#8891) #8893

CI: remove dependency on cabal-plan (#8891) #8893

Uh oh!

Conversation

ulysses4ever commented Apr 3, 2023

Uh oh!

fgaz Apr 3, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ulysses4ever Apr 3, 2023

Choose a reason for hiding this comment

Uh oh!

ulysses4ever Apr 3, 2023

Choose a reason for hiding this comment

Uh oh!

ulysses4ever Apr 6, 2023

Choose a reason for hiding this comment

Uh oh!

fgaz Apr 6, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ulysses4ever Apr 8, 2023

Choose a reason for hiding this comment

Uh oh!

andreasabel Apr 8, 2023

Choose a reason for hiding this comment

Uh oh!

ulysses4ever Apr 8, 2023

Choose a reason for hiding this comment

Uh oh!

ulysses4ever commented Apr 3, 2023

Uh oh!

fgaz commented Apr 3, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ulysses4ever commented Apr 5, 2023

Uh oh!

ulysses4ever commented Apr 5, 2023

Uh oh!

ulysses4ever commented Apr 5, 2023

Uh oh!

ulysses4ever commented Apr 7, 2023

Uh oh!

andreasabel left a comment

Choose a reason for hiding this comment

Uh oh!

andreasabel Apr 8, 2023

Choose a reason for hiding this comment

Uh oh!

ulysses4ever commented Apr 8, 2023

Uh oh!

ulysses4ever commented Apr 11, 2023

Uh oh!

Uh oh!

andreasabel left a comment

Choose a reason for hiding this comment

Uh oh!

fgaz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

fgaz Apr 3, 2023 •

edited

Loading

fgaz Apr 6, 2023 •

edited

Loading

fgaz commented Apr 3, 2023 •

edited

Loading