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

haskell.packages.ghc94.gi-gtk: unbreak dependencies and package #205814

Closed
wants to merge 1 commit into from

Conversation

dark-ether
Copy link
Contributor

Description of changes

on current nixos-unstable and on haskell-updates the gi-gtk package and some of its dependencies are broken

Things done

for some reason the haskell-gi packages have more dependencies on ghc-9.4 luckily they all are packaged already and it is a single line per package to make them build.

  • 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/)
  • 23.05 Release Notes (or backporting 22.11 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.

@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux labels Dec 12, 2022
@cdepillabout
Copy link
Member

for some reason the haskell-gi packages have more dependencies on ghc-9.4

I'm somewhat curious here, do you know why this is? I'm surprised something like this isn't picked up automatically by our tooling.

@dark-ether
Copy link
Contributor Author

i will be frank i have no idea, i just tried to use gi-gtk and saw the error was a clear missing x so i tried to add the dependency to see if it would build, and it did. it probably is worth investigating why it has more dependencies or at least why they aren't automatically picked up.

@dark-ether
Copy link
Contributor Author

in which directory is the code for determining the build dependencies of a package? gi graphene is marked broken but the error it gives is similar to this one haskell-gi/haskell-gi#308 and sure enough gi-graphene builds if in configuration we add graphene as a build dependency

@cdepillabout
Copy link
Member

it probably is worth investigating why it has more dependencies or at least why they aren't automatically picked up.

Just wanted to say that I thought we should probably hold off on merging this until we figure this out. It is somewhat concerning that the gi-* packages don't need these explicit overrides on ghc92, but they do on ghc94. I'm interested if there is some sort of bug in our tooling that makes this necessary.

in which directory is the code for determining the build dependencies of a package?

The actual derivations for a Haskell package are in https://github.com/NixOS/nixpkgs/blob/db01e7b2207e0de45d9c346036f22322495ab249/pkgs/development/haskell-modules/hackage-packages.nix (and you should be able to grep for things like "gi-gtk" and confirm that all system-level dependencies are passed correctly), and then there are various overrides in other files in that directory, most notably https://github.com/NixOS/nixpkgs/blob/db01e7b2207e0de45d9c346036f22322495ab249/pkgs/development/haskell-modules/configuration-common.nix and https://github.com/NixOS/nixpkgs/blob/db01e7b2207e0de45d9c346036f22322495ab249/pkgs/development/haskell-modules/configuration-nix.nix.

The hackage-packages.nix file gets created with the tool https://github.com/NixOS/cabal2nix/tree/master/cabal2nix/hackage2nix, which is basically a wrapper around cabal2nix.

You can play around with cabal2nix on the CLI with a command like:

$ nix-shell -p cabal2nix --command 'cabal2nix cabal://gi-gtk'

@cdepillabout
Copy link
Member

gi-graphene builds if in configuration we add graphene as a build dependency

It is possible that no one has added a graphene dependency for gi-graphene yet, which may require being sent as a PR here:

https://github.com/NixOS/cabal2nix/blob/021a48f4b4942462154b06fd81429a248638f87f/cabal2nix/src/Distribution/Nixpkgs/Haskell/FromCabal/Name.hs#L16-L27

@cdepillabout
Copy link
Member

Sorry for taking a while to get back to this. sterni added a workaround for this problem in #214446.

Would you be willing to rewrite this PR to use __CabalEagerPkgConfigWorkaround?

@dark-ether
Copy link
Contributor Author

yeah no problem. it was really easy the first time so whenever i am free i will do it. please ping me if i take more than 1 week i tend to get hyperfocused in whatever i am currently doing and forget other things.

@dark-ether
Copy link
Contributor Author

ok i took some of my time to work on this and i got almost everything to work, however gi-gdkpixbuf fails due to our version of libdeflate not having a.pc file, there is a pr to bump its version so we need to wait that do i just push my version that should work even if it can't be merged or should i wait that pr to merge to push my changes?

Copy link
Member

@sternenseemann sternenseemann left a comment

Choose a reason for hiding this comment

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

Adding the workaround for gdkpixbuf should work in the sense that it'll fix itself as soon as the PR arrives in the relevant branch(es).

@@ -203,4 +203,12 @@ in {
fourmolu = overrideCabal (drv: {
libraryHaskellDepends = drv.libraryHaskellDepends ++ [ self.file-embed ];
}) (disableCabalFlag "fixity-th" super.fourmolu);
# haskell-gi
Copy link
Member

@sternenseemann sternenseemann Feb 15, 2023

Choose a reason for hiding this comment

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

You'll want to use __CabalEagerPkgConfigWorkaround for all of these like this:

# Apply workaround for Cabal 3.8 bug https://github.com/haskell/cabal/issues/8455
# by making `pkg-config --static` happy. Note: Cabal 3.9 is also affected, so
# the GHC 9.6 configuration may need similar overrides eventually.
X11-xft = __CabalEagerPkgConfigWorkaround super.X11-xft;
# Jailbreaks for https://github.com/gtk2hs/gtk2hs/issues/323#issuecomment-1416723309
glib = __CabalEagerPkgConfigWorkaround (doJailbreak super.glib);
cairo = __CabalEagerPkgConfigWorkaround (doJailbreak super.cairo);
pango = __CabalEagerPkgConfigWorkaround (doJailbreak super.pango);

Also please put your overrides next to those, so they'll be easier to clean up.

@dark-ether
Copy link
Contributor Author

ok please review this version, as it is the current one i am working on, no reason to have a wrong version in my pull request i tested and although gi-gtk hasn't built due to gi-gdk failing except for gi-gdk and gi-gdkpixbuf all dependencies build properly

@@ -485,7 +485,7 @@ rec {
builtins.genericClosure {
startSet = builtins.map (drv:
{ key = drv.outPath; val = drv; }
) drvs;
) (builtins.filter (i: !builtins.isNull i) drvs);
Copy link
Member

Choose a reason for hiding this comment

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

Is this necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, when it receives null it chokes so we need to filter it out, as mentioned in my other comment some derivations like gi-gmodule have a dependency set to null which means this function will error without this change.

Copy link
Member

Choose a reason for hiding this comment

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

bb62d57 should also fix that – in general it is unlikely that null ends up in libraryPkgConfigDepends and it's correct, so keeping it as it was probably will catch similar problems in the future.

gi-gdk_4_0_5 = __CabalEagerPkgConfigWorkaround super.gi-gdk_4_0_5;
gi-gio = __CabalEagerPkgConfigWorkaround super.gi-gio;
gi-glib = __CabalEagerPkgConfigWorkaround super.gi-glib;
gi-gmodule = __CabalEagerPkgConfigWorkaround (overrideCabal (drv: {libraryPkgconfigDepends = [pkgs.glib];}) super.gi-gmodule);
Copy link
Member

Choose a reason for hiding this comment

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

What's going on here?

Copy link
Contributor Author

@dark-ether dark-ether Feb 25, 2023

Choose a reason for hiding this comment

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

gi-gmodule has the following code

"gi-gmodule" = callPackage
    ({ mkDerivation, base, bytestring, Cabal, containers, gi-glib
     , gmodule, haskell-gi, haskell-gi-base, haskell-gi-overloading
     , text, transformers
     }:
     mkDerivation {
       pname = "gi-gmodule";
       version = "2.0.3";
       sha256 = "043n3nyxy29chzc7xzhinp40yxazlikqcjdbm3pvh344jv7m5xjx";
       setupHaskellDepends = [ base Cabal gi-glib haskell-gi ];
       libraryHaskellDepends = [
         base bytestring containers gi-glib haskell-gi haskell-gi-base
         haskell-gi-overloading text transformers
       ];
       libraryPkgconfigDepends = [ gmodule ];
       description = "GModule bindings";
       license = lib.licenses.lgpl21Only;
     }) {gmodule = null;};

because gmodule is null it doesn't find its dependency glib which is necessary for compiling. on second thought it should probably be commented

Copy link
Member

Choose a reason for hiding this comment

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

That is easily fixed properly in cabal2nix.

Copy link
Member

Choose a reason for hiding this comment

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

Done in bb62d57, you should be able to rebase and simplify this rebase.

@@ -214,4 +214,20 @@ in {
# failing during the Setup.hs phase: https://github.com/gtk2hs/gtk2hs/issues/323.
gtk2hs-buildtools = appendPatch ./patches/gtk2hs-buildtools-fix-ghc-9.4.x.patch super.gtk2hs-buildtools;

haskell-gi-base = __CabalEagerPkgConfigWorkaround super.haskell-gi-base;
Copy link
Member

Choose a reason for hiding this comment

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

Still should be moved to be grouped with the other __CabalEagerPkgConfigWorkaround overrides above.

@dark-ether
Copy link
Contributor Author

sterni could you make a similar commit for gi-harfbuzz? it receives harfbuzz-gobject as null

@Janik-Haag Janik-Haag added the 12. first-time contribution This PR is the author's first one; please be gentle! label Jun 12, 2023
@cdepillabout
Copy link
Member

This has been fixed in #240387, except for gi-adwaita, which appears to fail to build for unrelated reasons.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6.topic: haskell 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux 12. first-time contribution This PR is the author's first one; please be gentle!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants