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

Fix #8377: allow text-2.0 and time-1.12 in cabal-install #8398

Merged
merged 1 commit into from
Aug 28, 2022

Conversation

andreasabel
Copy link
Member

Fix #8377: allow text-2.0 and time-1.12 in cabal-install.

Also allow latest aeson, network-wait, optparse-applicative in cabal-testsuite.

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!

@juhp
Copy link
Collaborator

juhp commented Aug 20, 2022

I think it looks fine, but need base bumps for haskellari/lukko#30 and phadej/singleton-bool#20, which I opened now :)

(For anyone wondering it is possible to build cabal-install with ghc-9.4.1 using --allow-newer.)

Also allow latest aeson, network-wait, optparse-applicative in cabal-testsuite.
@juhp
Copy link
Collaborator

juhp commented Aug 30, 2022

Can a revision be done on Hackage now?

@andreasabel
Copy link
Member Author

The only code change in this PR is addressing a -Werror in the test-suite. This isn't relevant for the released version.
So, a revision is likely possible, but someone should first make the experiment whether the released version builds & works with the relaxed bounds.

@Mikolaj
Copy link
Member

Mikolaj commented Aug 30, 2022

Yes, we'd need a manual (or in a special PR) test run with a constraint that forces these bounds. But that's on 3.8 branch, so only after this is backported. Let me backport...

@Mikolaj
Copy link
Member

Mikolaj commented Aug 30, 2022

@mergify backport 3.8

@mergify
Copy link
Contributor

mergify bot commented Aug 30, 2022

backport 3.8

✅ Backports have been created

@Mikolaj
Copy link
Member

Mikolaj commented Aug 30, 2022

Oh, actually the dev (default) version of cabal.project forces that, so it's enough to build and test with that one.

@andreasabel
Copy link
Member Author

Oh, actually the dev (default) version of cabal.project forces that, so it's enough to build and test with that one.

Yeah, I couldn't get cabal to pick up the latest versions otherwise, so I forced it.

@andreasabel
Copy link
Member Author

The version obtained with cabal get cabal-install and then patched with this PR builds:

cabal build -w ghc-9.2.4 -c "text > 2" -c "time >= 1.12"

The testsuites do not build:

$ cabal build --enable-tests -w ghc-9.2.4 -c "text > 2" -c "time >= 1.12"

Error: cabal: Could not resolve dependencies:
[__0] trying: cabal-install-3.8.1.0 (user goal)
[__1] trying: cabal-install:*test
[__2] unknown package: Cabal-described (dependency of cabal-install *test)

$ cabal info Cabal-described

Error: cabal: There is no package named 'Cabal-described'.

The dependency Cabal-described of test-suite long-tests does not exist.

Are the testsuites of the released versions supposed to build?

(If not we could delete them from the .cabal file before releasing.)

@Mikolaj
Copy link
Member

Mikolaj commented Aug 30, 2022

That's a bummer. If they are in the .cabal file, they should build. Agreed we should either remove them or publish enough little packages for them to build. I suppose the former is easier and we have more urgent issues.

Edit: ideally we'd remove them to yet another package so that they can stay there unmolested whenever we release cabal proper.

@andreasabel
Copy link
Member Author

Ok, pushed the revision: https://hackage.haskell.org/package/cabal-install-3.8.1.0/revisions/

    • Changed the library component's library dependency on 'text'
      from >=1.2.3 && <1.3
      to >=1.2.3 && <1.3 || >=2.0 && <2.1

    • Changed the library component's library dependency on 'time'
      from >=1.5.0.1 && <1.12
      to >=1.5.0.1 && <1.13

@andreasabel andreasabel self-assigned this Aug 30, 2022
@andreasabel
Copy link
Member Author

@Mikolaj: Indeed, cabal-install-3.6.2.0 was released without any test suites.

@Mikolaj
Copy link
Member

Mikolaj commented Aug 30, 2022

Ok, pushed the revision

Oh, wouldn't it be better to test first? Not sure if the tests in the backport use the cabal.project or ignore it, but at least somebody could checkout 3.8 and test manually.

@Mikolaj: Indeed, cabal-install-3.6.2.0 was released without any test suites.

Let me open a ticket in that case.

@andreasabel
Copy link
Member Author

Oh, wouldn't it be better to test first? Not sure if the tests in the backport use the cabal.project or ignore it, but at least somebody could checkout 3.8 and test manually.

Indeed, this would be safer. Branch 3.8 seems to be ahead of the released version, but there is likely a tag from which one can branch off, patch, and run the CI.
We might be lucky; I haven't heard of any serious regression in text-2.0 yet...

Unfortunately, the CI for the backport does not show whether text-2.0 is picked up:

@Mikolaj
Copy link
Member

Mikolaj commented Aug 30, 2022

Unfortunately, the CI for the backport does not show whether text-2.0 is picked up

I've rummaged in the logs and it seems it did not build text 2 even the first time it started on GHC 9.2, so it probably ignores cabal.project. Too bad, otherwise we'd have a test for free. Anyway, it should get merged soon, so if a volunteer would like to test, that's a good moment. Otherwise, we will depend on our luck. :)

Mikolaj added a commit that referenced this pull request Aug 30, 2022
Fix #8377: allow text-2.0 and time-1.12 in cabal-install (backport #8398)
@Mikolaj
Copy link
Member

Mikolaj commented Aug 30, 2022

The backport is merged.

@mergify mergify bot added the merge delay passed Applied (usually by Mergify) when PR approved and received no updates for 2 days label Sep 1, 2022
@ulysses4ever ulysses4ever removed the merge delay passed Applied (usually by Mergify) when PR approved and received no updates for 2 days label Sep 3, 2022
@mergify mergify bot added the merge delay passed Applied (usually by Mergify) when PR approved and received no updates for 2 days label Sep 5, 2022
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Should cabal-install use text-2.0?
6 participants