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

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

Merged
merged 2 commits into from
Jul 18, 2023

Conversation

bgamari
Copy link
Contributor

@bgamari bgamari commented Jul 6, 2023

In #9064 we discovered that ghc-boot was added to the non-reinstallable package set due to #8501 despite there being no reason why it can't be built from its source distribution. This revealed the fact that there is quite some ambiguity around what constitutes a non-reinstallable package.

In #9064 we worked out a hopefully-more-clear picture of non-reinstallability. Here we update the commentary to describe this concept and update the lists to reflect the new definition.

Closes #9064.

In haskell#9064 we discovered that `ghc-boot` was added to the
non-reinstallable package set due to haskell#8051 despite there being no
reason why it can't be built from its source distribution. This
revealed the fact that there is quite some ambiguity around what
constitutes a non-reinstallable package.

In haskell#9064 we worked out a hopefully-more-clear picture of
non-reinstallability. Here we update the commentary to describe this
concept and update the lists to reflect the new definition.

Closes haskell#9064.
Copy link
Collaborator

@ulysses4ever ulysses4ever left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@geekosaur
Copy link
Collaborator

Looks like CI tests specifically test for the libraries being non-upgradeable?

Comment on lines 487 to 488
, mkPackageName "integer-gmp"
, mkPackageName "integer-simple"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both integer-* packages are reinstallable now afaik.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hsyl20 Is there an authoritative source about this? I can ship this change in the PR if you have a source :)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No authoritative source, sorry.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even if this were true, that's only for latest ghcs, and this package set needs to span all supported ghcs, so we can't do it regardless. If we do start doing it going forward we'll need a per-ghc-version refactor of how we manage the set.

@ulysses4ever
Copy link
Collaborator

@bgamari do you need help with this?

Copy link
Collaborator

@grayjay grayjay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for documenting this!

@Kleidukos Kleidukos self-assigned this Jul 14, 2023
@gbaz
Copy link
Collaborator

gbaz commented Jul 14, 2023

Is this blocked or can we merge?

@ulysses4ever
Copy link
Collaborator

@gbaz I think it has been unblocked since I applied the label, so I'm removing the label now.

If no one feels too strong about integer-simple, we can put the squash+merge label. This will still leave 2 days for contemplating, and the timer will reset if someone decides to add more comments.

Perhaps we need a ticket for making the list of non-upgradable packages dependent on the compiler version?

@gbaz gbaz added the squash+merge me Tell Mergify Bot to squash-merge label Jul 14, 2023
Comment on lines 1299 to 1300
, Right $ exAv "ghci" 2 []
, Right $ exAv "ghc-boot" 2 []
Copy link
Collaborator

@grayjay grayjay Jul 15, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The source versions of ghci and ghc-boot and packages B and C should also be removed.

@mergify mergify bot added the merge delay passed Applied (usually by Mergify) when PR approved and received no updates for 2 days label Jul 17, 2023
@ulysses4ever ulysses4ever merged commit 249374d into haskell:master Jul 18, 2023
@Kleidukos
Copy link
Member

@Mergifyio backport 3.10

@mergify
Copy link
Contributor

mergify bot commented Jul 20, 2023

backport 3.10

✅ Backports have been created

@Kleidukos
Copy link
Member

@grayjay damnit, sorry I missed your message :/

Kleidukos added a commit that referenced this pull request Jul 20, 2023
)

cabal-install: Fix non-reinstallable package set

In #9064 we discovered that `ghc-boot` was added to the
non-reinstallable package set due to #8051 despite there being no
reason why it can't be built from its source distribution. This
revealed the fact that there is quite some ambiguity around what
constitutes a non-reinstallable package.

In #9064 we worked out a hopefully-more-clear picture of
non-reinstallability. Here we update the commentary to describe this
concept and update the lists to reflect the new definition.

Closes #9064.

(cherry picked from commit 2e32a44)

# Conflicts:
#	cabal-install/src/Distribution/Client/Dependency.hs

* Fix tests

(cherry picked from commit 249374d)

# Conflicts:
#	cabal-install/tests/UnitTests/Distribution/Solver/Modular/Solver.hs

* Fix conflicts
mergify bot added a commit that referenced this pull request Jul 26, 2023
cabal-install: Fix non-reinstallable package set (backport #9092)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merge delay passed Applied (usually by Mergify) when PR approved and received no updates for 2 days squash+merge me Tell Mergify Bot to squash-merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ghc-boot is reinstallable
7 participants