tracker-miners: switch to meson, some tweaks #39534

lukateras · 2018-04-26T09:54:40Z

Motivation for this change

After this commit, GNOME Music will properly display FLAC tracks.

Things done

Tested using sandboxing (nix.useSandbox on NixOS, or option build-use-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 · 2018-04-26T10:05:57Z

pkgs/desktops/gnome-3/core/tracker-miners/default.nix

@@ -51,6 +84,18 @@ in stdenv.mkDerivation rec {
    };
  };

+  postPatch = ''
+    substituteInPlace src/tracker-extract/10-flac.rule --replace audio/x-flac audio/flac


Cool, is this why GNOME Music does not see my FLAC music? Edit: Right, should have read the OP.

Could you add some comments to each of this replacements so we know why it is needed.

Apparently, this was done to prioritize GStreamer extractor: https://git.gnome.org/browse/tracker-miners/commit/?id=b830fdf96a97e6f37a25a4bb58106748c585d8e3

jtojnar · 2018-04-26T10:07:26Z

pkgs/desktops/gnome-3/core/tracker-miners/default.nix

@@ -24,18 +20,55 @@ in stdenv.mkDerivation rec {

  enableParallelBuilding = true;


This is not be needed with meson.

jtojnar · 2018-04-26T10:08:04Z

pkgs/desktops/gnome-3/core/tracker-miners/default.nix

+  '';
+
+  postFixup = ''
+    ${glib.dev}/bin/glib-compile-schemas $out/share/gsettings-schemas/${name}/glib-2.0/schemas


Schema compilation traditionally belongs to postInstall.

Moving to postInstall causes this error:

Error opening directory “/nix/store/xck0468mbapjd98bg9dvavwx5gfyw80c-tracker-miners-2.0.4/share/gsettings-schemas/tracker-miners-2.0.4/glib-2.0/schemas”: No such file or directory

So, this path doesn't exist until fixupPhase :-(

Yes, it is moved in preFixup by glib setup hook. You would use glib-compile-schemas $out/share/glib-2.0/schemas.

jtojnar · 2018-04-26T10:10:25Z

pkgs/desktops/gnome-3/core/tracker-miners/default.nix

+  postFixup = ''
+    ${glib.dev}/bin/glib-compile-schemas $out/share/gsettings-schemas/${name}/glib-2.0/schemas
+    wrapProgram $out/libexec/tracker-writeback \
+      --prefix LD_LIBRARY_PATH : $out/lib/tracker-miners-2.0


This is weird too. Why does meson not produce a correct rpath? Is there an upstream bug report?

I'm not sure why, but it doesn't. I don't think there is an upstream bug report.

It looks like tracker-writeback is missing install_rpath. Shall I open a bug?

Yes, thank you :-)

Bug: https://bugzilla.gnome.org/show_bug.cgi?id=795574
Patch: https://bugzilla.gnome.org/attachment.cgi?id=371423

jtojnar · 2018-04-26T10:12:43Z

pkgs/desktops/gnome-3/core/tracker-miners/default.nix

