From ad98c36f1b1b681db433a6cb798e7ec21e8da33c Mon Sep 17 00:00:00 2001 From: Jan Malakhovski Date: Thu, 15 Mar 2018 00:00:01 +0000 Subject: [PATCH 1/8] stdenv: generic/setup.sh: simplify buildPhase Makefile check --- pkgs/stdenv/generic/setup.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkgs/stdenv/generic/setup.sh b/pkgs/stdenv/generic/setup.sh index d7a4781448ae..17aaa5b798ff 100644 --- a/pkgs/stdenv/generic/setup.sh +++ b/pkgs/stdenv/generic/setup.sh @@ -968,7 +968,7 @@ buildPhase() { # set to empty if unset : ${makeFlags=} - if [[ -z "$makeFlags" && ! ( -n "${makefile:-}" || -e Makefile || -e makefile || -e GNUmakefile ) ]]; then + if [[ -z "$makeFlags" && -z "${makefile:-}" && ! ( -e Makefile || -e makefile || -e GNUmakefile ) ]]; then echo "no Makefile, doing nothing" else # See https://github.com/NixOS/nixpkgs/pull/1354#issuecomment-31260409 From e9e06888edc0f79055e0f4e59867a1426d960895 Mon Sep 17 00:00:00 2001 From: Jan Malakhovski Date: Thu, 15 Mar 2018 00:00:02 +0000 Subject: [PATCH 2/8] stdenv: generic/setup.sh: cleanup installPhase --- pkgs/stdenv/generic/setup.sh | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/pkgs/stdenv/generic/setup.sh b/pkgs/stdenv/generic/setup.sh index 17aaa5b798ff..bb9e32486674 100644 --- a/pkgs/stdenv/generic/setup.sh +++ b/pkgs/stdenv/generic/setup.sh @@ -1018,14 +1018,12 @@ installPhase() { mkdir -p "$prefix" fi - installTargets="${installTargets:-install}" - # Old bash empty array hack # shellcheck disable=SC2086 local flagsArray=( - $installTargets $makeFlags ${makeFlagsArray+"${makeFlagsArray[@]}"} $installFlags ${installFlagsArray+"${installFlagsArray[@]}"} + ${installTargets:-install} ) echoCmd 'install flags' "${flagsArray[@]}" From 50af975d8595c7fce05bc8bbd9db93e3c4cece25 Mon Sep 17 00:00:00 2001 From: Jan Malakhovski Date: Thu, 15 Mar 2018 00:00:04 +0000 Subject: [PATCH 3/8] stdenv: implement `checkTarget` and `installCheckTarget` autodetection --- pkgs/stdenv/generic/setup.sh | 74 +++++++++++++++++++++++++----------- 1 file changed, 52 insertions(+), 22 deletions(-) diff --git a/pkgs/stdenv/generic/setup.sh b/pkgs/stdenv/generic/setup.sh index bb9e32486674..c85f05d9a919 100644 --- a/pkgs/stdenv/generic/setup.sh +++ b/pkgs/stdenv/generic/setup.sh @@ -971,6 +971,8 @@ buildPhase() { if [[ -z "$makeFlags" && -z "${makefile:-}" && ! ( -e Makefile || -e makefile || -e GNUmakefile ) ]]; then echo "no Makefile, doing nothing" else + foundMakefile=1 + # See https://github.com/NixOS/nixpkgs/pull/1354#issuecomment-31260409 makeFlags="SHELL=$SHELL $makeFlags" @@ -994,18 +996,38 @@ buildPhase() { checkPhase() { runHook preCheck - # Old bash empty array hack - # shellcheck disable=SC2086 - local flagsArray=( - ${enableParallelBuilding:+-j${NIX_BUILD_CORES} -l${NIX_BUILD_CORES}} - $makeFlags ${makeFlagsArray+"${makeFlagsArray[@]}"} - ${checkFlags:-VERBOSE=y} ${checkFlagsArray+"${checkFlagsArray[@]}"} - ${checkTarget:-check} - ) + if [[ -z "${foundMakefile:-}" ]]; then + echo "no Makefile or custom buildPhase, doing nothing" + runHook postCheck + return + fi - echoCmd 'check flags' "${flagsArray[@]}" - make ${makefile:+-f $makefile} "${flagsArray[@]}" - unset flagsArray + if [[ -z "${checkTarget:-}" ]]; then + #TODO(@oxij): should flagsArray influence make -n? + if make -n ${makefile:+-f $makefile} check >/dev/null 2>&1; then + checkTarget=check + elif make -n ${makefile:+-f $makefile} test >/dev/null 2>&1; then + checkTarget=test + fi + fi + + if [[ -z "${checkTarget:-}" ]]; then + echo "no check/test target in ${makefile:-Makefile}, doing nothing" + else + # Old bash empty array hack + # shellcheck disable=SC2086 + local flagsArray=( + ${enableParallelBuilding:+-j${NIX_BUILD_CORES} -l${NIX_BUILD_CORES}} + $makeFlags ${makeFlagsArray+"${makeFlagsArray[@]}"} + ${checkFlags:-VERBOSE=y} ${checkFlagsArray+"${checkFlagsArray[@]}"} + ${checkTarget} + ) + + echoCmd 'check flags' "${flagsArray[@]}" + make ${makefile:+-f $makefile} "${flagsArray[@]}" + + unset flagsArray + fi runHook postCheck } @@ -1104,18 +1126,26 @@ fixupPhase() { installCheckPhase() { runHook preInstallCheck - # Old bash empty array hack - # shellcheck disable=SC2086 - local flagsArray=( - ${enableParallelBuilding:+-j${NIX_BUILD_CORES} -l${NIX_BUILD_CORES}} - $makeFlags ${makeFlagsArray+"${makeFlagsArray[@]}"} - $installCheckFlags ${installCheckFlagsArray+"${installCheckFlagsArray[@]}"} - ${installCheckTarget:-installcheck} - ) + if [[ -z "${foundMakefile:-}" ]]; then + echo "no Makefile or custom buildPhase, doing nothing" + #TODO(@oxij): should flagsArray influence make -n? + elif [[ -z "${installCheckTarget:-}" ]] \ + && ! make -n ${makefile:+-f $makefile} ${installCheckTarget:-installcheck} >/dev/null 2>&1; then + echo "no installcheck target in ${makefile:-Makefile}, doing nothing" + else + # Old bash empty array hack + # shellcheck disable=SC2086 + local flagsArray=( + ${enableParallelBuilding:+-j${NIX_BUILD_CORES} -l${NIX_BUILD_CORES}} + $makeFlags ${makeFlagsArray+"${makeFlagsArray[@]}"} + $installCheckFlags ${installCheckFlagsArray+"${installCheckFlagsArray[@]}"} + ${installCheckTarget:-installcheck} + ) - echoCmd 'installcheck flags' "${flagsArray[@]}" - make ${makefile:+-f $makefile} "${flagsArray[@]}" - unset flagsArray + echoCmd 'installcheck flags' "${flagsArray[@]}" + make ${makefile:+-f $makefile} "${flagsArray[@]}" + unset flagsArray + fi runHook postInstallCheck } From d834ba6654fc5fa79c6cc8b0d5cdaa5d8f85fd0e Mon Sep 17 00:00:00 2001 From: Jan Malakhovski Date: Thu, 15 Mar 2018 00:00:04 +0000 Subject: [PATCH 4/8] stdenv: introduce and use `config.doCheckByDefault` option --- pkgs/stdenv/generic/make-derivation.nix | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/pkgs/stdenv/generic/make-derivation.nix b/pkgs/stdenv/generic/make-derivation.nix index 7b5f9f7d6b0b..c7a991572e9c 100644 --- a/pkgs/stdenv/generic/make-derivation.nix +++ b/pkgs/stdenv/generic/make-derivation.nix @@ -46,11 +46,13 @@ rec { (stdenv.hostPlatform != stdenv.buildPlatform) [ "build" "host" ] + # TODO(@Ericson2314): Make unconditional / resolve #33599 # Check phase - , doCheck ? false + , doCheck ? config.doCheckByDefault or false + # TODO(@Ericson2314): Make unconditional / resolve #33599 # InstallCheck phase - , doInstallCheck ? false + , doInstallCheck ? config.doCheckByDefault or false , crossConfig ? null , meta ? {} @@ -120,6 +122,11 @@ rec { ] ]; + # TODO(@oxij, @Ericson2314): This is here to keep the old semantics, remove when + # no package has `doCheck = true`. + doCheck' = doCheck && stdenv.hostPlatform == stdenv.buildPlatform; + doInstallCheck' = doInstallCheck && stdenv.hostPlatform == stdenv.buildPlatform; + outputs' = outputs ++ (if separateDebugInfo then assert stdenv.hostPlatform.isLinux; [ "debug" ] else []); @@ -127,6 +134,7 @@ rec { derivationArg = (removeAttrs attrs ["meta" "passthru" "crossAttrs" "pos" + "doCheck" "doInstallCheck" "__impureHostDeps" "__propagatedImpureHostDeps" "sandboxProfile" "propagatedSandboxProfile"]) // (let @@ -202,12 +210,10 @@ rec { __propagatedImpureHostDeps = computedPropagatedImpureHostDeps ++ __propagatedImpureHostDeps; } // lib.optionalAttrs (outputs' != [ "out" ]) { outputs = outputs'; - } // lib.optionalAttrs (attrs ? doCheck) { - # TODO(@Ericson2314): Make unconditional / resolve #33599 - doCheck = doCheck && (stdenv.hostPlatform == stdenv.buildPlatform); - } // lib.optionalAttrs (attrs ? doInstallCheck) { - # TODO(@Ericson2314): Make unconditional / resolve #33599 - doInstallCheck = doInstallCheck && (stdenv.hostPlatform == stdenv.buildPlatform); + } // lib.optionalAttrs doCheck' { + doCheck = true; + } // lib.optionalAttrs doInstallCheck' { + doInstallCheck = true; }); validity = import ./check-meta.nix { From 845fa5692164d0bbe2a1857ba27d1bbb4ae0c0e7 Mon Sep 17 00:00:00 2001 From: Jan Malakhovski Date: Wed, 25 Apr 2018 18:49:44 +0000 Subject: [PATCH 5/8] stdenv: cleanup things a little bit --- pkgs/stdenv/generic/make-derivation.nix | 55 +++++++++++++------------ 1 file changed, 29 insertions(+), 26 deletions(-) diff --git a/pkgs/stdenv/generic/make-derivation.nix b/pkgs/stdenv/generic/make-derivation.nix index c7a991572e9c..96e40a78e518 100644 --- a/pkgs/stdenv/generic/make-derivation.nix +++ b/pkgs/stdenv/generic/make-derivation.nix @@ -131,31 +131,33 @@ rec { outputs ++ (if separateDebugInfo then assert stdenv.hostPlatform.isLinux; [ "debug" ] else []); + computedSandboxProfile = + lib.concatMap (input: input.__propagatedSandboxProfile or []) + (stdenv.extraNativeBuildInputs + ++ stdenv.extraBuildInputs + ++ lib.concatLists dependencies); + + computedPropagatedSandboxProfile = + lib.concatMap (input: input.__propagatedSandboxProfile or []) + (lib.concatLists propagatedDependencies); + + computedImpureHostDeps = + lib.unique (lib.concatMap (input: input.__propagatedImpureHostDeps or []) + (stdenv.extraNativeBuildInputs + ++ stdenv.extraBuildInputs + ++ lib.concatLists dependencies)); + + computedPropagatedImpureHostDeps = + lib.unique (lib.concatMap (input: input.__propagatedImpureHostDeps or []) + (lib.concatLists propagatedDependencies)); + derivationArg = (removeAttrs attrs ["meta" "passthru" "crossAttrs" "pos" "doCheck" "doInstallCheck" "__impureHostDeps" "__propagatedImpureHostDeps" "sandboxProfile" "propagatedSandboxProfile"]) - // (let - computedSandboxProfile = - lib.concatMap (input: input.__propagatedSandboxProfile or []) - (stdenv.extraNativeBuildInputs - ++ stdenv.extraBuildInputs - ++ lib.concatLists dependencies); - computedPropagatedSandboxProfile = - lib.concatMap (input: input.__propagatedSandboxProfile or []) - (lib.concatLists propagatedDependencies); - computedImpureHostDeps = - lib.unique (lib.concatMap (input: input.__propagatedImpureHostDeps or []) - (stdenv.extraNativeBuildInputs - ++ stdenv.extraBuildInputs - ++ lib.concatLists dependencies)); - computedPropagatedImpureHostDeps = - lib.unique (lib.concatMap (input: input.__propagatedImpureHostDeps or []) - (lib.concatLists propagatedDependencies)); - in - { + // { # A hack to make `nix-env -qa` and `nix search` ignore broken packages. # TODO(@oxij): remove this assert when something like NixOS/nix#1771 gets merged into nix. name = assert validity.handled; name + lib.optionalString @@ -194,6 +196,13 @@ rec { } // lib.optionalAttrs (hardeningDisable != [] || hardeningEnable != []) { NIX_HARDENING_ENABLE = enabledHardeningOptions; + } // lib.optionalAttrs (outputs' != [ "out" ]) { + outputs = outputs'; + } // lib.optionalAttrs doCheck' { + doCheck = true; + } // lib.optionalAttrs doInstallCheck' { + doInstallCheck = true; + } // lib.optionalAttrs (stdenv.buildPlatform.isDarwin) { # TODO: remove lib.unique once nix has a list canonicalization primitive __sandboxProfile = @@ -208,13 +217,7 @@ rec { "/bin/sh" ]; __propagatedImpureHostDeps = computedPropagatedImpureHostDeps ++ __propagatedImpureHostDeps; - } // lib.optionalAttrs (outputs' != [ "out" ]) { - outputs = outputs'; - } // lib.optionalAttrs doCheck' { - doCheck = true; - } // lib.optionalAttrs doInstallCheck' { - doInstallCheck = true; - }); + }; validity = import ./check-meta.nix { inherit lib config meta; From 912cfb8aaaaeff60c25a685dc41b143952cfb4d3 Mon Sep 17 00:00:00 2001 From: Jan Malakhovski Date: Wed, 25 Apr 2018 17:15:48 +0000 Subject: [PATCH 6/8] buildPythonPackage: use `config.doCheckByDefault` --- pkgs/development/interpreters/python/build-python-package.nix | 3 ++- pkgs/development/interpreters/python/mk-python-derivation.nix | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/pkgs/development/interpreters/python/build-python-package.nix b/pkgs/development/interpreters/python/build-python-package.nix index 6a07a006c6b5..b55d6d874bbb 100644 --- a/pkgs/development/interpreters/python/build-python-package.nix +++ b/pkgs/development/interpreters/python/build-python-package.nix @@ -2,6 +2,7 @@ # and can build packages that use distutils, setuptools or flit. { lib +, config , python , wrapPython , setuptools @@ -19,7 +20,7 @@ let wheel-specific = import ./build-python-package-wheel.nix { }; common = import ./build-python-package-common.nix { inherit python bootstrapped-pip; }; mkPythonDerivation = import ./mk-python-derivation.nix { - inherit lib python wrapPython setuptools unzip ensureNewerSourcesForZipFilesHook toPythonModule namePrefix; + inherit lib config python wrapPython setuptools unzip ensureNewerSourcesForZipFilesHook toPythonModule namePrefix; }; in diff --git a/pkgs/development/interpreters/python/mk-python-derivation.nix b/pkgs/development/interpreters/python/mk-python-derivation.nix index 96a9cdf0c615..9835ba32d170 100644 --- a/pkgs/development/interpreters/python/mk-python-derivation.nix +++ b/pkgs/development/interpreters/python/mk-python-derivation.nix @@ -1,6 +1,7 @@ # Generic builder. { lib +, config , python , wrapPython , setuptools @@ -53,7 +54,7 @@ , passthru ? {} -, doCheck ? false +, doCheck ? config.doCheckByDefault or false , ... } @ attrs: From 9345fc51d111bb4a35afa86fb1617855bcade43d Mon Sep 17 00:00:00 2001 From: Jan Malakhovski Date: Thu, 15 Mar 2018 00:00:00 +0000 Subject: [PATCH 7/8] haskell-generic-builder: be explicit about `doCheck`, cleanup --- pkgs/development/haskell-modules/generic-builder.nix | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/pkgs/development/haskell-modules/generic-builder.nix b/pkgs/development/haskell-modules/generic-builder.nix index 52d596da701a..09cbab13930e 100644 --- a/pkgs/development/haskell-modules/generic-builder.nix +++ b/pkgs/development/haskell-modules/generic-builder.nix @@ -20,7 +20,7 @@ in , buildTools ? [], libraryToolDepends ? [], executableToolDepends ? [], testToolDepends ? [], benchmarkToolDepends ? [] , configureFlags ? [] , description ? "" -, doCheck ? !isCross && (stdenv.lib.versionOlder "7.4" ghc.version) +, doCheck ? !isCross && stdenv.lib.versionOlder "7.4" ghc.version , doBenchmark ? false , doHoogle ? true , editedCabalFile ? null @@ -172,7 +172,7 @@ let buildTools ++ libraryToolDepends ++ executableToolDepends; propagatedBuildInputs = buildDepends ++ libraryHaskellDepends ++ executableHaskellDepends; otherBuildInputs = setupHaskellDepends ++ extraLibraries ++ librarySystemDepends ++ executableSystemDepends ++ - optionals (allPkgconfigDepends != []) allPkgconfigDepends ++ + allPkgconfigDepends ++ optionals doCheck (testDepends ++ testHaskellDepends ++ testSystemDepends ++ testToolDepends) ++ optionals doBenchmark (benchmarkDepends ++ benchmarkHaskellDepends ++ benchmarkSystemDepends ++ benchmarkToolDepends); allBuildInputs = propagatedBuildInputs ++ otherBuildInputs; @@ -314,6 +314,8 @@ stdenv.mkDerivation ({ runHook postBuild ''; + inherit doCheck; + checkPhase = '' runHook preCheck ${setupCommand} test ${testTarget} @@ -428,7 +430,6 @@ stdenv.mkDerivation ({ // optionalAttrs (postConfigure != "") { inherit postConfigure; } // optionalAttrs (preBuild != "") { inherit preBuild; } // optionalAttrs (postBuild != "") { inherit postBuild; } -// optionalAttrs (doCheck) { inherit doCheck; } // optionalAttrs (doBenchmark) { inherit doBenchmark; } // optionalAttrs (checkPhase != "") { inherit checkPhase; } // optionalAttrs (preCheck != "") { inherit preCheck; } From 87651b32fe44b9cdeb5dc55d6197d5c5158ce42a Mon Sep 17 00:00:00 2001 From: Jan Malakhovski Date: Thu, 15 Mar 2018 00:00:04 +0000 Subject: [PATCH 8/8] stdenv: steal `checkInputs` from buildPythonPackage Note that a bunch of non-python packages use this attribute already. Some of those are clearly unaware of the fact that this attribute does not exists in stdenv because they define it but don't to add it to their `bulidInputs` :) Also note that I use `buildInputs` here and only handle regular builds because python and haskell builders do it this way and I'm not sure how to properly handle the cross-compilation case. --- doc/stdenv.xml | 14 ++++++++++++++ .../interpreters/python/mk-python-derivation.nix | 2 +- pkgs/stdenv/generic/make-derivation.nix | 8 +++++++- 3 files changed, 22 insertions(+), 2 deletions(-) diff --git a/doc/stdenv.xml b/doc/stdenv.xml index 2a3316b8d018..31d3d0ca3c4e 100644 --- a/doc/stdenv.xml +++ b/doc/stdenv.xml @@ -1005,6 +1005,13 @@ but only if the doCheck variable is enabled. + + checkInputs + + A list of dependencies used by the phase. This gets included in buildInputs when doCheck is set. + + + makeFlags / makeFlagsArray / @@ -1291,6 +1298,13 @@ installcheck. + + installCheckInputs + + A list of dependencies used by the phase. This gets included in buildInputs when doInstallCheck is set. + + + preInstallCheck Hook executed at the start of the installCheck diff --git a/pkgs/development/interpreters/python/mk-python-derivation.nix b/pkgs/development/interpreters/python/mk-python-derivation.nix index 9835ba32d170..63ffdbb8c0ac 100644 --- a/pkgs/development/interpreters/python/mk-python-derivation.nix +++ b/pkgs/development/interpreters/python/mk-python-derivation.nix @@ -75,7 +75,6 @@ toPythonModule (python.stdenv.mkDerivation (builtins.removeAttrs attrs [ buildInputs = [ wrapPython ] ++ lib.optional (lib.hasSuffix "zip" (attrs.src.name or "")) unzip - ++ lib.optionals doCheck checkInputs ++ lib.optional catchConflicts setuptools # If we no longer propagate setuptools ++ buildInputs ++ pythonPath; @@ -86,6 +85,7 @@ toPythonModule (python.stdenv.mkDerivation (builtins.removeAttrs attrs [ # Python packages don't have a checkPhase, only an installCheckPhase doCheck = false; doInstallCheck = doCheck; + installCheckInputs = checkInputs; postFixup = lib.optionalString (!dontWrapPythonPrograms) '' wrapPythonPrograms diff --git a/pkgs/stdenv/generic/make-derivation.nix b/pkgs/stdenv/generic/make-derivation.nix index 96e40a78e518..b523374454fc 100644 --- a/pkgs/stdenv/generic/make-derivation.nix +++ b/pkgs/stdenv/generic/make-derivation.nix @@ -36,6 +36,9 @@ rec { , depsTargetTarget ? [] # 1 -> 1 , depsTargetTargetPropagated ? [] # 1 -> 1 + , checkInputs ? [] + , installCheckInputs ? [] + # Configure Phase , configureFlags ? [] , # Target is not included by default because most programs don't care. @@ -101,7 +104,9 @@ rec { ] [ (map (drv: drv.__spliced.hostHost or drv) depsHostHost) - (map (drv: drv.crossDrv or drv) buildInputs) + (map (drv: drv.crossDrv or drv) (buildInputs + ++ lib.optionals doCheck' checkInputs + ++ lib.optionals doInstallCheck' installCheckInputs)) ] [ (map (drv: drv.__spliced.targetTarget or drv) depsTargetTarget) @@ -155,6 +160,7 @@ rec { (removeAttrs attrs ["meta" "passthru" "crossAttrs" "pos" "doCheck" "doInstallCheck" + "checkInputs" "installCheckInputs" "__impureHostDeps" "__propagatedImpureHostDeps" "sandboxProfile" "propagatedSandboxProfile"]) // {