`ghc`, `ghc-boot`, `ghci` packages should be non-upgradable (#8489) #8501

Abastro · 2022-09-29T08:22:48Z

Please include the following checklist in your PR:

Patches conform to the coding conventions.
Any changes that could be relevant to users have been recorded in the changelog.
The documentation has been updated, if necessary.

Fixes #8489 (was not able to check if it is fixed myself, though)

Just adding a few in a non-upgradable package list, so this should be a simple change without major breakages.

I would love to add tests to check if those packages are correctly marked as non-upgradable, but I do not know how.
I would like to know how to add such tests.

Abastro · 2022-09-29T08:54:23Z

Uh, why does it timeout and why does it fail to install changelog-d?

gbaz · 2022-09-29T16:40:42Z

I think I was mistaken that nonInstallable doesn't need to be updated. I think it should be as well. I would also consider it a good refactor to export the list canonically from Cabal and then to have the solver import and use it, rather than needing to update two places independently and keep them in sync. (perhaps worth asking whoever left the comments why they didn't do so to begin with?)

Abastro · 2022-09-30T11:17:27Z

Agreed, such refactor would be great if feasible. How could I find who left the comments?

belka-ew · 2022-10-01T09:17:10Z

Agreed, such refactor would be great if feasible. How could I find who left the comments?

With git blame cabal-install/src/Distribution/Client/Dependency.hs you'll find the commit that introduced the change, it's 0eb4e51, @hvr.

gbaz · 2022-10-01T18:17:18Z

The duplication was introduced prior to the comments, which just documented it. I think this wasn't planned, just accidental complexity. So I vote try the consolidation refactor and see what happens.

Abastro · 2022-10-02T14:12:22Z

If there is going to be single source of truth for this, which module should it go? It seems cabal-install-solver is a common dependency, but I guess it could go into Cabal itself. I do not know much about how these libraries are split around.

Also, perhaps this is going outside my scope, as I was originally planned to do simple changes at 1 or 2 places.

gbaz · 2022-10-02T15:52:59Z

Sure -- if you don't want to do a bigger refactor, just solving the immediate issue is fine, and that refactor can be done at a later date.

grayjay · 2022-10-06T08:07:16Z

I would love to add tests to check if those packages are correctly marked as non-upgradable, but I do not know how.
I would like to know how to add such tests.

You could add a unit test to cabal-install/tests/UnitTests/Distribution/Solver/Modular/Solver.hs. I think you could write a test that uses a package repository with an installed version of ghc, a newer source version of ghc, and a package that depends on the newer version of ghc. The solver should fail to find a solution, and the test could also check the error message. Here are some related tests:

cabal/cabal-install/tests/UnitTests/Distribution/Solver/Modular/Solver.hs

Lines 151 to 156 in 52cc82d

    
           , testGroup "Base" [ 
        
                 runTest $ mkTest dbBase "Refuse to install base without --allow-boot-library-installs" ["base"] $ 
        
                             solverFailure (isInfixOf "only already installed instances can be used") 
        
               , runTest $ allowBootLibInstalls $ mkTest dbBase "Install base with --allow-boot-library-installs" ["base"] $ 
        
                             solverSuccess [("base", 1), ("ghc-prim", 1), ("integer-gmp", 1), ("integer-simple", 1)] 
        
               ]

ulysses4ever · 2022-10-18T21:42:20Z

@Abastro do you have any trouble with grayjay’s suggestion above? It should be reasonably straightforward, and we’re always happy to explain in more detail. Otherwise, great job here, and I’m looking forward for your patch to be merged!

Abastro · 2022-10-22T08:18:58Z

Sorry for long hiatus, I was busy with academy work. As soon as I get some time, I will add tests with grayjay's suggestions!

Abastro · 2022-10-23T09:16:21Z

Should I create test case for each of "ghc", "ghci", and "ghc-boot"? What error message should I check?
Also, which test group should this test belong to?

grayjay · 2022-10-24T05:42:45Z

Should I create test case for each of "ghc", "ghci", and "ghc-boot"?

Sure, that sounds fine.

What error message should I check?

It looks like the error will contain either "only already installed instances can be used" or "constraint from non-upgradeable
package requires installed instance". I'm not sure which error takes precedence.

