gitAndTools.pre-commit: init at 1.10.4 #44239

borisbabic · 2018-07-30T18:39:13Z

Motivation for this change

Add pre-commit - a useful tool for managing and sharing git commit hooks.

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/)
Determined the impact on package closure size (by running nix path-info -S before and after)
Fits CONTRIBUTING.md.

FRidh · 2018-07-30T18:51:28Z

pkgs/development/python-modules/aspy.yaml/default.nix

@@ -0,0 +1,19 @@
+{ lib, buildPythonPackage, fetchPypi, pythonPackages }:


no pythonPackages, list the individual requirements

FRidh · 2018-07-30T18:51:35Z

pkgs/development/python-modules/cfgv/default.nix

@@ -0,0 +1,19 @@
+{ lib, buildPythonPackage, fetchPypi, pythonPackages }:


I fixed these issues.

FRidh · 2018-07-30T18:51:48Z

pkgs/development/tools/pre-commit/default.nix

@@ -0,0 +1,30 @@
+{ stdenv, pythonPackages, pew }:
+with pythonPackages; buildPythonApplication rec {
+  name = "${pname}-${version}";


The name attribute is added by buildPython* and should therefore be removed.

Yeah, that's not necessary. removed it

FRidh · 2018-07-30T18:51:58Z

pkgs/development/tools/pre-commit/default.nix

+    description = "A framework for managing and maintaining multi-language pre-commit hooks.";
+    homepage = https://pre-commit.com/;
+    license = licenses.mit;
+    platforms = platforms.all;


drop platforms, is already set

Oh, buildPythonApplication does that? I dropped it.

dotlambda · 2018-07-30T20:30:43Z

pkgs/top-level/python-packages.nix

@@ -186,6 +186,8 @@ in {

  asn1crypto = callPackage ../development/python-modules/asn1crypto { };

+  aspy.yaml = callPackage ../development/python-modules/aspy.yaml { };


The . should be replaced by e.g. _ (also in the commit message).

preferable even a -, so to follow the PEP on normalized names.

Yeah, I wasn't sure what to do with the period, I changed it now to -.

dotlambda · 2018-07-30T21:42:00Z

pkgs/development/python-modules/aspy.yaml/default.nix

+  propagatedBuildInputs = [ pyyaml ];
+
+  meta = with lib; {
+    description = "A few extensions to pyyaml.";


No period: https://nixos.org/nixpkgs/manual/#sec-standard-meta-attributes (also for the other descriptions)

dotlambda · 2018-07-30T21:43:06Z

pkgs/top-level/all-packages.nix

@@ -8420,6 +8420,8 @@ with pkgs;

  pyprof2calltree = pythonPackages.callPackage ../development/tools/profiling/pyprof2calltree { };

+  pre-commit = pythonPackages.callPackage ../development/tools/pre-commit { };


Please don't use pythonPackages here, but as argument in the expression.

Also, since pre-commit seems to support Python 3, please use that if possible.

So I don't have to list individual requirements for pre-commit, just for the modules? Or am I misunderstanding something.

I'll use python3. Is the preferred method to do the following here:

pre-commit = callPackage ../development/tools/pre-commit { pythonPackages = python3Packages; };

Ye, correctly. The advantage is that applications can override individual packages.
No, the latter is not preferred. I'd just use python3 or python3Packages as argument.

dotlambda · 2018-07-30T22:43:44Z

pkgs/development/tools/pre-commit/default.nix

+    description = "A framework for managing and maintaining multi-language pre-commit hooks.";
+    homepage = https://pre-commit.com/;
+    license = licenses.mit;
+    maintainers = with maintainers; [ borisbabic ];


You need to add yourself to https://github.com/NixOS/nixpkgs/blob/master/maintainers/maintainer-list.nix. Please do so in an additional commit.

dotlambda · 2018-07-31T05:36:05Z

pkgs/development/tools/pre-commit/default.nix

+    virtualenv
+  ];
+
+  doCheck = false;


Why do you disable tests? Please add a comment.

The tests fail on python3 due to a windll dependency. I will add a comment explaining, I should have done that right away 😅

dotlambda · 2018-07-31T10:05:00Z

There are two final things to do. Sorry that I didn't think of these earlier:

The expression for pre-commit should be moved to pkgs/applications/version-management/git-and-tools/pre-commit/default.nix and called from pkgs/applications/version-management/git-and-tools/default.nix.
All three newly packaged Python modules seem not ship their testsuite in the PyPI tarball. Please add something like this to their expressions:

# Tests not included in PyPI tarball
doCheck = false;

Anyway, thanks a lot for your work!

dotlambda · 2018-07-31T12:29:54Z

@GrahamcOfBorg build pre-commit

GrahamcOfBorg · 2018-07-31T12:30:23Z

No attempt on aarch64-linux (full log)

The following builds were skipped because they don't evaluate on aarch64-linux: pre-commit

Partial log (click to expand)

Cannot nix-instantiate `pre-commit' because:
error: attribute 'pre-commit' in selection path 'pre-commit' not found

GrahamcOfBorg · 2018-07-31T12:31:12Z

No attempt on x86_64-linux (full log)

The following builds were skipped because they don't evaluate on x86_64-linux: pre-commit

Partial log (click to expand)

Cannot nix-instantiate `pre-commit' because:
error: attribute 'pre-commit' in selection path 'pre-commit' not found

dotlambda · 2018-07-31T12:31:48Z

@GrahamcOfBorg build gitAndTools.pre-commit

GrahamcOfBorg · 2018-07-31T12:32:50Z

Success on aarch64-linux (full log)

Attempted: gitAndTools.pre-commit

Partial log (click to expand)

shrinking RPATHs of ELF executables and libraries in /nix/store/ccnm03kwjb59231p0gws3vvz8799q4sn-pre_commit-1.10.4
strip is /nix/store/zrs21zqcchgyabjf4xfimncdq16njizc-binutils-2.30/bin/strip
stripping (with command strip and flags -S) in /nix/store/ccnm03kwjb59231p0gws3vvz8799q4sn-pre_commit-1.10.4/lib  /nix/store/ccnm03kwjb59231p0gws3vvz8799q4sn-pre_commit-1.10.4/bin
patching script interpreter paths in /nix/store/ccnm03kwjb59231p0gws3vvz8799q4sn-pre_commit-1.10.4
/nix/store/ccnm03kwjb59231p0gws3vvz8799q4sn-pre_commit-1.10.4/lib/python3.6/site-packages/pre_commit/resources/hook-tmpl: interpreter directive changed from "/usr/bin/env python" to "/nix/store/grlcs2s6hzmra5r6sdb2l612q3002ssn-python3-3.6.6/bin/python"
checking for references to /build in /nix/store/ccnm03kwjb59231p0gws3vvz8799q4sn-pre_commit-1.10.4...
wrapping `/nix/store/ccnm03kwjb59231p0gws3vvz8799q4sn-pre_commit-1.10.4/bin/pre-commit-validate-manifest'...
wrapping `/nix/store/ccnm03kwjb59231p0gws3vvz8799q4sn-pre_commit-1.10.4/bin/pre-commit-validate-config'...
wrapping `/nix/store/ccnm03kwjb59231p0gws3vvz8799q4sn-pre_commit-1.10.4/bin/pre-commit'...
/nix/store/ccnm03kwjb59231p0gws3vvz8799q4sn-pre_commit-1.10.4

borisbabic · 2018-07-31T12:51:48Z

Oops, I forgot to change pre-commit to gitAndTools.pre-commit.

Thanks everybody for your help and patience!

borisbabic requested a review from FRidh as a code owner July 30, 2018 18:39

GrahamcOfBorg added the 6.topic: python label Jul 30, 2018

FRidh requested changes Jul 30, 2018

View reviewed changes

dotlambda reviewed Jul 30, 2018

View reviewed changes

borisbabic force-pushed the feature/init-pre-commit branch from 9346457 to 0fe9407 Compare July 30, 2018 21:33

dotlambda requested changes Jul 30, 2018

View reviewed changes

dotlambda reviewed Jul 30, 2018

View reviewed changes

borisbabic force-pushed the feature/init-pre-commit branch 4 times, most recently from 7fac828 to fc4a6a6 Compare July 30, 2018 23:32

GrahamcOfBorg added 8.has: package (new) 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 labels Jul 30, 2018

dotlambda reviewed Jul 31, 2018

View reviewed changes

borisbabic force-pushed the feature/init-pre-commit branch from fc4a6a6 to 4ce8101 Compare July 31, 2018 08:42

borisbabic added 5 commits July 31, 2018 13:57

pythonPackages.identify: init at 1.1.4

632d3b5

pythonPackages.nodeenv: init at 1.3.2

a1f03c0

pythonPackages.cfgv: init at 1.1.0

eed74d1

pythonPackages.aspy-yaml: init at 1.1.1

339f789

maintainers: add @borisbabic

37bfdc1

borisbabic force-pushed the feature/init-pre-commit branch from 4ce8101 to 62f7130 Compare July 31, 2018 12:00

gitAndTools.pre-commit: init at 1.10.4

585b0ac

dotlambda force-pushed the feature/init-pre-commit branch from 62f7130 to 585b0ac Compare July 31, 2018 12:32

dotlambda changed the title ~~Init pre-commit~~ gitAndTools.pre-commit: init at 1.10.4 Jul 31, 2018

dotlambda merged commit e858b28 into NixOS:master Jul 31, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

gitAndTools.pre-commit: init at 1.10.4 #44239

gitAndTools.pre-commit: init at 1.10.4 #44239

borisbabic commented Jul 30, 2018

FRidh Jul 30, 2018

FRidh Jul 30, 2018

borisbabic Jul 30, 2018

FRidh Jul 30, 2018

borisbabic Jul 30, 2018 •

edited

FRidh Jul 30, 2018

borisbabic Jul 30, 2018

dotlambda Jul 30, 2018

FRidh Jul 30, 2018

borisbabic Jul 30, 2018

dotlambda Jul 30, 2018

dotlambda Jul 30, 2018

dotlambda Jul 30, 2018

borisbabic Jul 30, 2018

dotlambda Jul 31, 2018

dotlambda Jul 30, 2018

dotlambda Jul 31, 2018

borisbabic Jul 31, 2018

dotlambda commented Jul 31, 2018

dotlambda commented Jul 31, 2018

GrahamcOfBorg commented Jul 31, 2018

GrahamcOfBorg commented Jul 31, 2018

dotlambda commented Jul 31, 2018

GrahamcOfBorg commented Jul 31, 2018

borisbabic commented Jul 31, 2018

		@@ -0,0 +1,19 @@
		{ lib, buildPythonPackage, fetchPypi, pythonPackages }:

		@@ -186,6 +186,8 @@ in {

		asn1crypto = callPackage ../development/python-modules/asn1crypto { };

		aspy.yaml = callPackage ../development/python-modules/aspy.yaml { };

		@@ -8420,6 +8420,8 @@ with pkgs;

		pyprof2calltree = pythonPackages.callPackage ../development/tools/profiling/pyprof2calltree { };

		pre-commit = pythonPackages.callPackage ../development/tools/pre-commit { };

gitAndTools.pre-commit: init at 1.10.4 #44239

gitAndTools.pre-commit: init at 1.10.4 #44239

Conversation

borisbabic commented Jul 30, 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

borisbabic Jul 30, 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

dotlambda commented Jul 31, 2018

dotlambda commented Jul 31, 2018

GrahamcOfBorg commented Jul 31, 2018

GrahamcOfBorg commented Jul 31, 2018

dotlambda commented Jul 31, 2018

GrahamcOfBorg commented Jul 31, 2018

borisbabic commented Jul 31, 2018

borisbabic Jul 30, 2018 •

edited