yabar: minor derivation improvements #29184

Ma27 · 2017-09-10T09:26:53Z

Motivation for this change

uses hooks for pre/post installation
provides make flags rather than a direct make call
adds myself as maintainer
updates the package

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
- Linux
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.

mention-bot · 2017-09-10T09:26:54Z

@Ma27, thanks for your PR! By analyzing the history of the files in this pull request, we identified @hiberno and @globin to be potential reviewers.

rycee · 2017-09-10T18:23:59Z

pkgs/applications/window-managers/yabar/default.nix


 stdenv.mkDerivation rec {
  name    = "yabar-${version}";
-  version = "0.4.0";
+  version = "2017-09-09";


I'd suggest introducing a package yabar-unstable that tracks the git version and leave the existing yabar package at the released version.

(at least if upstream plan to make future stable releases)

There is an open discussion about it (geommer/yabar#136)

As there's no activity, I think it's indeed the better approach to add an unstable.nix.

rycee · 2017-09-10T18:45:57Z

pkgs/applications/window-managers/yabar/default.nix

+    ${lib.optionalString (configFile != null)
+      ''
+        wrapProgram "$out/bin/yabar" \
+          --add-flags "-c ${configFile}"


It's a bit unusual to make command flags configurable in the package derivation, looking at the yabar man page it seems to read ~/.config/yabar/yabar.conf by default so the -c flag is not mandatory.

why though? You might want to configure the configuration file as a derivation (I do this ATM) and then it's placed in ~/.nix-profile for instance.

Fair enough, I don't have any strong feelings about it. The main downside I can think of is that it makes it tricky to use the binary cache when having a non-null configFile value.

rycee · 2017-09-11T12:26:22Z

Looking good now :-) The final thing would be to amend the "yabar: update to 2017-09-09" commit to something like "yabar: cleanup package" and remove the version = "2017-09-09"; change (since it is now actually done in the "yabar: add unstable package" commit.

Ma27 · 2017-09-11T16:09:20Z

you're right, I have to clean up the history a bit :p

The latest changes from `yabar` require several changes in the derivation to build the package successfully.

rycee · 2017-09-11T17:00:10Z

pkgs/applications/window-managers/yabar/default.nix

-    platforms   = platforms.linux;
-  };
-}
+  rev = "d3934344ba27f5bdf122bf74daacee6d49284dab";


Are you sure you want the same rev for 0.4.0 and unstable?

I think you want rev = version;.

Sorry, just noticed you had changed it. I still think it is a good idea to use rev = version; to avoid future mistakes :-)

rycee · 2017-09-11T17:01:10Z

Tried building and get some error:

$ nix-build -A yabar-unstable
these derivations will be built:
  /nix/store/g0lsbyzj51if2n7kskg7wh2xilpkrlrh-yabar-unstable-2017-09-09.drv
