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

Bump dependencies for GHC 9.8 #9372

Merged
merged 2 commits into from
Nov 2, 2023
Merged

Bump dependencies for GHC 9.8 #9372

merged 2 commits into from
Nov 2, 2023

Conversation

andreasabel
Copy link
Member

@andreasabel andreasabel commented Oct 25, 2023

Bump to latest dependencies for GHC 9.8.1, closes #9368.

Update: I added a allow-newer for rere into cabal.project now.
TODO:

  • Temporarily added cabal.project.local for outdated deps, needs to be removed once deps have caught up to GHC 9.8.

Upstream issues:

@ulysses4ever
Copy link
Collaborator

There once was #9330 but it's fine to proceed here…

@andreasabel
Copy link
Member Author

There once was #9330 but it's fine to proceed here…

Oh, apologies @ulysses4ever, I didn't notice your PR. I now tagged it with ghc-9.8 to be more outstanding.
Anyhow, it does much more than my PR which is just bumps, so we should come back to yours after.

I am frankly quite surprised this PR here went through CI like a charm. The good work that has been done on CI is paying off here.

We'll have to wait for the upstream issues though before merging...

@andreasabel andreasabel changed the title ghc 9.8 Bump dependencies for GHC 9.8 Oct 26, 2023
@ulysses4ever
Copy link
Collaborator

No problem at all! Yeah, upstream is a little annoying, and in some cases it can take quite some time, which is why I think for the latest GHC it's fine to have a couple of allow-newer's and audit them every time we update for a new GHC release. But I don't have a firm opinion on this matter. I just think it's nice to show people what to do to build the project with the latest GHC.

@andreasabel
Copy link
Member Author

andreasabel commented Oct 26, 2023

Let's ask some big bosses for their opinion on allow-newer: @Mikolaj & (can't remember the handle of Hecate now).
I must confess a git add -f cabal.project.local, overriding the .gitignore... 😊

@geekosaur
Copy link
Collaborator

@Kleidukos

And cabal.project.local might indeed be a problem for people with cabal checkouts, since it's intended for local changes to the project.

@Mikolaj
Copy link
Member

Mikolaj commented Oct 27, 2023

IMHO, temporary allow-newer is fine. However, it would be better to place it not in cabal.project.local. I think, even cabal.project.9.8.1 is better and a comment somewhere that recommends cabal --project-file=cabal.project.9.8.1 (or whatever the syntax really is).

@andreasabel
Copy link
Member Author

@Mikolaj wrote:

cabal.project.9.8.1 is better and a comment somewhere that recommends cabal --project-file=cabal.project.9.8.1

Good suggestion, however, I don't want to fiddle with the CI to make this work.

Maybe the time is better invested in contributing to the upstream issues so they get closed.

@Mikolaj
Copy link
Member

Mikolaj commented Oct 27, 2023

So, is the plan to add cabal.project.local to the cabal repo? Isn't the absence of this file in git repos a valuable invariant simplifying the life of git users since time immemorial? How about cabal.project, if we really have to?

@ulysses4ever
Copy link
Collaborator

I'll add a new cabal.project.blah in #9330

This one needs a merge label. Please, feel free to add it. I'll rebase 9330 when this one is merged.

@andreasabel
Copy link
Member Author

So, is the plan to add cabal.project.local to the cabal repo? Isn't the absence of this file in git repos a valuable invariant simplifying the life of git users since time immemorial? How about cabal.project, if we really have to?

Ah, no I meant to say that I should work on the upstream issues so that we do not need a workaround here.
But I agree that cabal.project would be the better place.

@andreasabel andreasabel added the merge me Tell Mergify Bot to merge label Oct 29, 2023
@andreasabel
Copy link
Member Author

How about cabal.project, if we really have to?

Ok, did that now, for rere.

@andreasabel
Copy link
Member Author

I am out of luck now with CI. Maybe I just started it in an auspicious moment last time.

This time I am getting strange errors, which I think I have seen before but don't remember when and why:

Error:     Ambiguous module name ‘System.Directory’:
      it was found in multiple packages:
      directory-1.3.8.1 directory-1.3.8.1
  |
2 | import System.Directory
  | ^^^^^^^^^^^^^^^^^^^^^^^

https://github.com/haskell/cabal/actions/runs/6682526449/job/18161843458?pr=9372

@andreasabel andreasabel added the attention: needs-help Help wanted with this issue/PR label Oct 30, 2023
@ulysses4ever
Copy link
Collaborator

It's a famous heisenbuh, see #8883 and #8356. Clearing the cache using https://github.com/haskell/cabal/actions/caches may help.

While I'm here. I'd prefer a separate project file for these allow-newer's (e.g. cabal.project.ghc98), I think, so that we don't pollute the history of the main one.

@Mikolaj
Copy link
Member

Mikolaj commented Oct 30, 2023

I see somebody has cleared the caches already. Let me restart the builds...

@Mikolaj
Copy link
Member

