Merge pull request #39464 from oxij/stdenv/docheck-infra

stdenv: implement most of #33599

doc/stdenv.xml

@@ -1005,6 +1005,13 @@ but only if the <varname>doCheck</varname> variable is enabled.</para>
     </para></listitem>
   </varlistentry>
 
+  <varlistentry>
+    <term><varname>checkInputs</varname></term>
+    <listitem><para>
+      A list of dependencies used by the phase. This gets included in <varname>buildInputs</varname> when <varname>doCheck</varname> is set.
+    </para></listitem>
+  </varlistentry>
+
   <varlistentry>
     <term><varname>makeFlags</varname> /
     <varname>makeFlagsArray</varname> /
@@ -1291,6 +1298,13 @@ installcheck</command>.</para>
     </para></listitem>
   </varlistentry>
 
+  <varlistentry>
+    <term><varname>installCheckInputs</varname></term>
+    <listitem><para>
+      A list of dependencies used by the phase. This gets included in <varname>buildInputs</varname> when <varname>doInstallCheck</varname> is set.
+    </para></listitem>
+  </varlistentry>
+
   <varlistentry>
     <term><varname>preInstallCheck</varname></term>
     <listitem><para>Hook executed at the start of the installCheck

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; }

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
 

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:
 
@@ -74,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;
@@ -85,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

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.
@@ -46,11 +49,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 ? {}
@@ -99,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)
@@ -120,34 +127,43 @@ 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 []);
 
+      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"
+           "checkInputs" "installCheckInputs"
            "__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
@@ -186,6 +202,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 =
@@ -200,15 +223,7 @@ rec {
             "/bin/sh"
           ];
           __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);
-        });
+        };
 
       validity = import ./check-meta.nix {
         inherit lib config meta;

pkgs/stdenv/generic/setup.sh

@@ -968,9 +968,11 @@ 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
+        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
 }
@@ -1018,14 +1040,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[@]}"
@@ -1106,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
 }