Add presentation app Spice Up #34546

samdroid-apps · 2018-02-03T02:22:12Z

Motivation for this change

Spice Up is a nice presentations app: https://medium.com/elementaryos/appcenter-spotlight-spice-up-65b7c169b3bd

It required also updating the dependency granite derivation. The only other user of granite is pantheon.pantheon-terminal, which was also tested.

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.

Upstream has moved to GitHub, as the elementary.io page [1] now links to the GitHub repository. The only current dependant derivation is Pantheon Terminal (pantheon.pantheon-terminal), which continues to work. [1]: https://elementary.io/open-source

jtojnar · 2018-02-03T02:38:43Z

pkgs/applications/office/spice-up/default.nix

+, intltool
+, autoreconfHook
+, pkgconfig
+, libqalculate


jtojnar · 2018-02-03T02:38:47Z

pkgs/applications/office/spice-up/default.nix

+  patches = [ ./0001-Remove-environment-dependant-CMake-configuration.patch ];
+
+  # Add extra packages to the VAPI path
+  # There is no environment variable, so we must edit the CMakeLists.txt


Does not XDG_DATA_DIRS work?

yes, it does. I totally forgot that nix-build removes the environment variables

jtojnar · 2018-02-03T02:40:28Z

pkgs/development/libraries/granite/default.nix

  };
+
  cmakeFlags = "-DINTROSPECTION_GIRDIR=share/gir-1.0/ -DINTROSPECTION_TYPELIBDIR=lib/girepository-1.0";


Could you also please change this into an array, when at it.

jtojnar · 2018-02-03T02:46:44Z

pkgs/applications/office/spice-up/default.nix

+    gnome3.gnome_themes_standard
+  ];
+
+  enableParallelBuilding = true;


CMake builds in parallel by default: #32271

jtojnar · 2018-02-03T02:48:02Z

pkgs/applications/office/spice-up/default.nix

+
+  enableParallelBuilding = true;
+
+  meta = with stdenv.lib; {


Missing license.

jtojnar · 2018-02-03T02:55:41Z

pkgs/applications/office/spice-up/0001-Remove-environment-dependant-CMake-configuration.patch

+Date: Sat, 3 Feb 2018 13:16:16 +1100
+Subject: [PATCH] Remove environment dependant CMake configuration
+
+This is broken by Nix's CMake patching scripts


Do you have a reference for this? In the setup hook, I only see a borked path replacer.

yeah sorry that was bullshit

jtojnar · 2018-02-03T02:58:10Z

pkgs/applications/office/spice-up/default.nix

+  '';
+
+  nativeBuildInputs = [
+    intltool


gettext should be enough.

jtojnar · 2018-02-03T02:59:54Z

pkgs/applications/office/spice-up/default.nix

+    done
+  '';
+
+  nativeBuildInputs = [


XMLLINT not set and xmllint not found in path; skipping xml preprocessing.

adding libxml2 would be nice.

jtojnar · 2018-02-03T03:00:39Z

pkgs/development/libraries/granite/default.nix

-  buildInputs = [perl cmake vala gobjectIntrospection glib gtk3 gnome3.libgee gettext];
+  nativeBuildInputs = [ vala pkgconfig cmake perl ];
+  buildInputs = [ gobjectIntrospection glib gtk3 gnome3.libgee gettext ];
+
  meta = {


Maybe also add with stdenv.lib to clean it up a little.

jtojnar · 2018-02-03T03:02:23Z

pkgs/development/libraries/granite/default.nix

-  nativeBuildInputs = [ pkgconfig ];
-  buildInputs = [perl cmake vala gobjectIntrospection glib gtk3 gnome3.libgee gettext];
+  nativeBuildInputs = [ vala pkgconfig cmake perl ];
+  buildInputs = [ gobjectIntrospection glib gtk3 gnome3.libgee gettext ];


gettext and gobjectIntrospection are only needed during build.

jtojnar · 2018-02-03T03:13:04Z

pkgs/applications/office/spice-up/default.nix

+    pkgconfig
+    wrapGAppsHook
+    vala
+    cmake


cmake should be used along with ninja for faster builds.

samdroid-apps · 2018-02-05T04:17:49Z

Thanks for the review @jtojnar - I've force-pushed an update

jtojnar · 2018-02-05T06:32:49Z

pkgs/development/libraries/granite/default.nix

+  nativeBuildInputs = [
+    vala
+    pkgconfig
+    cmake


Ninja here as well.

jtojnar · 2018-02-05T06:34:42Z

pkgs/applications/office/spice-up/0001-Remove-environment-dependant-CMake-configuration.patch

+Date: Sat, 3 Feb 2018 13:16:16 +1100
+Subject: [PATCH] Remove environment dependant CMake configuration
+
+This is broken as nix unsets the $USER environment variable


Would setting the variable fix this? We should also set patch upstream, would quoting the variables work?

jtojnar · 2018-02-05T06:35:28Z

pkgs/applications/office/spice-up/default.nix

+
+  # Add extra packages to the VAPI path
+  # There is no environment variable, so we must edit the CMakeLists.txt
+  postPatch = ''


No longer necessary?

Err, so when testing without removing the postPatch, it is broken.

So vala uses the this function to lookup vapis in the path. It calls g_get_system_data_dirs (). That uses XDG spec:

$XDG_DATA_DIRS defines the preference-ordered set of base directories to search for data files in addition to the $XDG_DATA_HOME base directory. The directories in $XDG_DATA_DIRS should be **seperated with a colon ':'. **

So it was just broken because of a colon (whereas nix seems to default to space separated).

* move build time dependencies into nativeBuildInputs * convert cmakeFlags to an array * use `with` in the meta section * add `ninja`

samdroid-apps · 2018-02-09T11:42:02Z

@jtojnar thanks for the review again... this should fix your review issues. But it seems to break the icons; I'm not sure if that is inherited from my system environment or if that is an issue with the package.

jtojnar · 2018-02-09T12:57:30Z

Some icons (e.g. document-export-symbolic) do not seem to be available outside of elementary icon theme.

LGTM otherwise.

granite: 0.4.1 -> 0.5

0f15b33

Upstream has moved to GitHub, as the elementary.io page [1] now links to the GitHub repository. The only current dependant derivation is Pantheon Terminal (pantheon.pantheon-terminal), which continues to work. [1]: https://elementary.io/open-source

GrahamcOfBorg added 10.rebuild-darwin: 0 10.rebuild-linux: 1-10 labels Feb 3, 2018

jtojnar requested changes Feb 3, 2018

View reviewed changes

jtojnar reviewed Feb 3, 2018

View reviewed changes

samdroid-apps force-pushed the add-spice-up branch from b0138cb to 6c53aa2 Compare February 5, 2018 04:16

samdroid-apps force-pushed the add-spice-up branch from 6c53aa2 to 21665de Compare February 5, 2018 04:18

jtojnar reviewed Feb 5, 2018

View reviewed changes

samdroid-apps added 2 commits February 9, 2018 22:23

granite: refactor package

197d9a8

* move build time dependencies into nativeBuildInputs * convert cmakeFlags to an array * use `with` in the meta section * add `ninja`

spice-up: init at 1.2.1

8a7fd9d

samdroid-apps force-pushed the add-spice-up branch from 21665de to 8a7fd9d Compare February 9, 2018 11:37

jtojnar approved these changes Feb 9, 2018

View reviewed changes

Mic92 merged commit fdc9312 into NixOS:master Feb 10, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add presentation app Spice Up #34546

Add presentation app Spice Up #34546

samdroid-apps commented Feb 3, 2018

jtojnar Feb 3, 2018

jtojnar Feb 3, 2018

samdroid-apps Feb 5, 2018

jtojnar Feb 3, 2018

jtojnar Feb 3, 2018

jtojnar Feb 3, 2018

jtojnar Feb 3, 2018

samdroid-apps Feb 5, 2018

jtojnar Feb 3, 2018

jtojnar Feb 3, 2018

jtojnar Feb 3, 2018

jtojnar Feb 3, 2018

jtojnar Feb 3, 2018

samdroid-apps commented Feb 5, 2018

jtojnar Feb 5, 2018

jtojnar Feb 5, 2018

jtojnar Feb 5, 2018

samdroid-apps Feb 9, 2018

samdroid-apps commented Feb 9, 2018

jtojnar commented Feb 9, 2018

		};

		cmakeFlags = "-DINTROSPECTION_GIRDIR=share/gir-1.0/ -DINTROSPECTION_TYPELIBDIR=lib/girepository-1.0";

Add presentation app Spice Up #34546

Add presentation app Spice Up #34546

Conversation

samdroid-apps commented Feb 3, 2018

Motivation for this change

Things done

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

samdroid-apps commented Feb 5, 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

samdroid-apps commented Feb 9, 2018

jtojnar commented Feb 9, 2018