From eb49fb355e8269690bb100fae060af50138b2de3 Mon Sep 17 00:00:00 2001 From: Doron Behar Date: Sun, 16 Jun 2024 17:08:57 +0300 Subject: [PATCH 1/5] versionCheckHook: init --- doc/hooks/index.md | 1 + doc/hooks/versionCheckHook.section.md | 35 ++++++++++++ pkgs/by-name/ve/versionCheckHook/hook.sh | 60 ++++++++++++++++++++ pkgs/by-name/ve/versionCheckHook/package.nix | 12 ++++ 4 files changed, 108 insertions(+) create mode 100644 doc/hooks/versionCheckHook.section.md create mode 100644 pkgs/by-name/ve/versionCheckHook/hook.sh create mode 100644 pkgs/by-name/ve/versionCheckHook/package.nix diff --git a/doc/hooks/index.md b/doc/hooks/index.md index 1534ef85ccb9..6d01c6cbcbe5 100644 --- a/doc/hooks/index.md +++ b/doc/hooks/index.md @@ -29,6 +29,7 @@ scons.section.md tetex-tex-live.section.md unzip.section.md validatePkgConfig.section.md +versionCheckHook.section.md waf.section.md zig.section.md xcbuild.section.md diff --git a/doc/hooks/versionCheckHook.section.md b/doc/hooks/versionCheckHook.section.md new file mode 100644 index 000000000000..55b9fa916c40 --- /dev/null +++ b/doc/hooks/versionCheckHook.section.md @@ -0,0 +1,35 @@ +# versionCheckHook {#versioncheckhook} + +This hook adds a `versionCheckPhase` to the [`preInstallCheckHooks`](#ssec-installCheck-phase) that runs the main program of the derivation with a `--help` or `--version` argument, and checks that the `${version}` string is found in that output. You use it like this: + +```nix +{ + lib, + stdenv, + versionCheckHook, + # ... +}: + +stdenv.mkDerivation (finalAttrs: { + # ... + + nativeInstallCheckInputs = [ + versionCheckHook + ]; + doInstallCheck = true; + + # ... +}) +``` + +Note that for [`buildPythonPackage`](#buildpythonpackage-function) and [`buildPythonApplication`](#buildpythonapplication-function), `doInstallCheck` is enabled by default. + +It does so in a clean environment (using `env --ignore-environment`), and it checks for the `${version}` string in both the `stdout` and the `stderr` of the command. It will report to you in the build log the output it received and it will fail the build if it failed to find `${version}`. + +The variables that this phase control are: + +- `dontVersionCheck`: Disable adding this hook to the [`preDistPhases`](#var-stdenv-preDist). Useful if you do want to load the bash functions of the hook, but run them differently. +- `versionCheckProgram`: The full path to the program that should print the `${version}` string. Defaults roughly to `${placeholder "out"}/bin/${pname}`. Using `$out` in the value of this variable won't work, as environment variables from this variable are not expanded by the hook. Hence using `placeholder` is unavoidable. +- `versionCheckProgramArg`: The argument that needs to be passed to `versionCheckProgram`. If undefined the hook tries first `--help` and then `--version`. Examples: `version`, `-V`, `-v`. +- `preVersionCheck`: A hook to run before the check is done. +- `postVersionCheck`: A hook to run after the check is done. diff --git a/pkgs/by-name/ve/versionCheckHook/hook.sh b/pkgs/by-name/ve/versionCheckHook/hook.sh new file mode 100644 index 000000000000..cd417493e6c2 --- /dev/null +++ b/pkgs/by-name/ve/versionCheckHook/hook.sh @@ -0,0 +1,60 @@ +_handleCmdOutput(){ + local versionOutput + versionOutput="$(env --chdir=/ --argv0="$(basename "$1")" --ignore-environment "$@" 2>&1 || true)" + if [[ "$versionOutput" =~ "$version" ]]; then + echoPrefix="Successfully managed to" + else + echoPrefix="Did not" + fi + # The return value of this function is this variable: + echo "$echoPrefix" + # And in anycase we want these to be printed in the build log, useful for + # debugging, so we print these to stderr. + echo "$echoPrefix" find version "$version" in the output of the command \ + "$@" >&2 + echo "$versionOutput" >&2 +} +versionCheckHook(){ + runHook preVersionCheck + echo Executing versionCheckPhase + + local cmdProgram cmdArg echoPrefix + if [[ -z "${versionCheckProgram-}" ]]; then + if [[ -z "${pname-}" ]]; then + echo "both \$pname and \$versionCheckProgram are empty, so" \ + "we don't know which program to run the versionCheckPhase" \ + "upon" >&2 + exit 2 + else + cmdProgram="${!outputBin}/bin/$pname" + fi + else + cmdProgram="$versionCheckProgram" + fi + if [[ ! -x "$cmdProgram" ]]; then + echo "versionCheckHook: $cmdProgram was not found, or is not an executable" >&2 + exit 2 + fi + if [[ -z "${versionCheckProgramArg}" ]]; then + for cmdArg in "--help" "--version"; do + echoPrefix="$(_handleCmdOutput "$cmdProgram" "$cmdArg")" + if [[ "$echoPrefix" == "Successfully managed to" ]]; then + break + fi + done + else + cmdArg="$versionCheckProgramArg" + echoPrefix="$(_handleCmdOutput "$cmdProgram" "$cmdArg")" + fi + if [[ "$echoPrefix" == "Did not" ]]; then + exit 2 + fi + + runHook postVersionCheck + echo Finished versionCheckPhase +} + +if [[ -z "${dontVersionCheck-}" ]]; then + echo "Using versionCheckHook" + preInstallCheckHooks+=(versionCheckHook) +fi diff --git a/pkgs/by-name/ve/versionCheckHook/package.nix b/pkgs/by-name/ve/versionCheckHook/package.nix new file mode 100644 index 000000000000..ed35384d7960 --- /dev/null +++ b/pkgs/by-name/ve/versionCheckHook/package.nix @@ -0,0 +1,12 @@ +{ + lib, + makeSetupHook, +}: + +makeSetupHook { + name = "version-check-hook"; + meta = { + description = "Lookup for $version in the output of --help and --version"; + maintainers = with lib.maintainers; [ doronbehar ]; + }; +} ./hook.sh From b353c6d82ab939b6fd12fa8cb5854cab7b3e1537 Mon Sep 17 00:00:00 2001 From: Doron Behar Date: Sun, 16 Jun 2024 17:18:17 +0300 Subject: [PATCH 2/5] python312Packages.ftfy: use versionCheckHook --- pkgs/development/python-modules/ftfy/default.nix | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/pkgs/development/python-modules/ftfy/default.nix b/pkgs/development/python-modules/ftfy/default.nix index a52c32614657..2ca4ab01e716 100644 --- a/pkgs/development/python-modules/ftfy/default.nix +++ b/pkgs/development/python-modules/ftfy/default.nix @@ -12,6 +12,7 @@ # tests pytestCheckHook, + versionCheckHook, }: buildPythonPackage rec { @@ -30,7 +31,10 @@ buildPythonPackage rec { propagatedBuildInputs = [ wcwidth ]; - nativeCheckInputs = [ pytestCheckHook ]; + nativeCheckInputs = [ + versionCheckHook + pytestCheckHook + ]; preCheck = '' export PATH=$out/bin:$PATH From f42d4debf076b03f48b24fb59a7a75d0d8c29363 Mon Sep 17 00:00:00 2001 From: Doron Behar Date: Tue, 18 Jun 2024 08:54:26 +0300 Subject: [PATCH 3/5] doc: recommend versionCheckHook whenever relevant --- doc/build-helpers/testers.chapter.md | 7 ++--- doc/stdenv/passthru.chapter.md | 39 ++++++---------------------- doc/stdenv/stdenv.chapter.md | 2 ++ 3 files changed, 14 insertions(+), 34 deletions(-) diff --git a/doc/build-helpers/testers.chapter.md b/doc/build-helpers/testers.chapter.md index 34cfc00a4953..a10e60de8c6d 100644 --- a/doc/build-helpers/testers.chapter.md +++ b/doc/build-helpers/testers.chapter.md @@ -120,9 +120,10 @@ It has two modes: Checks that the output from running a command contains the specified version string in it as a whole word. -Although simplistic, this test assures that the main program can run. -While there's no substitute for a real test case, it does catch dynamic linking errors and such. -It also provides some protection against accidentally building the wrong version, for example when using an "old" hash in a fixed-output derivation. +NOTE: In most cases, [`versionCheckHook`](#versioncheckhook) should be preferred, but this function is provided and documented here anyway. The motivation for adding either tests would be: + +- Catch dynamic linking errors and such and missing environment variables that should be added by wrapping. +- Probable protection against accidentally building the wrong version, for example when using an "old" hash in a fixed-output derivation. By default, the command to be run will be inferred from the given `package` attribute: it will check `meta.mainProgram` first, and fall back to `pname` or `name`. diff --git a/doc/stdenv/passthru.chapter.md b/doc/stdenv/passthru.chapter.md index 008b908b2f48..142b978d5880 100644 --- a/doc/stdenv/passthru.chapter.md +++ b/doc/stdenv/passthru.chapter.md @@ -75,40 +75,17 @@ The Nixpkgs systems for continuous integration [Hydra](https://hydra.nixos.org/) #### Package tests {#var-passthru-tests-packages} []{#var-meta-tests-packages} -Tests that are part of the source package, if they run quickly, are typically executed in the [`installCheckPhase`](#var-stdenv-phases). -This phase is also suitable for performing a `--version` test for packages that support such flag. -Most programs distributed by Nixpkgs support such a `--version` flag, and successfully calling the program with that flag indicates that the package at least got compiled properly. +Besides tests provided by upstream, that you run in the [`checkPhase`](#ssec-check-phase), you may want to define tests derivations in the `passthru.tests` attribute, which won't change the build. `passthru.tests` have several advantages over running tests during any of the [standard phases](#sec-stdenv-phases): -:::{.example #ex-checking-build-installCheckPhase} +- They access the package as consumers would, independently from the environment in which it was built +- They can be run and debugged without rebuilding the package, which is useful if that takes a long time +- They don't add overhead to each build, as opposed checks added to the [`distPhase`](#ssec-distribution-phase), such as [`versionCheckHook`](#versioncheckhook). -## Checking builds with `installCheckPhase` +It is also possible to use `passthru.tests` to test the version with [`testVersion`](#tester-testVersion), but since that is pretty trivial and recommended thing to do, we recommend using [`versionCheckHook`](#versioncheckhook) for that, which has the following advantages over `passthru.tests`: -When building `git`, a rudimentary test for successful compilation would be running `git --version`: - -```nix -stdenv.mkDerivation (finalAttrs: { - pname = "git"; - version = "1.2.3"; - # ... - doInstallCheck = true; - installCheckPhase = '' - runHook preInstallCheck - echo checking if 'git --version' mentions ${finalAttrs.version} - $out/bin/git --version | grep ${finalAttrs.version} - runHook postInstallCheck - ''; - # ... -}) -``` -::: - -However, tests that are non-trivial will better fit into `passthru.tests` because they: - -- Access the package as consumers would, independently from the environment in which it was built -- Can be run and debugged without rebuilding the package, which is useful if that takes a long time -- Don't add overhad to each build, as opposed to `installCheckPhase` - -It is also possible to use `passthru.tests` to test the version with [`testVersion`](#tester-testVersion). +- If the `versionCheckPhase` (the phase defined by [`versionCheckHook`](#versioncheckhook)) fails, it triggers a failure which can't be ignored if you use the package, or if you find out about it in a [`nixpkgs-review`](https://github.com/Mic92/nixpkgs-review) report. +- Sometimes packages become silently broken - meaning they fail to launch but their build passes because they don't perform any tests in the `checkPhase`. If you use this tool infrequently, such a silent breakage may rot in your system / profile configuration, and you will not notice the failure until you will want to use this package. Testing such basic functionality ensures you have to deal with the failure when you update your system / profile. +- When you open a PR, [ofborg](https://github.com/NixOS/ofborg)'s CI _will_ run `passthru.tests` of [packages that are directly changed by your PR (according to your commits' messages)](https://github.com/NixOS/ofborg?tab=readme-ov-file#automatic-building), but if you'd want to use the [`@ofborg build`](https://github.com/NixOS/ofborg?tab=readme-ov-file#build) command for dependent packages, you won't have to specify in addition the `.tests` attribute of the packages you want to build, and no body will be able to avoid these tests. For more on how to write and run package tests for Nixpkgs, see the [testing section in the package contributor guide](https://github.com/NixOS/nixpkgs/blob/master/pkgs/README.md#package-tests). diff --git a/doc/stdenv/stdenv.chapter.md b/doc/stdenv/stdenv.chapter.md index 400fa2de1e76..6d26f1dad3d4 100644 --- a/doc/stdenv/stdenv.chapter.md +++ b/doc/stdenv/stdenv.chapter.md @@ -762,6 +762,8 @@ Before and after running `make`, the hooks `preBuild` and `postBuild` are called The check phase checks whether the package was built correctly by running its test suite. The default `checkPhase` calls `make $checkTarget`, but only if the [`doCheck` variable](#var-stdenv-doCheck) is enabled. +It is highly recommended, for packages' sources that are not distributed with any tests, to at least use [`versionCheckHook`](#versioncheckhook) to test that the resulting executable is basically functional. + #### Variables controlling the check phase {#variables-controlling-the-check-phase} ##### `doCheck` {#var-stdenv-doCheck} From d69284079e863e14aca31f7ca10e18548a6da492 Mon Sep 17 00:00:00 2001 From: Doron Behar Date: Tue, 18 Jun 2024 09:36:52 +0300 Subject: [PATCH 4/5] syncthingtray: use versionCheckHook --- pkgs/applications/misc/syncthingtray/default.nix | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/pkgs/applications/misc/syncthingtray/default.nix b/pkgs/applications/misc/syncthingtray/default.nix index b5bf0696f764..b66c4f74d724 100644 --- a/pkgs/applications/misc/syncthingtray/default.nix +++ b/pkgs/applications/misc/syncthingtray/default.nix @@ -31,6 +31,7 @@ during runtime. Alternatively, one can edit the desktop file themselves after it is generated See: https://github.com/NixOS/nixpkgs/issues/199596#issuecomment-1310136382 */ , autostartExecPath ? "syncthingtray" +, versionCheckHook }: stdenv.mkDerivation (finalAttrs: { @@ -85,9 +86,10 @@ stdenv.mkDerivation (finalAttrs: { # Make binary available in PATH like on other platforms ln -s $out/Applications/syncthingtray.app/Contents/MacOS/syncthingtray $out/bin/syncthingtray ''; - installCheckPhase = '' - $out/bin/syncthingtray --help | grep ${finalAttrs.version} - ''; + nativeInstallCheckInputs = [ + versionCheckHook + ]; + doInstallCheck = true; cmakeFlags = [ "-DQT_PACKAGE_PREFIX=Qt${lib.versions.major qtbase.version}" From 42a85f473529f2fd7bd6af41b1272c7b0feacbd9 Mon Sep 17 00:00:00 2001 From: Doron Behar Date: Tue, 18 Jun 2024 09:41:18 +0300 Subject: [PATCH 5/5] hello: use also versionCheckHook --- pkgs/by-name/he/hello/package.nix | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/pkgs/by-name/he/hello/package.nix b/pkgs/by-name/he/hello/package.nix index ca585ce03bed..7b8db4c7c3c2 100644 --- a/pkgs/by-name/he/hello/package.nix +++ b/pkgs/by-name/he/hello/package.nix @@ -4,6 +4,7 @@ , fetchurl , nixos , testers +, versionCheckHook , hello }: @@ -19,6 +20,9 @@ stdenv.mkDerivation (finalAttrs: { doCheck = true; doInstallCheck = true; + nativeInstallCheckInputs = [ + versionCheckHook + ]; # Give hello some install checks for testing purpose. postInstallCheck = ''