Also, which test group should this test belong to?

You could either rename the "Base" test group to something more general and put it there or create a new one.

Abastro · 2022-10-24T11:50:55Z

@grayjay Currently I check directly against the whole error message:

solverFailure . isInfixOf $
                    "Could not resolve dependencies:\n"
               ++ "[__0] trying: A-1.0.0 (user goal)\n"
               ++ "[__1] next goal: ghc (dependency of A)\n"
               ++ "[__1] rejecting: ghc-1.0.0/installed-1 (conflict: A => ghc==2.0.0)\n"
               ++ "[__1] rejecting: ghc-2.0.0 (constraint from non-upgradeable package requires installed instance)\n"
               ++ "[__1] fail (backjumping, conflict set: A, ghc)\n"
               ++ "After searching the rest of the dependency tree exhaustively, "
                    ++ "these were the goals I've had most trouble fulfilling: A, ghc

Should I rather check for solverFailure . isInfixOf "constraint from non-upgradeable package requires installed instance"?

grayjay · 2022-10-25T07:42:04Z

@Abastro I think it would be better to only check that the error message contains "rejecting: ghc-2.0.0 (constraint from non-upgradeable package requires installed instance)", because that ensures that the correct type of error occurs and that it relates to the source version of ghc. The rest of the message isn't really relevant and makes the test more susceptible to failing after other unrelated changes to solver error messages.

Abastro · 2022-10-25T11:01:40Z

Fixed accordingly. By the way, what is the checklist about?

ulysses4ever · 2022-10-25T12:25:40Z

How about ghc-boot-th, ghc-compact, ghc-heap? That's what I see here: https://gitlab.haskell.org/ghc/ghc/-/tree/master/libraries

Abastro · 2022-10-25T13:44:02Z

Are these also boot libraries? Sorry, I do not know what "boot library" precisely means.

Kleidukos · 2022-10-25T14:09:05Z

@Abastro Hi! A list of boot libraries can be viewed for each GHC release in its release notes. For example: https://downloads.haskell.org/ghc/9.2.2/docs/html/users_guide/9.2.2-notes.html#included-libraries

ulysses4ever · 2022-10-25T15:29:28Z

I'm afraid "non-upgradable" is not exactly the same as "boot". E.g. bytestring is boot (meaning, it comes with GHC), but you can compile and use a version of it that's newer than what comes with your GHC. On the other hand, you can't use template-haskell or ghc-prim of a version different from what comes with your GHC.

Please, someone with more knowledge, sanity-check what I'm saying here. That's just the way I understand it but I don't know of any documentation that proves what I say here.

gbaz · 2022-10-25T15:48:19Z

The above is correct. I'm fine with the PR as is. I agree that there are probably a few other non-upgradable libraries that still aren't in the list as we've amended it, but I'm not sure how to determine what they are for certain, and would much rather that we just solve things we know to be problems!

ulysses4ever

Gershom's reasoning is convincing to me, and I have no other objections. Thank you for the contribution!

grayjay

I think we should continue to also update the list in Distribution.Solver.Modular.Solver until we try to remove it. It would be confusing if cabal handled different non-upgradable packages differently, if the two lists had subtly different behavior.

grayjay · 2022-10-26T05:36:49Z

cabal-install/tests/UnitTests/Distribution/Solver/Modular/Solver.hs

@@ -1123,6 +1129,22 @@ dbBase = [
    , Right $ exAv "integer-gmp" 1 []
    ]

+dbNonupgrade :: ExampleDb
+dbNonupgrade =
+  let base = exInst "base" 1 "base-1" []


I don't think that the base dependency is necessary. It would be simpler for each test to only involve one non-upgradable package.

I thought it would add some realistic complexity, but I see it is better to remove it.

Abastro · 2022-10-27T07:27:03Z

Updated accordingly. Should I also add ghc-boot-th, ghc-compact, ghc-heap in? I guess ghc-boot-th is template-haskell related, so that one might need to be in. I am not sure what the other two does..

EDIT: Also, should I keep rebasing onto the master? Or does Github PR machinary make my PR neatly contained in one commit anyway?

grayjay

