buku: Enabled tests and added shell completion #26968

infinisil · 2017-06-29T14:30:32Z

Motivation for this change

Completion wasn't being installed for buku which provides one for zsh, fish and bash. Tests were disabled previously because they failed. Since buku has a lot more test cases since 3.0 it's nice to have them run and succeed.

Had to patch 3 things in the tests though:

One which requires internet connection I disabled
One test dependency was version freezed (see reqs: Lower bound hypothesis jarun/buku#167) which I patched (can be removed when that PR is merged and new version released)
Another issue was due to locale issues.

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.

infinisil · 2017-06-29T15:05:40Z

Pinging @matthiasbeyer as the mention bot seems to be sleeping.

matthiasbeyer

Builds and works for me. Re-evaluating this on another machine, to be sure. 👍

matthiasbeyer · 2017-06-29T16:10:31Z

On a debian machine I get

these derivations will be built:
  /nix/store/lls7dzqxs0dzlcqfwza376mphh41fqmd-shell.drv
building path(s) ‘/nix/store/1z6hvdphwvazy05fjn3g69a10m757lgq-shell’
builder for ‘/nix/store/lls7dzqxs0dzlcqfwza376mphh41fqmd-shell.drv’ failed to produce output path ‘/nix/store/1z6hvdphwvazy05fjn3g69a10m757lgq-shell’
error: build of ‘/nix/store/lls7dzqxs0dzlcqfwza376mphh41fqmd-shell.drv’ failed

And I don't know whether this is related or so...

matthewbauer · 2017-06-29T17:57:10Z

pkgs/applications/misc/buku/default.nix

@@ -12,20 +11,41 @@ with pythonPackages; buildPythonApplication rec {
    sha256 = "1a33x3197vi5s8rq5fvhy021jdlsc8ww8zc4kysss6r9mvdlk7ax";
  };

+  buildInputs = [


these should be nativeBuildInputs, I think

matthewbauer · 2017-06-29T17:58:39Z

pkgs/applications/misc/buku/default.nix


-  doCheck = false;
+    mkdir -p $out/share/zsh/site-functions $out/share/bash-completion/completions $out/share/fish/vendor_completions.d


Is there a reason make install is not installing these? Maybe open an issue if you think it should be the default.

Good point, I may do that

jarun/buku#168

matthewbauer · 2017-06-29T18:21:55Z

pkgs/applications/misc/buku/default.nix

+
+    # https://github.com/jarun/Buku/pull/167
+    substituteInPlace setup.py \
+      --replace "hypothesis==3.7.0" "hypothesis>=3.7.0"


This seems like something to fix upstream. I guess it's fine for now though... just make sure to get rid of it as soon as it's no longer needed.

Yeah, but my python knowledge is so limited I couldn't do it myself, I'll keep an eye on it though

infinisil · 2017-06-29T19:06:48Z

Built in an Ubuntu VM no problem, I'll have a look at Darwin now..

infinisil · 2017-06-29T19:56:02Z

Doesn't build on darwin, the dependency python3.6-cffi-1.10.0 fails on the tests, not buku itself, maybe this can be worked on in a separate PR.

infinisil · 2017-06-30T11:40:01Z

pkgs/applications/misc/buku/default.nix

+
+    # Should be upstream, see https://github.com/jarun/Buku/pull/167
+    substituteInPlace setup.py \
+      --replace "hypothesis==3.7.0" "hypothesis>=3.7.0"


Update: This would get fixed with jarun/buku#167, thanks @rachmadaniHaryono! Can therefore be removed when Buku is bumped next time

infinisil · 2017-06-30T12:50:18Z

pkgs/applications/misc/buku/default.nix

+    export PYTHONIOENCODING=utf-8
+
+    ### Remove this for 3.1 ###
+    # See https://github.com/jarun/Buku/pull/167 (merged)


jarun/buku#167 is merged, I added a note for the next version bump :)

buku: Enabled tests and added shell completion

fb62250

matthiasbeyer approved these changes Jun 29, 2017

View reviewed changes

matthewbauer reviewed Jun 29, 2017

View reviewed changes

buku: test & completions revision

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.

GPG key ID: 4AEE18F83AFDEB23
Expired

Verified
Learn about vigilant mode

bc81760

infinisil commented Jun 30, 2017

View reviewed changes

buku: added not to remove line on next version

ab162ee

infinisil commented Jun 30, 2017

View reviewed changes

grahamc merged commit 800d449 into NixOS:master Jul 8, 2017

infinisil deleted the buku-tests-and-completions branch July 14, 2017 16:11

nh2 mentioned this pull request Nov 11, 2018

python-msgpack test fails when building on my machine due to encoding #50231

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

buku: Enabled tests and added shell completion #26968

buku: Enabled tests and added shell completion #26968

infinisil commented Jun 29, 2017 •

edited

Loading

infinisil commented Jun 29, 2017

matthiasbeyer left a comment

matthiasbeyer commented Jun 29, 2017

matthewbauer Jun 29, 2017

infinisil Jun 29, 2017

matthewbauer Jun 29, 2017

infinisil Jun 29, 2017

infinisil Jun 29, 2017

matthewbauer Jun 29, 2017

infinisil Jun 29, 2017

infinisil commented Jun 29, 2017

infinisil commented Jun 29, 2017

infinisil Jun 30, 2017

infinisil Jun 30, 2017


		doCheck = false;
		mkdir -p $out/share/zsh/site-functions $out/share/bash-completion/completions $out/share/fish/vendor_completions.d

buku: Enabled tests and added shell completion #26968

buku: Enabled tests and added shell completion #26968

Conversation

infinisil commented Jun 29, 2017 • edited Loading

Motivation for this change

Things done

infinisil commented Jun 29, 2017

matthiasbeyer left a comment

Choose a reason for hiding this comment

matthiasbeyer commented Jun 29, 2017

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

infinisil commented Jun 29, 2017

infinisil commented Jun 29, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

infinisil commented Jun 29, 2017 •

edited

Loading