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

Fix haskellPackages.fltkhs #50828

Closed
wants to merge 2 commits into from
Closed

Fix haskellPackages.fltkhs #50828

wants to merge 2 commits into from

Conversation

chris-martin
Copy link
Contributor

Motivation for this change

haskellPackages.fltkhs was marked as broken. It apparently requires FLTK >= 1.4 (see deech/fltkhs#93), which we didn't have yet. As far as I can tell, FLTK 1.4 isn't actually released yet, so I didn't think it was appropriate to replace the FLTK 1.3 package that we already have, so I added a new version. This fixes the Haskell package.

While I was at it, I also added the dependencies required to enable OpenGL support in fltkhs.

Things done
  • 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 via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-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)
  • Fits CONTRIBUTING.md.

/cc @deech

peti
peti requested changes Nov 20, 2018
View reviewed changes
Copy link
Member

@peti peti left a comment

Choose a reason for hiding this comment

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

I think that most of those changes should be made in cabal2nix to generate a proper Nix expression from the outset rather than to fix it up after the fact.

pkgs/development/haskell-modules/configuration-common.nix
# linking fails because the build doesn't pull in the libGLU_combined libraries
fltkhs = markBroken super.fltkhs;
fltkhs = let inherit (pkgs.lib) flip foldr; in foldr (f: x: f x) super.fltkhs [
(flip appendConfigureFlag "-fopengl")
Copy link
Member

Choose a reason for hiding this comment

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

Please enable this flag in the generator at https://github.com/NixOS/cabal2nix/blob/master/src/Distribution/Nixpkgs/Haskell/FromCabal/Flags.hs#L14.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done - NixOS/cabal2nix#388

chris-martin added a commit to chris-martin/cabal2nix that referenced this pull request Nov 20, 2018
@chris-martin
fltkhs: enable opengl
701ada9 
As requested at NixOS/nixpkgs#50828
@chris-martin chris-martin mentioned this pull request Nov 20, 2018
Merged
@chris-martin
Copy link
Contributor Author

The cabal2nix PR should take care of appendConfigureFlag "-fopengl" and addExtraLibrary super.OpenGLRaw (which I'm hoping now ends up implied by the flag.

What about addPkgconfigDepend pkgs.libGLU_combined and addSetupDepend pkgs.fltk14, do those go somewhere in cabal2nix as well?

peti added a commit that referenced this pull request Nov 20, 2018
@peti
haskell-fltkhs: re-enable the build
8272d06 
#50828 makes an effort to fix this
package.
@peti
Copy link
Member

peti commented Nov 20, 2018

The cabal2nix PR should take care of appendConfigureFlag "-fopengl" and addExtraLibrary super.OpenGLRaw (which I'm hoping now ends up implied by the flag.

OK. I've re-generated the hackage package set in haskell-updates branch @ 8272d06 with the new version.

What about addPkgconfigDepend pkgs.libGLU_combined and addSetupDepend pkgs.fltk14, do those go somewhere in cabal2nix as well?

There are two places that might be relevant: the post-process hooks at https://github.com/NixOS/cabal2nix/blob/master/src/Distribution/Nixpkgs/Haskell/FromCabal/PostProcess.hs#L72 allow you to modify the generated Nix expression as you see fit, i.e. to fix up insufficient Cabal files.

Then there is the "name lookup" code in https://github.com/NixOS/cabal2nix/blob/master/src/Distribution/Nixpkgs/Haskell/FromCabal/Name.hs which you can extend to make sure cabal2nix maps system library names used in the Cabal file to Nixpkgs names properly. I'm not sure which of the two is more appropriate your this particular use case. Using post-process hooks to add those dependencies is almost certainly going to work fine, though.

peti
peti requested changes Nov 20, 2018
View reviewed changes
Copy link
Member

@peti peti left a comment

Choose a reason for hiding this comment

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

It's probably a good idea to target haskell-updates with this PR because it has the newly generated code for hackage-packages.nix.

peti added a commit to NixOS/cabal2nix that referenced this pull request Nov 21, 2018
@peti
fltkhs: fix build in post-processing
042625c 
The necessary know-how was kindly provided in NixOS/nixpkgs#50828.
peti added a commit to NixOS/cabal2nix that referenced this pull request Nov 21, 2018
@peti
fltkhs: fix build in post-processing
2103989 
The necessary know-how was kindly provided in NixOS/nixpkgs#50828.
@peti
Copy link
Member

peti commented Nov 21, 2018

I cherry-picked the addition of fltkhs 1.4.x in 0aff1d86800570799ac16912fc3cda9e177ebdb6. The necessary fixes in cabal2nix are in NixOS/cabal2nix@2103989. The updated package set is in 59a8047dcb9916f342cc9c1146ef333d77316bd5. fltkhs should now compile fine in the haskell-updates branch. I'll merge to master soon!

@peti peti closed this Nov 21, 2018
peti added a commit that referenced this pull request Nov 21, 2018
@peti
haskell-fltkhs: re-enable the build
31de774 
#50828 makes an effort to fix this
package.
@chris-martin
Copy link
Contributor Author

Awesome, thanks for carrying this through to completion.

Profpatsch pushed a commit to Profpatsch/cabal2nix that referenced this pull request Mar 7, 2022
@chris-martin @Profpatsch
fltkhs: enable opengl
caed040 
As requested at NixOS/nixpkgs#50828
Profpatsch pushed a commit to Profpatsch/cabal2nix that referenced this pull request Mar 7, 2022
@peti @Profpatsch
fltkhs: fix build in post-processing
cc163af 
The necessary know-how was kindly provided in NixOS/nixpkgs#50828.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@peti peti peti requested changes

@basvandijk basvandijk Awaiting requested review from basvandijk

@ryantm ryantm Awaiting requested review from ryantm

Assignees
No one assigned
Labels
6.topic: haskell 8.has: package (new) 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@chris-martin @peti @GrahamcOfBorg