Updated accordingly. Should I also add ghc-boot-th, ghc-compact, ghc-heap in? I guess ghc-boot-th is template-haskell related, so that one might need to be in. I am not sure what the other two does..

I'm not sure how to determine whether a GHC package is upgradable, so I'll let others review that part.

EDIT: Also, should I keep rebasing onto the master? Or does Github PR machinary make my PR neatly contained in one commit anyway?

If I understand correctly, Mergify can be made to squash a PR by applying a specific label.

grayjay · 2022-10-28T02:58:48Z

cabal-install/tests/UnitTests/Distribution/Solver/Modular/Solver.hs

+  , Right $ exAv "ghc" 2 [ExFix "base" 1]
+  , Right $ exAv "ghci" 2 [ExFix "base" 1]
+  , Right $ exAv "ghc-boot" 2 [ExFix "base" 1]


There are still three references to base here.

Hmm, strange that the test passed. Will fix!

grayjay · 2022-10-28T04:17:15Z

cabal-install-solver/src/Distribution/Solver/Modular/Solver.hs

@@ -162,7 +162,11 @@ solve sc cinfo idx pkgConfigDB userPrefs userConstraints userGoals =
    nonInstallable =
        L.map mkPackageName
             [ "base"
+             , "ghc-bignum"


Thanks for syncing ghc-bignum, too.

grayjay · 2022-11-07T03:47:41Z

@Abastro Can I apply the "squash+merge me" label, since this PR looks ready to merge, and the commits look like they should be combined?

Abastro · 2022-11-07T03:52:59Z

@grayjay Yep, of course. Thank you!

grayjay · 2022-11-07T05:09:12Z

Thanks for the fix!

Mikolaj · 2022-11-07T17:22:23Z

@Abastro: thanks for the PR. I've given you Triage rights so that you can apply labels yourself.

…8489) (haskell#8501) * ghc, ghc-boot, ghci should be non-upgradable * (PR number) * Checks for depending on GHC * Also tests ghc-boot, ghci * Simplify condition * Added "rejecting:" bits * Synced cabal-install-solver's list & Simplify testcase * Updated changelog * Fully remove base dependency in test dbNonUpgrade Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

Abastro force-pushed the ghc-non-upgrade branch from e836d93 to 959ed3d Compare October 23, 2022 10:02

Abastro force-pushed the ghc-non-upgrade branch from 959ed3d to ed80f71 Compare October 25, 2022 11:00

Abastro changed the title ~~WIP: ghc, ghc-boot, ghci packages should be non-upgradable (#8489)~~ ghc, ghc-boot, ghci packages should be non-upgradable (#8489) Oct 25, 2022

ulysses4ever approved these changes Oct 25, 2022

View reviewed changes

grayjay reviewed Oct 26, 2022

View reviewed changes

Abastro added 4 commits October 27, 2022 16:28

ghc, ghc-boot, ghci should be non-upgradable

4bc509e

(PR number)

93eb6bf

Checks for depending on GHC

4cd65ad

Also tests ghc-boot, ghci

9b751e5

Abastro added 4 commits October 27, 2022 16:28

Simplify condition

b836247

Added "rejecting:" bits

54a01fc

Synced cabal-install-solver's list & Simplify testcase

a65771f

Updated changelog

91e9137

Abastro force-pushed the ghc-non-upgrade branch from fd2e62f to 91e9137 Compare October 27, 2022 07:28

grayjay approved these changes Oct 28, 2022

View reviewed changes

Fully remove base dependency in test dbNonUpgrade

db82990

grayjay approved these changes Oct 30, 2022

View reviewed changes

grayjay added the squash+merge me Tell Mergify Bot to squash-merge label Nov 7, 2022

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

Merge branch 'master' into ghc-non-upgrade

4130984

Mikolaj mentioned this pull request Nov 9, 2022

Update contrib haskellari/strict-containers#10

Merged

mergify bot merged commit 09c90de into haskell:master Nov 9, 2022

grayjay mentioned this pull request Nov 9, 2022

Try to combine lists of non-installable and non-upgradable packages #8581

Closed

andreasabel mentioned this pull request Dec 2, 2022

hspec-core unconditionally depends on the ghc package which pins down versions of ghc boot libraries hspec/hspec#752

Closed

andreabedini mentioned this pull request Jun 26, 2023

ghc-boot is reinstallable #9064

Closed

gbaz mentioned this pull request Jul 6, 2023

cabal-install: Fix non-reinstallable package set #9092

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

`ghc`, `ghc-boot`, `ghci` packages should be non-upgradable (#8489) #8501

`ghc`, `ghc-boot`, `ghci` packages should be non-upgradable (#8489) #8501

Abastro commented Sep 29, 2022

Abastro commented Sep 29, 2022

gbaz commented Sep 29, 2022 •

edited

Loading

Abastro commented Sep 30, 2022

belka-ew commented Oct 1, 2022

gbaz commented Oct 1, 2022

Abastro commented Oct 2, 2022

gbaz commented Oct 2, 2022

grayjay commented Oct 6, 2022

ulysses4ever commented Oct 18, 2022

Abastro commented Oct 22, 2022

Abastro commented Oct 23, 2022 •

edited

Loading

grayjay commented Oct 24, 2022

Abastro commented Oct 24, 2022 •

edited

Loading

grayjay commented Oct 25, 2022

Abastro commented Oct 25, 2022

ulysses4ever commented Oct 25, 2022

Abastro commented Oct 25, 2022

Kleidukos commented Oct 25, 2022

ulysses4ever commented Oct 25, 2022 •

edited

Loading

gbaz commented Oct 25, 2022

ulysses4ever left a comment

grayjay left a comment

grayjay Oct 26, 2022

Abastro Oct 27, 2022

Abastro commented Oct 27, 2022 •

edited

Loading

grayjay left a comment

grayjay Oct 28, 2022

Abastro Oct 28, 2022

grayjay Oct 28, 2022

grayjay commented Nov 7, 2022

Abastro commented Nov 7, 2022

grayjay commented Nov 7, 2022

Mikolaj commented Nov 7, 2022

ghc, ghc-boot, ghci packages should be non-upgradable (#8489) #8501

ghc, ghc-boot, ghci packages should be non-upgradable (#8489) #8501

Conversation

Abastro commented Sep 29, 2022

Abastro commented Sep 29, 2022

gbaz commented Sep 29, 2022 • edited Loading

Abastro commented Sep 30, 2022

belka-ew commented Oct 1, 2022

gbaz commented Oct 1, 2022

Abastro commented Oct 2, 2022

gbaz commented Oct 2, 2022

grayjay commented Oct 6, 2022

ulysses4ever commented Oct 18, 2022

Abastro commented Oct 22, 2022

Abastro commented Oct 23, 2022 • edited Loading

grayjay commented Oct 24, 2022

Abastro commented Oct 24, 2022 • edited Loading

grayjay commented Oct 25, 2022

Abastro commented Oct 25, 2022

ulysses4ever commented Oct 25, 2022

Abastro commented Oct 25, 2022

Kleidukos commented Oct 25, 2022

ulysses4ever commented Oct 25, 2022 • edited Loading

gbaz commented Oct 25, 2022

ulysses4ever left a comment

Choose a reason for hiding this comment

grayjay left a comment

Choose a reason for hiding this comment

grayjay Oct 26, 2022

Choose a reason for hiding this comment

Abastro Oct 27, 2022

Choose a reason for hiding this comment

Abastro commented Oct 27, 2022 • edited Loading

grayjay left a comment

Choose a reason for hiding this comment

grayjay Oct 28, 2022

Choose a reason for hiding this comment

Abastro Oct 28, 2022

Choose a reason for hiding this comment

grayjay Oct 28, 2022

Choose a reason for hiding this comment

grayjay commented Nov 7, 2022

Abastro commented Nov 7, 2022

grayjay commented Nov 7, 2022

Mikolaj commented Nov 7, 2022

`ghc`, `ghc-boot`, `ghci` packages should be non-upgradable (#8489) #8501

`ghc`, `ghc-boot`, `ghci` packages should be non-upgradable (#8489) #8501

gbaz commented Sep 29, 2022 •

edited

Loading

Abastro commented Oct 23, 2022 •

edited

Loading

Abastro commented Oct 24, 2022 •

edited

Loading

ulysses4ever commented Oct 25, 2022 •

edited

Loading

Abastro commented Oct 27, 2022 •

edited

Loading