diff --git a/pkgs/shells/bash/5.nix b/pkgs/shells/bash/5.nix index bb26bcd77336..08b4e4bff2fc 100644 --- a/pkgs/shells/bash/5.nix +++ b/pkgs/shells/bash/5.nix @@ -6,6 +6,10 @@ updateAutotoolsGnuConfigScriptsHook, bison, util-linux, + coreutils, + libredirect, + glibcLocales, + gnused, interactive ? true, readline, @@ -29,13 +33,13 @@ lib.warnIf (withDocs != null) bash: `.override { withDocs = true; }` is deprecated, the docs are always included. '' stdenv.mkDerivation - rec { + (fa: { pname = "bash${lib.optionalString interactive "-interactive"}"; - version = "5.3${patch_suffix}"; + version = "5.3${fa.patch_suffix}"; patch_suffix = "p${toString (builtins.length upstreamPatches)}"; src = fetchurl { - url = "mirror://gnu/bash/bash-${lib.removeSuffix patch_suffix version}.tar.gz"; + url = "mirror://gnu/bash/bash-${lib.removeSuffix fa.patch_suffix fa.version}.tar.gz"; hash = "sha256-DVzYaWX4aaJs9k9Lcb57lvkKO6iz104n6OnZ1VUPMbo="; }; @@ -137,8 +141,7 @@ lib.warnIf (withDocs != null) "SHOBJ_LIBS=-lbash" ]; - nativeCheckInputs = [ util-linux ]; - doCheck = false; # dependency cycle, needs to be interactive + doCheck = false; # Can't be enabled by default due to dependency cycle, use passthru.tests.withChecks instead postInstall = '' ln -s bash "$out/bin/sh" @@ -160,6 +163,96 @@ lib.warnIf (withDocs != null) passthru = { shellPath = "/bin/bash"; tests.static = pkgsStatic.bash; + tests.withChecks = fa.finalPackage.overrideAttrs (attrs: { + doCheck = true; + + nativeCheckInputs = attrs.nativeCheckInputs or [ ] ++ [ + util-linux + libredirect.hook + glibcLocales + gnused + ]; + + meta = attrs.meta // { + # Ignore Darwin for now, because the tests fail in many more ways than on Linux + broken = attrs.meta.broken or false || stdenv.buildPlatform.isDarwin; + }; + + patches = attrs.patches or [ ] ++ [ + # See commit comment, also submitted upstream: https://lists.gnu.org/archive/html/bug-bash/2025-10/msg00054.html + ./fail-tests.patch + # See commit comment, also submitted upstream: https://lists.gnu.org/archive/html/bug-bash/2025-10/msg00055.html + ./failed-tests-output.patch + # The run-builtins test _almost_ succeeds, only has a bit of PATH trouble + # and some odd terminal column mismatch + ./fix-builtins-tests.patch + # The run-invocation test _almost_ succeeds, only has a bit of PATH trouble + ./fix-invocation-tests.patch + ]; + + preCheck = attrs.preCheck or "" + '' + # Allows looking at actual outputs for failed tests + export BASH_TSTOUT_KEEPDIR=$(mktemp -d) + export HOME=$(mktemp -d) + export NIX_REDIRECTS=${ + lib.concatMapAttrsStringSep ":" (name: value: "${name}=${value}") { + "/bin/echo" = lib.getExe' coreutils "echo"; + "/bin/cat" = lib.getExe' coreutils "cat"; + "/bin/rm" = lib.getExe' coreutils "rm"; + "/usr" = "$(mktemp -d)"; + } + } + + disabled_checks=( + # Unsets PATH and breaks, not clear + run-execscript + + # Fails on ZFS & needs a ja_JP.SJIS locale, which glibcLocales doesn't have + run-intl + + # These error with "echo: write error: Broken pipe" + run-histexpand + run-lastpipe + run-comsub + run-comsub2 + + # For some reason has an extra 'declare -x version="5.2p37"' + run-nameref + + # These print some extra 'trap -- ''' SIGPIPE' + run-trap + run-varenv + + # These rely on /dev/tty + run-read + run-test + run-vredir + + # Might also be related to not having a tty: "Inappropriate ioctl for device" + run-history + + # Can be enabled in 5.4 + run-printf + + # This is probably fixable without too much trouble, but just not having a hardcoded PATH in type5.sub doesn't cut it + # 142,143c142,147 + # < type5.sub: line 23: mkdir: command not found + # < type5.sub: line 24: cd: /build/type-23722: No such file or directory + # --- + # > cat is /bin/cat + # > cat is aliased to `echo cat' + # > /bin/cat + # > break is a shell builtin + # > break is a special shell builtin + # > ./e + run-type + ) + for check in "''${disabled_checks[@]}"; do + # Exit before running the test script + sed -i "1iecho 'Skipping test $check' >&2 && exit 0" "tests/$check" + done + ''; + }); }; meta = with lib; { @@ -181,7 +274,7 @@ lib.warnIf (withDocs != null) platforms = platforms.all; # https://github.com/NixOS/nixpkgs/issues/333338 badPlatforms = [ lib.systems.inspect.patterns.isMinGW ]; - maintainers = [ ]; + maintainers = with lib.maintainers; [ infinisil ]; mainProgram = "bash"; identifiers.cpeParts = let @@ -194,4 +287,4 @@ lib.warnIf (withDocs != null) update = lib.elemAt versionSplit 2; }; }; - } + }) diff --git a/pkgs/shells/bash/fail-tests.patch b/pkgs/shells/bash/fail-tests.patch new file mode 100644 index 000000000000..c50c41553bee --- /dev/null +++ b/pkgs/shells/bash/fail-tests.patch @@ -0,0 +1,58 @@ +From 750cfd9af2174e1fa8164e433be1379e3e367c78 Mon Sep 17 00:00:00 2001 +From: Silvan Mosberger +Date: Tue, 7 Oct 2025 15:01:00 +0200 +Subject: [PATCH 1/2] Fail run-all on non-zero exit codes and summarise the + results + +Previously it was hard to determine whether the tests passed or not. +With this change, run-all exiting with 0 means that no tests failed. + +This allows us to set up CI for our bash package in the NixOS distribution: +https://github.com/NixOS/nixpkgs/pull/435033 + +We hope that it will also be useful for other distributions to make sure +their packaging is correct. + +Co-Authored-By: Silvan Mosberger +Co-Authored-By: Aleksander Bantyev +--- + tests/run-all | 17 ++++++++++++++++- + 1 file changed, 16 insertions(+), 1 deletion(-) + +diff --git tests/run-all tests/run-all +index f9dfa604..c8f503a2 100644 +--- tests/run-all ++++ tests/run-all +@@ -58,13 +58,28 @@ rm -f ${BASH_TSTOUT} + + echo Any output from any test, unless otherwise noted, indicates a possible anomaly + ++passed=0 ++total=0 + for x in run-* + do + case $x in + $0|run-minimal|run-gprof) ;; + *.orig|*~) ;; +- *) echo $x ; sh $x ; rm -f ${BASH_TSTOUT} ;; ++ *) ++ echo $x ++ total=$(( total + 1 )) ++ if sh $x; then ++ passed=$(( passed + 1 )) ++ else ++ echo "$x EXITED NON-ZERO ($?) - possible anomaly unless otherwise noted" ++ fi ++ rm -f "${BASH_TSTOUT}" ++ ;; + esac + done + ++echo "$passed/$total tests had exit code zero" ++if [ "$passed" -ne "$total" ]; then ++ exit 1 ++fi + exit 0 +-- +2.49.0 + diff --git a/pkgs/shells/bash/failed-tests-output.patch b/pkgs/shells/bash/failed-tests-output.patch new file mode 100644 index 000000000000..a2783422778c --- /dev/null +++ b/pkgs/shells/bash/failed-tests-output.patch @@ -0,0 +1,40 @@ +From 30c4d7fb34a09cf0a711c43231bd13fc380844d0 Mon Sep 17 00:00:00 2001 +From: Silvan Mosberger +Date: Tue, 7 Oct 2025 15:08:49 +0200 +Subject: [PATCH 2/2] allow keeping output of potentially failed tests + +Previously if a test produced a diff, the only way to fix it was to +inspect the test output. + +We introduce the BASH_TSTOUT_KEEPDIR environment variable which is +expected to point to a directory. +If set, that directory will be populated with output from all failed +tests, with test names as file names. + +This makes debugging or updating the expected test outputs easier. + +Co-Authored-By: Silvan Mosberger +Co-Authored-By: Aleksander Bantyev +--- + tests/run-all | 5 +++++ + 1 file changed, 5 insertions(+) + +diff --git tests/run-all tests/run-all +index c8f503a2..5d769d0f 100644 +--- tests/run-all ++++ tests/run-all +@@ -72,6 +72,11 @@ do + passed=$(( passed + 1 )) + else + echo "$x EXITED NON-ZERO ($?) - possible anomaly unless otherwise noted" ++ if [ "${BASH_TSTOUT_KEEPDIR+set}" = "set" ]; then ++ cp "${BASH_TSTOUT}" "${BASH_TSTOUT_KEEPDIR}/$x" ++ else ++ echo "Hint: set BASH_TSTOUT_KEEPDIR environment variable to a directory to keep test output there" ++ fi + fi + rm -f "${BASH_TSTOUT}" + ;; +-- +2.49.0 + diff --git a/pkgs/shells/bash/fix-builtins-tests.patch b/pkgs/shells/bash/fix-builtins-tests.patch new file mode 100644 index 000000000000..cd7b598a7713 --- /dev/null +++ b/pkgs/shells/bash/fix-builtins-tests.patch @@ -0,0 +1,26 @@ +diff --git tests/builtins.right tests/builtins.right +index d708c18..32b2cfd 100644 +--- tests/builtins.right ++++ tests/builtins.right +@@ -362,7 +362,7 @@ A star (*) next to a name means that the command is disabled. + complete [-abcdefgjksuv] [-pr] [-DEI]> set [-abefhkmnptuvxBCEHPT] [-o optio> + compopt [-o|+o option] [-DEI] [name .> shift [n] + continue [n] shopt [-pqsu] [-o] [optname ...] +- coproc [NAME] command [redirections] source [-p path] filename [argument> ++ coproc [NAME] command [redirections] source [-p path] filename [arguments> + declare [-aAfFgiIlnrtux] [name[=value> suspend [-f] + dirs [-clpv] [+N] [-N] test [expr] + disown [-h] [-ar] [jobspec ... | pid > time [-p] pipeline +diff --git tests/source8.sub tests/source8.sub +index a6826da..fc4ae8c 100644 +--- tests/source8.sub ++++ tests/source8.sub +@@ -15,7 +15,7 @@ + # test various uses of source -p + + : ${THIS_SH:=./bash} +-PATH=/bin:/usr/bin:/sbin:/usr/sbin ++PATH="$(dirname "$(type -P mkdir)")" + + : ${TMPDIR:=/var/tmp} + export TDIR=${TMPDIR}/cwd-$$ diff --git a/pkgs/shells/bash/fix-invocation-tests.patch b/pkgs/shells/bash/fix-invocation-tests.patch new file mode 100644 index 000000000000..8c97566ffb54 --- /dev/null +++ b/pkgs/shells/bash/fix-invocation-tests.patch @@ -0,0 +1,13 @@ +diff --git tests/invocation.tests tests/invocation.tests +index c629c29..537e470 100644 +--- tests/invocation.tests ++++ tests/invocation.tests +@@ -71,7 +71,7 @@ command cd -L $TDIR + ./x23 + + # this should result in a cannot execute binary file error since ls is in $PATH +-PATH=/bin:/usr/bin ++#PATH=/bin:/usr/bin + ${THIS_SH} ls |& sed 's|^.*: ||' + + cd $SAVEPWD