-    libtiff libuuid libvorbis libxml2 poppler taglib upower gexiv2
+  mesonFlags = [
+    "-Ddbus_services=share/dbus-1/services"
+    "-Dminer_rss=false" # needs libgrss


gif and jpeg seem to be missing in the new directory tree.

jtojnar · 2018-04-26T10:17:42Z

pkgs/desktops/gnome-3/core/tracker-miners/default.nix

-    json-glib libcue libexif libgsf libiptcdata libjpeg libpng libseccomp libsoup
-    libtiff libuuid libvorbis libxml2 poppler taglib upower gexiv2
+  mesonFlags = [
+    "-Ddbus_services=share/dbus-1/services"


This seems to produce dangling symlinks in share/tracker/miners/. We really need #37693 and ${placeholder "out"}. For now, we could use the horrendous

preConfigure = '' mesonFlagsArray+("-Ddbus_services=$out/share/dbus-1/services") '';

pkgs/desktops/gnome-3/core/tracker-miners/default.nix

@@ -51,6 +84,18 @@ in stdenv.mkDerivation rec {
    };


jtojnar · 2018-04-26T11:19:09Z

pkgs/desktops/gnome-3/core/tracker-miners/default.nix


  patches = [
    (substituteAll {
      src = ./fix-paths.patch;
      inherit (gnome3) tracker;
    })
+    (fetchurl {
+      url = https://bugzilla.gnome.org/attachment.cgi?id=371422;


Could you at least add a link to the tracking bug, see #39392

jtojnar · 2018-04-26T11:33:26Z

pkgs/desktops/gnome-3/core/tracker-miners/default.nix

@@ -51,11 +86,33 @@ in stdenv.mkDerivation rec {
    };
  };

+  postPatch = ''
+    # audio/flac is the proper MIME type for FLAC audio (fixes GNOME Music):
+    substituteInPlace src/tracker-extract/10-flac.rule --replace audio/x-flac audio/flac


It might make more sense to investigate why the GStreamer extractor does not work. Maybe we need to add gst-plugins-good or something.

This incorrect MIME type still is an upstream bug.

But intentional one, see https://git.gnome.org/browse/tracker-miners/commit/?id=b830fdf96a97e6f37a25a4bb58106748c585d8e3.

This doesn't make any sense :-) libflac extractor does work, and this commit deliberately breaks it. Would make more sense to put it behind a switch.

Well, GStreamer extractor is supposed to work better according to the commit. If we manage to fix the bug, that is.

Maybe open an upstream bug about this as well.

jtojnar · 2018-04-26T11:34:32Z

pkgs/desktops/gnome-3/core/tracker-miners/default.nix

+
+    # bug (typo) in upstream meson.build
+    substituteInPlace src/tracker-extract/meson.build \
+      --replace tracker_common_dep tracker_miners_common_dep


Could you open an upstream bug with this?

jtojnar · 2018-04-26T11:35:41Z

pkgs/desktops/gnome-3/core/tracker-miners/default.nix

+  '';
+
+  preConfigure = ''
+    mesonFlagsArray+=("-Ddbus_services=$out/share/dbus-1/services")


Maybe add todo comment so that we do not forget to replace it with placeholder when they are available.

jtojnar · 2018-04-26T11:36:48Z

pkgs/desktops/gnome-3/core/tracker-miners/default.nix


  patches = [
    (substituteAll {
      src = ./fix-paths.patch;
      inherit (gnome3) tracker;
    })
+    # https://github.com/NixOS/nixpkgs/issues/39392


Hmm, I meant https://bugzilla.gnome.org/show_bug.cgi?id=795573.

jtojnar · 2018-04-26T11:52:41Z

pkgs/desktops/gnome-3/core/tracker-miners/default.nix


-  nativeBuildInputs = [ vala pkgconfig intltool itstool libxslt wrapGAppsHook ];
-  # TODO: add libgrss, libenca, libosinfo
+  # TODO: add libgrss, libenca


What happened to libosinfo?

Oversight, I'm not sure why I didn't have that.

jtojnar · 2018-04-26T11:53:40Z

pkgs/desktops/gnome-3/core/tracker-miners/default.nix


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

-  NIX_CFLAGS_COMPILE = "-I${poppler.dev}/include/poppler";
+  LIBRARY_PATH = stdenv.lib.makeLibraryPath [ giflib ];


I do not like this, how did this work before? Do we need to open an upstream bug?

It seems that is an issue with our Meson hooks. We should check how autoconf AC_CHECK_LIB handles this.

jtojnar · 2018-04-26T11:55:01Z

pkgs/desktops/gnome-3/core/tracker-miners/default.nix

+      --replace tracker_common_dep tracker_miners_common_dep
+
+    # fix libjpeg discovery
+    substituteInPlace meson.build --replace "cc.find_library('libjpeg'" "dependency('libjpeg'"


Bah, the same thing as gif.

This should probably be upstreamed. If I put libjpeg on LIBRARY_PATH, build fails with exit code 1.

After this commit, GNOME Music should identify FLAC tracks.

lukateras · 2018-04-26T12:30:57Z

Thanks a lot for your thorough review! I've sent all patches upstream. LIBRARY_PATH workaround still remains, but I've opened #39547 and linked to it.

jtojnar · 2018-04-26T13:53:07Z

pkgs/desktops/gnome-3/core/tracker-miners/default.nix

  ];

-  LANG = "en_US.UTF-8"; # for running tests
-
-  doCheck = true;


What happened to the tests?

See libdazzle for an example how to run tests under dbus.

Actually, adding

for f in tests/functional-tests/*.py; do patchShebangs "$f" done

to postPatch might be enough. We need to force python2, though.

Opened an issue about it, we can probably ignore the tests for now https://bugzilla.gnome.org/show_bug.cgi?id=795594

jtojnar · 2018-04-27T02:38:47Z

$out/share/tracker/miners/org.freedesktop.Tracker1.Miner.RSS.service probably should not be installed.

lukateras · 2018-04-27T10:24:12Z

Probably (should be considered a minor upstream bug?).

jtojnar · 2018-05-15T16:57:04Z

Could you please report it and add a comment with the link?

lukateras · 2018-05-15T17:41:15Z

Done. Can I squash and merge this?

lukateras requested a review from jtojnar April 26, 2018 09:54

GrahamcOfBorg added 6.topic: GNOME GNOME desktop environment and its underlying platform 10.rebuild-darwin: 0 10.rebuild-linux: 11-100 labels Apr 26, 2018

jtojnar requested changes Apr 26, 2018

View reviewed changes

jtojnar reviewed Apr 26, 2018

View reviewed changes

pkgs/desktops/gnome-3/core/tracker-miners/default.nix

@@ -51,6 +84,18 @@ in stdenv.mkDerivation rec {

};

This comment was marked as resolved.

Sign in to view

This comment was marked as resolved.

Sign in to view

lukateras force-pushed the 20180426.094748/tracker-miners branch from 0158095 to 7c5c4a3 Compare April 26, 2018 11:12

jtojnar reviewed Apr 26, 2018

View reviewed changes

lukateras force-pushed the 20180426.094748/tracker-miners branch from 7c5c4a3 to 1af1ac1 Compare April 26, 2018 11:29

jtojnar reviewed Apr 26, 2018

View reviewed changes

lukateras force-pushed the 20180426.094748/tracker-miners branch from 1af1ac1 to 9a9aaf0 Compare April 26, 2018 11:42

jtojnar reviewed Apr 26, 2018

View reviewed changes

lukateras force-pushed the 20180426.094748/tracker-miners branch 3 times, most recently from cda6902 to 10ef5ac Compare April 26, 2018 12:22

lukateras mentioned this pull request Apr 26, 2018

meson: cc.find_library doesn't work #39547

Closed

tracker-miners: switch to meson, some tweaks

b67ff94

After this commit, GNOME Music should identify FLAC tracks.

lukateras force-pushed the 20180426.094748/tracker-miners branch from 10ef5ac to b67ff94 Compare April 26, 2018 12:30

lukateras mentioned this pull request Apr 26, 2018

gnome.tracker-miners: GStreamer extractor doesn't work #39545

Open

jtojnar reviewed Apr 26, 2018

View reviewed changes

Update default.nix

f6cfa77

Update default.nix

728013a

jtojnar approved these changes May 15, 2018

View reviewed changes

lukateras merged commit edbce18 into NixOS:master May 15, 2018

		@@ -24,18 +20,55 @@ in stdenv.mkDerivation rec {

		enableParallelBuilding = true;

tracker-miners: switch to meson, some tweaks #39534

tracker-miners: switch to meson, some tweaks #39534

Conversation

lukateras commented Apr 26, 2018 • edited

Motivation for this change

Things done

jtojnar Apr 26, 2018 • edited

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment was marked as resolved.

This comment was marked as resolved.

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lukateras Apr 26, 2018 • edited

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

jtojnar Apr 26, 2018 • edited

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

lukateras Apr 26, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lukateras commented Apr 26, 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

jtojnar commented Apr 27, 2018

lukateras commented Apr 27, 2018

jtojnar commented May 15, 2018

lukateras commented May 15, 2018

lukateras commented Apr 26, 2018 •

edited

jtojnar Apr 26, 2018 •

edited

lukateras Apr 26, 2018 •

edited

jtojnar Apr 26, 2018 •

edited

lukateras Apr 26, 2018 •

edited