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.

Sign up for GitHub

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

Jump to bottom

haskellPackages.glpk-hs: fix for ghc-8.6 #64895

Closed
wants to merge 0 commits into from
Closed

haskellPackages.glpk-hs: fix for ghc-8.6 #64895

wants to merge 0 commits into from

Conversation

pschuprikov
Copy link
Contributor

Motivation for this change

From what I was able to understand, the hackage version of glpk-hs does not work with the new gasp. Moreover, due to deprecated calls glpk-hs doesn't compile with ghc-8.6, which is fixed by the patch.

The last upstream change was 8 months ago.

Things done

Compiles fine with ghc-8.4.4, ghc-8.6.5, ghc-8.8.1.

  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nix-review --run "nix-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@turion
Copy link
Contributor

turion commented Dec 10, 2019

There has been a release to version 0.7 a month ago. Is this still needed?

@pschuprikov
Copy link
Contributor Author

There are still deprecation errors. A pending upstream PR jyp/glpk-hs#11 from which I've taken the patch hasn't been merged yet. I'll remove overrideSrc part then.

cdepillabout
cdepillabout reviewed Dec 22, 2019
View reviewed changes
pkgs/development/haskell-modules/configuration-common.nix
# https://github.com/jyp/glpk-hs/pull/11
# patch is the above PR with conflicts resolved
glpk-hs = assert super.glpk-hs.version == "0.7";
appendPatch super.glpk-hs ./patches/glpk-hs-ghc86.patch;
Copy link
Member

Choose a reason for hiding this comment

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

If possible, in nixpkgs we generally prefer to pull patches from upstream directly with fetchpatch. There should be some examples of doing this in this file.

Can you rewrite this to use fetchpatch?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That was exactly my previous approach, but the PR in its current form has merge conflicts with 0.7 and the patching fails. If there is a cleaner solution other than creating a patch manually, I would certainly use it.

Copy link
Member

Choose a reason for hiding this comment

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

@pschuprikov Could you send the patch in this PR as a PR to upstream?

@cdepillabout
Copy link
Member

@pschuprikov Can you rebase this on the haskell-updates branch?

@pschuprikov pschuprikov changed the base branch from master to haskell-updates December 22, 2019 10:17
@pschuprikov pschuprikov changed the title haskellPackages.glpk-hs: 0.5 -> 0.6-pre-release (+ deprecation fix) haskellPackages.glpk-hs: fix for ghc-8.6 Dec 22, 2019
@peti peti force-pushed the haskell-updates branch 4 times, most recently from aa46cdd to fdd64fb Compare December 27, 2019 19:37
@peti peti force-pushed the haskell-updates branch 9 times, most recently from ff24de1 to 8842566 Compare January 10, 2020 14:12
@peti peti force-pushed the haskell-updates branch 5 times, most recently from 5011d9e to 0685be7 Compare January 10, 2020 19:57
@peti
Copy link
Member

peti commented Jan 17, 2020

Please submit a cleaned-up version of the change if you're still interested in fixing this issue.

@peti peti closed this Jan 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cdepillabout cdepillabout cdepillabout left review comments

@basvandijk basvandijk Awaiting requested review from basvandijk

Assignees
No one assigned
Labels
2.status: merge conflict 6.topic: haskell 10.rebuild-darwin: 0 10.rebuild-linux: 0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@pschuprikov @turion @cdepillabout @peti