add gtk3 as propagatedBuildInput to vte #44803

cdepillabout · 2018-08-09T03:03:44Z

Motivation for this change

Vte.2.91.gir imports Gtk.3.0.gir. So in order to use Vte.2.91.gir, you must also have Gtk.3.0.gir on the GI_TYPELIB_PATH. Adding gtk3 to the propagatedBuildInputs of vte accomplishes this.

This affects the the gi-vte Haskell package, which currently only depends on vte but not gtk. gi-vte does not currently build on NixOS, and this PR fixes one of the reasons. Here is the current definition of gi-vte for reference:

  "gi-vte" = callPackage
    ({ mkDerivation, base, bytestring, Cabal, containers, gi-atk
     , gi-gdk, gi-gio, gi-glib, gi-gobject, gi-gtk, gi-pango, haskell-gi
     , haskell-gi-base, haskell-gi-overloading, text, transformers, vte
     }:
     mkDerivation {
       pname = "gi-vte";
       version = "2.91.18";
       sha256 = "0rixrkw0k2vz59y20lsd8zw54n7l069mij0n76dnmah2bjjk1r7w";
       setupHaskellDepends = [ base Cabal haskell-gi ];
       libraryHaskellDepends = [
         base bytestring containers gi-atk gi-gdk gi-gio gi-glib gi-gobject
         gi-gtk gi-pango haskell-gi haskell-gi-base haskell-gi-overloading
         text transformers
       ];
       libraryPkgconfigDepends = [ vte ];
       doHaddock = false;
       description = "Vte bindings";
       license = stdenv.lib.licenses.lgpl21;
       hydraPlatforms = stdenv.lib.platforms.none;
     }) {inherit (pkgs.gnome3) vte;};

Merging in this PR will help gi-vte build.

This fix is exactly the same as the fix for other Gnome packages, for instance gtksourceview:

nixpkgs/pkgs/development/libraries/gtksourceview/3.x.nix

Lines 15 to 20 in 42eca7c

    
           propagatedBuildInputs = [ 
        
             # Required by gtksourceview-3.0.pc 
        
             gtk3 
        
             # Used by gtk_source_language_manager_guess_language 
        
             shared-mime-info 
        
           ];

This was discussed on the following issue:

haskell-gi/haskell-gi#183

That issue was inspired by the following PR:

#44529

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.

The `Vte.2.91.gir` imports `Gtk.3.0.gir`. So in order to use `Vte.2.91.gir`, you must also have `Gtk.3.0.gir` on the GI_TYPELIB_PATH. Adding gtk3 to the `propagatedBuildInputs` of vte accomplishes this.

jtojnar · 2018-08-09T09:58:57Z

pkgs/desktops/gnome-3/core/vte/default.nix

@@ -19,7 +19,12 @@ stdenv.mkDerivation rec {
  nativeBuildInputs = [ gobjectIntrospection intltool pkgconfig vala gperf libxml2 ];
  buildInputs = [ gnome3.glib gnome3.gtk3 ncurses ];

-  propagatedBuildInputs = [ gnutls pcre2 ];
+  propagatedBuildInputs = [
+    # Needed because Vte-2.91.gir depends on Gtk-3.0.gir


I think it is required by vte-2.91.pc, not the gir (at least the gnutls).

On a side note, it is weird that pcre is used instead of the regexes from GLib but it does:

$ rg '#include <pcre' src/vtepcre2.h 21:#include <pcre2.h>

Hi @jtojnar, thanks for reviewing this.

The comment "Needed because Vte-2.91.gir depends on Gtk-3.0.gir" might be a little misleading. I only meant it to apply to the gnome3.gtk dependency.

Since the comment is a little misleading, I will take it out.

I think it is required by vte-2.91.pc, not the gir.

The Vte-2.91.gir file is importing the GDK and GTK gir files, as explained here. If some package tries to make use of Vte-2.91.gir, they will also necessarily need the GDK and GTK gir files.

Adding gnome3.gtk3 as a propagatedBuildInput for VTE accomplishes this.

I'm not sure whether the vte-2.91.pc file requires the GTK .pc file as well. This didn't affect any of the stuff I was doing (and to be honest I'm not sure exactly how the .pc files work). Although I can totally buy that the vte .pc file depends on the GTK .pc file.

As for gnutls and pcre2, I'm not sure why they are in the propagatedBuildInputs. They were already there, so I just left them there. I would be okay with taking them out if they are not actually needed, but I think this should happen in a future PR, since it is not really related to the reason I opened this PR.

Yeah, they are required by the .pc file:

$ nix-shell -p gnome3.vte gnome3.gtk pkgconfig --run 'pkg-config --print-requires vte-2.91' glib-2.0 >= 2.40.0 gobject-2.0 pango >= 1.22.0 gtk+-3.0 >= 3.8.0 gobject-2.0 gio-2.0 gio-unix-2.0 zlib libpcre2-8 >= 10.21 gnutls >= 3.2.7

Please change the comment to .pc file like in gtksourceview.

I think that would make the comment somewhat misleading, since GTK appears to be required by both the .gir file and the .pc file.

Although I'm much more interested in getting this merged in than having the best comment.

What do you recommend is the best way forward?

Leave it like it is (no comment as implemented in commit 5241c5a)

Add a comment explaining that the .pc file requires all three libraries (gtk, gnutls, and pcre2)

Add a comment explaining that the .pc file requires all three libraries, and the .gir file requires just gtk.

I would choose the second variant. After all, the goal is to distinguish the dependencies required to use the package from those only used by the package and not required for the consumers to have – the second already tells that. The third would be also nice but I feel it does not add enough new information to justify the extra clutter.

@jtojnar Okay, this sounds good. I'll update this PR.

cdepillabout · 2018-08-09T13:45:17Z

@jtojnar Commit 5241c5a98db removes the somewhat confusing comment.

Would you be able to take another look at this?

cdepillabout · 2018-08-09T14:24:01Z

@jtojnar Thanks again for taking a look at this.

In commit 05c0c3f I got rid of the comment about the .gir file and replaced it with a comment about the .pc file.

Does this look alright?

vte: Add gtk3 as propagatedBuildInput

8edc67c

The `Vte.2.91.gir` imports `Gtk.3.0.gir`. So in order to use `Vte.2.91.gir`, you must also have `Gtk.3.0.gir` on the GI_TYPELIB_PATH. Adding gtk3 to the `propagatedBuildInputs` of vte accomplishes this.

GrahamcOfBorg added 6.topic: GNOME GNOME desktop environment and its underlying platform 10.rebuild-darwin: 1-10 10.rebuild-linux: 11-100 labels Aug 9, 2018

This was referenced Aug 9, 2018

Fix gi vte haskell pkg #44529

Merged

problem with gi-vte in nixos: gi-vte does have a pkgconfig depend on gtk+-3.0 haskell-gi/haskell-gi#183

Closed

Mic92 requested a review from jtojnar August 9, 2018 05:21

jtojnar reviewed Aug 9, 2018

View reviewed changes

vte: Change comment to talk about vte-2.91.pc file instead of .gir file.

05c0c3f

cdepillabout force-pushed the add-correct-propagatedbuildinput-to-vte branch from 5241c5a to 05c0c3f Compare August 9, 2018 14:22

jtojnar approved these changes Aug 9, 2018

View reviewed changes

jtojnar merged commit 59186af into NixOS:master Aug 9, 2018

cdepillabout deleted the add-correct-propagatedbuildinput-to-vte branch August 10, 2018 01:27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add gtk3 as propagatedBuildInput to vte #44803

add gtk3 as propagatedBuildInput to vte #44803

cdepillabout commented Aug 9, 2018 •

edited

jtojnar Aug 9, 2018

cdepillabout Aug 9, 2018

jtojnar Aug 9, 2018

cdepillabout Aug 9, 2018 •

edited

jtojnar Aug 9, 2018

cdepillabout Aug 9, 2018

cdepillabout commented Aug 9, 2018

cdepillabout commented Aug 9, 2018

	propagatedBuildInputs = [
	# Required by gtksourceview-3.0.pc
	gtk3
	# Used by gtk_source_language_manager_guess_language
	shared-mime-info
	];

add gtk3 as propagatedBuildInput to vte #44803

add gtk3 as propagatedBuildInput to vte #44803

Conversation

cdepillabout commented Aug 9, 2018 • edited

Motivation for this change

Things done

jtojnar Aug 9, 2018

Choose a reason for hiding this comment

cdepillabout Aug 9, 2018

Choose a reason for hiding this comment

jtojnar Aug 9, 2018

Choose a reason for hiding this comment

cdepillabout Aug 9, 2018 • edited

Choose a reason for hiding this comment

jtojnar Aug 9, 2018

Choose a reason for hiding this comment

cdepillabout Aug 9, 2018

Choose a reason for hiding this comment

cdepillabout commented Aug 9, 2018

cdepillabout commented Aug 9, 2018

cdepillabout commented Aug 9, 2018 •

edited

cdepillabout Aug 9, 2018 •

edited