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

Allow building cabal-3.6 with GHC 9.2 #8025

Merged
merged 2 commits into from
Mar 3, 2022
Merged

Allow building cabal-3.6 with GHC 9.2 #8025

merged 2 commits into from
Mar 3, 2022

Conversation

Bodigrim
Copy link
Collaborator

@Bodigrim Bodigrim commented Mar 2, 2022

Otherwise GHC 9.2.2 cannot be bootstrapped by GHC 9.2.1.

@Mikolaj could we please have it merged and released soon as Cabal-3.6.3.0? Just a revision would not be enough, because GHC source tree refers to Cabal repo, not to Cabal package on Hackage.

@Mikolaj
Copy link
Member

Mikolaj commented Mar 2, 2022

Thanks for the PR. Why not use master? Did you compare these fixes to what we've just finished on master to make it work with GHC 9.2?

@jneira
Copy link
Member

jneira commented Mar 2, 2022

There are some risks in using a dev (3.7) not released version. I guess ghc prefers a more stable one.

@Bodigrim
Copy link
Collaborator Author

Bodigrim commented Mar 2, 2022

AFAIK it is usually undesirable to bump a major version of a boot package within the same GHC series. Patch releases of GHC are to provide more stability, not new features. So it is safer and simpler to release Cabal-3.6.3.0 (no need for cabal-install release), covering just newer dependencies and #7852.

@Mikolaj
Copy link
Member

Mikolaj commented Mar 2, 2022

Fair enough. So, did you compare your PR to master? Regarding releases, @emilypi is our release manager.

@Bodigrim
Copy link
Collaborator Author

Bodigrim commented Mar 2, 2022

Fair enough. So, did you compare your PR to master?

I guess it's more or less the same, I just ran cabal build -w ghc-9.2 and bumped prohibitive bounds. I cannot find a specific commit in master to compare against, could you possibly point me to it?

@Mikolaj
Copy link
Member

Mikolaj commented Mar 2, 2022

I cannot find a specific commit in master to compare against, could you possibly point me to it?

I'm afraid, there is no single commit, but a tangled mess from iteratively porting to 9.2, overhauling CI to work with 9.2, spotting errors, fixing the porting, etc. in https://github.com/haskell/cabal/pull/7952/commits

One thing I can't find in your PR but I remember we did is removing allow-newer from project files so that the upper bounds are not side-stepped.

There are some risks in using a dev (3.7) not released version. I guess ghc prefers a more stable one.

I'd be worried that using a version that's completely not tested (tests in 3.6 branch are not ported to GHC 9.2 and, understandably, don't run on 9.2) is an even greater risk. Especially that we have discovered some test failures specific to 9.2 (or 9.0?) and some of these are not even resolved yet. And porting the new CI harness to 3.6.3 may be hard without porting all the changed tests, some of which use new API or have changed expected results, etc. I'm not saying this is impossible, but it may be hard.

@Bodigrim
Copy link
Collaborator Author

Bodigrim commented Mar 2, 2022

One thing I can't find in your PR but I remember we did is removing allow-newer from project files so that the upper bounds are not side-stepped.

Done, allow-newer were just redundant.

I'd be worried that using a version that's completely not tested (tests in 3.6 branch are not ported to GHC 9.2 and, understandably, don't run on 9.2) is an even greater risk.

I'm afraid I don't quite follow what exactly is not ported: cabal test -w ghc-9.2 Cabal-tests succeeds.

@bgamari
Copy link
Contributor

bgamari commented Mar 2, 2022

Thanks for the PR. Why not use master? Did you compare these fixes to what we've just finished on master to make it work with GHC 9.2?

Indeed @Bodigrim hit the nail on the head. In general we do not perform major upgrades of core libraries in minor GHC releases to avoid breaking users. Upgrading between GHC releases in the same major series should be a painless task.

@Mikolaj
Copy link
Member

Mikolaj commented Mar 2, 2022

I'm afraid I don't quite follow what exactly is not ported: cabal test -w ghc-9.2 Cabal-tests succeeds.

I have no idea what this does. There are many testsuites and to fully exercise Cabal library you also need cabal-install tests and various integration tests. Also, does cabal test -w ghc-9.2 even test cabal-3.6 with GHC 9.2 or perhaps some older cabal version (e.g., the installed one) with some older GHC (e.g., the one the installed one is built with)? In case of some of our testsuites, such questions don't have the answer you'd expect.

@Mikolaj
Copy link
Member

Mikolaj commented Mar 2, 2022

However, given that branch 3.6 almost didn't change since 3.6.2 and that users of GHC 9.2.1 did not complain, I think it's fine to release 3.6.3 --- it won't break cabal-install 3.6.2, because the bounds on cabal-instal-3.6.2 deps will keep it from working on 9.2.

As a minimum precaution, I'd check if the failed tests we discovered recently when porting to 9.2 (and 9.0) are likely to affect branch 3.6 and if there's anything that needs to be done about it. @jneira: do you remember the ticket where we list the loose ends after the 9.2. CI port? Do you remember if we had to fix the Cabal library code due to the test failures discovered when porting? Is it likely that any such fix would be needed?

@Bodigrim
Copy link
Collaborator Author

Bodigrim commented Mar 2, 2022

Well, Cabal-3.6.2.0 as a boot library is already built with GHC 9.2 and included in bindists, so I do not really expect Cabal-3.6.3.0 to be a problem - the only essential change in #7852.

@Bodigrim
Copy link
Collaborator Author

Bodigrim commented Mar 3, 2022

I'm sorry for pushing on a short notice, but it would be really awesome to prepare Cabal-3.6.3.0 in time for GHC 9.2.2, which could happen as soon as this Friday. An actual Hackage release (or documentation/changelog tweaks) are not that urgent, we need just a commit with a version bump, which can be referred from GHC.

@jneira
Copy link
Member

jneira commented Mar 3, 2022

do you remember the ticket where we list the loose ends after the 9.2. CI port? Do you remember if we had to fix the Cabal library code due to the test failures discovered when porting? Is it likely that any such fix would be needed?

The pr which added 9.2 support for building Cabal and cabal-install was #7952 and the relevant commit 1d98686.

Then the 9.2.1 job was marked as experimental due to haddock related test errors in the cli (cabal-install) suite. It was recorded in the issue #7987

In that pr i dont see any change over Cabal the library, it already had base < 5 since long long time ago according to blame.
The 9.2.1 job in master is passing the lib-tests validate.sh which includes:

cabal/validate.sh

Lines 377 to 405 in 0077a69

step_lib_tests() {
print_header "Cabal: tests"
CMD="$($CABALPLANLISTBIN Cabal-tests:test:unit-tests) $TESTSUITEJOBS --hide-successes --with-ghc=$HC"
(cd Cabal-tests && timed $CMD) || exit 1
CMD="$($CABALPLANLISTBIN Cabal-tests:test:check-tests) $TESTSUITEJOBS --hide-successes"
(cd Cabal-tests && timed $CMD) || exit 1
CMD="$($CABALPLANLISTBIN Cabal-tests:test:parser-tests) $TESTSUITEJOBS --hide-successes"
(cd Cabal-tests && timed $CMD) || exit 1
CMD="$($CABALPLANLISTBIN Cabal-tests:test:rpmvercmp) $TESTSUITEJOBS --hide-successes"
(cd Cabal-tests && timed $CMD) || exit 1
CMD="$($CABALPLANLISTBIN Cabal-tests:test:no-thunks-test) $TESTSUITEJOBS --hide-successes"
(cd Cabal-tests && timed $CMD) || exit 1
CMD=$($CABALPLANLISTBIN Cabal-tests:test:hackage-tests)
(cd Cabal-tests && timed $CMD read-fields) || exit 1
if $HACKAGETESTSALL; then
(cd Cabal-tests && timed $CMD parsec) || exit 1
(cd Cabal-tests && timed $CMD roundtrip) || exit 1
else
(cd Cabal-tests && timed $CMD parsec d) || exit 1
(cd Cabal-tests && timed $CMD roundtrip k) || exit 1
fi
}

But we have to take in account the cabal-testsuite also does integration tests for Cabal the library.

The 3.6 branch does not have a job testing 9.2.1 so either we should tested it locally (as @Bodigrim already has done i guess) or try to add the job at least for Linux (or both things). The validate.sh test targeting Cabal the library is sh validate.sh -j 2 -w ghc-9.2.1 -v -s lib-tests.
Given the short deadline and what changes has been backported (as noted by @Bodigrim above) maybe some local tests will be the best option.

@Mikolaj
Copy link
Member

Mikolaj commented Mar 3, 2022

Then the 9.2.1 job was marked as experimental due to haddock related test errors in the cli (cabal-install) suite. It was recorded in the issue #7987

If that's all, this doesn't sound critical. In the worst case, in order to generate some (backpack?) haddocks, GHC devs or GHC users will need some extra fixes or workarounds (e.g., use cabal-install compiled with GHC 8.10.7). Not a good reason to delay GHC 9.2.2.

The 3.6 branch does not have a job testing 9.2.1 so either we should tested it locally (as @Bodigrim already has done i guess)

I agree, local tests should be enough. As you can see above, @Bodigrim doesn't seem to have touched https://github.com/haskell/cabal/tree/master/cabal-testsuite, so there's still work to do. I think we should just mimic master branch CI and note down any CI runs that were not performed or not successful.

Let's merge this PR and continue in #8026.

@Mikolaj Mikolaj merged commit c395be7 into haskell:3.6 Mar 3, 2022
@jneira
Copy link
Member

jneira commented Mar 3, 2022

Just in case i am adding a 9.2.1 job against 3.6 here: jneira#6
Not sure if it worths to add it upstream though

I agree, local tests should be enough

I suppose local tests has been done only in linux so there is some risk of unexpected bugs in windows/macos. I hope the risk is low enough to go forward

@Mikolaj
Copy link
Member

Mikolaj commented Mar 3, 2022

I suppose local tests has been done only in linux

Could you point me to a mention of that anywhere? E.g., has https://github.com/haskell/cabal/tree/master/cabal-testsuite been run by anybody?

@jneira
Copy link
Member

jneira commented Mar 3, 2022

See #8025 (comment)

I'm afraid I don't quite follow what exactly is not ported: cabal test -w ghc-9.2 Cabal-tests succeeds.

I am supposing @Bodigrim did Cabal specific local tests using 9.2.1 (no cabal-testsuite) and i guess only in linux (but not sure)

@blackgnezdo
Copy link
Contributor

Not sure why I couldn't get Cabal-v3.6.3.0 tag to bootstrap with ghc-9.2.2, but relaxing these two bounds helped me get a functional build of cabal-install:

--- cabal-install/cabal-install.cabal.orig
+++ cabal-install/cabal-install.cabal
@@ -278,7 +278,7 @@ executable cabal
         echo       >= 0.1.3    && < 0.2,
         edit-distance >= 0.2.2 && < 0.3,
         filepath   >= 1.4.0.0  && < 1.5,
-        hashable   >= 1.0      && < 1.4,
+        hashable   >= 1.0      && < 1.5,
         HTTP       >= 4000.1.5 && < 4000.4,
         mtl        >= 2.0      && < 2.3,
         network-uri >= 2.6.0.2 && < 2.7,
@@ -287,7 +287,7 @@ executable cabal
         random     >= 1.2      && < 1.3,
         stm        >= 2.0      && < 2.6,
         tar        >= 0.5.0.3  && < 0.6,
-        time       >= 1.5.0.1  && < 1.11,
+        time       >= 1.5.0.1  && < 1.12,
         transformers >= 0.4.2.0 && < 0.6,
         zlib       >= 0.5.3    && < 0.7,
         hackage-security >= 0.6.0.1 && < 0.7,

@Bodigrim
Copy link
Collaborator Author

@blackgnezdo this PR relaxed bounds for Cabal only, not for cabal-install.

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

Successfully merging this pull request may close these issues.

5 participants