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

haskellPackages: unbreak autodocodec, autdocodec-yaml, cabal2json #181242

Open
wants to merge 1 commit into
base: haskell-updates
Choose a base branch
from

Conversation

DavHau
Copy link
Member

@DavHau DavHau commented Jul 12, 2022

Description of changes
  • unbreak cabal2json and its dependencies for ghc8107.
Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 22.11 Release Notes (or backporting 22.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
    • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
  • Fits CONTRIBUTING.md.

@@ -139,4 +139,10 @@ self: super: {
# Not possible to build in the main GHC 9.0 package set
# https://github.com/awakesecurity/spectacle/issues/49
spectacle = doDistribute (markUnbroken super.spectacle);

# fails on missing doccheck dependency on ghc < 9.0
autodocodec = dontCheck super.autodocodec;
Copy link
Member

Choose a reason for hiding this comment

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

This should do the trick, the issue is just that the test suite is marked not buildable for GHC >= 9.0, so cabal2nix can't generate the dependencies for it.

Suggested change
autodocodec = dontCheck super.autodocodec;
autodocodec = addBuildDepends [ self.doctest ] super.autodocodec;

Copy link
Member Author

Choose a reason for hiding this comment

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

After applying that, the test suite now fails due to a failing unit test. I'd like to just disable the tests for now as my package of interest cabal2json just works fine.

@NorfairKing , any idea why we are running into this here?

autodocodec> src/Autodocodec/Codec.hs:1140: failure in expression `JSON.parseMaybe scientificCodec (Number 3)'
autodocodec> expected: Just 3
autodocodec>  but got:
autodocodec>           ^
autodocodec>           <interactive>:621:17: error:
autodocodec>               • Couldn't match type ‘Codec Value Scientific Scientific’
autodocodec>                                with ‘Value -> JSON.Parser b’
autodocodec>                 Expected type: Value -> JSON.Parser b
autodocodec>                   Actual type: JSONCodec Scientific
autodocodec>               • In the first argument of ‘JSON.parseMaybe’, namely
autodocodec>                   ‘scientificCodec’
autodocodec>                 In the expression: JSON.parseMaybe scientificCodec (Number 3)
autodocodec>                 In an equation for ‘it’:
autodocodec>                     it = JSON.parseMaybe scientificCodec (Number 3)
autodocodec>               • Relevant bindings include
autodocodec>                   it :: Maybe b (bound at <interactive>:621:1)
autodocodec> src/Autodocodec/Codec.hs:1174: failure in expression `JSON.parseMaybe (parseJSONVia c) (Number 3)'
autodocodec> expected: Just 3
autodocodec>  but got: Just 3.0
autodocodec>                 ^
autodocodec> Examples: 112  Tried: 111  Errors: 0  Failures: 2

Copy link
Member Author

Choose a reason for hiding this comment

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

I now kept the tests disabled, but put a comment mentioning the failing unit test.

Copy link
Member Author

Choose a reason for hiding this comment

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

If you are OK with this, I think the PR is ready to merge.

Copy link
Contributor

Choose a reason for hiding this comment

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

@DavHau doctest is broken on ghc > 9: sol/doctest#327

Copy link
Contributor

Choose a reason for hiding this comment

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

I just released 0.1.0.3 which shouldn't have this issue.

Copy link
Member Author

Choose a reason for hiding this comment

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

@sternenseemann How would I go about including that new release? Should I make a separate PR updating the hackage set via update-hackage.sh?

Copy link
Member

Choose a reason for hiding this comment

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

It'll be updated automatically by the next haskell-updates rotation, you don't need to do anything. I think someone will start one in the next few days, we're just all a little busy atm.

Copy link
Member

Choose a reason for hiding this comment

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

Should be available after rebasing on haskell-updates again (#181822).

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks, I rebased it and now the autodocodec package builds fine and tests pass.

@DavHau DavHau force-pushed the unbreak-haskell branch from d80993b to ad2554d Compare July 12, 2022 19:38
@DavHau DavHau changed the title haskellPackages: unbreak autodocodec, autdocodec-yaml, cabal2json, validity haskellPackages: unbreak autodocodec, autdocodec-yaml, cabal2json Jul 12, 2022
@DavHau DavHau requested a review from sternenseemann July 12, 2022 19:45
@DavHau DavHau force-pushed the unbreak-haskell branch 3 times, most recently from 3946d6b to d214337 Compare July 14, 2022 12:31
@DavHau DavHau force-pushed the unbreak-haskell branch from d214337 to debde0d Compare July 18, 2022 13:43
@maralorn
Copy link
Member

I just tried to compile cabal2json and got:

sydtest> Warning:
sydtest>     This package indirectly depends on multiple versions of the same package. This is very likely to cause a compile failure.
sydtest>       package safe-coloured-text-terminfo (safe-coloured-text-terminfo-0.0.0.0-AJUJKQbAuX7Hi4IJu2ZUm4) requires safe-coloured-text-0.1.0.0-1oKuyjwaSBzMmogYiTwIJ
sydtest>       package sydtest (sydtest-0.11.0.0) requires safe-coloured-text-0.2.0.1-Cm2NciAxHdy6PGojf331dq
sydtest>       package sydtest (sydtest-0.11.0.0) requires safe-coloured-text-0.2.0.1-Cm2NciAxHdy6PGojf331dq
sydtest>       package sydtest (sydtest-0.11.0.0) requires safe-coloured-text-0.2.0.1-Cm2NciAxHdy6PGojf331dq
sydtest>       package autodocodec-yaml (autodocodec-yaml-0.2.0.0-CBBlKbKu9eGKMuhas9c9H) requires safe-coloured-text-0.2.0.1-Cm2NciAxHdy6PGojf331dq

@DavHau
Copy link
Member Author

DavHau commented Jul 29, 2022

I just tried to compile cabal2json and got:

sydtest> Warning:
sydtest>     This package indirectly depends on multiple versions of the same package. This is very likely to cause a compile failure.
sydtest>       package safe-coloured-text-terminfo (safe-coloured-text-terminfo-0.0.0.0-AJUJKQbAuX7Hi4IJu2ZUm4) requires safe-coloured-text-0.1.0.0-1oKuyjwaSBzMmogYiTwIJ
sydtest>       package sydtest (sydtest-0.11.0.0) requires safe-coloured-text-0.2.0.1-Cm2NciAxHdy6PGojf331dq
sydtest>       package sydtest (sydtest-0.11.0.0) requires safe-coloured-text-0.2.0.1-Cm2NciAxHdy6PGojf331dq
sydtest>       package sydtest (sydtest-0.11.0.0) requires safe-coloured-text-0.2.0.1-Cm2NciAxHdy6PGojf331dq
sydtest>       package autodocodec-yaml (autodocodec-yaml-0.2.0.0-CBBlKbKu9eGKMuhas9c9H) requires safe-coloured-text-0.2.0.1-Cm2NciAxHdy6PGojf331dq

This PR fixes the package only for ghc 8.10.x.

@maralorn
Copy link
Member

Oh, my bad.

Then the commit message should replace haskellPackages with haskell.packages.ghc8107 imo.
Besides, if haskellPackages.cabal2json stays broken it should not be removed from the broken.yaml. Instead, we should use markUnbroken in the configiguration-8.10.x.nix.

@DavHau
Copy link
Member Author

DavHau commented Jul 31, 2022

Instead, we should use markUnbroken in the configiguration-8.10.x.nix.

Will markUnbroken be enough to make the package build with hydra, or do I have to do something with the hydraPlatforms as well?

@maralorn
Copy link
Member

Good point, doDistribute.

@sternenseemann
Copy link
Member

haskell.packages.ghc8107 is also not built by Hydra by default.

I kind of wanna question why such a trivial utility needs to have such a non trivial dependency tree and can't be built with GHC 9.* despite being released like yesterday?

@ParetoOptimalDev
Copy link

I kind of wanna question why such a trivial utility needs to have such a non trivial dependency tree and can't be built with GHC 9.* despite being released like yesterday?

@sternenseemann I think it's tied to the Cabal library and therefore requires lots of updates each new release after reading this:

cabal2json should be updated to support newer versions of the cabal lib.

However the suggestion below that comment seems like it could free that library of that constraint maybe?

Maybe you can just depend on Cabal-syntax in the future. This package was singled out from Cabal to give third-party tools a bit more lightweight dependency.

I'm not really sure though.

@sternenseemann
Copy link
Member

cabal2nix also depends on Cabal, but supports all GHC versions in nixpkgs – it's just a matter of doing a bit of legwork and adding a bit of CPP here and there – which is easier to maintain than this.

In any case, maybe things are easier now with Stackage LTS 20 – I think the main problem is the autodocodec universe.

@wegank wegank added 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md 2.status: merge conflict This PR has merge conflicts with the target branch labels Mar 19, 2024
@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Mar 20, 2024
@wegank wegank added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jul 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.status: merge conflict This PR has merge conflicts with the target branch 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md 6.topic: haskell 8.has: package (new) This PR adds a new package 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants