Gnome-latex: init at 3.28.1 #42325

manveru · 2018-06-21T11:10:37Z

Motivation for this change

Package request in #42261

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/)
Fits CONTRIBUTING.md.

jtojnar

Thank you for taking time to package this. Except for minor issues this looks quite good. One thing to consider, though, is how will the application find LaTeX tools and packages.

jtojnar · 2018-06-21T11:36:13Z

pkgs/development/libraries/amtk/default.nix

+in stdenv.mkDerivation {
+  name = "amtk-${version}";
+  src = fetchTarball {
+    url = "https://gitlab.gnome.org/GNOME/amtk/-/archive/${version}/amtk-${version}.tar.bz2";


You should probably use mirror://gnome/sources/amtk/${gnome3.versionBranch version}/${name}.tar.xz, since it will be served by geographically distributed mirrors.

Also sorry about misinformation, we now have stdenv.lib.versions.majorMinor which should be preferred.

jtojnar · 2018-06-21T11:37:41Z

pkgs/development/libraries/amtk/default.nix

+
+  configurePhase = ''
+    mkdir -p $out
+    ./autogen.sh --prefix=$out


Usually, you would use preConfigure = ''NOCONFIGURE=1 ./autogen.sh''; and let the generic builder take care of the rest.

jtojnar · 2018-06-21T11:38:04Z

pkgs/development/libraries/amtk/default.nix

+  '';
+
+  meta = with stdenv.lib; {
+    homepage = "https://wiki.gnome.org/Projects/Amtk";


No need to quote simple URLs.

jtojnar · 2018-06-21T11:39:45Z

pkgs/development/libraries/amtk/default.nix

+    gobjectIntrospection
+    automake
+    intltool
+    gnome2.gtkdoc


Use top-level gtk-doc, this is just an alias.

jtojnar · 2018-06-21T11:42:51Z

pkgs/applications/editors/gnome-latex/default.nix

+    sha256 = "17rbwg0v3m0myl797vrynk7wpz1fysp5px45bl7l4fpqwan2vgv8";
+  };
+
+  NIX_CFLAGS_COMPILE = "-I${glib.dev}/include/gio-unix-2.0";


This should be reported as per #36468

jtojnar · 2018-06-21T11:43:47Z

pkgs/applications/editors/gnome-latex/default.nix

+  '';
+
+  preFixup = ''
+    gappsWrapperArgs+=(--prefix XDG_DATA_DIRS : $out/share:$XDG_ICON_DIRS:$GSETTINGS_SCHEMAS_PATH)


This should already be a part of wrapGAppsHook.

jtojnar · 2018-06-21T11:45:04Z

pkgs/applications/editors/gnome-latex/default.nix

+  ];
+
+  buildInputs = [
+    vala


The following should go to nativeBuildInputs: vala, appstream-glib, gobjectIntrospection, gnome2.gtkdoc, gnome3.yelp-tools, itstool

jtojnar · 2018-06-21T11:46:50Z

pkgs/development/libraries/tepl/default.nix

+    mkdir -p $out
+    ./autogen.sh --prefix=$out --disable-Werror
+  '';
+


Could you also add updateScripts as per #36150

jtojnar · 2018-06-21T11:49:43Z

pkgs/development/libraries/amtk/default.nix

+    mkdir -p $out
+    ./autogen.sh --prefix=$out
+  '';
+


This package seems to have tests, could you add doCheck = true (or doCheck = false with a TODO comment if they fail).

manveru · 2018-06-21T12:36:07Z

I already discussed in with @Mic92 about the issue of finding mklatex, but it seems like a common thing to leave providing that binary to the user, since we don't want to decide for them which latex distribution they want to use.

Some other packages provide a texLive argument where you can pass it in, but since we don't actually need it for building it seemed overkill.

manveru · 2018-06-21T12:42:27Z

two more things that worry me are these warnings:

(gnome-latex:26858): Gtk-WARNING **: 14:41:48.496: Locale not supported by C library.
	Using the fallback 'C' locale.

(gnome-latex:26858): dconf-WARNING **: 14:41:48.524: failed to commit changes to dconf: GDBus.Error:org.freedesktop.DBus.Error.ServiceUnknown: The name ca.desrt.dconf was not provided by any .service files

Any idea what that means?

jtojnar · 2018-06-21T12:47:40Z

The latter is caused by missing dconf service. You would need to add dconf to environment.systemPackages.

manveru · 2018-06-21T12:51:48Z

OK, thanks a lot for your help, your suggestions made this a whole lot cleaner (and apparently switching to the mirror got rid of the autotools dependency)

jtojnar · 2018-06-21T13:50:48Z

pkgs/applications/editors/gnome-latex/default.nix

+    sha256 = "1z481izrx057wraphnr82kxnpmmi8nvl7jswyylzm22kfs0mw402";
+  };
+
+  passthru.updateScript = gnome3.updateScript { packageName = pname; };


Could you move this down before meta? We are trying to structure the expressions so that building instructions are first and meta things are last.

jtojnar · 2018-06-21T13:53:45Z

pkgs/development/libraries/tepl/default.nix

+  passthru.updateScript = gnome3.updateScript { packageName = pname; };
+
+  nativeBuildInputs = [
+    amtk


Should these be in native inputs?

I really don't know the difference between buildInputs and nativeBuildInputs... i can try shuffling them around but it seems to build either way.

The difference occurs during cross-compilation. nativeBuildInputs use the architecture of the builder (build platform), while buildInputs use the architecture of the host platform (the one the resulting program will be run). That means programs run during build (build managers, compilers, asset generators) need to go to nativeBuildInputs.

jtojnar · 2018-06-21T13:55:41Z

pkgs/development/libraries/amtk/default.nix

+  ];
+
+  doCheck = false;
+  # TODO: seems like tests need a display:


Maybe running those in xvfb would help:

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

Line 36 in f7397f2

xvfb-run -s '-screen 0 800x600x24' dbus-run-session \

seems to fail with this:

FAIL: test-file-metadata ======================== /file/get_set_metadata: OK /file/load_save_metadata_sync: (./test-file-metadata:20931): Tepl-WARNING **: 14:41:36.942: GVfs metadata is not supported. Fallback to TeplMetadataManager. Either GVfs is not correctly installed or GVfs metadata are not supported on this platform. In the latter case, you should configure Tepl with --disable-gvfs-metadata. FAIL test-file-metadata (exit status: 133)

I'll try to find a solution

amtk tests work with this trick though, so that's also nice

We are building on tmpfs so some file attributes might not work. Probably nothing we can do about it.

jtojnar · 2018-06-21T14:01:01Z

pkgs/development/libraries/amtk/default.nix

+    homepage = https://wiki.gnome.org/Projects/Amtk;
+    description = "Actions, Menus and Toolbars Kit for GTK+ applications";
+    maintainers = [ maintainers.manveru ];
+    license = licenses.lgpl21;


That would be lgpl21Plus: https://gitlab.gnome.org/GNOME/amtk/blob/master/amtk/amtk-action-map.c#L8
The COPYING file usually contains only the text of the lower bound, since including the text of later versions is not possible.

manveru · 2018-06-21T19:59:03Z

I think that's about all I can fix... couldn't make 100% sure the buildInputs/nativeBuildInputs work because my cross-compilation failed for other reasons, but hopefully it's better now.

jtojnar · 2018-06-21T20:04:49Z

pkgs/applications/editors/gnome-latex/default.nix

+
+  buildInputs = with gnome3; [
+    amtk
+    dconf.dev


Why are you specifically requesting dconf.dev? Actually, we should probably build with --disable-dconf-migration as we did not have the package before so there are no old GSettings paths to migrate.

jtojnar · 2018-06-21T20:07:05Z

pkgs/development/libraries/amtk/default.nix

+  ];
+
+  buildInputs = [
+    dbus.daemon


I think this would go to nativeBuildInputs, as dbus-run-session is run by the build platform. Also the binary is not part of dbus.daemon but dbus.

jtojnar · 2018-06-21T20:07:26Z

pkgs/development/libraries/amtk/default.nix

+
+  buildInputs = [
+    dbus.daemon
+    gnome3.gtk.dev


Why specifically request dev output?

That's what I found when looking for the missing file using nix-locate

I think we usually do not use dev outputs, since this will probably also use a library that will be in a different output (out). It works because the builder somehow inserts other outputs as well or something.

jtojnar · 2018-06-21T22:19:52Z

pkgs/applications/editors/gnome-latex/default.nix

+  name = "${pname}-${version}";
+
+  src = fetchurl {
+    url = "mirror://gnome/sources/${pname}/${gnome3.versionBranch version}/${pname}-${version}.tar.xz";


Sorry about misinformation, we now have stdenv.lib.versions.majorMinor which should be preferred.

jtojnar · 2018-06-21T22:21:07Z

pkgs/applications/editors/gnome-latex/default.nix

+  passthru.updateScript = gnome3.updateScript { packageName = pname; };
+
+  meta = with stdenv.lib; {
+    homepage = "https://wiki.gnome.org/Apps/GNOME-LaTeX";


You can remove quotes here as well.

manveru · 2018-06-22T08:15:32Z

One more update ;)

jtojnar requested changes Jun 21, 2018

View reviewed changes

manveru force-pushed the add-gnome-latex branch from eeecb63 to 4502339 Compare June 21, 2018 12:50

GrahamcOfBorg added 8.has: package (new) 10.rebuild-darwin: 0 10.rebuild-linux: 1-10 labels Jun 21, 2018

jtojnar reviewed Jun 21, 2018

View reviewed changes

manveru force-pushed the add-gnome-latex branch from 4502339 to 30ed00b Compare June 21, 2018 18:41

jtojnar reviewed Jun 21, 2018

View reviewed changes

Gnome-latex: init at 3.28.1

6f831a7

manveru force-pushed the add-gnome-latex branch from 30ed00b to 6f831a7 Compare June 22, 2018 08:14

jtojnar merged commit 0463c20 into NixOS:master Jun 25, 2018

manveru deleted the add-gnome-latex branch July 6, 2018 07:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Gnome-latex: init at 3.28.1 #42325

Gnome-latex: init at 3.28.1 #42325

manveru commented Jun 21, 2018 •

edited

jtojnar left a comment

jtojnar Jun 21, 2018

jtojnar Jun 21, 2018

jtojnar Jun 21, 2018

jtojnar Jun 21, 2018

jtojnar Jun 21, 2018

jtojnar Jun 21, 2018

jtojnar Jun 21, 2018

jtojnar Jun 21, 2018

jtojnar Jun 21, 2018

jtojnar Jun 21, 2018

manveru commented Jun 21, 2018

manveru commented Jun 21, 2018

jtojnar commented Jun 21, 2018

manveru commented Jun 21, 2018

jtojnar Jun 21, 2018

jtojnar Jun 21, 2018

manveru Jun 21, 2018

jtojnar Jun 21, 2018

jtojnar Jun 21, 2018

manveru Jun 21, 2018

manveru Jun 21, 2018

jtojnar Jun 21, 2018

jtojnar Jun 21, 2018

manveru commented Jun 21, 2018

jtojnar Jun 21, 2018

jtojnar Jun 21, 2018

jtojnar Jun 21, 2018

manveru Jun 21, 2018

jtojnar Jun 21, 2018

jtojnar Jun 21, 2018

jtojnar Jun 21, 2018

manveru commented Jun 22, 2018

Gnome-latex: init at 3.28.1 #42325

Gnome-latex: init at 3.28.1 #42325

Conversation

manveru commented Jun 21, 2018 • edited

Motivation for this change

Things done

jtojnar left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

manveru commented Jun 21, 2018

manveru commented Jun 21, 2018

jtojnar commented Jun 21, 2018

manveru commented Jun 21, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

manveru commented Jun 21, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

manveru commented Jun 22, 2018

manveru commented Jun 21, 2018 •

edited