search

haskell-infra / hackage-trustees

Issue tracker for Hackage maintainance and trustee operations
https://hackage.haskell.org/packages/trustees/
42 stars 7 forks source link

Requesting bytestring bump for ftp-client #325

Closed ysangkok closed 2 years ago

ysangkok commented 2 years ago

Hello, I have noticed that ftp-client on Hackage doesn't allow bytestring v0.11.

But in it's repo, it does actually allow it. I have been told on IRC that Stack may have inserted this upper bound automatically on upload.

I have reported the issue on ftp-client's issue tracker but Megan Robinson hasn't replied for 18 days: https://github.com/mr/ftp-client/issues/22

So I'd really appreciate it if a trustee could bump make a revision to allow bytestring v0.11. This fixes the build of our project on GHC 9.2. I can dig into why exactly v0.10 is excluded for me, if you'd like me to.

The repo sources do build on GHC 9.2 as they are.

Happy holidays!

sjakobi commented 2 years ago

@ysangkok If you can demonstrate that the ftp-client-0.5.1.4 tests pass when built with bytestring >= 0.11, I can make a revision.

ysangkok commented 2 years ago

@sjakobi

Using the following cabal.project:

packages: .
source-repository-package
  location: git@github.com:tfausak/hs-memory.git
  tag: 3cf661a8a9a8ac028df77daa88e8d65c55a3347a
  type: git

source-repository-package
  location: git@github.com:TomMD/foundation.git
  tag: 0bb195e1fea06d144dafc5af9a0ff79af0a5f4a0
  type: git
  subdir: basement

source-repository-package
  location: git@github.com:josephcsible/cryptonite.git
  tag: 3b081e3ad027b0550fc87f171dffecbb20dedafe
  type: git

testing with an additional constraint passes:

 % cabal test --constraint='tasty-hspec<1.2' 
Resolving dependencies...
Build profile: -w ghc-8.6.5 -O1
In order, the following will be built (use -v for more details):
 - tasty-hspec-1.1.6 (lib) (requires download & build)
 - ftp-client-0.5.1.4 (test:ftp-client-test) (configuration changed)
Downloading  tasty-hspec-1.1.6
Downloaded   tasty-hspec-1.1.6
Starting     tasty-hspec-1.1.6 (lib)
Building     tasty-hspec-1.1.6 (lib)
Installing   tasty-hspec-1.1.6 (lib)
Completed    tasty-hspec-1.1.6 (lib)
Configuring test suite 'ftp-client-test' for ftp-client-0.5.1.4..
Preprocessing test suite 'ftp-client-test' for ftp-client-0.5.1.4..
Building test suite 'ftp-client-test' for ftp-client-0.5.1.4..
[1 of 1] Compiling Main             ( test/test.hs, /home/janus/flipstone/ftp-client/ftp-client/dist-newstyle/build/x86_64-linux/ghc-8.6.5/ftp-client-0.5.1.4/t/ftp-client-test/build/ftp-client-test/ftp-client-test-tmp/Main.o ) [Test.Tasty.Hspec changed]
Linking /home/janus/flipstone/ftp-client/ftp-client/dist-newstyle/build/x86_64-linux/ghc-8.6.5/ftp-client-0.5.1.4/t/ftp-client-test/build/ftp-client-test/ftp-client-test ...
Running 1 test suites...
Test suite ftp-client-test: RUNNING...
Test suite ftp-client-test: PASS
Test suite logged to:
/home/janus/flipstone/ftp-client/ftp-client/dist-newstyle/build/x86_64-linux/ghc-8.6.5/ftp-client-0.5.1.4/t/ftp-client-test/test/ftp-client-0.5.1.4-ftp-client-test.log
1 of 1 test suites (1 of 1 test cases) passed.

But using the newest tasty-hspec (which ftp-client allows), it fails because of a missing reexport of hspec and similar functions from package hspec in tasty-hspec.

So if the trustees prefer, they could constrain tasty-hspec to <1.2.

It can be made to build on the newest tasty-hspec using the following patch (which I will now submit upstream):

commit 907845a45333783c861af83564cd9f35694c295d (refs/stash)
Merge: 7329084 d49ce3e
Author: Janus Troelsen <janus@flipstone.com>
Date:   Fri Dec 17 10:13:05 2021 -0600

    WIP on master: 7329084 Stack upgrade