building path(s) ‘/nix/store/y75vx5zp8n98yl1hfgxhr6m47hhc56bk-yabar-unstable-2017-09-09’
unpacking sources
unpacking source archive /nix/store/9cjz85ir9b2zq7df4d08yn2kb79srqg0-yabar-d3934344ba27f5bdf122bf74daacee6d49284dab-src
source root is yabar-d3934344ba27f5bdf122bf74daacee6d49284dab-src
patching sources
configuring
no configure script, doing nothing
building
build flags: SHELL=/nix/store/nm9h53kd0ngh28wm42rivh2lc2d69nzl-bash-4.4-p12/bin/bash DESTDIR=\$\(out\) PREFIX=/
gcc -std=c99 -Iinclude -pedantic -Wall -flto -O2 `pkg-config --cflags pango pangocairo libconfig gdk-pixbuf-2.0 alsa` -DVERSION=\"unstable-2017-09-09\" -D_POSIX_C_SOURCE=199309L -DYA_INTERNAL -DYA_DYN_COL -DYA_ENV_VARS -DYA_INTERNAL_EWMH -DYA_ICON -DYA_NOWIN_COL -DYA_MUTEX -DYA_VAR_WIDTH -DYA_BSPWM  -c -o src/ya_parse.o src/ya_parse.c
gcc -std=c99 -Iinclude -pedantic -Wall -flto -O2 `pkg-config --cflags pango pangocairo libconfig gdk-pixbuf-2.0 alsa` -DVERSION=\"unstable-2017-09-09\" -D_POSIX_C_SOURCE=199309L -DYA_INTERNAL -DYA_DYN_COL -DYA_ENV_VARS -DYA_INTERNAL_EWMH -DYA_ICON -DYA_NOWIN_COL -DYA_MUTEX -DYA_VAR_WIDTH -DYA_BSPWM  -c -o src/ya_exec.o src/ya_exec.c
gcc -std=c99 -Iinclude -pedantic -Wall -flto -O2 `pkg-config --cflags pango pangocairo libconfig gdk-pixbuf-2.0 alsa` -DVERSION=\"unstable-2017-09-09\" -D_POSIX_C_SOURCE=199309L -DYA_INTERNAL -DYA_DYN_COL -DYA_ENV_VARS -DYA_INTERNAL_EWMH -DYA_ICON -DYA_NOWIN_COL -DYA_MUTEX -DYA_VAR_WIDTH -DYA_BSPWM  -c -o src/ya_draw.o src/ya_draw.c
gcc -std=c99 -Iinclude -pedantic -Wall -flto -O2 `pkg-config --cflags pango pangocairo libconfig gdk-pixbuf-2.0 alsa` -DVERSION=\"unstable-2017-09-09\" -D_POSIX_C_SOURCE=199309L -DYA_INTERNAL -DYA_DYN_COL -DYA_ENV_VARS -DYA_INTERNAL_EWMH -DYA_ICON -DYA_NOWIN_COL -DYA_MUTEX -DYA_VAR_WIDTH -DYA_BSPWM  -c -o src/ya_main.o src/ya_main.c
gcc -std=c99 -Iinclude -pedantic -Wall -flto -O2 `pkg-config --cflags pango pangocairo libconfig gdk-pixbuf-2.0 alsa` -DVERSION=\"unstable-2017-09-09\" -D_POSIX_C_SOURCE=199309L -DYA_INTERNAL -DYA_DYN_COL -DYA_ENV_VARS -DYA_INTERNAL_EWMH -DYA_ICON -DYA_NOWIN_COL -DYA_MUTEX -DYA_VAR_WIDTH -DYA_BSPWM  -c -o src/intern_blks/ya_intern.o src/intern_blks/ya_intern.c
gcc -flto -O2 -o yabar src/ya_parse.o src/ya_exec.o src/ya_draw.o src/ya_main.o src/intern_blks/ya_intern.o -liw -lxcb -lpthread -lxcb-randr -lxcb-ewmh -lxcb-icccm -lm `pkg-config --libs pango pangocairo libconfig gdk-pixbuf-2.0 alsa`
/nix/store/nagij3p85d7f8z9fzhz4aygls264b7wx-asciidoc-8.6.9/bin/a2x --no-xmllint --doctype manpage --format manpage "doc/yabar.1.asciidoc"
a2x: ERROR: "xsltproc"  --stringparam callout.graphics 0 --stringparam navig.graphics 0 --stringparam admon.textlabel 1 --stringparam admon.graphics 0  "/nix/store/nagij3p85d7f8z9fzhz4aygls264b7wx-asciidoc-8.6.9/etc/asciidoc/docbook-xsl/manpage.xsl" "/tmp/nix-build-yabar-unstable-2017-09-09.drv-0/yabar-d3934344ba27f5bdf122bf74daacee6d49284dab-src/doc/yabar.1.xml" returned non-zero exit status 5
make: *** [Makefile:32: yabar.1] Error 1
builder for ‘/nix/store/g0lsbyzj51if2n7kskg7wh2xilpkrlrh-yabar-unstable-2017-09-09.drv’ failed with exit code 2
error: build of ‘/nix/store/g0lsbyzj51if2n7kskg7wh2xilpkrlrh-yabar-unstable-2017-09-09.drv’ failed

rycee · 2017-09-11T21:15:18Z

@Ma27 I tried adding docbook_xsl to buildInputs and then a2x can build the documentation OK:

$ nix-build --check -A yabar-unstable
checking path(s) ‘/nix/store/w6mq8gw6kbs50y5iy1y5j1f6l3156q1n-yabar-unstable-2017-09-09’
unpacking sources
unpacking source archive /nix/store/9cjz85ir9b2zq7df4d08yn2kb79srqg0-yabar-d3934344ba27f5bdf122bf74daacee6d49284dab-src
source root is yabar-d3934344ba27f5bdf122bf74daacee6d49284dab-src
patching sources
configuring
no configure script, doing nothing
building
build flags: SHELL=/nix/store/nm9h53kd0ngh28wm42rivh2lc2d69nzl-bash-4.4-p12/bin/bash DESTDIR=\$\(out\) PREFIX=/
gcc -std=c99 -Iinclude -pedantic -Wall -flto -O2 `pkg-config --cflags pango pangocairo libconfig gdk-pixbuf-2.0 alsa` -DVERSION=\"unstable-2017-09-09\" -D_POSIX_C_SOURCE=199309L -DYA_INTERNAL -DYA_DYN_COL -DYA_ENV_VARS -DYA_INTERNAL_EWMH -DYA_ICON -DYA_NOWIN_COL -DYA_MUTEX -DYA_VAR_WIDTH -DYA_BSPWM  -c -o src/ya_parse.o src/ya_parse.c
gcc -std=c99 -Iinclude -pedantic -Wall -flto -O2 `pkg-config --cflags pango pangocairo libconfig gdk-pixbuf-2.0 alsa` -DVERSION=\"unstable-2017-09-09\" -D_POSIX_C_SOURCE=199309L -DYA_INTERNAL -DYA_DYN_COL -DYA_ENV_VARS -DYA_INTERNAL_EWMH -DYA_ICON -DYA_NOWIN_COL -DYA_MUTEX -DYA_VAR_WIDTH -DYA_BSPWM  -c -o src/ya_exec.o src/ya_exec.c
gcc -std=c99 -Iinclude -pedantic -Wall -flto -O2 `pkg-config --cflags pango pangocairo libconfig gdk-pixbuf-2.0 alsa` -DVERSION=\"unstable-2017-09-09\" -D_POSIX_C_SOURCE=199309L -DYA_INTERNAL -DYA_DYN_COL -DYA_ENV_VARS -DYA_INTERNAL_EWMH -DYA_ICON -DYA_NOWIN_COL -DYA_MUTEX -DYA_VAR_WIDTH -DYA_BSPWM  -c -o src/ya_draw.o src/ya_draw.c
gcc -std=c99 -Iinclude -pedantic -Wall -flto -O2 `pkg-config --cflags pango pangocairo libconfig gdk-pixbuf-2.0 alsa` -DVERSION=\"unstable-2017-09-09\" -D_POSIX_C_SOURCE=199309L -DYA_INTERNAL -DYA_DYN_COL -DYA_ENV_VARS -DYA_INTERNAL_EWMH -DYA_ICON -DYA_NOWIN_COL -DYA_MUTEX -DYA_VAR_WIDTH -DYA_BSPWM  -c -o src/ya_main.o src/ya_main.c
gcc -std=c99 -Iinclude -pedantic -Wall -flto -O2 `pkg-config --cflags pango pangocairo libconfig gdk-pixbuf-2.0 alsa` -DVERSION=\"unstable-2017-09-09\" -D_POSIX_C_SOURCE=199309L -DYA_INTERNAL -DYA_DYN_COL -DYA_ENV_VARS -DYA_INTERNAL_EWMH -DYA_ICON -DYA_NOWIN_COL -DYA_MUTEX -DYA_VAR_WIDTH -DYA_BSPWM  -c -o src/intern_blks/ya_intern.o src/intern_blks/ya_intern.c
gcc -flto -O2 -o yabar src/ya_parse.o src/ya_exec.o src/ya_draw.o src/ya_main.o src/intern_blks/ya_intern.o -liw -lxcb -lpthread -lxcb-randr -lxcb-ewmh -lxcb-icccm -lm `pkg-config --libs pango pangocairo libconfig gdk-pixbuf-2.0 alsa`
a2x --no-xmllint --doctype manpage --format manpage "doc/yabar.1.asciidoc"
glibPreInstallPhase
installing
install flags: install SHELL=/nix/store/nm9h53kd0ngh28wm42rivh2lc2d69nzl-bash-4.4-p12/bin/bash DESTDIR=\$\(out\) PREFIX=/ gsettingsschemadir=/nix/store/w6mq8gw6kbs50y5iy1y5j1f6l3156q1n-yabar-unstable-2017-09-09/share/gsettings-schemas/yabar-unstable-2017-09-09/glib-2.0/schemas/
mkdir -p "/nix/store/w6mq8gw6kbs50y5iy1y5j1f6l3156q1n-yabar-unstable-2017-09-09//bin"
cp -pf yabar "/nix/store/w6mq8gw6kbs50y5iy1y5j1f6l3156q1n-yabar-unstable-2017-09-09//bin"
mkdir -p "/nix/store/w6mq8gw6kbs50y5iy1y5j1f6l3156q1n-yabar-unstable-2017-09-09//share/man"/man1
cp -pf "doc/yabar.1" "/nix/store/w6mq8gw6kbs50y5iy1y5j1f6l3156q1n-yabar-unstable-2017-09-09//share/man/man1"
'examples/example.config' -> '/nix/store/w6mq8gw6kbs50y5iy1y5j1f6l3156q1n-yabar-unstable-2017-09-09/share/yabar/examples/example.config'
glibPreFixupPhase
post-installation fixup
shrinking RPATHs of ELF executables and libraries in /nix/store/w6mq8gw6kbs50y5iy1y5j1f6l3156q1n-yabar-unstable-2017-09-09
shrinking /nix/store/w6mq8gw6kbs50y5iy1y5j1f6l3156q1n-yabar-unstable-2017-09-09/bin/yabar
gzipping man pages under /nix/store/w6mq8gw6kbs50y5iy1y5j1f6l3156q1n-yabar-unstable-2017-09-09/share/man/
stripping (with flags -S) in /nix/store/w6mq8gw6kbs50y5iy1y5j1f6l3156q1n-yabar-unstable-2017-09-09/bin 
patching script interpreter paths in /nix/store/w6mq8gw6kbs50y5iy1y5j1f6l3156q1n-yabar-unstable-2017-09-09
checking for references to /tmp/nix-build-yabar-unstable-2017-09-09.drv-0 in /nix/store/w6mq8gw6kbs50y5iy1y5j1f6l3156q1n-yabar-unstable-2017-09-09...
/nix/store/w6mq8gw6kbs50y5iy1y5j1f6l3156q1n-yabar-unstable-2017-09-09

rycee · 2017-09-11T21:17:48Z

pkgs/applications/window-managers/yabar/build.nix

+  postPatch = ''
+    substituteInPlace ./Makefile \
+      --replace "\$(shell git describe)" "${version}" \
+      --replace "a2x" "${asciidoc}/bin/a2x --no-xmllint"


Just as an aside, since asciidoc is in buildInputs its bin directory will be in PATH. So it would be sufficient to have

--replace "a2x" "a2x --no-xmllint"

here. No need to change unless you feel like it. Just thought I'd highlight it.

Ma27 · 2017-09-12T19:41:27Z

@rycee does it build now?

rycee · 2017-09-12T19:55:04Z

Yes, now it builds successfully! 👍

rycee · 2017-09-12T20:09:07Z

All ready for merge?

Ma27 · 2017-09-12T20:15:28Z

👍 from my side :-)

rycee · 2017-09-12T20:28:34Z

Thanks for all your work on fixing this up 😃

Ma27 force-pushed the yabar/install-hooks branch from 9d56f44 to ac253f7 Compare September 10, 2017 09:45

Ma27 changed the title ~~yabar: minor derivation improvements~~ WIP yabar: minor derivation improvements Sep 10, 2017

Ma27 changed the title ~~WIP yabar: minor derivation improvements~~ yabar: minor derivation improvements Sep 10, 2017

rycee reviewed Sep 10, 2017

View reviewed changes

Ma27 force-pushed the yabar/install-hooks branch 2 times, most recently from da3981e to f3e1429 Compare September 10, 2017 23:06

Ma27 force-pushed the yabar/install-hooks branch from f3e1429 to 605a445 Compare September 11, 2017 16:25

Ma27 added 2 commits September 11, 2017 18:26

yabar: cleanup derivation

0761e74

The latest changes from `yabar` require several changes in the derivation to build the package successfully.

yabar: add optional configFile value to derivation

12e79f1

Ma27 force-pushed the yabar/install-hooks branch from 605a445 to 6bc1323 Compare September 11, 2017 16:27

rycee reviewed Sep 11, 2017

View reviewed changes

Ma27 changed the title ~~yabar: minor derivation improvements~~ WIP yabar: minor derivation improvements Sep 11, 2017

rycee reviewed Sep 11, 2017

View reviewed changes

Ma27 force-pushed the yabar/install-hooks branch from 6bc1323 to 3c2ad38 Compare September 12, 2017 19:41

Ma27 force-pushed the yabar/install-hooks branch from 3c2ad38 to aabef69 Compare September 12, 2017 19:52

Ma27 force-pushed the yabar/install-hooks branch from aabef69 to 03b27dd Compare September 12, 2017 20:04

Ma27 changed the title ~~WIP yabar: minor derivation improvements~~ yabar: minor derivation improvements Sep 12, 2017

yabar: add unstable package

1782510

Ma27 force-pushed the yabar/install-hooks branch from 03b27dd to 1782510 Compare September 12, 2017 20:05

rycee merged commit 3c14ef0 into NixOS:master Sep 12, 2017

Ma27 deleted the yabar/install-hooks branch September 12, 2017 20:32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

yabar: minor derivation improvements #29184

yabar: minor derivation improvements #29184

Ma27 commented Sep 10, 2017 •

edited

mention-bot commented Sep 10, 2017

rycee Sep 10, 2017 •

edited

rycee Sep 10, 2017

Ma27 Sep 10, 2017

rycee Sep 10, 2017

Ma27 Sep 10, 2017

rycee Sep 10, 2017

rycee commented Sep 11, 2017

Ma27 commented Sep 11, 2017

rycee Sep 11, 2017

rycee Sep 12, 2017

rycee Sep 12, 2017

rycee commented Sep 11, 2017

rycee commented Sep 11, 2017

rycee Sep 11, 2017 •

edited

Ma27 commented Sep 12, 2017

rycee commented Sep 12, 2017

rycee commented Sep 12, 2017

Ma27 commented Sep 12, 2017

rycee commented Sep 12, 2017

yabar: minor derivation improvements #29184

yabar: minor derivation improvements #29184

Conversation

Ma27 commented Sep 10, 2017 • edited

Motivation for this change

Things done

mention-bot commented Sep 10, 2017

rycee Sep 10, 2017 • 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

rycee commented Sep 11, 2017

Ma27 commented Sep 11, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rycee commented Sep 11, 2017

rycee commented Sep 11, 2017

rycee Sep 11, 2017 • edited

Choose a reason for hiding this comment

Ma27 commented Sep 12, 2017

rycee commented Sep 12, 2017

rycee commented Sep 12, 2017

Ma27 commented Sep 12, 2017

rycee commented Sep 12, 2017

Ma27 commented Sep 10, 2017 •

edited

rycee Sep 10, 2017 •

edited

rycee Sep 11, 2017 •

edited