diff --git a/doc/manual/rl-next/added-shell-dir-length.md b/doc/manual/rl-next/added-shell-dir-length.md new file mode 100644 index 000000000..1de244b70 --- /dev/null +++ b/doc/manual/rl-next/added-shell-dir-length.md @@ -0,0 +1,13 @@ +--- +synopsis: "Nix shells' $NIX_BUILD_TOP are shorter" +cls: [4663] +issues: [fj#1044] +category: "Fixes" +credits: [raito] +--- + +Following the changes in 2.94.0 to shorten build directory paths, aimed at [resolving UNIX domain socket length issues](https://gerrit.lix.systems/c/lix/+/4168/13) and [improving nix-shell](https://git.lix.systems/lix-project/lix/issues/940), we inadvertently introduced an excessively long path for the `$NIX_BUILD_TOP` environment variable used by Nix shells (their effective temporary `/build` directory). + +To fix this, we replaced the `build-top-$HASH` directory name with simply `build-top`, reducing these paths by at least 30 characters. + +We also added a test to ensure that Nix shells do not introduce more than 50 extra characters relative to their base directory (e.g., `/tmp` when `$TMPDIR` is not set). diff --git a/lix/legacy/nix-build.cc b/lix/legacy/nix-build.cc index e0f0916e8..7554c03a6 100644 --- a/lix/legacy/nix-build.cc +++ b/lix/legacy/nix-build.cc @@ -192,7 +192,11 @@ static int main_nix_build(AsyncIoRoot & aio, std::string programName, Strings ar throw UsageError("'-p' and '-E' are mutually exclusive"); AutoDelete tmpDir(createTempDir(myName)); - AutoDelete buildTopTmpDir(createTempSubdir(tmpDir, "build-top")); + // NOTE: we assume there's no `build-top` directory created inside of `tmpDir` and we have + // ownership of this. + auto buildTopTmpDir = tmpDir + "/build-top"; + createDirs(buildTopTmpDir); + if (outLink.empty()) outLink = (Path) tmpDir + "/result"; diff --git a/tests/functional/nix-shell/basic.sh b/tests/functional/nix-shell/basic.sh index 54b4752d2..784eb2617 100644 --- a/tests/functional/nix-shell/basic.sh +++ b/tests/functional/nix-shell/basic.sh @@ -75,6 +75,44 @@ output=$(NIX_PATH=nixpkgs="$shellDotNix" nix-shell --pure -p foo --argstr fooCon test ! -d "$(dirname $IN_SHELL_TEMPDIR)" && echo "nix-shell deleted the parent directory of \$TEMPDIR: clean up successful" || { echo "nix-shell did not delete the parent directory of \$TEMPDIR: clean up failure"; exit 1; } ) +# Test whether the added length w.r.t. base directories is reasonable enough. +# This prevents regressions that can affect negatively things like opening UNIX +# domain sockets in non-isolated builds. +( + unset TMPDIR + BASE_DIR="$(realpath /tmp)" + + LENGTH_RESULT=$( + NIX_PATH=nixpkgs="$shellDotNix" nix-shell --pure -p foo --run " + BASE_DIR_ENV=\"$BASE_DIR\" + ADDED_LENGTH=\$(( \${#NIX_BUILD_TOP} - \${#BASE_DIR_ENV} )) + echo \"\$ADDED_LENGTH:\$NIX_BUILD_TOP\" + " + ) + + ADDED_LENGTH="${LENGTH_RESULT%%:*}" + IN_SHELL_NIX_BUILD_TOP="${LENGTH_RESULT#*:}" + + # The idea of this value is len("nix-shell-$hash/build-top") + ≤5 chars of margin. + MAX_ALLOWED=50 + # The added length must be minimum 2 chars. + MIN_ALLOWED=2 + + if (( ADDED_LENGTH < MIN_ALLOWED )); then + echo "Added length ($ADDED_LENGTH) is impossibly low. The test is not correct." + exit 1 + fi + + if (( ADDED_LENGTH <= MAX_ALLOWED )); then + echo "Added length to \$NIX_BUILD_TOP in shells ($ADDED_LENGTH chars) is within acceptable limit ($MAX_ALLOWED chars)." + else + echo "ERROR: Added length too large ($ADDED_LENGTH chars > $MAX_ALLOWED chars)." + echo "Base: $BASE_DIR" + echo "NIX_BUILD_TOP: $IN_SHELL_NIX_BUILD_TOP" + exit 1 + fi +) + # FIXME: testing that Ctrl-C to a (non-)interactive nix-shell stops it would be appreciated, # but it is hard to send accurately the signal to the right process group.