diff --cc ftp-client/ftp-client.cabal
index a6f30e9,a6f30e9..57172b5
--- a/ftp-client/ftp-client.cabal
+++ b/ftp-client/ftp-client.cabal
@@@ -35,6 -35,6 +35,7 @@@ test-suite ftp-client-tes
                       , tasty >= 1.2.3
                       , tasty-hspec >= 1.1.5.1
                       , ftp-client
++                     , hspec
    ghc-options:         -threaded -rtsopts -with-rtsopts=-N
    default-language:    Haskell2010

diff --cc ftp-client/test/test.hs
index b416130,b416130..b7cf900
--- a/ftp-client/test/test.hs
+++ b/ftp-client/test/test.hs
@@@ -2,6 -2,6 +2,7 @@@ import Data.ByteString (ByteString
  import qualified Data.ByteString.Char8 as C
  import Test.Tasty
  import Test.Tasty.Hspec
++import Test.Hspec
  import Network.FTP.Client hiding (Success)
  import qualified Network.FTP.Client as F
  import Control.Monad.IO.Class
sjakobi commented 2 years ago

@ysangkok Thanks! Did you ensure that bytestring >= 0.11 is used though? I don't see a relevant constraint.

ysangkok commented 2 years ago

@sjakobi I have redone the test with this additional bytestring constraint and GHC 8.10:


 % cabal test --constraint='tasty-hspec<1.2' --constraint='bytestring>=0.11'
Resolving dependencies...
Build profile: -w ghc-8.10.7 -O1
In order, the following will be built (use -v for more details):
 - basement-0.0.12 (lib) (requires build)
 - bytestring-0.11.1.0 (lib) (requires build)
[...]
/home/janus/flipstone/ftp-client/ftp-client/dist-newstyle/build/x86_64-linux/ghc-8.10.7/ftp-client-0.5.1.4/t/ftp-client-test/test/ftp-client-0.5.1.4-ftp-client-test.log
1 of 1 test suites (1 of 1 test cases) passed.
sjakobi commented 2 years ago

Alright. I've made the revision: https://hackage.haskell.org/package/ftp-client-0.5.1.4/revisions/

ysangkok commented 2 years ago

@sjakobi I just noticed that my test methodology was wrong, because I should have added all the constraints from the Hackage when building ftp-client from Git source. This caused me not to notice that attoparsec also needed bumping.

I have tested the bump of attoparsec of the cabal get'ed sources, and if attoparsec is also bumped, this final revision bump indeed allows it to be used on my GHC 9.2 project: https://github.com/mr/ftp-client/issues/23#issuecomment-996902294

Can this also be done now or should I just wait a few weeks and file to take over the package? In any case, thanks a lot for your help.

sjakobi commented 2 years ago

I've bumped the attoparsec bound too: https://hackage.haskell.org/package/ftp-client-0.5.1.4/revisions/

phadej commented 2 years ago

TBH, this feels a bit too much. Why some from-git memory, foundation and cryptonite versions? Why can't ftp-client be used from git version until then?

18 days, you are inpatient. @mr doesn't seem to be active on github (see https://github.com/mr), @sjakobi please send an email to that you made a revision. Maybe an email would been enough in the first place.

sjakobi commented 2 years ago

@phadej See my comments https://github.com/mr/ftp-client/issues/22#issuecomment-996859316 and https://github.com/mr/ftp-client/issues/23#issuecomment-996968704.

phadej commented 2 years ago

I don't think in this case GitHub comment is enough. The maintainer doesn't seem to be using GitHub that much. I think our policy is to lax by allowing to shout on GitHub (people do disable, or simple ignore GitHub notifications).

Also

Trustees are expected to use this power judiciously and make sure they understand the packages involved and their APIs. In the first instance they should get maintainers to make changes.

I don't think that was done.

Also https://matrix.hackage.haskell.org/#/package/ftp-client shows dependencies failures with GHC-7.10.3. These shouldn't been fixed as well, IMHO. (there isn't an install plan with GHC-7.10.3 and bytestring-0.11, and hopefully won't ever be in the future, as if ftp-client uses internals of bytestring, then the PatternSynonym compat won't work).

I'd like trustees to be very conservative when considering relaxing bounds.

gbaz commented 2 years ago

With all due respect, I disagree with the above. In this case it seems like relaxing bounds was done with enough diligence, and the attempt to get a response from the maintainer was sufficient.

sjakobi commented 2 years ago

@phadej I don't think an email is necessary. @mr was active on GitHub in September, merging https://github.com/mr/reactive-banana-gi-gtk/pull/7 only 12 days after it was created. https://github.com/mr/ftp-client/watchers indicates that she'll receive notifications about https://github.com/mr/ftp-client/issues/22 and https://github.com/mr/ftp-client/issues/23.

Trustees are expected to use this power judiciously and make sure they understand the packages involved and their APIs. In the first instance they should get maintainers to make changes.

I don't think that was done.

I'll try to be more careful next time, although I don't see any harm inflicted by the revisions I have published.

Also https://matrix.hackage.haskell.org/#/package/ftp-client shows dependencies failures with GHC-7.10.3. These shouldn't been fixed as well, IMHO. (there isn't an install plan with GHC-7.10.3 and bytestring-0.11, and hopefully won't ever be in the future, as if ftp-client uses internals of bytestring, then the PatternSynonym compat won't work).

I wasn't aware of the build failure with GHC-7.10.3 and I don't understand how it's related to this issue. Feel free to fix it yourself.

I'd like trustees to be very conservative when considering relaxing bounds.

I think that conservatism with respect to relaxing bounds would only increase the pain of upgrading dependencies. And AFAIK bad revisions can easily be fixed by follow-up revisions.

phadej commented 2 years ago

Then I kindly request

(in the future) maintainers should be able to explicitly indicate whether trustees are able to relax constraints for their package

part of the policy to be implemented soon, I don't want relaxations happen to my packages in such short time frames. Only restricting revisions when there is a build failure evidence. I have reasons to hold some revisions, which are well over 20 or so days.

andreasabel commented 2 years ago

@phadej wrote:

I don't want relaxations happen to my packages in such short time frames.

In general, I agree, 18 days is clearly a time span where one could be away on holiday etc.

However, in the given case, a relaxation was already due for 1 year, since bytestring-0.11 came out in September 2020. I think in such cases it is justified that trustees act to get a stuck package unstuck.

Then I kindly request

(in the future) maintainers should be able to explicitly indicate whether trustees are able to relax constraints for their package

part of the policy to be implemented soon,

I'd rather want to move in the opposite direction, to include in a hackage-upload a pledge to maintain. That would give hackage trustees a stronger backing of their work in cases where maintenance is neglected. Including maintenance imperatives would make hackage more a community place. It would pay respect to the fact that there are people depending on package and they suffer if it is neglected.

Such maintenance imperatives would be soft ("should"), but their presence makes it clear there is a duty, and if it is neglected, then it is justified that others step in.

One could object that if someone shares code on hackage, it is already an act of benevolence and grace, and no duties should derive from that. I'd like to argue that this is not the whole story: By getting users, you also build a name and fame, you get items to list in your CV etc., the usual social payback...

andreasabel commented 2 years ago

@phadej wrote:

Also matrix.hackage.haskell.org/#/package/ftp-client shows dependencies failures with GHC-7.10.3. These shouldn't been fixed as well, IMHO.

Yes, these should be investigated. Unfortunately, matrix.hackage does not show a log for when dependencies failed to build.

On my machine, build with GHC 7.10.3 succeeds using the following plan:

  base-4.8.2.0
  bytestring-0.10.12.1
  connection-0.3.1
  containers-0.6.5.1
  network-3.1.0.1
  transformers-0.5.6.2
  attoparsec-0.14.3
  exceptions-0.10.4
phadej commented 2 years ago

However, in the given case, a relaxation was already due for 1 year, since bytestring-0.11 came out in September 2020. I

But wasn't included with any GHC release (EDIT: until recently in GHC-9.2, released october 29 this year), and therefore weren't in Stackage snapshot to date (nightly is on GHC-9.0 atm, and mr seems to use stack). I'd argue that people could simply be still unaware of bytestring-0.11.

Yes, these should be investigated.

I did already.