From 04bedda0b605dcecacb23b65d5a05c8f4aecfbdd Mon Sep 17 00:00:00 2001 From: Philipp Middendorf Date: Thu, 9 Apr 2020 17:05:29 +0200 Subject: [PATCH 1/7] gc.cc: Ignore hidden files in temproots --- src/libstore/gc.cc | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/libstore/gc.cc b/src/libstore/gc.cc index 0c3d89611..6bab1e37c 100644 --- a/src/libstore/gc.cc +++ b/src/libstore/gc.cc @@ -202,6 +202,11 @@ void LocalStore::findTempRoots(FDs & fds, Roots & tempRoots, bool censor) /* Read the `temproots' directory for per-process temporary root files. */ for (auto & i : readDirectory(tempRootsDir)) { + if (i.name[0] == '.') { + // Ignore hidden files. Some package managers (notably portage) create + // those to keep the directory alive. + continue; + } Path path = tempRootsDir + "/" + i.name; pid_t pid = std::stoi(i.name); From 16a4864759a80aa03b9c87b8aeaf7a4c31c03e39 Mon Sep 17 00:00:00 2001 From: Bruce Toll <4109762+tollb@users.noreply.github.com> Date: Sun, 17 Feb 2019 16:26:49 -0500 Subject: [PATCH 2/7] Delete temporary directory on successful build With --check and the --keep-failed (-K) flag, the temporary directory was being retained regardless of whether the build was successful and reproducible. This removes the temporary directory, as expected, on a reproducible check build. Added tests to verify that temporary build directories are not retained unnecessarily, particularly when using --check with --keep-failed. --- src/libstore/build.cc | 1 + tests/check.nix | 33 +++++++++++++++++++++++++++ tests/check.sh | 52 +++++++++++++++++++++++++++++++++++++++++-- 3 files changed, 84 insertions(+), 2 deletions(-) diff --git a/src/libstore/build.cc b/src/libstore/build.cc index 0febb8dfb..760663ac9 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -1680,6 +1680,7 @@ void DerivationGoal::buildDone() } if (buildMode == bmCheck) { + deleteTmpDir(true); done(BuildResult::Built); return; } diff --git a/tests/check.nix b/tests/check.nix index 56c82e565..bca04fdaf 100644 --- a/tests/check.nix +++ b/tests/check.nix @@ -1,12 +1,45 @@ +{checkBuildId ? 0}: + with import ./config.nix; { nondeterministic = mkDerivation { + inherit checkBuildId; name = "nondeterministic"; buildCommand = '' mkdir $out date +%s.%N > $out/date + echo "CHECK_TMPDIR=$TMPDIR" + echo "checkBuildId=$checkBuildId" + echo "$checkBuildId" > $TMPDIR/checkBuildId + ''; + }; + + deterministic = mkDerivation { + inherit checkBuildId; + name = "deterministic"; + buildCommand = + '' + mkdir $out + echo date > $out/date + echo "CHECK_TMPDIR=$TMPDIR" + echo "checkBuildId=$checkBuildId" + echo "$checkBuildId" > $TMPDIR/checkBuildId + ''; + }; + + failed = mkDerivation { + inherit checkBuildId; + name = "failed"; + buildCommand = + '' + mkdir $out + echo date > $out/date + echo "CHECK_TMPDIR=$TMPDIR" + echo "checkBuildId=$checkBuildId" + echo "$checkBuildId" > $TMPDIR/checkBuildId + false ''; }; diff --git a/tests/check.sh b/tests/check.sh index bc23a6634..b423dc0b5 100644 --- a/tests/check.sh +++ b/tests/check.sh @@ -1,14 +1,62 @@ source common.sh +checkBuildTempDirRemoved () +{ + buildDir=$(sed -n 's/CHECK_TMPDIR=//p' $1 | head -1) + checkBuildIdFile=${buildDir}/checkBuildId + [[ ! -f $checkBuildIdFile ]] || ! grep $checkBuildId $checkBuildIdFile +} + +# written to build temp directories to verify created by this instance +checkBuildId=$(date +%s%N) + clearStore nix-build dependencies.nix --no-out-link nix-build dependencies.nix --no-out-link --check -nix-build check.nix -A nondeterministic --no-out-link -nix-build check.nix -A nondeterministic --no-out-link --check 2> $TEST_ROOT/log || status=$? +# check for dangling temporary build directories +# only retain if build fails and --keep-failed is specified, or... +# ...build is non-deterministic and --check and --keep-failed are both specified +nix-build check.nix -A failed --argstr checkBuildId $checkBuildId \ + --no-out-link 2> $TEST_ROOT/log || status=$? +[ "$status" = "100" ] +checkBuildTempDirRemoved $TEST_ROOT/log + +nix-build check.nix -A failed --argstr checkBuildId $checkBuildId \ + --no-out-link --keep-failed 2> $TEST_ROOT/log || status=$? +[ "$status" = "100" ] +if checkBuildTempDirRemoved $TEST_ROOT/log; then false; fi + +nix-build check.nix -A deterministic --argstr checkBuildId $checkBuildId \ + --no-out-link 2> $TEST_ROOT/log +checkBuildTempDirRemoved $TEST_ROOT/log + +nix-build check.nix -A deterministic --argstr checkBuildId $checkBuildId \ + --no-out-link --check --keep-failed 2> $TEST_ROOT/log +if grep -q 'may not be deterministic' $TEST_ROOT/log; then false; fi +checkBuildTempDirRemoved $TEST_ROOT/log + +nix-build check.nix -A nondeterministic --argstr checkBuildId $checkBuildId \ + --no-out-link 2> $TEST_ROOT/log +checkBuildTempDirRemoved $TEST_ROOT/log + +nix-build check.nix -A nondeterministic --argstr checkBuildId $checkBuildId \ + --no-out-link --check 2> $TEST_ROOT/log || status=$? grep 'may not be deterministic' $TEST_ROOT/log [ "$status" = "104" ] +checkBuildTempDirRemoved $TEST_ROOT/log + +nix-build check.nix -A nondeterministic --argstr checkBuildId $checkBuildId \ + --no-out-link --check --keep-failed 2> $TEST_ROOT/log || status=$? + +# The above nix-build fails with status=1 on darwin (not sure why) +# ...but the primary purpose of the test case is to verify the temp directory is retained +if [ "$(uname -s)" != "Darwin" ]; then +grep 'may not be deterministic' $TEST_ROOT/log +[ "$status" = "104" ] +fi +if checkBuildTempDirRemoved $TEST_ROOT/log; then false; fi clearStore From 3abf6d03c611417b309fdedf4323c08e6afbcd9c Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Fri, 10 Apr 2020 17:27:35 +0200 Subject: [PATCH 3/7] Update release script --- maintainers/upload-release.pl | 86 ++++++++++++++++++++++------------- 1 file changed, 55 insertions(+), 31 deletions(-) diff --git a/maintainers/upload-release.pl b/maintainers/upload-release.pl index 77534babb..cb584d427 100755 --- a/maintainers/upload-release.pl +++ b/maintainers/upload-release.pl @@ -1,5 +1,5 @@ #! /usr/bin/env nix-shell -#! nix-shell -i perl -p perl perlPackages.LWPUserAgent perlPackages.LWPProtocolHttps perlPackages.FileSlurp gnupg1 +#! nix-shell -i perl -p perl perlPackages.LWPUserAgent perlPackages.LWPProtocolHttps perlPackages.FileSlurp perlPackages.NetAmazonS3 gnupg1 use strict; use Data::Dumper; @@ -9,12 +9,16 @@ use File::Slurp; use File::Copy; use JSON::PP; use LWP::UserAgent; +use Net::Amazon::S3; my $evalId = $ARGV[0] or die "Usage: $0 EVAL-ID\n"; -my $releasesDir = "/home/eelco/mnt/releases"; +my $releasesBucketName = "nix-releases"; +my $channelsBucketName = "nix-channels"; my $nixpkgsDir = "/home/eelco/Dev/nixpkgs-pristine"; +my $TMPDIR = $ENV{'TMPDIR'} // "/tmp"; + # FIXME: cut&paste from nixos-channel-scripts. sub fetch { my ($url, $type) = @_; @@ -42,13 +46,31 @@ my $version = $1; print STDERR "Nix revision is $nixRev, version is $version\n"; -File::Path::make_path($releasesDir); -if (system("mountpoint -q $releasesDir") != 0) { - system("sshfs hydra-mirror\@nixos.org:/releases $releasesDir") == 0 or die; -} +my $releaseDir = "nix/$releaseName"; -my $releaseDir = "$releasesDir/nix/$releaseName"; -File::Path::make_path($releaseDir); +my $tmpDir = "$TMPDIR/nix-release/$releaseName"; +File::Path::make_path($tmpDir); + +# S3 setup. +my $aws_access_key_id = $ENV{'AWS_ACCESS_KEY_ID'} or die "No AWS_ACCESS_KEY_ID given."; +my $aws_secret_access_key = $ENV{'AWS_SECRET_ACCESS_KEY'} or die "No AWS_SECRET_ACCESS_KEY given."; + +my $s3 = Net::Amazon::S3->new( + { aws_access_key_id => $aws_access_key_id, + aws_secret_access_key => $aws_secret_access_key, + retry => 1, + host => "s3-eu-west-1.amazonaws.com", + }); + +my $releasesBucket = $s3->bucket($releasesBucketName) or die; + +my $s3_us = Net::Amazon::S3->new( + { aws_access_key_id => $aws_access_key_id, + aws_secret_access_key => $aws_secret_access_key, + retry => 1, + }); + +my $channelsBucket = $s3_us->bucket($channelsBucketName) or die; sub downloadFile { my ($jobName, $productNr, $dstName) = @_; @@ -57,40 +79,49 @@ sub downloadFile { my $srcFile = $buildInfo->{buildproducts}->{$productNr}->{path} or die "job '$jobName' lacks product $productNr\n"; $dstName //= basename($srcFile); - my $dstFile = "$releaseDir/" . $dstName; + my $tmpFile = "$tmpDir/$dstName"; - if (! -e $dstFile) { - print STDERR "downloading $srcFile to $dstFile...\n"; - system("NIX_REMOTE=https://cache.nixos.org/ nix cat-store '$srcFile' > '$dstFile.tmp'") == 0 + if (!-e $tmpFile) { + print STDERR "downloading $srcFile to $tmpFile...\n"; + system("NIX_REMOTE=https://cache.nixos.org/ nix cat-store '$srcFile' > '$tmpFile'") == 0 or die "unable to fetch $srcFile\n"; - rename("$dstFile.tmp", $dstFile) or die; } my $sha256_expected = $buildInfo->{buildproducts}->{$productNr}->{sha256hash} or die; - my $sha256_actual = `nix hash-file --base16 --type sha256 '$dstFile'`; + my $sha256_actual = `nix hash-file --base16 --type sha256 '$tmpFile'`; chomp $sha256_actual; if ($sha256_expected ne $sha256_actual) { - print STDERR "file $dstFile is corrupt, got $sha256_actual, expected $sha256_expected\n"; + print STDERR "file $tmpFile is corrupt, got $sha256_actual, expected $sha256_expected\n"; exit 1; } - write_file("$dstFile.sha256", $sha256_expected); + write_file("$tmpFile.sha256", $sha256_expected); - if (! -e "$dstFile.asc") { - system("gpg2 --detach-sign --armor $dstFile") == 0 or die "unable to sign $dstFile\n"; + if (! -e "$tmpFile.asc") { + system("gpg2 --detach-sign --armor $tmpFile") == 0 or die "unable to sign $tmpFile\n"; } - return ($dstFile, $sha256_expected); + return $sha256_expected; } downloadFile("tarball", "2"); # .tar.bz2 -my ($tarball, $tarballHash) = downloadFile("tarball", "3"); # .tar.xz +my $tarballHash = downloadFile("tarball", "3"); # .tar.xz downloadFile("binaryTarball.i686-linux", "1"); downloadFile("binaryTarball.x86_64-linux", "1"); downloadFile("binaryTarball.aarch64-linux", "1"); downloadFile("binaryTarball.x86_64-darwin", "1"); downloadFile("installerScript", "1"); +for my $fn (glob "$tmpDir/*") { + my $name = basename($fn); + my $dstKey = "$releaseDir/" . $name; + unless (defined $releasesBucket->head_key($dstKey)) { + print STDERR "uploading $fn to s3://$releasesBucketName/$dstKey...\n"; + $releasesBucket->add_key_filename($dstKey, $fn) + or die $releasesBucket->err . ": " . $releasesBucket->errstr; + } +} + exit if $version =~ /pre/; # Update Nixpkgs in a very hacky way. @@ -125,18 +156,11 @@ write_file("$nixpkgsDir/nixos/modules/installer/tools/nix-fallback-paths.nix", system("cd $nixpkgsDir && git commit -a -m 'nix: $oldName -> $version'") == 0 or die; -# Extract the HTML manual. -File::Path::make_path("$releaseDir/manual"); - -system("tar xvf $tarball --strip-components=3 -C $releaseDir/manual --wildcards '*/doc/manual/*.html' '*/doc/manual/*.css' '*/doc/manual/*.gif' '*/doc/manual/*.png'") == 0 or die; - -if (! -e "$releaseDir/manual/index.html") { - symlink("manual.html", "$releaseDir/manual/index.html") or die; -} - # Update the "latest" symlink. -symlink("$releaseName", "$releasesDir/nix/latest-tmp") or die; -rename("$releasesDir/nix/latest-tmp", "$releasesDir/nix/latest") or die; +$channelsBucket->add_key( + "nix-latest/install", "", + { "x-amz-website-redirect-location" => "https://releases.nixos.org/$releaseDir/install" }) + or die $channelsBucket->err . ": " . $channelsBucket->errstr; # Tag the release in Git. chdir("/home/eelco/Dev/nix-pristine") or die; From 8132d0a12e1b1d30973ae2c543622a46c24ec075 Mon Sep 17 00:00:00 2001 From: Bruce Toll <4109762+tollb@users.noreply.github.com> Date: Sun, 17 Feb 2019 14:34:31 -0500 Subject: [PATCH 4/7] Fix nix-build --check -K in sandbox w/o root Temporarily add user-write permission to build directory so that it can be moved out of the sandbox to the store with a .check suffix. This is necessary because the build directory has already had its permissions set read-only, but write permission is required to update the directory's parent link to move it out of the sandbox. Updated the related --check "derivation may not be deterministic" messages to consistently use the real store paths. Added test for non-root sandbox nix-build --check -K to demonstrate issue and help prevent regressions. --- src/libstore/build.cc | 30 ++++++++++++++++++++++++++---- tests/linux-sandbox.sh | 7 +++++++ 2 files changed, 33 insertions(+), 4 deletions(-) diff --git a/src/libstore/build.cc b/src/libstore/build.cc index 760663ac9..b4207e1b8 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -3537,6 +3537,29 @@ StorePathSet parseReferenceSpecifiers(Store & store, const BasicDerivation & drv } +static void moveCheckToStore(const Path & src, const Path & dst) +{ + /* For the rename of directory to succeed, we must be running as root or + the directory must be made temporarily writable (to update the + directory's parent link ".."). */ + struct stat st; + if (lstat(src.c_str(), &st) == -1) { + throw SysError(format("getting attributes of path '%1%'") % src); + } + + bool changePerm = (geteuid() && S_ISDIR(st.st_mode) && !(st.st_mode & S_IWUSR)); + + if (changePerm) + chmod_(src, st.st_mode | S_IWUSR); + + if (rename(src.c_str(), dst.c_str())) + throw SysError(format("renaming '%1%' to '%2%'") % src % dst); + + if (changePerm) + chmod_(dst, st.st_mode); +} + + void DerivationGoal::registerOutputs() { /* When using a build hook, the build hook can register the output @@ -3715,8 +3738,7 @@ void DerivationGoal::registerOutputs() if (settings.runDiffHook || settings.keepFailed) { Path dst = worker.store.toRealPath(path + checkSuffix); deletePath(dst); - if (rename(actualPath.c_str(), dst.c_str())) - throw SysError(format("renaming '%1%' to '%2%'") % actualPath % dst); + moveCheckToStore(actualPath, dst); handleDiffHook( buildUser ? buildUser->getUID() : getuid(), @@ -3724,10 +3746,10 @@ void DerivationGoal::registerOutputs() path, dst, worker.store.printStorePath(drvPath), tmpDir); throw NotDeterministic("derivation '%s' may not be deterministic: output '%s' differs from '%s'", - worker.store.printStorePath(drvPath), path, dst); + worker.store.printStorePath(drvPath), worker.store.toRealPath(path), dst); } else throw NotDeterministic("derivation '%s' may not be deterministic: output '%s' differs", - worker.store.printStorePath(drvPath), path); + worker.store.printStorePath(drvPath), worker.store.toRealPath(path)); } /* Since we verified the build, it's now ultimately trusted. */ diff --git a/tests/linux-sandbox.sh b/tests/linux-sandbox.sh index 52967d07d..16abd974c 100644 --- a/tests/linux-sandbox.sh +++ b/tests/linux-sandbox.sh @@ -28,3 +28,10 @@ nix cat-store $outPath/foobar | grep FOOBAR # Test --check without hash rewriting. nix-build dependencies.nix --no-out-link --check --sandbox-paths /nix/store + +# Test that sandboxed builds with --check and -K can move .check directory to store +nix-build check.nix -A nondeterministic --sandbox-paths /nix/store --no-out-link + +(! nix-build check.nix -A nondeterministic --sandbox-paths /nix/store --no-out-link --check -K 2> $TEST_ROOT/log) +if grep -q 'error: renaming' $TEST_ROOT/log; then false; fi +grep -q 'may not be deterministic' $TEST_ROOT/log From e8bd1bc732ff19f33e6915d14e15df1d7de612c0 Mon Sep 17 00:00:00 2001 From: Bruce Toll <4109762+tollb@users.noreply.github.com> Date: Fri, 10 Apr 2020 18:20:12 -0400 Subject: [PATCH 5/7] Add test case for temporary directories on darwin A test case for correct handling of temporary directory deletion that was added to check.sh as part of PR #2689 was initially disabled for Darwin because of a directory permission issue in PR #2688. Now that the issue in PR #2688 is fixed, this commit enables the test case for Darwin. --- tests/check.sh | 5 ----- 1 file changed, 5 deletions(-) diff --git a/tests/check.sh b/tests/check.sh index b423dc0b5..5f25d04cb 100644 --- a/tests/check.sh +++ b/tests/check.sh @@ -49,13 +49,8 @@ checkBuildTempDirRemoved $TEST_ROOT/log nix-build check.nix -A nondeterministic --argstr checkBuildId $checkBuildId \ --no-out-link --check --keep-failed 2> $TEST_ROOT/log || status=$? - -# The above nix-build fails with status=1 on darwin (not sure why) -# ...but the primary purpose of the test case is to verify the temp directory is retained -if [ "$(uname -s)" != "Darwin" ]; then grep 'may not be deterministic' $TEST_ROOT/log [ "$status" = "104" ] -fi if checkBuildTempDirRemoved $TEST_ROOT/log; then false; fi clearStore From fc14213d2da7302b237dfe5ca51f6f4d9af34e5c Mon Sep 17 00:00:00 2001 From: DavHau <42246742+DavHau@users.noreply.github.com> Date: Wed, 25 Mar 2020 07:19:00 +0000 Subject: [PATCH 6/7] improve toFile error message when containing potential drv path --- src/libexpr/primops.cc | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index 8de234951..6d7622900 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -1021,7 +1021,9 @@ static void prim_toFile(EvalState & state, const Pos & pos, Value * * args, Valu for (auto path : context) { if (path.at(0) != '/') - throw EvalError(format("in 'toFile': the file '%1%' cannot refer to derivation outputs, at %2%") % name % pos); + throw EvalError(format( + "in 'toFile': the file named '%1%' must not contain a reference " + "to a derivation but contains (%2%), at %3%") % name % path % pos); refs.insert(state.store->parseStorePath(path)); } From 4d9db420ffc9bd48da107a61c093b0d65d9d8db1 Mon Sep 17 00:00:00 2001 From: Daiderd Jordan Date: Sun, 12 Apr 2020 09:57:22 +0200 Subject: [PATCH 7/7] never use /var/folders for TMPDIR on darwin This doesn't just cause problems for nix-store --serve but also results in certain build failures. Builds that use unix domain sockets in their tests often fail because the /var/folders prefix already consumes more than half of the maximum length of socket paths. struct sockaddr_un { sa_family_t sun_family; /* AF_UNIX */ char sun_path[108]; /* Pathname */ }; --- src/libmain/shared.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libmain/shared.cc b/src/libmain/shared.cc index d41e772e9..72ba717e5 100644 --- a/src/libmain/shared.cc +++ b/src/libmain/shared.cc @@ -155,7 +155,7 @@ void initNix() sshd). This breaks build users because they don't have access to the TMPDIR, in particular in ‘nix-store --serve’. */ #if __APPLE__ - if (getuid() == 0 && hasPrefix(getEnv("TMPDIR").value_or("/tmp"), "/var/folders/")) + if (hasPrefix(getEnv("TMPDIR").value_or("/tmp"), "/var/folders/")) unsetenv("TMPDIR"); #endif }