haskell / hackage-security

Hackage security framework based on TUF (The Update Framework)
http://hackage.haskell.org/package/hackage-security
56 stars 47 forks source link

Support mtl-2.3 #289

Closed Bodigrim closed 2 years ago

Bodigrim commented 2 years ago

Resolves #288.

Tested with

cabal build all -c 'transformers >= 0.6' -c 'mtl >= 2.3' --allow-newer='Cabal-syntax:mtl,Cabal-syntax:transformers,Cabal:mtl,Cabal:transformers'
Bodigrim commented 2 years ago

@Mikolaj CI is green.

andreasabel commented 1 year ago

@Mikolaj : Can we get this released? I came by here when trying hackage-server with mtl-2.3.

Mikolaj commented 1 year ago

@andreasabel: yes, we'll try cutting a branch in Jan and releasing after a month of testing perhaps.

I'm afraid we don't have the resources for making and maintaining a point release of 3.8.

andreasabel commented 1 year ago

@Mikolaj: Mmh, I am a bit lost here. You released hackage-security in November, but not hackage-security-HTTP. This PR allows mtl-2.3 for hackage-security-HTTP. Isn't the latter rather easy to release? (Sorry, I might not understand the bigger picture.)

P.S.: Merry Christmas belated. 🎄

Mikolaj commented 1 year ago

Merry Christmas, Andreas!

I got confused that it's about a cabal release, because the only changes to the hackage-security-HTTP package since last release are are bumped bounds on base, HTTP and mtl. However, we need to bump the dep on hackage-security as well (to 0.6.2.3), in order for the mtl bump to be valid. Do you have any other changes in mind? Do you have a preference if the remaining bumps (some you already applied on Hackage) should be done via a release or a revision?

Mikolaj commented 1 year ago

Actually, perhaps the current bound on hackage-security makes sense. With old hackage-security, hackage-security-HTTP doesn't support new mtl, with new hackage-security, it does. Depending on how you interpret the bound on mtl --- as a statement it works with new mtl always, or as a potential, conditional on other packages versions. The cabal solver interprets it in the latter way, but I'd prefer the former (and then the solver would pick older hackage-security-HTTP just fine is somebody insists on older hackage-security).

andreasabel commented 1 year ago

Depending on how you interpret the bound on mtl --- as a statement it works with new mtl always, or as a potential, conditional on other packages versions. The cabal solver interprets it in the latter way, but I'd prefer the former

I agree, esp. for mtl one should relax to <2.4 only if there is evidence that it can build with mtl-2.3.1. I wouldn't go as far as that it must be built with mtl-2.3, because if I am not mistaken, there are no semantic changes in mtl-2.3 over mtl-2.2; the breakage is only due to stuff removed from the API.

andreasabel commented 1 year ago

@Mikolaj wrote:

I got confused that it's about a cabal release, because the only changes to the hackage-security-HTTP package since last release are are bumped bounds on base, HTTP and mtl. However, we need to bump the dep on hackage-security as well (to 0.6.2.3), in order for the mtl bump to be valid. Do you have any other changes in mind? Do you have a preference if the remaining bumps (some you already applied on Hackage) should be done via a release or a revision?

I was maybe under the false impression that this PR makes changes to hackage-security-HTTP that are needed for mtl-2.3, rather than just bumping bounds, so that a revision would not be possible...

However, this works on the released hackage-security-HTTP-0.1.1.1:

$ cabal build -c 'mtl >= 2.3' --allow-newer='hackage-security-HTTP:mtl,Cabal-syntax:mtl,Cabal-syntax:transformers'

So it seems possible to just make the hackage revisions to hackage-security-HTTP and Cabal-syntax.

What is your take on revising Cabal-syntax-3.8.1.0 this way? It appears mtl-2.3-ready, so its bounds should reflect that, shouldn't they?

(And yes, the hackage-security bounds >=0.5 && <0.7 are fine the way they are.)

Mikolaj commented 1 year ago

Regarding hackage-security-HTTP, please feel free to revise.

Looking at master branch of cabal, the story is not that simple:

https://github.com/haskell/cabal/blob/master/Cabal-syntax/Cabal-syntax.cabal#L44-L46

but if it's useful to anybody, I can revise even omitting the comment (but we should probably amend 3.8 branch, too, which has old bounds, so that somebody compiling from 3.8 branch source doesn't have worse bounds than from Hackage).

andreasabel commented 1 year ago

I did the revision now: https://hackage.haskell.org/package/hackage-security-HTTP-0.1.1.1/revisions/

Mikolaj commented 1 year ago

Looks fine. Thank you.

andreasabel commented 1 year ago

@Mikolaj wrote:

we should probably amend 3.8 branch, too, which has old bounds, so that somebody compiling from 3.8 branch source doesn't have worse bounds than from Hackage

I have made a PR to this extend:

Once this PR is accepted, we can make the respective revision to Cabal-syntax-3.8.1.0.