Mikolaj commented Oct 30, 2023

@andreasabel: and it worked this time!

@andreasabel
Copy link
Member Author

andreasabel commented Oct 30, 2023

@ulysses4ever wrote:

I'd prefer a separate project file for these allow-newer's (e.g. cabal.project.ghc98), I think, so that we don't pollute the history of the main one.

Will these be picked up by CI?

Adding and deleting again will at least not pollute the blame.

hackage-server uses just cabal.project: https://github.com/haskell/hackage-server/commits/master/cabal.project Maybe we can also be pragmatic here?
(P.S.: I shouldn't have linked this file here, now you can see that we have not cleaned up under our sofa. 🤣 )

@Mikolaj
Copy link
Member

Mikolaj commented Oct 30, 2023

Our cabal.project already contains allow-newer: hackage-security:Cabal, so I wouldn't worry and just stick it there temporarily.

@Mikolaj
Copy link
Member

Mikolaj commented Oct 30, 2023

Oh, but I have no idea which cabal.project.* CI actually uses. Probably depends on the test. E.g., cabal.project.validate sounds plausible. So perhaps add it there and see what happens?

@andreasabel
Copy link
Member Author

So perhaps add it there and see what happens?

But atm allow-newer: rere:... is added to the main cabal.project and CI works.

@andreasabel andreasabel removed the attention: needs-help Help wanted with this issue/PR label Oct 30, 2023
@Mikolaj
Copy link
Member

Mikolaj commented Oct 30, 2023

I see

.github/workflows/validate.yml:          echo "allow-newer: rere:base, rere:transformers" >> cabal.project.validate

Edit: on branch master.

@ulysses4ever
Copy link
Collaborator

.github/workflows/validate.yml: echo "allow-newer: rere:base, rere:transformers" >> cabal.project.validate

That was my hack, which I'm not proud about. I'd like to localize these in a separate file, so that they're easier to track. And all other ones can import the one with allow-newer's.

@andreasabel
Copy link
Member Author

I think CI shouldn't do anything that surprising. In particular, if CI passes here for GHC 9.8, and I clone haskell/cabal to my machine, it should also work with GHC 9.8 on my machine. Anything else is very confusing.
This means that CI shouldn't do tweaks that make the code base compile while it otherwise doesn't compile.

@Mikolaj
Copy link
Member

Mikolaj commented Oct 30, 2023

This is all made even more convoluted by cabal.project normally not being packaged for Hackage (and Hackage builds ignoring it in any case). I think it's hard to do this really cleanly.

@ulysses4ever
Copy link
Collaborator

CI shouldn't do tweaks that make the code base compile while it otherwise doesn't compile.

Agreed. Question is: what's the most ergonomic way to recover this property.

@philderbeast
Copy link
Collaborator

Update: I added a allow-newer for rere into cabal.project now.

This also works and picks up ulysses4ever/rere#25:

source-repository-package
  type: git
  location: https://github.com/phadej/rere
  tag: dd78e2945a67f9cd32d16560c49780ae6227eed9

@andreasabel
Copy link
Member Author

andreasabel commented Oct 31, 2023

Since @phadej now released rere for GHC 9.8 (thanks for that!), there are no missing dependencies anymore, and no allow-newers needed (I removed them).
Thus I deem this PR uncontroversial now (just bumps).
I propose to merge this one quickly after CI succeeds, to unlock #9330.

P.S.: I took the opportunity to remove some cruft from cabal.project (my own cruft and one ancient allow-newer by Herbert).

@andreasabel andreasabel added the merge delay passed Applied (usually by Mergify) when PR approved and received no updates for 2 days label Oct 31, 2023
Copy link
Member

@Mikolaj Mikolaj left a comment

Choose a reason for hiding this comment

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

Well done.

@andreasabel
Copy link
Member Author

Supposedly needs another cache reset...

@Mikolaj
Copy link
Member

Mikolaj commented Oct 31, 2023

Yes, it seems so. Restarting...

@Mikolaj
Copy link
Member

Mikolaj commented Nov 2, 2023

Another cache wipe is needed...

@Mikolaj
Copy link
Member

Mikolaj commented Nov 2, 2023

@mergify rebase

Copy link
Contributor

mergify bot commented Nov 2, 2023

rebase

✅ Branch has been successfully rebased

@Mikolaj
Copy link
Member

Mikolaj commented Nov 2, 2023

It didn't help this time, but let me try wiping out the cache again...

Edit: and now it did work. :)

@mergify mergify bot merged commit 1157461 into master Nov 2, 2023
45 checks passed
@mergify mergify bot deleted the ghc-9.8 branch November 2, 2023 15:08
@Kleidukos
Copy link
Member

@Mergifyio backport 3.10

Copy link
Contributor

mergify bot commented Nov 7, 2023

backport 3.10

✅ Backports have been created

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 merge me Tell Mergify Bot to merge re: ghc 9.8
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Build with GHC 9.8
6 participants