From 04967dee9d08befe4661e6fa5da4a00da0538a13 Mon Sep 17 00:00:00 2001 From: Alexander Bantyev Date: Tue, 5 May 2020 13:04:36 +0300 Subject: [PATCH 01/56] Wait for build users when none are available --- src/libstore/build.cc | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/src/libstore/build.cc b/src/libstore/build.cc index 147093fae..1b7e6d75e 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -507,6 +507,9 @@ private: Path fnUserLock; AutoCloseFD fdUserLock; + bool findFreeUser(); + + string user; uid_t uid; gid_t gid; @@ -526,10 +529,19 @@ public: }; - UserLock::UserLock() { assert(settings.buildUsersGroup != ""); + createDirs(settings.nixStateDir + "/userpool"); + + if (findFreeUser()) return; + + printError("waiting for build users"); + + do std::this_thread::sleep_for(std::chrono::seconds(2)); while (! findFreeUser()); +} + +bool UserLock::findFreeUser() { /* Get the members of the build-users-group. */ struct group * gr = getgrnam(settings.buildUsersGroup.get().c_str()); @@ -559,7 +571,6 @@ UserLock::UserLock() throw Error(format("the user '%1%' in the group '%2%' does not exist") % i % settings.buildUsersGroup); - createDirs(settings.nixStateDir + "/userpool"); fnUserLock = (format("%1%/userpool/%2%") % settings.nixStateDir % pw->pw_uid).str(); @@ -590,16 +601,12 @@ UserLock::UserLock() supplementaryGIDs.resize(ngroups); #endif - return; + return true; } } - - throw Error(format("all build users are currently in use; " - "consider creating additional users and adding them to the '%1%' group") - % settings.buildUsersGroup); + return false; } - void UserLock::kill() { killUser(uid); From 9be46859a90d5a0771e079615d324644a08efb41 Mon Sep 17 00:00:00 2001 From: Peter Kolloch Date: Wed, 6 May 2020 11:21:12 +0200 Subject: [PATCH 02/56] libstore/build.cc: more explicit about form of output Be more explicit about why we expect a regular file as output when outputHashMode=flat for a fixed output derivation. --- src/libstore/build.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/libstore/build.cc b/src/libstore/build.cc index 572634765..e0c7324c7 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -3678,7 +3678,8 @@ void DerivationGoal::registerOutputs() /* The output path should be a regular file without execute permission. */ if (!S_ISREG(st.st_mode) || (st.st_mode & S_IXUSR) != 0) throw BuildError( - format("output path '%1%' should be a non-executable regular file") % path); + format("output path '%1%' should be a non-executable regular file " + "since recursive hashing is not enabled (outputHashMode=flat).") % path); } /* Check the hash. In hash mode, move the path produced by From 58ed1e6d6842688ac2d32355265e0d89049f7e36 Mon Sep 17 00:00:00 2001 From: Tobias Pflug Date: Tue, 5 May 2020 17:56:46 +0200 Subject: [PATCH 03/56] WIP: add unit tests for libutil This is a proof on concept to evaluate writing unit tests for Nix using google test (https://github.com/google/googletest). In order to execute tests: $ make unit-tests $ ./unit-tests The Makefile rules for `unit-tests` is a complete hack. --- Makefile | 3 +- release-common.nix | 1 + tests/unit-tests/local.mk | 4 + tests/unit-tests/test-util.cc | 594 ++++++++++++++++++++++++++++++++++ 4 files changed, 601 insertions(+), 1 deletion(-) create mode 100644 tests/unit-tests/local.mk create mode 100644 tests/unit-tests/test-util.cc diff --git a/Makefile b/Makefile index e3057c36c..06de35a27 100644 --- a/Makefile +++ b/Makefile @@ -16,7 +16,8 @@ makefiles = \ misc/upstart/local.mk \ doc/manual/local.mk \ tests/local.mk \ - tests/plugins/local.mk + tests/plugins/local.mk \ + tests/unit-tests/local.mk -include Makefile.config diff --git a/release-common.nix b/release-common.nix index b6c8f3d81..7e7de005d 100644 --- a/release-common.nix +++ b/release-common.nix @@ -55,6 +55,7 @@ rec { # Tests git mercurial + gmock ] ++ lib.optionals stdenv.isLinux [libseccomp utillinuxMinimal] ++ lib.optional (stdenv.isLinux || stdenv.isDarwin) libsodium diff --git a/tests/unit-tests/local.mk b/tests/unit-tests/local.mk new file mode 100644 index 000000000..ed11261c1 --- /dev/null +++ b/tests/unit-tests/local.mk @@ -0,0 +1,4 @@ +LIBS=-larchive -lcrypto -llzma -lbz2 -lz -lbrotlienc -lbrotlidec +unit-tests: + echo $(nix_LDFLAGS) + $(CXX) -o unit-test $(nix_CXXFLAGS) $(nix_LDFLAGS) $(LIBS) --std=c++17 $$(pkg-config --libs gtest_main) ./src/libutil/*.o tests/unit-tests/test-util.cc diff --git a/tests/unit-tests/test-util.cc b/tests/unit-tests/test-util.cc new file mode 100644 index 000000000..05d8a0039 --- /dev/null +++ b/tests/unit-tests/test-util.cc @@ -0,0 +1,594 @@ +#include "../../src/libutil/util.hh" +#include "../../src/libutil/types.hh" + +#include + +namespace nix { + + /* ---------------------------------------------------------------------------- + * absPath + * --------------------------------------------------------------------------*/ + + TEST(absPath, doesntChangeRoot) { + auto p = absPath("/"); + + ASSERT_STREQ(p.c_str(), "/"); + } + + TEST(absPath, turnsEmptyPathIntoCWD) { + char cwd[PATH_MAX+1]; + auto p = absPath(""); + + ASSERT_STREQ(p.c_str(), getcwd((char*)&cwd, PATH_MAX)); + } + + TEST(absPath, usesOptionalBasePathWhenGiven) { + char _cwd[PATH_MAX+1]; + char* cwd = getcwd((char*)&_cwd, PATH_MAX); + + auto p = absPath("", cwd); + + ASSERT_STREQ(p.c_str(), cwd); + } + + TEST(absPath, isIdempotent) { + char _cwd[PATH_MAX+1]; + char* cwd = getcwd((char*)&_cwd, PATH_MAX); + auto p1 = absPath(cwd); + auto p2 = absPath(p1); + + ASSERT_STREQ(p1.c_str(), p2.c_str()); + } + + + TEST(absPath, pathIsCanonicalised) { + auto path = "/some/path/with/trailing/dot/."; + auto p1 = absPath(path); + auto p2 = absPath(p1); + + ASSERT_STREQ(p1.c_str(), "/some/path/with/trailing/dot"); + ASSERT_STREQ(p1.c_str(), p2.c_str()); + } + + /* ---------------------------------------------------------------------------- + * canonPath + * --------------------------------------------------------------------------*/ + + TEST(canonPath, removesTrailingSlashes) { + auto path = "/this/is/a/path//"; + auto p = canonPath(path); + + ASSERT_STREQ(p.c_str(), "/this/is/a/path"); + } + + TEST(canonPath, removesDots) { + auto path = "/this/./is/a/path/./"; + auto p = canonPath(path); + + ASSERT_STREQ(p.c_str(), "/this/is/a/path"); + } + + TEST(canonPath, removesDots2) { + auto path = "/this/a/../is/a////path/foo/.."; + auto p = canonPath(path); + + ASSERT_STREQ(p.c_str(), "/this/is/a/path"); + } + + TEST(canonPath, requiresAbsolutePath) { + ASSERT_ANY_THROW(canonPath(".")); + ASSERT_ANY_THROW(canonPath("..")); + ASSERT_ANY_THROW(canonPath("../")); + ASSERT_DEATH({ canonPath(""); }, "path != \"\""); + } + + /* ---------------------------------------------------------------------------- + * dirOf + * --------------------------------------------------------------------------*/ + + // XXX: according to the doc of `dirOf`, dirOf("/") and dirOf("/foo") + // should both return "" but it actually returns "/" in both cases + TEST(dirOf, DISABLED_returnsEmptyStringForRoot) { + auto p = dirOf("/"); + + ASSERT_STREQ(p.c_str(), ""); + } + + TEST(dirOf, returnsFirstPathComponent) { + auto p1 = dirOf("/dir/"); + ASSERT_STREQ(p1.c_str(), "/dir"); + auto p2 = dirOf("/dir"); + ASSERT_STREQ(p2.c_str(), "/"); + auto p3 = dirOf("/dir/.."); + ASSERT_STREQ(p3.c_str(), "/dir"); + auto p4 = dirOf("/dir/../"); + ASSERT_STREQ(p4.c_str(), "/dir/.."); + } + + /* ---------------------------------------------------------------------------- + * baseNameOf + * --------------------------------------------------------------------------*/ + + TEST(baseNameOf, emptyPath) { + auto p1 = baseNameOf(""); + ASSERT_STREQ(std::string(p1).c_str(), ""); + } + + TEST(baseNameOf, pathOnRoot) { + auto p1 = baseNameOf("/dir"); + ASSERT_STREQ(std::string(p1).c_str(), "dir"); + } + + TEST(baseNameOf, relativePath) { + auto p1 = baseNameOf("dir/foo"); + ASSERT_STREQ(std::string(p1).c_str(), "foo"); + } + + TEST(baseNameOf, pathWithTrailingSlashRoot) { + auto p1 = baseNameOf("/"); + ASSERT_STREQ(std::string(p1).c_str(), ""); + } + + // XXX: according to the doc of `baseNameOf`, baseNameOf("/dir/") should return + // "" but it actually returns "dir" + TEST(baseNameOf, DISABLED_trailingSlash) { + auto p1 = baseNameOf("/dir/"); + ASSERT_STREQ(std::string(p1).c_str(), ""); + } + + /* ---------------------------------------------------------------------------- + * isInDir + * --------------------------------------------------------------------------*/ + + TEST(isInDir, trivialCase) { + auto p1 = isInDir("/foo/bar", "/foo"); + ASSERT_EQ(p1, true); + } + + TEST(isInDir, notInDir) { + auto p1 = isInDir("/zes/foo/bar", "/foo"); + ASSERT_EQ(p1, false); + } + + // XXX: hm, bug or feature? :) Looking at the implementation + // this might be problematic. + TEST(isInDir, emptyDir) { + auto p1 = isInDir("/zes/foo/bar", ""); + ASSERT_EQ(p1, true); + } + + /* ---------------------------------------------------------------------------- + * isDirOrInDir + * --------------------------------------------------------------------------*/ + + TEST(isDirOrInDir, trueForSameDirectory) { + ASSERT_EQ(isDirOrInDir("/nix", "/nix"), true); + ASSERT_EQ(isDirOrInDir("/", "/"), true); + } + + TEST(isDirOrInDir, trueForEmptyPaths) { + ASSERT_EQ(isDirOrInDir("", ""), true); + } + + TEST(isDirOrInDir, falseForDisjunctPaths) { + ASSERT_EQ(isDirOrInDir("/foo", "/bar"), false); + } + + TEST(isDirOrInDir, relativePaths) { + ASSERT_EQ(isDirOrInDir("/foo/..", "/foo"), true); + } + + // XXX: while it is possible to use "." or ".." in the + // first argument this doesn't seem to work in the second. + TEST(isDirOrInDir, DISABLED_shouldWork) { + ASSERT_EQ(isDirOrInDir("/foo/..", "/foo/."), true); + + } + + /* ---------------------------------------------------------------------------- + * pathExists + * --------------------------------------------------------------------------*/ + + TEST(pathExists, rootExists) { + ASSERT_TRUE(pathExists("/")); + } + + TEST(pathExists, cwdExists) { + ASSERT_TRUE(pathExists(".")); + } + + TEST(pathExists, bogusPathDoesNotExist) { + ASSERT_FALSE(pathExists("/home/schnitzel/darmstadt/pommes")); + } + + /* ---------------------------------------------------------------------------- + * concatStringsSep + * --------------------------------------------------------------------------*/ + + TEST(concatStringsSep, buildCommaSeparatedString) { + Strings strings; + strings.push_back("this"); + strings.push_back("is"); + strings.push_back("great"); + + ASSERT_EQ(concatStringsSep(",", strings), "this,is,great"); + } + + TEST(concatStringsSep, buildStringWithEmptySeparator) { + Strings strings; + strings.push_back("this"); + strings.push_back("is"); + strings.push_back("great"); + + ASSERT_EQ(concatStringsSep("", strings), "thisisgreat"); + } + + TEST(concatStringsSep, buildSingleString) { + Strings strings; + strings.push_back("this"); + + ASSERT_EQ(concatStringsSep(",", strings), "this"); + } + + /* ---------------------------------------------------------------------------- + * hasPrefix + * --------------------------------------------------------------------------*/ + + TEST(hasPrefix, emptyStringHasNoPrefix) { + ASSERT_FALSE(hasPrefix("", "foo")); + } + + TEST(hasPrefix, emptyStringIsAlwaysPrefix) { + ASSERT_TRUE(hasPrefix("foo", "")); + ASSERT_TRUE(hasPrefix("jshjkfhsadf", "")); + } + + TEST(hasPrefix, trivialCase) { + ASSERT_TRUE(hasPrefix("foobar", "foo")); + } + + /* ---------------------------------------------------------------------------- + * hasSuffix + * --------------------------------------------------------------------------*/ + + TEST(hasSuffix, emptyStringHasNoSuffix) { + ASSERT_FALSE(hasSuffix("", "foo")); + } + + TEST(hasSuffix, trivialCase) { + ASSERT_TRUE(hasSuffix("foo", "foo")); + ASSERT_TRUE(hasSuffix("foobar", "bar")); + } + + /* ---------------------------------------------------------------------------- + * base64Encode + * --------------------------------------------------------------------------*/ + + TEST(base64Encode, emptyString) { + ASSERT_STREQ(base64Encode("").c_str(), ""); + } + + TEST(base64Encode, encodesAString) { + ASSERT_STREQ(base64Encode("quod erat demonstrandum").c_str(), "cXVvZCBlcmF0IGRlbW9uc3RyYW5kdW0="); + } + + TEST(base64Encode, encodeAndDecode) { + auto s = "quod erat demonstrandum"; + auto encoded = base64Encode(s); + auto decoded = base64Decode(encoded); + + ASSERT_STREQ(decoded.c_str(), s); + } + + /* ---------------------------------------------------------------------------- + * base64Decode + * --------------------------------------------------------------------------*/ + + TEST(base64Decode, emptyString) { + ASSERT_STREQ(base64Decode("").c_str(), ""); + } + + TEST(base64Decode, decodeAString) { + ASSERT_STREQ(base64Decode("cXVvZCBlcmF0IGRlbW9uc3RyYW5kdW0=").c_str(), "quod erat demonstrandum"); + } + + /* ---------------------------------------------------------------------------- + * toLower + * --------------------------------------------------------------------------*/ + + TEST(toLower, emptyString) { + ASSERT_STREQ(toLower("").c_str(), ""); + } + + TEST(toLower, nonLetters) { + auto s = "!@(*$#)(@#=\\234_"; + ASSERT_STREQ(toLower(s).c_str(), s); + } + + // XXX: std::tolower() doesn't cover this. This probably doesn't matter + // since the context is paths and store paths specifically where such + // characters are not allowed. + TEST(toLower, DISABLED_umlauts) { + auto s = "ÄÖÜ"; + ASSERT_STREQ(toLower(s).c_str(), "äöü"); + } + + /* ---------------------------------------------------------------------------- + * string2Float + * --------------------------------------------------------------------------*/ + + TEST(string2Float, emptyString) { + double n; + ASSERT_EQ(string2Float("", n), false); + } + + TEST(string2Float, trivialConversions) { + double n; + ASSERT_EQ(string2Float("1.0", n), true); + ASSERT_EQ(n, 1.0); + + ASSERT_EQ(string2Float("0.0", n), true); + ASSERT_EQ(n, 0.0); + + ASSERT_EQ(string2Float("-100.25", n), true); + ASSERT_EQ(n, (-100.25)); + } + + /* ---------------------------------------------------------------------------- + * string2Int + * --------------------------------------------------------------------------*/ + + TEST(string2Int, emptyString) { + double n; + ASSERT_EQ(string2Int("", n), false); + } + + TEST(string2Int, trivialConversions) { + double n; + ASSERT_EQ(string2Int("1", n), true); + ASSERT_EQ(n, 1); + + ASSERT_EQ(string2Int("0", n), true); + ASSERT_EQ(n, 0); + + ASSERT_EQ(string2Int("-100", n), true); + ASSERT_EQ(n, (-100)); + } + + /* ---------------------------------------------------------------------------- + * statusOk + * --------------------------------------------------------------------------*/ + + TEST(statusOk, zeroIsOk) { + ASSERT_EQ(statusOk(0), true); + ASSERT_EQ(statusOk(1), false); + } + + /* ---------------------------------------------------------------------------- + * replaceInSet : XXX: this function isn't called anywhere! + * --------------------------------------------------------------------------*/ + + + /* ---------------------------------------------------------------------------- + * rewriteStrings + * --------------------------------------------------------------------------*/ + + TEST(rewriteStrings, emptyString) { + StringMap rewrites; + rewrites["this"] = "that"; + + ASSERT_STREQ(rewriteStrings("", rewrites).c_str(), ""); + } + + TEST(rewriteStrings, emptyRewrites) { + StringMap rewrites; + + ASSERT_STREQ(rewriteStrings("this and that", rewrites).c_str(), "this and that"); + } + + TEST(rewriteStrings, successfulRewrite) { + StringMap rewrites; + rewrites["this"] = "that"; + + ASSERT_STREQ(rewriteStrings("this and that", rewrites).c_str(), "that and that"); + } + + TEST(rewriteStrings, doesntOccur) { + StringMap rewrites; + rewrites["foo"] = "bar"; + + ASSERT_STREQ(rewriteStrings("this and that", rewrites).c_str(), "this and that"); + } + + /* ---------------------------------------------------------------------------- + * replaceStrings + * --------------------------------------------------------------------------*/ + + TEST(replaceStrings, emptyString) { + ASSERT_STREQ(replaceStrings("", "this", "that").c_str(), ""); + ASSERT_STREQ(replaceStrings("this and that", "", "").c_str(), "this and that"); + } + + TEST(replaceStrings, successfulReplace) { + ASSERT_STREQ(replaceStrings("this and that", "this", "that").c_str(), "that and that"); + } + + TEST(replaceStrings, doesntOccur) { + ASSERT_STREQ(replaceStrings("this and that", "foo", "bar").c_str(), "this and that"); + } + + /* ---------------------------------------------------------------------------- + * trim + * --------------------------------------------------------------------------*/ + + TEST(trim, emptyString) { + ASSERT_STREQ(trim("").c_str(), ""); + } + + TEST(trim, removesWhitespace) { + ASSERT_STREQ(trim("foo").c_str(), "foo"); + ASSERT_STREQ(trim(" foo ").c_str(), "foo"); + ASSERT_STREQ(trim(" foo bar baz").c_str(), "foo bar baz"); + ASSERT_STREQ(trim(" \t foo bar baz\n").c_str(), "foo bar baz"); + } + + /* ---------------------------------------------------------------------------- + * chomp + * --------------------------------------------------------------------------*/ + + TEST(chomp, emptyString) { + ASSERT_STREQ(chomp("").c_str(), ""); + } + + TEST(chomp, removesWhitespace) { + ASSERT_STREQ(chomp("foo").c_str(), "foo"); + ASSERT_STREQ(chomp("foo ").c_str(), "foo"); + ASSERT_STREQ(chomp(" foo ").c_str(), " foo"); + ASSERT_STREQ(chomp(" foo bar baz ").c_str(), " foo bar baz"); + ASSERT_STREQ(chomp("\t foo bar baz\n").c_str(), "\t foo bar baz"); + } + + /* ---------------------------------------------------------------------------- + * quoteStrings + * --------------------------------------------------------------------------*/ + + TEST(quoteStrings, empty) { + Strings s = { }; + Strings expected = { }; + + ASSERT_EQ(quoteStrings(s), expected); + } + + TEST(quoteStrings, emptyStrings) { + Strings s = { "", "", "" }; + Strings expected = { "''", "''", "''" }; + ASSERT_EQ(quoteStrings(s), expected); + + } + + TEST(quoteStrings, trivialQuote) { + Strings s = { "foo", "bar", "baz" }; + Strings expected = { "'foo'", "'bar'", "'baz'" }; + + ASSERT_EQ(quoteStrings(s), expected); + } + + TEST(quoteStrings, quotedStrings) { + Strings s = { "'foo'", "'bar'", "'baz'" }; + Strings expected = { "''foo''", "''bar''", "''baz''" }; + + ASSERT_EQ(quoteStrings(s), expected); + } + + /* ---------------------------------------------------------------------------- + * tokenizeString + * --------------------------------------------------------------------------*/ + + TEST(tokenizeString, empty) { + auto s = ""; + Strings expected = { }; + + ASSERT_EQ(tokenizeString(""), expected); + } + + TEST(tokenizeString, tokenizeSpacesWithDefaults) { + auto s = "foo bar baz"; + Strings expected = { "foo", "bar", "baz" }; + + ASSERT_EQ(tokenizeString(s), expected); + } + + TEST(tokenizeString, tokenizeTabsWithDefaults) { + auto s = "foo\tbar\tbaz"; + Strings expected = { "foo", "bar", "baz" }; + + ASSERT_EQ(tokenizeString(s), expected); + } + + TEST(tokenizeString, tokenizeTabsSpacesWithDefaults) { + auto s = "foo\t bar\t baz"; + Strings expected = { "foo", "bar", "baz" }; + + ASSERT_EQ(tokenizeString(s), expected); + } + + TEST(tokenizeString, tokenizeTabsSpacesNewlineWithDefaults) { + auto s = "foo\t\n bar\t\n baz"; + Strings expected = { "foo", "bar", "baz" }; + + ASSERT_EQ(tokenizeString(s), expected); + } + + TEST(tokenizeString, tokenizeTabsSpacesNewlineRetWithDefaults) { + auto s = "foo\t\n\r bar\t\n\r baz"; + Strings expected = { "foo", "bar", "baz" }; + + ASSERT_EQ(tokenizeString(s), expected); + + auto s2 = "foo \t\n\r bar \t\n\r baz"; + Strings expected2 = { "foo", "bar", "baz" }; + + ASSERT_EQ(tokenizeString(s2), expected2); + } + + TEST(tokenizeString, tokenizeWithCustomSep) { + auto s = "foo\n,bar\n,baz\n"; + Strings expected = { "foo\n", "bar\n", "baz\n" }; + + ASSERT_EQ(tokenizeString(s, ","), expected); + } + + /* ---------------------------------------------------------------------------- + * get + * --------------------------------------------------------------------------*/ + + TEST(get, emptyContainer) { + StringMap s = { }; + auto expected = std::nullopt; + + ASSERT_EQ(get(s, "one"), expected); + } + + TEST(get, getFromContainer) { + StringMap s; + s["one"] = "yi"; + s["two"] = "er"; + auto expected = "yi"; + + ASSERT_EQ(get(s, "one"), expected); + } + + /* ---------------------------------------------------------------------------- + * filterANSIEscapes + * --------------------------------------------------------------------------*/ + + TEST(filterANSIEscapes, emptyString) { + auto s = ""; + auto expected = ""; + + ASSERT_STREQ(filterANSIEscapes(s).c_str(), expected); + } + + TEST(filterANSIEscapes, doesntChangePrintableChars) { + auto s = "09 2q304ruyhr slk2-19024 kjsadh sar f"; + + ASSERT_STREQ(filterANSIEscapes(s).c_str(), s); + } + + TEST(filterANSIEscapes, filtersColorCodes) { + auto s = "\u001b[30m A \u001b[31m B \u001b[32m C \u001b[33m D \u001b[0m"; + + ASSERT_STREQ(filterANSIEscapes(s, true, 2).c_str(), " A" ); + ASSERT_STREQ(filterANSIEscapes(s, true, 3).c_str(), " A " ); + ASSERT_STREQ(filterANSIEscapes(s, true, 4).c_str(), " A " ); + ASSERT_STREQ(filterANSIEscapes(s, true, 5).c_str(), " A B" ); + ASSERT_STREQ(filterANSIEscapes(s, true, 8).c_str(), " A B C" ); + } + + TEST(filterANSIEscapes, expandsTabs) { + auto s = "foo\tbar\tbaz"; + + ASSERT_STREQ(filterANSIEscapes(s, true).c_str(), "foo bar baz" ); + } + +} From 479e8bf00b680343231d2b618e130141079a2097 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Thu, 7 May 2020 16:08:15 +0200 Subject: [PATCH 04/56] Manual: Fix typo --- doc/manual/advanced-topics/post-build-hook.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/manual/advanced-topics/post-build-hook.xml b/doc/manual/advanced-topics/post-build-hook.xml index 08a7a772f..57ee99e84 100644 --- a/doc/manual/advanced-topics/post-build-hook.xml +++ b/doc/manual/advanced-topics/post-build-hook.xml @@ -139,7 +139,7 @@ $ nix-store --delete /nix/store/ibcyipq5gf91838ldx40mjsp0b8w9n18-example Now, copy the path back from the cache: -$ nix store --realize /nix/store/ibcyipq5gf91838ldx40mjsp0b8w9n18-example +$ nix-store --realise /nix/store/ibcyipq5gf91838ldx40mjsp0b8w9n18-example copying path '/nix/store/m8bmqwrch6l3h8s0k3d673xpmipcdpsa-example from 's3://example-nix-cache'... warning: you did not specify '--add-root'; the result might be removed by the garbage collector /nix/store/m8bmqwrch6l3h8s0k3d673xpmipcdpsa-example From 41caaaad365b8e5260069c20458a2043254a6989 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Thu, 7 May 2020 16:37:33 +0200 Subject: [PATCH 05/56] Manual: Typo --- doc/manual/advanced-topics/post-build-hook.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/manual/advanced-topics/post-build-hook.xml b/doc/manual/advanced-topics/post-build-hook.xml index 57ee99e84..acfe9e3cc 100644 --- a/doc/manual/advanced-topics/post-build-hook.xml +++ b/doc/manual/advanced-topics/post-build-hook.xml @@ -61,7 +61,7 @@ substituters = https://cache.nixos.org/ s3://example-nix-cache trusted-public-keys = cache.nixos.org-1:6NCHdD59X431o0gWypbMrAURkbJ16ZPMQFGspcDShjY= example-nix-cache-1:1/cKDz3QCCOmwcztD2eV6Coggp6rqc9DGjWv7C0G+rM= -we will restart the Nix daemon a later step. +We will restart the Nix daemon in a later step.
From 987b3d6469f03bff59614bd3b9097df858d07263 Mon Sep 17 00:00:00 2001 From: Tobias Pflug Date: Thu, 7 May 2020 18:10:07 +0200 Subject: [PATCH 06/56] Use ASSERT_EQ instead of ASSERT_STREQ No need to use `c_str()` in combination with `ASSERT_STREQ`. It's possible to just use ASSERT_EQ on std::string --- tests/unit-tests/test-util.cc | 108 +++++++++++++++++----------------- 1 file changed, 54 insertions(+), 54 deletions(-) diff --git a/tests/unit-tests/test-util.cc b/tests/unit-tests/test-util.cc index 05d8a0039..62ca8f11c 100644 --- a/tests/unit-tests/test-util.cc +++ b/tests/unit-tests/test-util.cc @@ -12,14 +12,14 @@ namespace nix { TEST(absPath, doesntChangeRoot) { auto p = absPath("/"); - ASSERT_STREQ(p.c_str(), "/"); + ASSERT_EQ(p, "/"); } TEST(absPath, turnsEmptyPathIntoCWD) { char cwd[PATH_MAX+1]; auto p = absPath(""); - ASSERT_STREQ(p.c_str(), getcwd((char*)&cwd, PATH_MAX)); + ASSERT_EQ(p, getcwd((char*)&cwd, PATH_MAX)); } TEST(absPath, usesOptionalBasePathWhenGiven) { @@ -28,7 +28,7 @@ namespace nix { auto p = absPath("", cwd); - ASSERT_STREQ(p.c_str(), cwd); + ASSERT_EQ(p, cwd); } TEST(absPath, isIdempotent) { @@ -37,7 +37,7 @@ namespace nix { auto p1 = absPath(cwd); auto p2 = absPath(p1); - ASSERT_STREQ(p1.c_str(), p2.c_str()); + ASSERT_EQ(p1, p2); } @@ -46,8 +46,8 @@ namespace nix { auto p1 = absPath(path); auto p2 = absPath(p1); - ASSERT_STREQ(p1.c_str(), "/some/path/with/trailing/dot"); - ASSERT_STREQ(p1.c_str(), p2.c_str()); + ASSERT_EQ(p1, "/some/path/with/trailing/dot"); + ASSERT_EQ(p1, p2); } /* ---------------------------------------------------------------------------- @@ -58,21 +58,21 @@ namespace nix { auto path = "/this/is/a/path//"; auto p = canonPath(path); - ASSERT_STREQ(p.c_str(), "/this/is/a/path"); + ASSERT_EQ(p, "/this/is/a/path"); } TEST(canonPath, removesDots) { auto path = "/this/./is/a/path/./"; auto p = canonPath(path); - ASSERT_STREQ(p.c_str(), "/this/is/a/path"); + ASSERT_EQ(p, "/this/is/a/path"); } TEST(canonPath, removesDots2) { auto path = "/this/a/../is/a////path/foo/.."; auto p = canonPath(path); - ASSERT_STREQ(p.c_str(), "/this/is/a/path"); + ASSERT_EQ(p, "/this/is/a/path"); } TEST(canonPath, requiresAbsolutePath) { @@ -91,18 +91,18 @@ namespace nix { TEST(dirOf, DISABLED_returnsEmptyStringForRoot) { auto p = dirOf("/"); - ASSERT_STREQ(p.c_str(), ""); + ASSERT_EQ(p, ""); } TEST(dirOf, returnsFirstPathComponent) { auto p1 = dirOf("/dir/"); - ASSERT_STREQ(p1.c_str(), "/dir"); + ASSERT_EQ(p1, "/dir"); auto p2 = dirOf("/dir"); - ASSERT_STREQ(p2.c_str(), "/"); + ASSERT_EQ(p2, "/"); auto p3 = dirOf("/dir/.."); - ASSERT_STREQ(p3.c_str(), "/dir"); + ASSERT_EQ(p3, "/dir"); auto p4 = dirOf("/dir/../"); - ASSERT_STREQ(p4.c_str(), "/dir/.."); + ASSERT_EQ(p4, "/dir/.."); } /* ---------------------------------------------------------------------------- @@ -111,29 +111,29 @@ namespace nix { TEST(baseNameOf, emptyPath) { auto p1 = baseNameOf(""); - ASSERT_STREQ(std::string(p1).c_str(), ""); + ASSERT_EQ(std::string(p1), ""); } TEST(baseNameOf, pathOnRoot) { auto p1 = baseNameOf("/dir"); - ASSERT_STREQ(std::string(p1).c_str(), "dir"); + ASSERT_EQ(std::string(p1), "dir"); } TEST(baseNameOf, relativePath) { auto p1 = baseNameOf("dir/foo"); - ASSERT_STREQ(std::string(p1).c_str(), "foo"); + ASSERT_EQ(std::string(p1), "foo"); } TEST(baseNameOf, pathWithTrailingSlashRoot) { auto p1 = baseNameOf("/"); - ASSERT_STREQ(std::string(p1).c_str(), ""); + ASSERT_EQ(std::string(p1), ""); } // XXX: according to the doc of `baseNameOf`, baseNameOf("/dir/") should return // "" but it actually returns "dir" TEST(baseNameOf, DISABLED_trailingSlash) { auto p1 = baseNameOf("/dir/"); - ASSERT_STREQ(std::string(p1).c_str(), ""); + ASSERT_EQ(std::string(p1), ""); } /* ---------------------------------------------------------------------------- @@ -265,11 +265,11 @@ namespace nix { * --------------------------------------------------------------------------*/ TEST(base64Encode, emptyString) { - ASSERT_STREQ(base64Encode("").c_str(), ""); + ASSERT_EQ(base64Encode(""), ""); } TEST(base64Encode, encodesAString) { - ASSERT_STREQ(base64Encode("quod erat demonstrandum").c_str(), "cXVvZCBlcmF0IGRlbW9uc3RyYW5kdW0="); + ASSERT_EQ(base64Encode("quod erat demonstrandum"), "cXVvZCBlcmF0IGRlbW9uc3RyYW5kdW0="); } TEST(base64Encode, encodeAndDecode) { @@ -277,7 +277,7 @@ namespace nix { auto encoded = base64Encode(s); auto decoded = base64Decode(encoded); - ASSERT_STREQ(decoded.c_str(), s); + ASSERT_EQ(decoded, s); } /* ---------------------------------------------------------------------------- @@ -285,11 +285,11 @@ namespace nix { * --------------------------------------------------------------------------*/ TEST(base64Decode, emptyString) { - ASSERT_STREQ(base64Decode("").c_str(), ""); + ASSERT_EQ(base64Decode(""), ""); } TEST(base64Decode, decodeAString) { - ASSERT_STREQ(base64Decode("cXVvZCBlcmF0IGRlbW9uc3RyYW5kdW0=").c_str(), "quod erat demonstrandum"); + ASSERT_EQ(base64Decode("cXVvZCBlcmF0IGRlbW9uc3RyYW5kdW0="), "quod erat demonstrandum"); } /* ---------------------------------------------------------------------------- @@ -297,12 +297,12 @@ namespace nix { * --------------------------------------------------------------------------*/ TEST(toLower, emptyString) { - ASSERT_STREQ(toLower("").c_str(), ""); + ASSERT_EQ(toLower(""), ""); } TEST(toLower, nonLetters) { auto s = "!@(*$#)(@#=\\234_"; - ASSERT_STREQ(toLower(s).c_str(), s); + ASSERT_EQ(toLower(s), s); } // XXX: std::tolower() doesn't cover this. This probably doesn't matter @@ -310,7 +310,7 @@ namespace nix { // characters are not allowed. TEST(toLower, DISABLED_umlauts) { auto s = "ÄÖÜ"; - ASSERT_STREQ(toLower(s).c_str(), "äöü"); + ASSERT_EQ(toLower(s), "äöü"); } /* ---------------------------------------------------------------------------- @@ -377,27 +377,27 @@ namespace nix { StringMap rewrites; rewrites["this"] = "that"; - ASSERT_STREQ(rewriteStrings("", rewrites).c_str(), ""); + ASSERT_EQ(rewriteStrings("", rewrites), ""); } TEST(rewriteStrings, emptyRewrites) { StringMap rewrites; - ASSERT_STREQ(rewriteStrings("this and that", rewrites).c_str(), "this and that"); + ASSERT_EQ(rewriteStrings("this and that", rewrites), "this and that"); } TEST(rewriteStrings, successfulRewrite) { StringMap rewrites; rewrites["this"] = "that"; - ASSERT_STREQ(rewriteStrings("this and that", rewrites).c_str(), "that and that"); + ASSERT_EQ(rewriteStrings("this and that", rewrites), "that and that"); } TEST(rewriteStrings, doesntOccur) { StringMap rewrites; rewrites["foo"] = "bar"; - ASSERT_STREQ(rewriteStrings("this and that", rewrites).c_str(), "this and that"); + ASSERT_EQ(rewriteStrings("this and that", rewrites), "this and that"); } /* ---------------------------------------------------------------------------- @@ -405,16 +405,16 @@ namespace nix { * --------------------------------------------------------------------------*/ TEST(replaceStrings, emptyString) { - ASSERT_STREQ(replaceStrings("", "this", "that").c_str(), ""); - ASSERT_STREQ(replaceStrings("this and that", "", "").c_str(), "this and that"); + ASSERT_EQ(replaceStrings("", "this", "that"), ""); + ASSERT_EQ(replaceStrings("this and that", "", ""), "this and that"); } TEST(replaceStrings, successfulReplace) { - ASSERT_STREQ(replaceStrings("this and that", "this", "that").c_str(), "that and that"); + ASSERT_EQ(replaceStrings("this and that", "this", "that"), "that and that"); } TEST(replaceStrings, doesntOccur) { - ASSERT_STREQ(replaceStrings("this and that", "foo", "bar").c_str(), "this and that"); + ASSERT_EQ(replaceStrings("this and that", "foo", "bar"), "this and that"); } /* ---------------------------------------------------------------------------- @@ -422,14 +422,14 @@ namespace nix { * --------------------------------------------------------------------------*/ TEST(trim, emptyString) { - ASSERT_STREQ(trim("").c_str(), ""); + ASSERT_EQ(trim(""), ""); } TEST(trim, removesWhitespace) { - ASSERT_STREQ(trim("foo").c_str(), "foo"); - ASSERT_STREQ(trim(" foo ").c_str(), "foo"); - ASSERT_STREQ(trim(" foo bar baz").c_str(), "foo bar baz"); - ASSERT_STREQ(trim(" \t foo bar baz\n").c_str(), "foo bar baz"); + ASSERT_EQ(trim("foo"), "foo"); + ASSERT_EQ(trim(" foo "), "foo"); + ASSERT_EQ(trim(" foo bar baz"), "foo bar baz"); + ASSERT_EQ(trim(" \t foo bar baz\n"), "foo bar baz"); } /* ---------------------------------------------------------------------------- @@ -437,15 +437,15 @@ namespace nix { * --------------------------------------------------------------------------*/ TEST(chomp, emptyString) { - ASSERT_STREQ(chomp("").c_str(), ""); + ASSERT_EQ(chomp(""), ""); } TEST(chomp, removesWhitespace) { - ASSERT_STREQ(chomp("foo").c_str(), "foo"); - ASSERT_STREQ(chomp("foo ").c_str(), "foo"); - ASSERT_STREQ(chomp(" foo ").c_str(), " foo"); - ASSERT_STREQ(chomp(" foo bar baz ").c_str(), " foo bar baz"); - ASSERT_STREQ(chomp("\t foo bar baz\n").c_str(), "\t foo bar baz"); + ASSERT_EQ(chomp("foo"), "foo"); + ASSERT_EQ(chomp("foo "), "foo"); + ASSERT_EQ(chomp(" foo "), " foo"); + ASSERT_EQ(chomp(" foo bar baz "), " foo bar baz"); + ASSERT_EQ(chomp("\t foo bar baz\n"), "\t foo bar baz"); } /* ---------------------------------------------------------------------------- @@ -566,29 +566,29 @@ namespace nix { auto s = ""; auto expected = ""; - ASSERT_STREQ(filterANSIEscapes(s).c_str(), expected); + ASSERT_EQ(filterANSIEscapes(s), expected); } TEST(filterANSIEscapes, doesntChangePrintableChars) { auto s = "09 2q304ruyhr slk2-19024 kjsadh sar f"; - ASSERT_STREQ(filterANSIEscapes(s).c_str(), s); + ASSERT_EQ(filterANSIEscapes(s), s); } TEST(filterANSIEscapes, filtersColorCodes) { auto s = "\u001b[30m A \u001b[31m B \u001b[32m C \u001b[33m D \u001b[0m"; - ASSERT_STREQ(filterANSIEscapes(s, true, 2).c_str(), " A" ); - ASSERT_STREQ(filterANSIEscapes(s, true, 3).c_str(), " A " ); - ASSERT_STREQ(filterANSIEscapes(s, true, 4).c_str(), " A " ); - ASSERT_STREQ(filterANSIEscapes(s, true, 5).c_str(), " A B" ); - ASSERT_STREQ(filterANSIEscapes(s, true, 8).c_str(), " A B C" ); + ASSERT_EQ(filterANSIEscapes(s, true, 2), " A" ); + ASSERT_EQ(filterANSIEscapes(s, true, 3), " A " ); + ASSERT_EQ(filterANSIEscapes(s, true, 4), " A " ); + ASSERT_EQ(filterANSIEscapes(s, true, 5), " A B" ); + ASSERT_EQ(filterANSIEscapes(s, true, 8), " A B C" ); } TEST(filterANSIEscapes, expandsTabs) { auto s = "foo\tbar\tbaz"; - ASSERT_STREQ(filterANSIEscapes(s, true).c_str(), "foo bar baz" ); + ASSERT_EQ(filterANSIEscapes(s, true), "foo bar baz" ); } } From 1f3602a2c9c3bfeea7cc8ee4d478c77dec349206 Mon Sep 17 00:00:00 2001 From: Tobias Pflug Date: Thu, 7 May 2020 18:15:13 +0200 Subject: [PATCH 07/56] Remove replaceInSet The function isn't being used anywhere so it seems safe to remove --- src/libutil/util.hh | 11 ----------- tests/unit-tests/test-util.cc | 4 ---- 2 files changed, 15 deletions(-) diff --git a/src/libutil/util.hh b/src/libutil/util.hh index 1b263abcc..8770add64 100644 --- a/src/libutil/util.hh +++ b/src/libutil/util.hh @@ -389,17 +389,6 @@ string replaceStrings(const std::string & s, std::string rewriteStrings(const std::string & s, const StringMap & rewrites); -/* If a set contains 'from', remove it and insert 'to'. */ -template -void replaceInSet(std::set & set, const T & from, const T & to) -{ - auto i = set.find(from); - if (i == set.end()) return; - set.erase(i); - set.insert(to); -} - - /* Convert the exit status of a child as returned by wait() into an error string. */ string statusToString(int status); diff --git a/tests/unit-tests/test-util.cc b/tests/unit-tests/test-util.cc index 62ca8f11c..e0738614e 100644 --- a/tests/unit-tests/test-util.cc +++ b/tests/unit-tests/test-util.cc @@ -364,10 +364,6 @@ namespace nix { ASSERT_EQ(statusOk(1), false); } - /* ---------------------------------------------------------------------------- - * replaceInSet : XXX: this function isn't called anywhere! - * --------------------------------------------------------------------------*/ - /* ---------------------------------------------------------------------------- * rewriteStrings From 73d0b5d807edee05910cff4a30549393d5375d89 Mon Sep 17 00:00:00 2001 From: Tobias Pflug Date: Thu, 7 May 2020 19:29:10 +0200 Subject: [PATCH 08/56] Drop unnecessary std::string --- tests/unit-tests/test-util.cc | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/unit-tests/test-util.cc b/tests/unit-tests/test-util.cc index e0738614e..a0fa7c109 100644 --- a/tests/unit-tests/test-util.cc +++ b/tests/unit-tests/test-util.cc @@ -111,29 +111,29 @@ namespace nix { TEST(baseNameOf, emptyPath) { auto p1 = baseNameOf(""); - ASSERT_EQ(std::string(p1), ""); + ASSERT_EQ(p1, ""); } TEST(baseNameOf, pathOnRoot) { auto p1 = baseNameOf("/dir"); - ASSERT_EQ(std::string(p1), "dir"); + ASSERT_EQ(p1, "dir"); } TEST(baseNameOf, relativePath) { auto p1 = baseNameOf("dir/foo"); - ASSERT_EQ(std::string(p1), "foo"); + ASSERT_EQ(p1, "foo"); } TEST(baseNameOf, pathWithTrailingSlashRoot) { auto p1 = baseNameOf("/"); - ASSERT_EQ(std::string(p1), ""); + ASSERT_EQ(p1, ""); } // XXX: according to the doc of `baseNameOf`, baseNameOf("/dir/") should return // "" but it actually returns "dir" TEST(baseNameOf, DISABLED_trailingSlash) { auto p1 = baseNameOf("/dir/"); - ASSERT_EQ(std::string(p1), ""); + ASSERT_EQ(p1, ""); } /* ---------------------------------------------------------------------------- From 14073fb76be0f46cb9335edf747fcfa1a5275b7b Mon Sep 17 00:00:00 2001 From: Alexander Bantyev Date: Fri, 8 May 2020 12:22:39 +0300 Subject: [PATCH 09/56] Don't block while waiting for build users --- src/libstore/build.cc | 54 +++++++++++++++++++++++-------------------- 1 file changed, 29 insertions(+), 25 deletions(-) diff --git a/src/libstore/build.cc b/src/libstore/build.cc index 1b7e6d75e..00101f553 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -507,9 +507,6 @@ private: Path fnUserLock; AutoCloseFD fdUserLock; - bool findFreeUser(); - - string user; uid_t uid; gid_t gid; @@ -525,20 +522,19 @@ public: uid_t getGID() { assert(gid); return gid; } std::vector getSupplementaryGIDs() { return supplementaryGIDs; } + bool findFreeUser(); + bool enabled() { return uid != 0; } }; + UserLock::UserLock() { assert(settings.buildUsersGroup != ""); createDirs(settings.nixStateDir + "/userpool"); - - if (findFreeUser()) return; - - printError("waiting for build users"); - - do std::this_thread::sleep_for(std::chrono::seconds(2)); while (! findFreeUser()); + /* Mark that user is not enabled by default */ + uid = 0; } bool UserLock::findFreeUser() { @@ -1398,6 +1394,30 @@ void DerivationGoal::tryToBuild() { trace("trying to build"); + /* If `build-users-group' is not empty, then we have to build as + one of the members of that group. */ + if (settings.buildUsersGroup != "" && getuid() == 0) { +#if defined(__linux__) || defined(__APPLE__) + if (!buildUser) buildUser = std::make_unique(); + + if (!buildUser->enabled()) { + if (!buildUser->findFreeUser()) { + debug("waiting for build users"); + worker.waitForAWhile(shared_from_this()); + return; + } + + /* Make sure that no other processes are executing under this + uid. */ + buildUser->kill(); + } +#else + /* Don't know how to block the creation of setuid/setgid + binaries on this platform. */ + throw Error("build users are not supported on this platform for security reasons"); +#endif + } + /* Obtain locks on all output paths. The locks are automatically released when we exit this function or Nix crashes. If we can't acquire the lock, then continue; hopefully some other @@ -1950,22 +1970,6 @@ void DerivationGoal::startBuilder() #endif } - /* If `build-users-group' is not empty, then we have to build as - one of the members of that group. */ - if (settings.buildUsersGroup != "" && getuid() == 0) { -#if defined(__linux__) || defined(__APPLE__) - buildUser = std::make_unique(); - - /* Make sure that no other processes are executing under this - uid. */ - buildUser->kill(); -#else - /* Don't know how to block the creation of setuid/setgid - binaries on this platform. */ - throw Error("build users are not supported on this platform for security reasons"); -#endif - } - /* Create a temporary directory where the build will take place. */ tmpDir = createTempDir("", "nix-build-" + std::string(drvPath.name()), false, false, 0700); From 772e5db828c9924c803b4d126bca72d7e123614b Mon Sep 17 00:00:00 2001 From: Alexander Bantyev Date: Fri, 8 May 2020 12:29:00 +0300 Subject: [PATCH 10/56] Mention build users in the 'waiting for' message --- src/libstore/build.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libstore/build.cc b/src/libstore/build.cc index 00101f553..a8c1cd565 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -4830,7 +4830,7 @@ void Worker::waitForInput() if (!waitingForAWhile.empty()) { useTimeout = true; if (lastWokenUp == steady_time_point::min()) - printError("waiting for locks or build slots..."); + printError("waiting for locks, build slots or build users..."); if (lastWokenUp == steady_time_point::min() || lastWokenUp > before) lastWokenUp = before; timeout = std::max(1L, (long) std::chrono::duration_cast( From 7cc7cef9502dc388706561cbdf275e070520e65d Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Fri, 8 May 2020 11:34:09 +0200 Subject: [PATCH 11/56] Move unit tests to sr/libutil/tests, use mk make rules --- Makefile | 4 ++-- src/libutil/tests/local.mk | 11 +++++++++++ .../test-util.cc => src/libutil/tests/tests.cc | 4 ++-- tests/unit-tests/local.mk | 4 ---- 4 files changed, 15 insertions(+), 8 deletions(-) create mode 100644 src/libutil/tests/local.mk rename tests/unit-tests/test-util.cc => src/libutil/tests/tests.cc (99%) delete mode 100644 tests/unit-tests/local.mk diff --git a/Makefile b/Makefile index 06de35a27..0ba011e2a 100644 --- a/Makefile +++ b/Makefile @@ -3,6 +3,7 @@ makefiles = \ local.mk \ nix-rust/local.mk \ src/libutil/local.mk \ + src/libutil/tests/local.mk \ src/libstore/local.mk \ src/libfetchers/local.mk \ src/libmain/local.mk \ @@ -16,8 +17,7 @@ makefiles = \ misc/upstart/local.mk \ doc/manual/local.mk \ tests/local.mk \ - tests/plugins/local.mk \ - tests/unit-tests/local.mk + tests/plugins/local.mk -include Makefile.config diff --git a/src/libutil/tests/local.mk b/src/libutil/tests/local.mk new file mode 100644 index 000000000..9c2d2a7bb --- /dev/null +++ b/src/libutil/tests/local.mk @@ -0,0 +1,11 @@ +programs += libutil-tests + +libutil-tests_DIR := $(d) + +libutil-tests_SOURCES := $(wildcard $(d)/*.cc) + +libutil-tests_CXXFLAGS += -I src/libutil + +libutil-tests_LIBS = libutil + +libutil-tests_LDFLAGS := $$(pkg-config --libs gtest_main) diff --git a/tests/unit-tests/test-util.cc b/src/libutil/tests/tests.cc similarity index 99% rename from tests/unit-tests/test-util.cc rename to src/libutil/tests/tests.cc index a0fa7c109..fb593b5e6 100644 --- a/tests/unit-tests/test-util.cc +++ b/src/libutil/tests/tests.cc @@ -1,5 +1,5 @@ -#include "../../src/libutil/util.hh" -#include "../../src/libutil/types.hh" +#include "util.hh" +#include "types.hh" #include diff --git a/tests/unit-tests/local.mk b/tests/unit-tests/local.mk deleted file mode 100644 index ed11261c1..000000000 --- a/tests/unit-tests/local.mk +++ /dev/null @@ -1,4 +0,0 @@ -LIBS=-larchive -lcrypto -llzma -lbz2 -lz -lbrotlienc -lbrotlidec -unit-tests: - echo $(nix_LDFLAGS) - $(CXX) -o unit-test $(nix_CXXFLAGS) $(nix_LDFLAGS) $(LIBS) --std=c++17 $$(pkg-config --libs gtest_main) ./src/libutil/*.o tests/unit-tests/test-util.cc From 72b9d971bc4ff5b5dcb9349d03526a5ef3e4300e Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Fri, 8 May 2020 11:35:57 +0200 Subject: [PATCH 12/56] Fix warning --- src/libutil/tests/tests.cc | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/libutil/tests/tests.cc b/src/libutil/tests/tests.cc index fb593b5e6..0fb4411e8 100644 --- a/src/libutil/tests/tests.cc +++ b/src/libutil/tests/tests.cc @@ -481,8 +481,7 @@ namespace nix { * --------------------------------------------------------------------------*/ TEST(tokenizeString, empty) { - auto s = ""; - Strings expected = { }; + Strings expected = { }; ASSERT_EQ(tokenizeString(""), expected); } From 7898cdb75a721cc21867ccdbcc523e95f2db0354 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Fri, 8 May 2020 11:49:40 +0200 Subject: [PATCH 13/56] make check: Run unit tests --- mk/programs.mk | 6 ++++++ mk/tracing.mk | 1 + nix-rust/local.mk | 2 +- src/libutil/tests/local.mk | 2 ++ 4 files changed, 10 insertions(+), 1 deletion(-) diff --git a/mk/programs.mk b/mk/programs.mk index d93df4468..2d0c988d4 100644 --- a/mk/programs.mk +++ b/mk/programs.mk @@ -76,4 +76,10 @@ define build-program programs-list += $$($(1)_PATH) clean-files += $$($(1)_PATH) $$(_d)/*.o $$(_d)/.*.dep $$($(1)_DEPS) $$($(1)_OBJS) dist-files += $$(_srcs) + + # Phony target to run this program (typically as a dependency of 'check'). + .PHONY: $(1)_RUN + $(1)_RUN: $$($(1)_PATH) + $(trace-test) $$($(1)_PATH) + endef diff --git a/mk/tracing.mk b/mk/tracing.mk index 13912d3c7..54c77ab60 100644 --- a/mk/tracing.mk +++ b/mk/tracing.mk @@ -11,6 +11,7 @@ ifeq ($(V), 0) trace-javac = @echo " JAVAC " $@; trace-jar = @echo " JAR " $@; trace-mkdir = @echo " MKDIR " $@; + trace-test = @echo " TEST " $@; suppress = @ diff --git a/nix-rust/local.mk b/nix-rust/local.mk index 1e006e500..e4bfde31b 100644 --- a/nix-rust/local.mk +++ b/nix-rust/local.mk @@ -41,5 +41,5 @@ ifneq ($(OS), Darwin) check: rust-tests rust-tests: - cd nix-rust && CARGO_HOME=$$(if [[ -d vendor ]]; then echo vendor; fi) cargo test --release $$(if [[ -d vendor ]]; then echo --offline; fi) + $(trace-test) cd nix-rust && CARGO_HOME=$$(if [[ -d vendor ]]; then echo vendor; fi) cargo test --release $$(if [[ -d vendor ]]; then echo --offline; fi) endif diff --git a/src/libutil/tests/local.mk b/src/libutil/tests/local.mk index 9c2d2a7bb..a188d5093 100644 --- a/src/libutil/tests/local.mk +++ b/src/libutil/tests/local.mk @@ -1,3 +1,5 @@ +check: libutil-tests_RUN + programs += libutil-tests libutil-tests_DIR := $(d) From ca657525b88e788b72ce53ecc6d140a573476644 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Fri, 8 May 2020 12:03:27 +0200 Subject: [PATCH 14/56] Don't install unit tests --- mk/programs.mk | 20 ++++++++++++-------- src/libutil/tests/local.mk | 2 ++ 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/mk/programs.mk b/mk/programs.mk index 2d0c988d4..3fa9685c3 100644 --- a/mk/programs.mk +++ b/mk/programs.mk @@ -35,24 +35,28 @@ define build-program $$(trace-ld) $(CXX) -o $$@ $$(LDFLAGS) $$(GLOBAL_LDFLAGS) $$($(1)_OBJS) $$($(1)_LDFLAGS) $$(foreach lib, $$($(1)_LIBS), $$($$(lib)_LDFLAGS_USE)) $(1)_INSTALL_DIR ?= $$(bindir) - $(1)_INSTALL_PATH := $$($(1)_INSTALL_DIR)/$(1) - $$(eval $$(call create-dir, $$($(1)_INSTALL_DIR))) + ifdef $(1)_INSTALL_DIR - install: $(DESTDIR)$$($(1)_INSTALL_PATH) + $(1)_INSTALL_PATH := $$($(1)_INSTALL_DIR)/$(1) - ifeq ($(BUILD_SHARED_LIBS), 1) + $$(eval $$(call create-dir, $$($(1)_INSTALL_DIR))) - _libs_final := $$(foreach lib, $$($(1)_LIBS), $$($$(lib)_INSTALL_PATH)) + install: $(DESTDIR)$$($(1)_INSTALL_PATH) - $(DESTDIR)$$($(1)_INSTALL_PATH): $$($(1)_OBJS) $$(_libs_final) | $(DESTDIR)$$($(1)_INSTALL_DIR)/ + ifeq ($(BUILD_SHARED_LIBS), 1) + + _libs_final := $$(foreach lib, $$($(1)_LIBS), $$($$(lib)_INSTALL_PATH)) + + $(DESTDIR)$$($(1)_INSTALL_PATH): $$($(1)_OBJS) $$(_libs_final) | $(DESTDIR)$$($(1)_INSTALL_DIR)/ $$(trace-ld) $(CXX) -o $$@ $$(LDFLAGS) $$(GLOBAL_LDFLAGS) $$($(1)_OBJS) $$($(1)_LDFLAGS) $$(foreach lib, $$($(1)_LIBS), $$($$(lib)_LDFLAGS_USE_INSTALLED)) - else + else - $(DESTDIR)$$($(1)_INSTALL_PATH): $$($(1)_PATH) | $(DESTDIR)$$($(1)_INSTALL_DIR)/ + $(DESTDIR)$$($(1)_INSTALL_PATH): $$($(1)_PATH) | $(DESTDIR)$$($(1)_INSTALL_DIR)/ install -t $(DESTDIR)$$($(1)_INSTALL_DIR) $$< + endif endif # Propagate CFLAGS and CXXFLAGS to the individual object files. diff --git a/src/libutil/tests/local.mk b/src/libutil/tests/local.mk index a188d5093..5f4b3edc5 100644 --- a/src/libutil/tests/local.mk +++ b/src/libutil/tests/local.mk @@ -4,6 +4,8 @@ programs += libutil-tests libutil-tests_DIR := $(d) +libutil-tests_INSTALL_DIR := + libutil-tests_SOURCES := $(wildcard $(d)/*.cc) libutil-tests_CXXFLAGS += -I src/libutil From 5b8883faac0cff928a4359b4260b5be09a311d4b Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Fri, 8 May 2020 12:09:00 +0200 Subject: [PATCH 15/56] configure: Look for gtest --- Makefile.config.in | 19 ++++++++++--------- configure.ac | 4 ++++ src/libutil/tests/local.mk | 2 +- 3 files changed, 15 insertions(+), 10 deletions(-) diff --git a/Makefile.config.in b/Makefile.config.in index e7a12089a..b632444e8 100644 --- a/Makefile.config.in +++ b/Makefile.config.in @@ -1,36 +1,38 @@ AR = @AR@ BDW_GC_LIBS = @BDW_GC_LIBS@ +BOOST_LDFLAGS = @BOOST_LDFLAGS@ BUILD_SHARED_LIBS = @BUILD_SHARED_LIBS@ CC = @CC@ CFLAGS = @CFLAGS@ CXX = @CXX@ CXXFLAGS = @CXXFLAGS@ -LDFLAGS = @LDFLAGS@ +EDITLINE_LIBS = @EDITLINE_LIBS@ ENABLE_S3 = @ENABLE_S3@ -HAVE_SODIUM = @HAVE_SODIUM@ +GTEST_LIBS = @GTEST_LIBS@ HAVE_SECCOMP = @HAVE_SECCOMP@ -BOOST_LDFLAGS = @BOOST_LDFLAGS@ +HAVE_SODIUM = @HAVE_SODIUM@ +LDFLAGS = @LDFLAGS@ +LIBARCHIVE_LIBS = @LIBARCHIVE_LIBS@ +LIBBROTLI_LIBS = @LIBBROTLI_LIBS@ LIBCURL_LIBS = @LIBCURL_LIBS@ +LIBLZMA_LIBS = @LIBLZMA_LIBS@ OPENSSL_LIBS = @OPENSSL_LIBS@ PACKAGE_NAME = @PACKAGE_NAME@ PACKAGE_VERSION = @PACKAGE_VERSION@ SODIUM_LIBS = @SODIUM_LIBS@ -LIBLZMA_LIBS = @LIBLZMA_LIBS@ SQLITE3_LIBS = @SQLITE3_LIBS@ -LIBBROTLI_LIBS = @LIBBROTLI_LIBS@ -LIBARCHIVE_LIBS = @LIBARCHIVE_LIBS@ -EDITLINE_LIBS = @EDITLINE_LIBS@ bash = @bash@ bindir = @bindir@ -lsof = @lsof@ datadir = @datadir@ datarootdir = @datarootdir@ +doc_generate = @doc_generate@ docdir = @docdir@ exec_prefix = @exec_prefix@ includedir = @includedir@ libdir = @libdir@ libexecdir = @libexecdir@ localstatedir = @localstatedir@ +lsof = @lsof@ mandir = @mandir@ pkglibdir = $(libdir)/$(PACKAGE_NAME) prefix = @prefix@ @@ -38,6 +40,5 @@ sandbox_shell = @sandbox_shell@ storedir = @storedir@ sysconfdir = @sysconfdir@ system = @system@ -doc_generate = @doc_generate@ xmllint = @xmllint@ xsltproc = @xsltproc@ diff --git a/configure.ac b/configure.ac index b868343d2..c3007b4b6 100644 --- a/configure.ac +++ b/configure.ac @@ -266,6 +266,10 @@ if test "$gc" = yes; then fi +# Look for gtest. +PKG_CHECK_MODULES([GTEST], [gtest_main]) + + # documentation generation switch AC_ARG_ENABLE(doc-gen, AC_HELP_STRING([--disable-doc-gen], [disable documentation generation]), diff --git a/src/libutil/tests/local.mk b/src/libutil/tests/local.mk index 5f4b3edc5..a297edb64 100644 --- a/src/libutil/tests/local.mk +++ b/src/libutil/tests/local.mk @@ -12,4 +12,4 @@ libutil-tests_CXXFLAGS += -I src/libutil libutil-tests_LIBS = libutil -libutil-tests_LDFLAGS := $$(pkg-config --libs gtest_main) +libutil-tests_LDFLAGS := $(GTEST_LIBS) From e3df9c2a6e867d0e038cb26af40501511607e007 Mon Sep 17 00:00:00 2001 From: Tobias Pflug Date: Fri, 8 May 2020 15:03:44 +0200 Subject: [PATCH 16/56] Enable `dirOf` test Adjusted the doc comment for `dirOf` to reflect the implementation behavior. --- src/libutil/tests/tests.cc | 6 ++---- src/libutil/util.hh | 4 ++-- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/src/libutil/tests/tests.cc b/src/libutil/tests/tests.cc index 0fb4411e8..1b016430d 100644 --- a/src/libutil/tests/tests.cc +++ b/src/libutil/tests/tests.cc @@ -86,12 +86,10 @@ namespace nix { * dirOf * --------------------------------------------------------------------------*/ - // XXX: according to the doc of `dirOf`, dirOf("/") and dirOf("/foo") - // should both return "" but it actually returns "/" in both cases - TEST(dirOf, DISABLED_returnsEmptyStringForRoot) { + TEST(dirOf, returnsEmptyStringForRoot) { auto p = dirOf("/"); - ASSERT_EQ(p, ""); + ASSERT_EQ(p, "/"); } TEST(dirOf, returnsFirstPathComponent) { diff --git a/src/libutil/util.hh b/src/libutil/util.hh index 8770add64..38a0f7a5c 100644 --- a/src/libutil/util.hh +++ b/src/libutil/util.hh @@ -58,8 +58,8 @@ Path canonPath(const Path & path, bool resolveSymlinks = false); /* Return the directory part of the given canonical path, i.e., everything before the final `/'. If the path is the root or an - immediate child thereof (e.g., `/foo'), this means an empty string - is returned. */ + immediate child thereof (e.g., `/foo'), this means `/' + is returned.*/ Path dirOf(const Path & path); /* Return the base name of the given canonical path, i.e., everything From 2191141274cfefcb9ffbafbbfcdc58414fb28f1a Mon Sep 17 00:00:00 2001 From: Tobias Pflug Date: Fri, 8 May 2020 15:07:40 +0200 Subject: [PATCH 17/56] Enable `baseNameOf` test Add note about removal of trailing slashes in the doc comment of baseNameOf and enabled the test. --- src/libutil/tests/tests.cc | 6 ++---- src/libutil/util.hh | 2 +- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/src/libutil/tests/tests.cc b/src/libutil/tests/tests.cc index 1b016430d..d46f6a5a4 100644 --- a/src/libutil/tests/tests.cc +++ b/src/libutil/tests/tests.cc @@ -127,11 +127,9 @@ namespace nix { ASSERT_EQ(p1, ""); } - // XXX: according to the doc of `baseNameOf`, baseNameOf("/dir/") should return - // "" but it actually returns "dir" - TEST(baseNameOf, DISABLED_trailingSlash) { + TEST(baseNameOf, trailingSlash) { auto p1 = baseNameOf("/dir/"); - ASSERT_EQ(p1, ""); + ASSERT_EQ(p1, "dir"); } /* ---------------------------------------------------------------------------- diff --git a/src/libutil/util.hh b/src/libutil/util.hh index 38a0f7a5c..a63ee05b3 100644 --- a/src/libutil/util.hh +++ b/src/libutil/util.hh @@ -63,7 +63,7 @@ Path canonPath(const Path & path, bool resolveSymlinks = false); Path dirOf(const Path & path); /* Return the base name of the given canonical path, i.e., everything - following the final `/'. */ + following the final `/' (trailing slashes are removed). */ std::string_view baseNameOf(std::string_view path); /* Check whether 'path' is a descendant of 'dir'. */ From 181a47d8845e06edc9653e99de589376a1a96bb6 Mon Sep 17 00:00:00 2001 From: Tobias Pflug Date: Fri, 8 May 2020 15:13:55 +0200 Subject: [PATCH 18/56] Enable toLower umlauts test Update comment and enable the test --- src/libutil/tests/tests.cc | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/libutil/tests/tests.cc b/src/libutil/tests/tests.cc index d46f6a5a4..5abfe2c9c 100644 --- a/src/libutil/tests/tests.cc +++ b/src/libutil/tests/tests.cc @@ -301,12 +301,12 @@ namespace nix { ASSERT_EQ(toLower(s), s); } - // XXX: std::tolower() doesn't cover this. This probably doesn't matter - // since the context is paths and store paths specifically where such - // characters are not allowed. - TEST(toLower, DISABLED_umlauts) { + // std::tolower() doesn't handle unicode characters. In the context of + // store paths this isn't relevant but doesn't hurt to record this behavior + // here. + TEST(toLower, umlauts) { auto s = "ÄÖÜ"; - ASSERT_EQ(toLower(s), "äöü"); + ASSERT_EQ(toLower(s), "ÄÖÜ"); } /* ---------------------------------------------------------------------------- From 52cffafd2462c03466db15909dde250f07677d35 Mon Sep 17 00:00:00 2001 From: Dani Date: Sat, 9 May 2020 13:48:31 +0200 Subject: [PATCH 19/56] Fix typo --- doc/manual/release-notes/rl-0.8.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/manual/release-notes/rl-0.8.xml b/doc/manual/release-notes/rl-0.8.xml index 784b26c6b..825798fa9 100644 --- a/doc/manual/release-notes/rl-0.8.xml +++ b/doc/manual/release-notes/rl-0.8.xml @@ -8,7 +8,7 @@ NOTE: the hashing scheme in Nix 0.8 changed (as detailed below). As a result, nix-pull manifests and channels built -for Nix 0.7 and below will now work anymore. However, the Nix +for Nix 0.7 and below will not work anymore. However, the Nix expression language has not changed, so you can still build from source. Also, existing user environments continue to work. Nix 0.8 will automatically upgrade the database schema of previous From 446649e5403f9c41601eee6438eaeed5212b9190 Mon Sep 17 00:00:00 2001 From: Shao Cheng Date: Sat, 9 May 2020 15:59:39 +0200 Subject: [PATCH 20/56] Update "Upgrading Nix" documentation This PR proposes two changes to the "Upgrading Nix" documentation: * Besides updating `nixpkgs.nix`, we also update `nixpkgs.cacert`, so that the certificates are up-to-date as well. * Add the instructions for multi-user mode on Linux. --- doc/manual/installation/upgrading.xml | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/doc/manual/installation/upgrading.xml b/doc/manual/installation/upgrading.xml index 30670d7fe..592f63895 100644 --- a/doc/manual/installation/upgrading.xml +++ b/doc/manual/installation/upgrading.xml @@ -17,6 +17,11 @@ Single-user installations of Nix should run this: - nix-channel --update; nix-env -iA nixpkgs.nix + nix-channel --update; nix-env -iA nixpkgs.nix nixpkgs.cacert + + + + Multi-user Nix users on Linux should run this with sudo: + nix-channel --update; nix-env -iA nixpkgs.nix nixpkgs.cacert; systemctl daemon-reload; systemctl restart nix-daemon From 146f9c114ff347d372b5ab6c5fc00fdd3f2895da Mon Sep 17 00:00:00 2001 From: Benjamin Hipple Date: Sat, 9 May 2020 10:58:43 -0400 Subject: [PATCH 21/56] doc: consistently refer to 'fixed-output' with a dash General cleanup that makes it easier to search for the term. --- doc/manual/command-ref/nix-store.xml | 2 +- src/libstore/build.cc | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/manual/command-ref/nix-store.xml b/doc/manual/command-ref/nix-store.xml index 1ddb5408d..d71f9d8e4 100644 --- a/doc/manual/command-ref/nix-store.xml +++ b/doc/manual/command-ref/nix-store.xml @@ -936,7 +936,7 @@ $ nix-store --add ./foo.c The operation adds the specified paths to the Nix store. Unlike paths are registered using the -specified hashing algorithm, resulting in the same output path as a fixed output +specified hashing algorithm, resulting in the same output path as a fixed-output derivation. This can be used for sources that are not available from a public url or broke since the download expression was written. diff --git a/src/libstore/build.cc b/src/libstore/build.cc index 147093fae..6258bcb33 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -3155,7 +3155,7 @@ void DerivationGoal::runChild() // Only use nss functions to resolve hosts and // services. Don’t use it for anything else that may // be configured for this system. This limits the - // potential impurities introduced in fixed outputs. + // potential impurities introduced in fixed-outputs. writeFile(chrootRootDir + "/etc/nsswitch.conf", "hosts: files dns\nservices: files\n"); ss.push_back("/etc/services"); From 1d8144e36b18d3467f1b29b80800ed3eb9b876c6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Domen=20Ko=C5=BEar?= Date: Mon, 11 May 2020 18:14:23 +0200 Subject: [PATCH 22/56] Update src/libstore/build.cc --- src/libstore/build.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libstore/build.cc b/src/libstore/build.cc index e0c7324c7..f9b701910 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -3679,7 +3679,7 @@ void DerivationGoal::registerOutputs() if (!S_ISREG(st.st_mode) || (st.st_mode & S_IXUSR) != 0) throw BuildError( format("output path '%1%' should be a non-executable regular file " - "since recursive hashing is not enabled (outputHashMode=flat).") % path); + "since recursive hashing is not enabled (outputHashMode=flat)") % path); } /* Check the hash. In hash mode, move the path produced by From 46be11b762d6048746728e6e546dd4f0fac581fc Mon Sep 17 00:00:00 2001 From: Pavol Rusnak Date: Tue, 12 May 2020 12:13:40 +0200 Subject: [PATCH 23/56] Introduce NIX_INSTALLER_NO_CHANNEL_ADD which skips nix-channel --add --- scripts/install-nix-from-closure.sh | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/scripts/install-nix-from-closure.sh b/scripts/install-nix-from-closure.sh index e00708f6c..21915b37d 100644 --- a/scripts/install-nix-from-closure.sh +++ b/scripts/install-nix-from-closure.sh @@ -130,13 +130,15 @@ if [ -z "$NIX_SSL_CERT_FILE" ] || ! [ -f "$NIX_SSL_CERT_FILE" ]; then fi # Subscribe the user to the Nixpkgs channel and fetch it. -if ! $nix/bin/nix-channel --list | grep -q "^nixpkgs "; then - $nix/bin/nix-channel --add https://nixos.org/channels/nixpkgs-unstable -fi -if [ -z "$_NIX_INSTALLER_TEST" ]; then - if ! $nix/bin/nix-channel --update nixpkgs; then - echo "Fetching the nixpkgs channel failed. (Are you offline?)" - echo "To try again later, run \"nix-channel --update nixpkgs\"." +if [ -z "$NIX_INSTALLER_NO_CHANNEL_ADD" ]; then + if ! $nix/bin/nix-channel --list | grep -q "^nixpkgs "; then + $nix/bin/nix-channel --add https://nixos.org/channels/nixpkgs-unstable + fi + if [ -z "$_NIX_INSTALLER_TEST" ]; then + if ! $nix/bin/nix-channel --update nixpkgs; then + echo "Fetching the nixpkgs channel failed. (Are you offline?)" + echo "To try again later, run \"nix-channel --update nixpkgs\"." + fi fi fi From 5722f9690c39d136ffbf452f3b85df09e8127712 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Tue, 12 May 2020 13:49:55 +0200 Subject: [PATCH 24/56] tests/binary-cache.sh: Improve incomplete closure test Issue #3373. --- tests/binary-cache.sh | 18 ++++++++++++++++-- tests/build-hook.nix | 4 ++-- tests/build-remote.sh | 4 ++-- tests/config.nix.in | 2 +- tests/dependencies.builder1.sh | 2 -- tests/dependencies.builder2.sh | 2 -- tests/dependencies.nix | 15 ++++++++++++--- tests/dependencies.sh | 2 +- tests/export-graph.sh | 2 +- tests/gc-concurrent.nix | 6 +++--- tests/nix-channel.sh | 8 ++++---- 11 files changed, 42 insertions(+), 23 deletions(-) delete mode 100644 tests/dependencies.builder1.sh delete mode 100644 tests/dependencies.builder2.sh diff --git a/tests/binary-cache.sh b/tests/binary-cache.sh index a3c3c7847..17b63d978 100644 --- a/tests/binary-cache.sh +++ b/tests/binary-cache.sh @@ -105,10 +105,24 @@ mv $cacheDir/nar2 $cacheDir/nar # incomplete closure. clearStore -rm $(grep -l "StorePath:.*dependencies-input-2" $cacheDir/*.narinfo) +rm -v $(grep -l "StorePath:.*dependencies-input-2" $cacheDir/*.narinfo) nix-build --substituters "file://$cacheDir" --no-require-sigs dependencies.nix -o $TEST_ROOT/result 2>&1 | tee $TEST_ROOT/log -grep -q "copying path" $TEST_ROOT/log +grep -q "copying path.*input-0" $TEST_ROOT/log +grep -q "copying path.*input-2" $TEST_ROOT/log +grep -q "copying path.*top" $TEST_ROOT/log + + +# Idem, but without cached .narinfo. +clearStore +clearCacheCache + +nix-build --substituters "file://$cacheDir" --no-require-sigs dependencies.nix -o $TEST_ROOT/result 2>&1 | tee $TEST_ROOT/log +grep -q "don't know how to build" $TEST_ROOT/log +grep -q "building.*input-1" $TEST_ROOT/log +grep -q "building.*input-2" $TEST_ROOT/log +grep -q "copying path.*input-0" $TEST_ROOT/log +grep -q "copying path.*top" $TEST_ROOT/log if [ -n "$HAVE_SODIUM" ]; then diff --git a/tests/build-hook.nix b/tests/build-hook.nix index 8bff0fe79..8c5ca8cd3 100644 --- a/tests/build-hook.nix +++ b/tests/build-hook.nix @@ -4,13 +4,13 @@ let input1 = mkDerivation { name = "build-hook-input-1"; - builder = ./dependencies.builder1.sh; + buildCommand = "mkdir $out; echo FOO > $out/foo"; requiredSystemFeatures = ["foo"]; }; input2 = mkDerivation { name = "build-hook-input-2"; - builder = ./dependencies.builder2.sh; + buildCommand = "mkdir $out; echo BAR > $out/bar"; }; in diff --git a/tests/build-remote.sh b/tests/build-remote.sh index ddd68f327..a550f4460 100644 --- a/tests/build-remote.sh +++ b/tests/build-remote.sh @@ -20,5 +20,5 @@ cat $outPath/foobar | grep FOOBAR # Ensure that input1 was built on store1 due to the required feature. p=$(readlink -f $outPath/input-2) -(! nix path-info --store $TEST_ROOT/store0 --all | grep dependencies.builder1.sh) -nix path-info --store $TEST_ROOT/store1 --all | grep dependencies.builder1.sh +(! nix path-info --store $TEST_ROOT/store0 --all | grep builder-build-hook-input-1.sh) +nix path-info --store $TEST_ROOT/store1 --all | grep builder-build-hook-input-1.sh diff --git a/tests/config.nix.in b/tests/config.nix.in index 0ec2eba6b..a57a8c596 100644 --- a/tests/config.nix.in +++ b/tests/config.nix.in @@ -11,7 +11,7 @@ rec { derivation ({ inherit system; builder = shell; - args = ["-e" args.builder or (builtins.toFile "builder.sh" "if [ -e .attrs.sh ]; then source .attrs.sh; fi; eval \"$buildCommand\"")]; + args = ["-e" args.builder or (builtins.toFile "builder-${args.name}.sh" "if [ -e .attrs.sh ]; then source .attrs.sh; fi; eval \"$buildCommand\"")]; PATH = path; } // removeAttrs args ["builder" "meta"]) // { meta = args.meta or {}; }; diff --git a/tests/dependencies.builder1.sh b/tests/dependencies.builder1.sh deleted file mode 100644 index 4b006a17d..000000000 --- a/tests/dependencies.builder1.sh +++ /dev/null @@ -1,2 +0,0 @@ -mkdir $out -echo FOO > $out/foo diff --git a/tests/dependencies.builder2.sh b/tests/dependencies.builder2.sh deleted file mode 100644 index 4f886fdb3..000000000 --- a/tests/dependencies.builder2.sh +++ /dev/null @@ -1,2 +0,0 @@ -mkdir $out -echo BAR > $out/bar diff --git a/tests/dependencies.nix b/tests/dependencies.nix index eca4b2964..e320d81c9 100644 --- a/tests/dependencies.nix +++ b/tests/dependencies.nix @@ -2,18 +2,27 @@ with import ./config.nix; let { + input0 = mkDerivation { + name = "dependencies-input-0"; + buildCommand = "mkdir $out; echo foo > $out/bar"; + }; + input1 = mkDerivation { name = "dependencies-input-1"; - builder = ./dependencies.builder1.sh; + buildCommand = "mkdir $out; echo FOO > $out/foo"; }; input2 = mkDerivation { name = "dependencies-input-2"; - builder = "${./dependencies.builder2.sh}"; + buildCommand = '' + mkdir $out + echo BAR > $out/bar + echo ${input0} > $out/input0 + ''; }; body = mkDerivation { - name = "dependencies"; + name = "dependencies-top"; builder = ./dependencies.builder0.sh + "/FOOBAR/../."; input1 = input1 + "/."; input2 = "${input2}/."; diff --git a/tests/dependencies.sh b/tests/dependencies.sh index 8d0fdc10f..092950aa7 100644 --- a/tests/dependencies.sh +++ b/tests/dependencies.sh @@ -6,7 +6,7 @@ drvPath=$(nix-instantiate dependencies.nix) echo "derivation is $drvPath" -nix-store -q --tree "$drvPath" | grep '───.*builder1.sh' +nix-store -q --tree "$drvPath" | grep '───.*builder-dependencies-input-1.sh' # Test Graphviz graph generation. nix-store -q --graph "$drvPath" > $TEST_ROOT/graph diff --git a/tests/export-graph.sh b/tests/export-graph.sh index a6fd69054..a1449b34e 100644 --- a/tests/export-graph.sh +++ b/tests/export-graph.sh @@ -11,7 +11,7 @@ checkRef() { outPath=$(nix-build ./export-graph.nix -A 'foo."bar.runtimeGraph"' -o $TEST_ROOT/result) -test $(nix-store -q --references $TEST_ROOT/result | wc -l) = 2 || fail "bad nr of references" +test $(nix-store -q --references $TEST_ROOT/result | wc -l) = 3 || fail "bad nr of references" checkRef input-2 for i in $(cat $outPath); do checkRef $i; done diff --git a/tests/gc-concurrent.nix b/tests/gc-concurrent.nix index c0595cc47..21671ea2c 100644 --- a/tests/gc-concurrent.nix +++ b/tests/gc-concurrent.nix @@ -4,12 +4,12 @@ rec { input1 = mkDerivation { name = "dependencies-input-1"; - builder = ./dependencies.builder1.sh; + buildCommand = "mkdir $out; echo FOO > $out/foo"; }; input2 = mkDerivation { name = "dependencies-input-2"; - builder = ./dependencies.builder2.sh; + buildCommand = "mkdir $out; echo BAR > $out/bar"; }; test1 = mkDerivation { @@ -23,5 +23,5 @@ rec { builder = ./gc-concurrent2.builder.sh; inherit input1 input2; }; - + } diff --git a/tests/nix-channel.sh b/tests/nix-channel.sh index 93f837bef..49c68981a 100644 --- a/tests/nix-channel.sh +++ b/tests/nix-channel.sh @@ -32,10 +32,10 @@ if [ "$xmllint" != false ]; then $xmllint --noout $TEST_ROOT/meta.xml || fail "malformed XML" fi grep -q 'meta.*description.*Random test package' $TEST_ROOT/meta.xml -grep -q 'item.*attrPath="foo".*name="dependencies"' $TEST_ROOT/meta.xml +grep -q 'item.*attrPath="foo".*name="dependencies-top"' $TEST_ROOT/meta.xml # Do an install. -nix-env -i dependencies +nix-env -i dependencies-top [ -e $TEST_HOME/.nix-profile/foobar ] clearProfiles @@ -51,9 +51,9 @@ if [ "$xmllint" != false ]; then $xmllint --noout $TEST_ROOT/meta.xml || fail "malformed XML" fi grep -q 'meta.*description.*Random test package' $TEST_ROOT/meta.xml -grep -q 'item.*attrPath="foo".*name="dependencies"' $TEST_ROOT/meta.xml +grep -q 'item.*attrPath="foo".*name="dependencies-top"' $TEST_ROOT/meta.xml # Do an install. -nix-env -i dependencies +nix-env -i dependencies-top [ -e $TEST_HOME/.nix-profile/foobar ] From 268ecf5b3f3efd4e2a1fd78d088bdc69c27b6c7e Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Thu, 7 May 2020 16:46:25 +0200 Subject: [PATCH 25/56] nix: Don't require --experimental-features=nix-command for some subcommands --- src/nix/main.cc | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/nix/main.cc b/src/nix/main.cc index 5cf09c4f0..ef301580a 100644 --- a/src/nix/main.cc +++ b/src/nix/main.cc @@ -167,12 +167,15 @@ void mainWrapped(int argc, char * * argv) args.parseCmdline(argvToStrings(argc, argv)); - settings.requireExperimentalFeature("nix-command"); - initPlugins(); if (!args.command) args.showHelpAndExit(); + if (args.command->first != "repl" + && args.command->first != "doctor" + && args.command->first != "upgrade-nix") + settings.requireExperimentalFeature("nix-command"); + Finally f([]() { stopProgressBar(); }); startProgressBar(args.printBuildLogs); From ebc024df2287085d48ed6194aa756fd70c07f76c Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Thu, 7 May 2020 17:25:27 +0200 Subject: [PATCH 26/56] Show hint how to enable experimental features --- src/libstore/globals.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libstore/globals.cc b/src/libstore/globals.cc index a0a2d850e..bee94cbd8 100644 --- a/src/libstore/globals.cc +++ b/src/libstore/globals.cc @@ -130,7 +130,7 @@ bool Settings::isExperimentalFeatureEnabled(const std::string & name) void Settings::requireExperimentalFeature(const std::string & name) { if (!isExperimentalFeatureEnabled(name)) - throw Error("experimental Nix feature '%s' is disabled", name); + throw Error("experimental Nix feature '%1%' is disabled; use '--experimental-features %1%' to override", name); } bool Settings::isWSL1() From 9e12b2f5b86963bc6d3a4ee37cc759357fb62263 Mon Sep 17 00:00:00 2001 From: Pavol Rusnak Date: Tue, 12 May 2020 18:58:13 +0200 Subject: [PATCH 27/56] Expose installer configuration environment variables via command line flags --- scripts/install-nix-from-closure.sh | 59 ++++++++++++++++++----------- 1 file changed, 36 insertions(+), 23 deletions(-) diff --git a/scripts/install-nix-from-closure.sh b/scripts/install-nix-from-closure.sh index 21915b37d..e06530ddf 100644 --- a/scripts/install-nix-from-closure.sh +++ b/scripts/install-nix-from-closure.sh @@ -40,30 +40,43 @@ elif [ "$(uname -s)" = "Linux" ] && [ -e /run/systemd/system ]; then fi INSTALL_MODE=no-daemon -# Trivially handle the --daemon / --no-daemon options -if [ "x${1:-}" = "x--no-daemon" ]; then - INSTALL_MODE=no-daemon -elif [ "x${1:-}" = "x--daemon" ]; then - INSTALL_MODE=daemon -elif [ "x${1:-}" != "x" ]; then - ( - echo "Nix Installer [--daemon|--no-daemon]" - echo "Choose installation method." - echo "" - echo " --daemon: Installs and configures a background daemon that manages the store," - echo " providing multi-user support and better isolation for local builds." - echo " Both for security and reproducibility, this method is recommended if" - echo " supported on your platform." - echo " See https://nixos.org/nix/manual/#sect-multi-user-installation" - echo "" - echo " --no-daemon: Simple, single-user installation that does not require root and is" - echo " trivial to uninstall." - echo " (default)" - echo "" - ) >&2 - exit -fi +# handle the command line flags +while [ "x${1:-}" != "x" ]; do + if [ "x${1:-}" = "x--no-daemon" ]; then + INSTALL_MODE=no-daemon + elif [ "x${1:-}" = "x--daemon" ]; then + INSTALL_MODE=daemon + elif [ "x${1:-}" = "x--no-channel-add" ]; then + NIX_INSTALLER_NO_CHANNEL_ADD=1 + elif [ "x${1:-}" = "x--no-modify-profile" ]; then + NIX_INSTALLER_NO_MODIFY_PROFILE=1 + elif [ "x${1:-}" != "x" ]; then + ( + echo "Nix Installer [--daemon|--no-daemon] [--no-channel-add] [--no-modify-profile]" + + echo "Choose installation method." + echo "" + echo " --daemon: Installs and configures a background daemon that manages the store," + echo " providing multi-user support and better isolation for local builds." + echo " Both for security and reproducibility, this method is recommended if" + echo " supported on your platform." + echo " See https://nixos.org/nix/manual/#sect-multi-user-installation" + echo "" + echo " --no-daemon: Simple, single-user installation that does not require root and is" + echo " trivial to uninstall." + echo " (default)" + echo "" + echo " --no-channel-add: Don't add any channels. nixpkgs-unstable is installed by default." + echo "" + echo " --no-modify-profile: Skip channel installation. When not provided nixpkgs-unstable" + echo " is installed by default." + echo "" + ) >&2 + exit + fi + shift +done if [ "$INSTALL_MODE" = "daemon" ]; then printf '\e[1;31mSwitching to the Daemon-based Installer\e[0m\n' From 183dd28266e0292116da6cfbc3e2643577adbfbe Mon Sep 17 00:00:00 2001 From: Alexander Bantyev Date: Thu, 14 May 2020 17:00:54 +0300 Subject: [PATCH 28/56] Don't lock a user while doing remote builds --- src/libstore/build.cc | 88 ++++++++++++++++++++++++------------------- 1 file changed, 49 insertions(+), 39 deletions(-) diff --git a/src/libstore/build.cc b/src/libstore/build.cc index a8c1cd565..f7fc23b35 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -507,6 +507,7 @@ private: Path fnUserLock; AutoCloseFD fdUserLock; + bool isEnabled; string user; uid_t uid; gid_t gid; @@ -524,7 +525,7 @@ public: bool findFreeUser(); - bool enabled() { return uid != 0; } + bool enabled() { return isEnabled; } }; @@ -535,9 +536,11 @@ UserLock::UserLock() createDirs(settings.nixStateDir + "/userpool"); /* Mark that user is not enabled by default */ uid = 0; + isEnabled = false; } bool UserLock::findFreeUser() { + if (enabled()) return true; /* Get the members of the build-users-group. */ struct group * gr = getgrnam(settings.buildUsersGroup.get().c_str()); @@ -597,6 +600,7 @@ bool UserLock::findFreeUser() { supplementaryGIDs.resize(ngroups); #endif + isEnabled = true; return true; } } @@ -931,6 +935,7 @@ private: void closureRepaired(); void inputsRealised(); void tryToBuild(); + void tryLocalBuild(); void buildDone(); /* Is the build hook willing to perform the build? */ @@ -1002,6 +1007,8 @@ private: Goal::amDone(result); } + void started(); + void done(BuildResult::Status status, const string & msg = ""); StorePathSet exportReferences(const StorePathSet & storePaths); @@ -1389,35 +1396,24 @@ void DerivationGoal::inputsRealised() result = BuildResult(); } +void DerivationGoal::started() { + auto msg = fmt( + buildMode == bmRepair ? "repairing outputs of '%s'" : + buildMode == bmCheck ? "checking outputs of '%s'" : + nrRounds > 1 ? "building '%s' (round %d/%d)" : + "building '%s'", worker.store.printStorePath(drvPath), curRound, nrRounds); + fmt("building '%s'", worker.store.printStorePath(drvPath)); + if (hook) msg += fmt(" on '%s'", machineName); + act = std::make_unique(*logger, lvlInfo, actBuild, msg, + Logger::Fields{worker.store.printStorePath(drvPath), hook ? machineName : "", curRound, nrRounds}); + mcRunningBuilds = std::make_unique>(worker.runningBuilds); + worker.updateProgress(); +} void DerivationGoal::tryToBuild() { trace("trying to build"); - /* If `build-users-group' is not empty, then we have to build as - one of the members of that group. */ - if (settings.buildUsersGroup != "" && getuid() == 0) { -#if defined(__linux__) || defined(__APPLE__) - if (!buildUser) buildUser = std::make_unique(); - - if (!buildUser->enabled()) { - if (!buildUser->findFreeUser()) { - debug("waiting for build users"); - worker.waitForAWhile(shared_from_this()); - return; - } - - /* Make sure that no other processes are executing under this - uid. */ - buildUser->kill(); - } -#else - /* Don't know how to block the creation of setuid/setgid - binaries on this platform. */ - throw Error("build users are not supported on this platform for security reasons"); -#endif - } - /* Obtain locks on all output paths. The locks are automatically released when we exit this function or Nix crashes. If we can't acquire the lock, then continue; hopefully some other @@ -1464,20 +1460,6 @@ void DerivationGoal::tryToBuild() supported for local builds. */ bool buildLocally = buildMode != bmNormal || parsedDrv->willBuildLocally(); - auto started = [&]() { - auto msg = fmt( - buildMode == bmRepair ? "repairing outputs of '%s'" : - buildMode == bmCheck ? "checking outputs of '%s'" : - nrRounds > 1 ? "building '%s' (round %d/%d)" : - "building '%s'", worker.store.printStorePath(drvPath), curRound, nrRounds); - fmt("building '%s'", worker.store.printStorePath(drvPath)); - if (hook) msg += fmt(" on '%s'", machineName); - act = std::make_unique(*logger, lvlInfo, actBuild, msg, - Logger::Fields{worker.store.printStorePath(drvPath), hook ? machineName : "", curRound, nrRounds}); - mcRunningBuilds = std::make_unique>(worker.runningBuilds); - worker.updateProgress(); - }; - /* Is the build hook willing to accept this job? */ if (!buildLocally) { switch (tryBuildHook()) { @@ -1510,6 +1492,34 @@ void DerivationGoal::tryToBuild() return; } + state = &DerivationGoal::tryLocalBuild; + worker.wakeUp(shared_from_this()); +} + +void DerivationGoal::tryLocalBuild() { + + /* If `build-users-group' is not empty, then we have to build as + one of the members of that group. */ + if (settings.buildUsersGroup != "" && getuid() == 0) { +#if defined(__linux__) || defined(__APPLE__) + if (!buildUser) buildUser = std::make_unique(); + + if (buildUser->findFreeUser()) { + /* Make sure that no other processes are executing under this + uid. */ + buildUser->kill(); + } else { + debug("waiting for build users"); + worker.waitForAWhile(shared_from_this()); + return; + } +#else + /* Don't know how to block the creation of setuid/setgid + binaries on this platform. */ + throw Error("build users are not supported on this platform for security reasons"); +#endif + } + try { /* Okay, we have to build. */ From 546b179d0a89e9f27a02e92004da0f8f08e5041a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Domen=20Ko=C5=BEar?= Date: Fri, 15 May 2020 10:06:14 +0200 Subject: [PATCH 29/56] actions: use latest OS --- .github/workflows/test.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 87997414d..7feefc855 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -6,7 +6,7 @@ jobs: tests: strategy: matrix: - os: [ubuntu-18.04, macos] + os: [ubuntu-latest, macos-latest] runs-on: ${{ matrix.os }} steps: - uses: actions/checkout@v2 From e223eeac09c6468db80ee0497c84960b4ec73e02 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Thalheim?= Date: Sat, 16 May 2020 08:40:55 +0100 Subject: [PATCH 30/56] Remove -j option from simple-build-testing By default Nix/NixOS already set a reasonable default `max-jobs = auto` so we don't need to mention it in this tutorial. The option is still documented in other parts of the documentation if users ever stumble over this. Fixes https://github.com/NixOS/nix/issues/2531 --- doc/manual/expressions/simple-building-testing.xml | 8 -------- 1 file changed, 8 deletions(-) diff --git a/doc/manual/expressions/simple-building-testing.xml b/doc/manual/expressions/simple-building-testing.xml index 7326a3e76..ce0a1636d 100644 --- a/doc/manual/expressions/simple-building-testing.xml +++ b/doc/manual/expressions/simple-building-testing.xml @@ -73,12 +73,4 @@ waiting for lock on `/nix/store/0h5b7hp8d4hqfrw8igvx97x1xawrjnac-hello-2.1.1x'make). -If you have a system with multiple CPUs, you may want to have -Nix build different derivations in parallel (insofar as possible). -Just pass the option , where -N is the maximum number of jobs to be run -in parallel, or set. Typically this should be the number of -CPUs. -
From 5ef64f05e6e493d5fe69c87127328f68500b9e50 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Mon, 18 May 2020 15:50:29 +0200 Subject: [PATCH 31/56] Cleanup --- src/libstore/build.cc | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/libstore/build.cc b/src/libstore/build.cc index 4f2f38d63..a2b16f95c 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -507,10 +507,10 @@ private: Path fnUserLock; AutoCloseFD fdUserLock; - bool isEnabled; + bool isEnabled = false; string user; - uid_t uid; - gid_t gid; + uid_t uid = 0; + gid_t gid = 0; std::vector supplementaryGIDs; public: @@ -534,9 +534,6 @@ UserLock::UserLock() { assert(settings.buildUsersGroup != ""); createDirs(settings.nixStateDir + "/userpool"); - /* Mark that user is not enabled by default */ - uid = 0; - isEnabled = false; } bool UserLock::findFreeUser() { From a73a820a5d73e0c595ff807a752f76ef1184ca2d Mon Sep 17 00:00:00 2001 From: Tobias Pflug Date: Wed, 20 May 2020 16:27:53 +0200 Subject: [PATCH 32/56] Add unit testes for url.cc This adds tests for - parseURL - percentDecode - decodeQuery --- src/libutil/tests/tests.cc | 6 +- src/libutil/tests/url.cc | 266 +++++++++++++++++++++++++++++++++++++ 2 files changed, 271 insertions(+), 1 deletion(-) create mode 100644 src/libutil/tests/url.cc diff --git a/src/libutil/tests/tests.cc b/src/libutil/tests/tests.cc index 5abfe2c9c..8e77ccbe1 100644 --- a/src/libutil/tests/tests.cc +++ b/src/libutil/tests/tests.cc @@ -5,6 +5,8 @@ namespace nix { +/* ----------- tests for util.hh ------------------------------------------------*/ + /* ---------------------------------------------------------------------------- * absPath * --------------------------------------------------------------------------*/ @@ -15,6 +17,9 @@ namespace nix { ASSERT_EQ(p, "/"); } + + + TEST(absPath, turnsEmptyPathIntoCWD) { char cwd[PATH_MAX+1]; auto p = absPath(""); @@ -581,5 +586,4 @@ namespace nix { ASSERT_EQ(filterANSIEscapes(s, true), "foo bar baz" ); } - } diff --git a/src/libutil/tests/url.cc b/src/libutil/tests/url.cc new file mode 100644 index 000000000..80646ad3e --- /dev/null +++ b/src/libutil/tests/url.cc @@ -0,0 +1,266 @@ +#include "url.hh" +#include + +namespace nix { + +/* ----------- tests for url.hh --------------------------------------------------*/ + + string print_map(std::map m) { + std::map::iterator it; + string s = "{ "; + for (it = m.begin(); it != m.end(); ++it) { + s += "{ "; + s += it->first; + s += " = "; + s += it->second; + s += " } "; + } + s += "}"; + return s; + } + + + std::ostream& operator<<(std::ostream& os, const ParsedURL& p) { + return os << "\n" + << "url: " << p.url << "\n" + << "base: " << p.base << "\n" + << "scheme: " << p.scheme << "\n" + << "authority: " << p.authority.value() << "\n" + << "path: " << p.path << "\n" + << "query: " << print_map(p.query) << "\n" + << "fragment: " << p.fragment << "\n"; + } + + TEST(parseURL, parsesSimpleHttpUrl) { + auto s = "http://www.example.org/file.tar.gz"; + auto parsed = parseURL(s); + + ParsedURL expected { + .url = "http://www.example.org/file.tar.gz", + .base = "http://www.example.org/file.tar.gz", + .scheme = "http", + .authority = "www.example.org", + .path = "/file.tar.gz", + .query = (StringMap) { }, + .fragment = "", + }; + + ASSERT_EQ(parsed, expected); + } + + TEST(parseURL, parsesSimpleHttpsUrl) { + auto s = "https://www.example.org/file.tar.gz"; + auto parsed = parseURL(s); + + ParsedURL expected { + .url = "https://www.example.org/file.tar.gz", + .base = "https://www.example.org/file.tar.gz", + .scheme = "https", + .authority = "www.example.org", + .path = "/file.tar.gz", + .query = (StringMap) { }, + .fragment = "", + }; + + ASSERT_EQ(parsed, expected); + } + + TEST(parseURL, parsesSimpleHttpUrlWithQueryAndFragment) { + auto s = "https://www.example.org/file.tar.gz?download=fast&when=now#hello"; + auto parsed = parseURL(s); + + ParsedURL expected { + .url = "https://www.example.org/file.tar.gz", + .base = "https://www.example.org/file.tar.gz", + .scheme = "https", + .authority = "www.example.org", + .path = "/file.tar.gz", + .query = (StringMap) { { "download", "fast" }, { "when", "now" } }, + .fragment = "hello", + }; + + ASSERT_EQ(parsed, expected); + } + + TEST(parseURL, parsesSimpleHttpUrlWithComplexFragment) { + auto s = "http://www.example.org/file.tar.gz?field=value#?foo=bar%23"; + auto parsed = parseURL(s); + + ParsedURL expected { + .url = "http://www.example.org/file.tar.gz", + .base = "http://www.example.org/file.tar.gz", + .scheme = "http", + .authority = "www.example.org", + .path = "/file.tar.gz", + .query = (StringMap) { { "field", "value" } }, + .fragment = "?foo=bar#", + }; + + ASSERT_EQ(parsed, expected); + } + + + TEST(parseURL, parseIPv4Address) { + auto s = "http://127.0.0.1:8080/file.tar.gz?download=fast&when=now#hello"; + auto parsed = parseURL(s); + + ParsedURL expected { + .url = "http://127.0.0.1:8080/file.tar.gz", + .base = "https://127.0.0.1:8080/file.tar.gz", + .scheme = "http", + .authority = "127.0.0.1:8080", + .path = "/file.tar.gz", + .query = (StringMap) { { "download", "fast" }, { "when", "now" } }, + .fragment = "hello", + }; + + ASSERT_EQ(parsed, expected); + } + + TEST(parseURL, parseIPv6Address) { + auto s = "http://[2a02:8071:8192:c100:311d:192d:81ac:11ea]:8080"; + auto parsed = parseURL(s); + + ParsedURL expected { + .url = "http://[2a02:8071:8192:c100:311d:192d:81ac:11ea]:8080", + .base = "http://[2a02:8071:8192:c100:311d:192d:81ac:11ea]:8080", + .scheme = "http", + .authority = "[2a02:8071:8192:c100:311d:192d:81ac:11ea]:8080", + .path = "", + .query = (StringMap) { }, + .fragment = "", + }; + + ASSERT_EQ(parsed, expected); + + } + + TEST(parseURL, parseEmptyQueryParams) { + auto s = "http://127.0.0.1:8080/file.tar.gz?&&&&&"; + auto parsed = parseURL(s); + ASSERT_EQ(parsed.query, (StringMap) { }); + } + + TEST(parseURL, parseUserPassword) { + auto s = "http://user:pass@www.example.org:8080/file.tar.gz"; + auto parsed = parseURL(s); + + ParsedURL expected { + .url = "http://user:pass@www.example.org/file.tar.gz", + .base = "http://user:pass@www.example.org/file.tar.gz", + .scheme = "http", + .authority = "user:pass@www.example.org:8080", + .path = "/file.tar.gz", + .query = (StringMap) { }, + .fragment = "", + }; + + + ASSERT_EQ(parsed, expected); + } + + TEST(parseURL, parseFileURLWithQueryAndFragment) { + auto s = "file:///none/of/your/business"; + auto parsed = parseURL(s); + + ParsedURL expected { + .url = "", + .base = "", + .scheme = "file", + .authority = "", + .path = "/none/of/your/business", + .query = (StringMap) { }, + .fragment = "", + }; + + ASSERT_EQ(parsed, expected); + + } + + TEST(parseURL, parsedUrlsIsEqualToItself) { + auto s = "http://www.example.org/file.tar.gz"; + auto url = parseURL(s); + + ASSERT_TRUE(url == url); + } + + TEST(parseURL, parseFTPUrl) { + auto s = "ftp://ftp.nixos.org/downloads/nixos.iso"; + auto parsed = parseURL(s); + + ParsedURL expected { + .url = "ftp://ftp.nixos.org/downloads/nixos.iso", + .base = "ftp://ftp.nixos.org/downloads/nixos.iso", + .scheme = "ftp", + .authority = "ftp.nixos.org", + .path = "/downloads/nixos.iso", + .query = (StringMap) { }, + .fragment = "", + }; + + ASSERT_EQ(parsed, expected); + } + + TEST(parseURL, parsesAnythingInUriFormat) { + auto s = "whatever://github.com/NixOS/nixpkgs.git"; + auto parsed = parseURL(s); + } + + TEST(parseURL, parsesAnythingInUriFormatWithoutDoubleSlash) { + auto s = "whatever:github.com/NixOS/nixpkgs.git"; + auto parsed = parseURL(s); + } + + TEST(parseURL, emptyStringIsInvalidURL) { + ASSERT_THROW(parseURL(""), Error); + } + + /* ---------------------------------------------------------------------------- + * decodeQuery + * --------------------------------------------------------------------------*/ + + TEST(decodeQuery, emptyStringYieldsEmptyMap) { + auto d = decodeQuery(""); + ASSERT_EQ(d, (StringMap) { }); + } + + TEST(decodeQuery, simpleDecode) { + auto d = decodeQuery("yi=one&er=two"); + ASSERT_EQ(d, ((StringMap) { { "yi", "one" }, { "er", "two" } })); + } + + TEST(decodeQuery, decodeUrlEncodedArgs) { + auto d = decodeQuery("arg=%3D%3D%40%3D%3D"); + ASSERT_EQ(d, ((StringMap) { { "arg", "==@==" } })); + } + + TEST(decodeQuery, decodeArgWithEmptyValue) { + auto d = decodeQuery("arg="); + ASSERT_EQ(d, ((StringMap) { { "arg", ""} })); + } + + /* ---------------------------------------------------------------------------- + * percentDecode + * --------------------------------------------------------------------------*/ + + TEST(percentDecode, decodesUrlEncodedString) { + string s = "==@=="; + string d = percentDecode("%3D%3D%40%3D%3D"); + ASSERT_EQ(d, s); + } + + TEST(percentDecode, multipleDecodesAreIdempotent) { + string once = percentDecode("%3D%3D%40%3D%3D"); + string twice = percentDecode(once); + + ASSERT_EQ(once, twice); + } + + TEST(percentDecode, trailingPercent) { + string s = "==@==%"; + string d = percentDecode("%3D%3D%40%3D%3D%25"); + + ASSERT_EQ(d, s); + } + +} From c8cb558849da8ef88f15cc2a70c570f1f5013a30 Mon Sep 17 00:00:00 2001 From: Krzysztof Gogolewski Date: Thu, 21 May 2020 19:29:13 +0200 Subject: [PATCH 33/56] documentation: avoid unquoted URLs --- doc/manual/command-ref/conf-file.xml | 2 +- doc/manual/expressions/advanced-attributes.xml | 4 ++-- doc/manual/expressions/builtins.xml | 4 ++-- doc/manual/expressions/expression-syntax.xml | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/doc/manual/command-ref/conf-file.xml b/doc/manual/command-ref/conf-file.xml index 1820598e5..1fa74a143 100644 --- a/doc/manual/command-ref/conf-file.xml +++ b/doc/manual/command-ref/conf-file.xml @@ -386,7 +386,7 @@ false. builtins.fetchurl { - url = https://example.org/foo-1.2.3.tar.xz; + url = "https://example.org/foo-1.2.3.tar.xz"; sha256 = "2c26b46b68ffc68ff99b453c1d30413413422d706483bfa0f98a5e886266e7ae"; } diff --git a/doc/manual/expressions/advanced-attributes.xml b/doc/manual/expressions/advanced-attributes.xml index 372d03de7..5759ff50e 100644 --- a/doc/manual/expressions/advanced-attributes.xml +++ b/doc/manual/expressions/advanced-attributes.xml @@ -178,7 +178,7 @@ impureEnvVars = [ "http_proxy" "https_proxy" ... ]; fetchurl { - url = http://ftp.gnu.org/pub/gnu/hello/hello-2.1.1.tar.gz; + url = "http://ftp.gnu.org/pub/gnu/hello/hello-2.1.1.tar.gz"; sha256 = "1md7jsfd8pa45z73bz1kszpp01yw6x5ljkjk2hx7wl800any6465"; } @@ -189,7 +189,7 @@ fetchurl { fetchurl { - url = ftp://ftp.nluug.nl/pub/gnu/hello/hello-2.1.1.tar.gz; + url = "ftp://ftp.nluug.nl/pub/gnu/hello/hello-2.1.1.tar.gz"; sha256 = "1md7jsfd8pa45z73bz1kszpp01yw6x5ljkjk2hx7wl800any6465"; } diff --git a/doc/manual/expressions/builtins.xml b/doc/manual/expressions/builtins.xml index f71a8f3be..6709c4081 100644 --- a/doc/manual/expressions/builtins.xml +++ b/doc/manual/expressions/builtins.xml @@ -349,7 +349,7 @@ stdenv.mkDerivation { … } with import (fetchTarball { - url = https://github.com/NixOS/nixpkgs-channels/archive/nixos-14.12.tar.gz; + url = "https://github.com/NixOS/nixpkgs-channels/archive/nixos-14.12.tar.gz"; sha256 = "1jppksrfvbk5ypiqdz4cddxdl8z6zyzdb2srq8fcffr327ld5jj2"; }) {}; @@ -1406,7 +1406,7 @@ stdenv.mkDerivation { "; src = fetchurl { - url = http://ftp.nluug.nl/pub/gnu/hello/hello-2.1.1.tar.gz; + url = "http://ftp.nluug.nl/pub/gnu/hello/hello-2.1.1.tar.gz"; sha256 = "1md7jsfd8pa45z73bz1kszpp01yw6x5ljkjk2hx7wl800any6465"; }; inherit perl; diff --git a/doc/manual/expressions/expression-syntax.xml b/doc/manual/expressions/expression-syntax.xml index 42b9dca36..a3de20713 100644 --- a/doc/manual/expressions/expression-syntax.xml +++ b/doc/manual/expressions/expression-syntax.xml @@ -15,7 +15,7 @@ stdenv.mkDerivation { name = "hello-2.1.1"; builder = ./builder.sh; src = fetchurl { - url = ftp://ftp.nluug.nl/pub/gnu/hello/hello-2.1.1.tar.gz; + url = "ftp://ftp.nluug.nl/pub/gnu/hello/hello-2.1.1.tar.gz"; sha256 = "1md7jsfd8pa45z73bz1kszpp01yw6x5ljkjk2hx7wl800any6465"; }; inherit perl; From 0726ad5825f60e543d9cf535c62673685adbf5c8 Mon Sep 17 00:00:00 2001 From: Daiderd Jordan Date: Sun, 15 Dec 2019 16:43:43 +0100 Subject: [PATCH 34/56] install: configure and bootstrap synthetic.conf on darwin Starting macOS 10.15 /nix can't be creasted directly anymore due to the readonly filesystem, but synthetic.conf was introduced to enable creating mountpoints or symlinks for special usecases like package managers. --- release.nix | 12 +++- scripts/create-darwin-volume.sh | 107 ++++++++++++++++++++++++++++ scripts/install-nix-from-closure.sh | 93 +++++++++++++++--------- 3 files changed, 176 insertions(+), 36 deletions(-) create mode 100755 scripts/create-darwin-volume.sh diff --git a/release.nix b/release.nix index f5729cee3..2cc4bb4c0 100644 --- a/release.nix +++ b/release.nix @@ -177,10 +177,10 @@ let } '' cp ${installerClosureInfo}/registration $TMPDIR/reginfo + cp ${./scripts/create-darwin-volume.sh} $TMPDIR/create-darwin-volume.sh substitute ${./scripts/install-nix-from-closure.sh} $TMPDIR/install \ --subst-var-by nix ${toplevel} \ --subst-var-by cacert ${cacert} - substitute ${./scripts/install-darwin-multi-user.sh} $TMPDIR/install-darwin-multi-user.sh \ --subst-var-by nix ${toplevel} \ --subst-var-by cacert ${cacert} @@ -195,6 +195,7 @@ let # SC1090: Don't worry about not being able to find # $nix/etc/profile.d/nix.sh shellcheck --exclude SC1090 $TMPDIR/install + shellcheck $TMPDIR/create-darwin-volume.sh shellcheck $TMPDIR/install-darwin-multi-user.sh shellcheck $TMPDIR/install-systemd-multi-user.sh @@ -210,6 +211,7 @@ let fi chmod +x $TMPDIR/install + chmod +x $TMPDIR/create-darwin-volume.sh chmod +x $TMPDIR/install-darwin-multi-user.sh chmod +x $TMPDIR/install-systemd-multi-user.sh chmod +x $TMPDIR/install-multi-user @@ -222,11 +224,15 @@ let --absolute-names \ --hard-dereference \ --transform "s,$TMPDIR/install,$dir/install," \ + --transform "s,$TMPDIR/create-darwin-volume.sh,$dir/create-darwin-volume.sh," \ --transform "s,$TMPDIR/reginfo,$dir/.reginfo," \ --transform "s,$NIX_STORE,$dir/store,S" \ - $TMPDIR/install $TMPDIR/install-darwin-multi-user.sh \ + $TMPDIR/install \ + $TMPDIR/create-darwin-volume.sh \ + $TMPDIR/install-darwin-multi-user.sh \ $TMPDIR/install-systemd-multi-user.sh \ - $TMPDIR/install-multi-user $TMPDIR/reginfo \ + $TMPDIR/install-multi-user \ + $TMPDIR/reginfo \ $(cat ${installerClosureInfo}/store-paths) ''); diff --git a/scripts/create-darwin-volume.sh b/scripts/create-darwin-volume.sh new file mode 100755 index 000000000..f3e0d46f1 --- /dev/null +++ b/scripts/create-darwin-volume.sh @@ -0,0 +1,107 @@ +#!/bin/sh +set -e + +root_disks() { + diskutil list -plist / +} + +apfs_volumes_for() { + disk=$1 + diskutil apfs list -plist "$disk" +} + +disk_identifier() { + xpath "/plist/dict/key[text()='WholeDisks']/following-sibling::array[1]/string/text()" 2>/dev/null +} + +volume_get() { + key=$1 i=$2 + xpath "/plist/dict/array/dict/key[text()='Volumes']/following-sibling::array/dict[$i]/key[text()='$key']/following-sibling::string[1]/text()" 2> /dev/null +} + +find_nix_volume() { + disk=$1 + i=1 + volumes=$(apfs_volumes_for "$disk") + while true; do + name=$(echo "$volumes" | volume_get "Name" "$i") + if [ -z "$name" ]; then + break + fi + case "$name" in + [Nn]ix*) + echo "$name" + break + ;; + esac + i=$((i+1)) + done +} + +test_fstab() { + grep -q "/nix" /etc/fstab 2>/dev/null +} + +test_synthetic_conf() { + grep -q "^nix" /etc/synthetic.conf 2>/dev/null +} + +test_nix() { + test -d "/nix" +} + +main() { + ( + echo "" + echo " ------------------------------------------------------------------ " + echo " | This installer will create a volume for the nix store and |" + echo " | configure it to mount at /nix. Follow these steps to uninstall. |" + echo " ------------------------------------------------------------------ " + echo "" + echo " 1. Remove the entry from fstab using 'sudo vifs'" + echo " 2. Destroy the data volume using 'diskutil apfs deleteVolume'" + echo " 3. Delete /etc/synthetic.conf" + echo "" + ) >&2 + + if [ -L "/nix" ]; then + echo "error: /nix is a symlink, please remove it or edit synthetic.conf (requires reboot)" >&2 + echo " /nix -> $(readlink "/nix")" >&2 + exit 2 + fi + + if ! test_synthetic_conf; then + echo "Configuring /etc/synthetic.conf..." >&2 + echo nix | sudo tee /etc/synthetic.conf + /System/Library/Filesystems/apfs.fs/Contents/Resources/apfs.util -B + fi + + if ! test_nix; then + echo "Creating mountpoint for /nix..." >&2 + sudo mkdir /nix + fi + + disk=$(root_disks | disk_identifier) + volume=$(find_nix_volume "$disk") + if [ -z "$volume" ]; then + echo "Creating a Nix Store volume..." >&2 + sudo diskutil apfs addVolume "$disk" APFS 'Nix Store' -mountpoint /nix + volume="Nix Store" + else + echo "Using existing '$volume' volume" >&2 + fi + + if ! test_fstab; then + echo "Configuring /etc/fstab..." >&2 + label=$(echo "$volume" | sed 's/ /\\040/g') + printf "\$a\nLABEL=%s /nix apfs rw\n.\nwq\n" "$label" | EDITOR=ed sudo vifs + fi + + echo "The following options can be enabled to disable spotlight indexing" >&2 + echo "of the volume, which might be desirable." >&2 + echo "" >&2 + echo " $ mdutil -i off /nix" >&2 + echo "" >&2 +} + +main "$@" diff --git a/scripts/install-nix-from-closure.sh b/scripts/install-nix-from-closure.sh index e06530ddf..34be7ee6a 100644 --- a/scripts/install-nix-from-closure.sh +++ b/scripts/install-nix-from-closure.sh @@ -40,44 +40,62 @@ elif [ "$(uname -s)" = "Linux" ] && [ -e /run/systemd/system ]; then fi INSTALL_MODE=no-daemon - +CREATE_DARWIN_VOLUME=0 # handle the command line flags -while [ "x${1:-}" != "x" ]; do - if [ "x${1:-}" = "x--no-daemon" ]; then - INSTALL_MODE=no-daemon - elif [ "x${1:-}" = "x--daemon" ]; then - INSTALL_MODE=daemon - elif [ "x${1:-}" = "x--no-channel-add" ]; then - NIX_INSTALLER_NO_CHANNEL_ADD=1 - elif [ "x${1:-}" = "x--no-modify-profile" ]; then - NIX_INSTALLER_NO_MODIFY_PROFILE=1 - elif [ "x${1:-}" != "x" ]; then - ( - echo "Nix Installer [--daemon|--no-daemon] [--no-channel-add] [--no-modify-profile]" +while [ $# -gt 0 ]; do + case $1 in + --daemon) + INSTALL_MODE=daemon;; + --no-daemon) + INSTALL_MODE=no-daemon;; + --no-channel-add) + NIX_INSTALLER_NO_CHANNEL_ADD=1;; + --no-modify-profile) + NIX_INSTALLER_NO_MODIFY_PROFILE=1;; + --create-volume) + CREATE_DARWIN_VOLUME=1;; + *) + ( + echo "Nix Installer [--daemon|--no-daemon] [--no-channel-add] [--no-modify-profile]" - echo "Choose installation method." - echo "" - echo " --daemon: Installs and configures a background daemon that manages the store," - echo " providing multi-user support and better isolation for local builds." - echo " Both for security and reproducibility, this method is recommended if" - echo " supported on your platform." - echo " See https://nixos.org/nix/manual/#sect-multi-user-installation" - echo "" - echo " --no-daemon: Simple, single-user installation that does not require root and is" - echo " trivial to uninstall." - echo " (default)" - echo "" - echo " --no-channel-add: Don't add any channels. nixpkgs-unstable is installed by default." - echo "" - echo " --no-modify-profile: Skip channel installation. When not provided nixpkgs-unstable" - echo " is installed by default." - echo "" - ) >&2 - exit - fi + echo "Choose installation method." + echo "" + echo " --daemon: Installs and configures a background daemon that manages the store," + echo " providing multi-user support and better isolation for local builds." + echo " Both for security and reproducibility, this method is recommended if" + echo " supported on your platform." + echo " See https://nixos.org/nix/manual/#sect-multi-user-installation" + echo "" + echo " --no-daemon: Simple, single-user installation that does not require root and is" + echo " trivial to uninstall." + echo " (default)" + echo "" + echo " --no-channel-add: Don't add any channels. nixpkgs-unstable is installed by default." + echo "" + echo " --no-modify-profile: Skip channel installation. When not provided nixpkgs-unstable" + echo " is installed by default." + echo "" + ) >&2 + + if [ "$(uname -s)" = "Darwin" ]; then + ( + echo " --create-volume: Create an APFS volume for the store and create the /nix" + echo " mountpoint for it using synthetic.conf." + echo " (required on macOS >=10.15)" + echo " See https://nixos.org/nix/manual/#sect-darwin-apfs-volume" + echo "" + ) >&2 + fi + exit;; + esac shift done +if [ "$(uname -s)" = "Darwin" ] && [ "$CREATE_DARWIN_VOLUME" = 1 ]; then + printf '\e[1;31mCreating volume and mountpoint /nix.\e[0m\n' + "$self/create-darwin-volume.sh" +fi + if [ "$INSTALL_MODE" = "daemon" ]; then printf '\e[1;31mSwitching to the Daemon-based Installer\e[0m\n' exec "$self/install-multi-user" @@ -95,6 +113,15 @@ if ! [ -e $dest ]; then echo "directory $dest does not exist; creating it by running '$cmd' using sudo" >&2 if ! sudo sh -c "$cmd"; then echo "$0: please manually run '$cmd' as root to create $dest" >&2 + if [ "$(uname -s)" = "Darwin" ]; then + ( + echo "" + echo "Installing on macOS >=10.15 requires relocating the store to an apfs volume." + echo "Use --create-volume or run the preparation steps manually." + echo "See https://nixos.org/nix/manual/#sect-darwin-apfs-volume." + echo "" + ) >&2 + fi exit 1 fi fi From 10202628b911980f05fc2c9460995af30f1bcbf3 Mon Sep 17 00:00:00 2001 From: Daiderd Jordan Date: Sun, 15 Dec 2019 21:11:14 +0100 Subject: [PATCH 35/56] install: also configure ~/.zshenv The default login shell for users on macOS 10.15 changed from bash to zsh. So while generally nonstandard we need to configure it to make nix function out of the box on macOS. --- scripts/install-nix-from-closure.sh | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/scripts/install-nix-from-closure.sh b/scripts/install-nix-from-closure.sh index 34be7ee6a..88275d1f0 100644 --- a/scripts/install-nix-from-closure.sh +++ b/scripts/install-nix-from-closure.sh @@ -197,6 +197,17 @@ if [ -z "$NIX_INSTALLER_NO_MODIFY_PROFILE" ]; then break fi done + for i in .zshenv .zshrc; do + fn="$HOME/$i" + if [ -w "$fn" ]; then + if ! grep -q "$p" "$fn"; then + echo "modifying $fn..." >&2 + echo "if [ -e $p ]; then . $p; fi # added by Nix installer" >> "$fn" + fi + added=1 + break + fi + done fi if [ -z "$added" ]; then From 083bb3bbfcdccebd06bde81a66f158d51ed6e455 Mon Sep 17 00:00:00 2001 From: Daiderd Jordan Date: Thu, 13 Feb 2020 12:57:35 +0100 Subject: [PATCH 36/56] install: show macOS 10.15 message with --daemon --- scripts/create-darwin-volume.sh | 8 ++++---- scripts/install-nix-from-closure.sh | 28 ++++++++++++++++------------ 2 files changed, 20 insertions(+), 16 deletions(-) diff --git a/scripts/create-darwin-volume.sh b/scripts/create-darwin-volume.sh index f3e0d46f1..d6fb44f43 100755 --- a/scripts/create-darwin-volume.sh +++ b/scripts/create-darwin-volume.sh @@ -1,8 +1,8 @@ #!/bin/sh set -e -root_disks() { - diskutil list -plist / +root_disk() { + diskutil info -plist / } apfs_volumes_for() { @@ -11,7 +11,7 @@ apfs_volumes_for() { } disk_identifier() { - xpath "/plist/dict/key[text()='WholeDisks']/following-sibling::array[1]/string/text()" 2>/dev/null + xpath "/plist/dict/key[text()='ParentWholeDisk']/following-sibling::string[1]/text()" 2>/dev/null } volume_get() { @@ -81,7 +81,7 @@ main() { sudo mkdir /nix fi - disk=$(root_disks | disk_identifier) + disk=$(root_disk | disk_identifier) volume=$(find_nix_volume "$disk") if [ -z "$volume" ]; then echo "Creating a Nix Store volume..." >&2 diff --git a/scripts/install-nix-from-closure.sh b/scripts/install-nix-from-closure.sh index 88275d1f0..7d32bf92e 100644 --- a/scripts/install-nix-from-closure.sh +++ b/scripts/install-nix-from-closure.sh @@ -91,9 +91,22 @@ while [ $# -gt 0 ]; do shift done -if [ "$(uname -s)" = "Darwin" ] && [ "$CREATE_DARWIN_VOLUME" = 1 ]; then - printf '\e[1;31mCreating volume and mountpoint /nix.\e[0m\n' - "$self/create-darwin-volume.sh" +if [ "$(uname -s)" = "Darwin" ]; then + if [ "$CREATE_DARWIN_VOLUME" = 1 ]; then + printf '\e[1;31mCreating volume and mountpoint /nix.\e[0m\n' + "$self/create-darwin-volume.sh" + fi + + info=$(diskutil info -plist / | xpath "/plist/dict/key[text()='Writable']/following-sibling::true[1]" 2> /dev/null) + if ! [ -e $dest ] && [ -n "$info" ]; then + ( + echo "" + echo "Installing on macOS >=10.15 requires relocating the store to an apfs volume." + echo "Use --create-volume or run the preparation steps manually." + echo "See https://nixos.org/nix/manual/#sect-darwin-apfs-volume." + echo "" + ) >&2 + fi fi if [ "$INSTALL_MODE" = "daemon" ]; then @@ -113,15 +126,6 @@ if ! [ -e $dest ]; then echo "directory $dest does not exist; creating it by running '$cmd' using sudo" >&2 if ! sudo sh -c "$cmd"; then echo "$0: please manually run '$cmd' as root to create $dest" >&2 - if [ "$(uname -s)" = "Darwin" ]; then - ( - echo "" - echo "Installing on macOS >=10.15 requires relocating the store to an apfs volume." - echo "Use --create-volume or run the preparation steps manually." - echo "See https://nixos.org/nix/manual/#sect-darwin-apfs-volume." - echo "" - ) >&2 - fi exit 1 fi fi From ee89b7797d4ec1db6dad9df5fb3bb8cc2f05de12 Mon Sep 17 00:00:00 2001 From: Daiderd Jordan Date: Fri, 17 Jan 2020 23:27:29 +0100 Subject: [PATCH 37/56] manual: add apfs volume section --- doc/manual/installation/installing-binary.xml | 103 ++++++++++++++++++ 1 file changed, 103 insertions(+) diff --git a/doc/manual/installation/installing-binary.xml b/doc/manual/installation/installing-binary.xml index 3f57f47b5..86cbce6bf 100644 --- a/doc/manual/installation/installing-binary.xml +++ b/doc/manual/installation/installing-binary.xml @@ -136,6 +136,109 @@ sudo rm /Library/LaunchDaemons/org.nixos.nix-daemon.plist +
+ APFS Volume Installation + + + The root filesystem is read-only as of macOS 10.15 Catalina, all writable + paths to a separate data volume. This means creating or writing to /nix + is not allowed. While changing the default prefix would be possible, it's + a very intrusive change that has side effects we want to avoid for now. + + + + For common writable locations firmlinks where introduced, + described by Apple as a "bi-directional wormhole" between two filesystems. + Essentially a bind mount for APFS volumes. However this is (currently) not + user configurable and only available for paths like /Users. + + + + For special cases like NFS mount points or package manager roots synthetic.conf(5) + provides a mechanism for some limited, user-controlled file-creation at /. + This only applies on a reboot, but apfs.util can be used + to trigger the creation (not deletion) of new entries. + + + +alice$ /System/Library/Filesystems/apfs.fs/Contents/Resources/apfs.util -B + + + + + + The simplest solution is creating a symlink with /etc/synthetic.conf + to the data volume. (not recommended) + + + +nix /System/Volumes/Data/nix + + + +alice$ ls -l / +lrwxr-xr-x 1 root wheel 25 Jan 1 2019 nix -> /System/Volumes/Data/nix + + + + However builds that detect or resolve this symlink will leak the canonical + location or even fail in certain cases, making this approach undesirable. + + + + + + An empty directory can also be created using /etc/synthetic.conf, + this won't be writable but can be used as a mount point. And with + APFS it's relatively easy to create an separate + volume for nix instead. + + + +nix + + + +alice$ sudo diskutil apfs addVolume diskX APFS 'Nix Store' -mountpoint /nix +alice$ mount +/dev/disk1s6 on /nix (apfs, local, journaled) + + + + This does make the installation more complicated, requiring both + /etc/synthetic.conf as well as /etc/fstab + + + +# +# Warning - this file should only be modified with vifs(8) +# +# Failure to do so is unsupported and may be destructive. +# +LABEL=Nix\040Store /nix apfs rw + + + + On macOS volumes are also mounted quite late, launchd services or other + things that start during login will start before our volume is mounted. + For these cases eg. wait4path must be used for + things that depend on /nix. + + + + This new volume also won't be encrypted by default, and enabling is + only possible interactively? + + + +diskutil apfs enableFileVault /nix -user disk + + + + + +
+
Installing a pinned Nix version from a URL From caface1980344347f2a50701ec133e98e6094c8c Mon Sep 17 00:00:00 2001 From: Daiderd Jordan Date: Thu, 5 Mar 2020 23:12:52 +0100 Subject: [PATCH 38/56] install: hide the store volume on darwin --- scripts/create-darwin-volume.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/create-darwin-volume.sh b/scripts/create-darwin-volume.sh index d6fb44f43..a1a67acea 100755 --- a/scripts/create-darwin-volume.sh +++ b/scripts/create-darwin-volume.sh @@ -94,7 +94,7 @@ main() { if ! test_fstab; then echo "Configuring /etc/fstab..." >&2 label=$(echo "$volume" | sed 's/ /\\040/g') - printf "\$a\nLABEL=%s /nix apfs rw\n.\nwq\n" "$label" | EDITOR=ed sudo vifs + printf "\$a\nLABEL=%s /nix apfs rw,nobrowse\n.\nwq\n" "$label" | EDITOR=ed sudo vifs fi echo "The following options can be enabled to disable spotlight indexing" >&2 From 04f597c3f4d0ac8b8a677c798642329a5a6768e3 Mon Sep 17 00:00:00 2001 From: Daiderd Jordan Date: Sat, 7 Mar 2020 12:01:40 +0100 Subject: [PATCH 39/56] install: improve output and error handling --- doc/manual/installation/installing-binary.xml | 3 ++- scripts/create-darwin-volume.sh | 17 ++++++++++++++--- scripts/install-nix-from-closure.sh | 7 ++++--- 3 files changed, 20 insertions(+), 7 deletions(-) diff --git a/doc/manual/installation/installing-binary.xml b/doc/manual/installation/installing-binary.xml index 86cbce6bf..7f4bba3a0 100644 --- a/doc/manual/installation/installing-binary.xml +++ b/doc/manual/installation/installing-binary.xml @@ -199,6 +199,7 @@ nix +alice$ /System/Library/Filesystems/apfs.fs/Contents/Resources/apfs.util -B alice$ sudo diskutil apfs addVolume diskX APFS 'Nix Store' -mountpoint /nix alice$ mount /dev/disk1s6 on /nix (apfs, local, journaled) @@ -231,7 +232,7 @@ LABEL=Nix\040Store /nix apfs rw -diskutil apfs enableFileVault /nix -user disk +alice$ diskutil apfs enableFileVault /nix -user disk diff --git a/scripts/create-darwin-volume.sh b/scripts/create-darwin-volume.sh index a1a67acea..e2b49c9a0 100755 --- a/scripts/create-darwin-volume.sh +++ b/scripts/create-darwin-volume.sh @@ -73,12 +73,22 @@ main() { if ! test_synthetic_conf; then echo "Configuring /etc/synthetic.conf..." >&2 echo nix | sudo tee /etc/synthetic.conf - /System/Library/Filesystems/apfs.fs/Contents/Resources/apfs.util -B + if ! test_synthetic_conf; then + echo "error: failed to configure synthetic.conf" >&2 + exit 1 + fi fi if ! test_nix; then echo "Creating mountpoint for /nix..." >&2 - sudo mkdir /nix + /System/Library/Filesystems/apfs.fs/Contents/Resources/apfs.util -B || true + if ! test_nix; then + sudo mkdir -p /nix 2>/dev/null || true + fi + if ! test_nix; then + echo "error: failed to bootstrap /nix, a reboot might be required" >&2 + exit 1 + fi fi disk=$(root_disk | disk_identifier) @@ -97,10 +107,11 @@ main() { printf "\$a\nLABEL=%s /nix apfs rw,nobrowse\n.\nwq\n" "$label" | EDITOR=ed sudo vifs fi + echo "" >&2 echo "The following options can be enabled to disable spotlight indexing" >&2 echo "of the volume, which might be desirable." >&2 echo "" >&2 - echo " $ mdutil -i off /nix" >&2 + echo " $ sudo mdutil -i off /nix" >&2 echo "" >&2 } diff --git a/scripts/install-nix-from-closure.sh b/scripts/install-nix-from-closure.sh index 7d32bf92e..2f291ed4c 100644 --- a/scripts/install-nix-from-closure.sh +++ b/scripts/install-nix-from-closure.sh @@ -82,7 +82,7 @@ while [ $# -gt 0 ]; do echo " --create-volume: Create an APFS volume for the store and create the /nix" echo " mountpoint for it using synthetic.conf." echo " (required on macOS >=10.15)" - echo " See https://nixos.org/nix/manual/#sect-darwin-apfs-volume" + echo " See https://nixos.org/nix/manual/#sect-apfs-volume-installation" echo "" ) >&2 fi @@ -102,10 +102,11 @@ if [ "$(uname -s)" = "Darwin" ]; then ( echo "" echo "Installing on macOS >=10.15 requires relocating the store to an apfs volume." - echo "Use --create-volume or run the preparation steps manually." - echo "See https://nixos.org/nix/manual/#sect-darwin-apfs-volume." + echo "Use sh <(curl https://nixos.org/nix/install) --create-volume or run the preparation steps manually." + echo "See https://nixos.org/nix/manual/#sect-apfs-volume-installation" echo "" ) >&2 + exit 1 fi fi From bc24c09968bb35fce599151f86d123cb5984f727 Mon Sep 17 00:00:00 2001 From: Daiderd Jordan Date: Wed, 18 Mar 2020 20:34:46 +0100 Subject: [PATCH 40/56] install: make synthetic.conf and fstab checks stricter --- scripts/create-darwin-volume.sh | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/scripts/create-darwin-volume.sh b/scripts/create-darwin-volume.sh index e2b49c9a0..ea4133444 100755 --- a/scripts/create-darwin-volume.sh +++ b/scripts/create-darwin-volume.sh @@ -39,11 +39,15 @@ find_nix_volume() { } test_fstab() { - grep -q "/nix" /etc/fstab 2>/dev/null + grep -q "/nix apfs rw" /etc/fstab 2>/dev/null +} + +test_nix_symlink() { + [ -L "/nix" ] || grep -q "^nix." /etc/synthetic.conf 2>/dev/null } test_synthetic_conf() { - grep -q "^nix" /etc/synthetic.conf 2>/dev/null + grep -q "^nix$" /etc/synthetic.conf 2>/dev/null } test_nix() { @@ -60,12 +64,12 @@ main() { echo "" echo " 1. Remove the entry from fstab using 'sudo vifs'" echo " 2. Destroy the data volume using 'diskutil apfs deleteVolume'" - echo " 3. Delete /etc/synthetic.conf" + echo " 3. Remove the 'nix' line from /etc/synthetic.conf or the file" echo "" ) >&2 - if [ -L "/nix" ]; then - echo "error: /nix is a symlink, please remove it or edit synthetic.conf (requires reboot)" >&2 + if test_nix_symlink; then + echo "error: /nix is a symlink, please remove it and make sure it's not in synthetic.conf (in which case a reboot is required)" >&2 echo " /nix -> $(readlink "/nix")" >&2 exit 2 fi From 33865752960c9a2ff28eb9024f20d2103918685c Mon Sep 17 00:00:00 2001 From: Daiderd Jordan Date: Thu, 26 Mar 2020 20:14:08 +0100 Subject: [PATCH 41/56] manual: clarify volume creation section --- doc/manual/installation/installing-binary.xml | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/doc/manual/installation/installing-binary.xml b/doc/manual/installation/installing-binary.xml index 7f4bba3a0..c11ba9cce 100644 --- a/doc/manual/installation/installing-binary.xml +++ b/doc/manual/installation/installing-binary.xml @@ -141,13 +141,14 @@ sudo rm /Library/LaunchDaemons/org.nixos.nix-daemon.plist The root filesystem is read-only as of macOS 10.15 Catalina, all writable - paths to a separate data volume. This means creating or writing to /nix - is not allowed. While changing the default prefix would be possible, it's - a very intrusive change that has side effects we want to avoid for now. + paths were moved to a separate data volume. This means creating or writing + to /nix is not allowed. While changing the default prefix + would be possible, it's a very intrusive change that has side effects we want to + avoid for now. - For common writable locations firmlinks where introduced, + For common writable locations firmlinks were introduced, described by Apple as a "bi-directional wormhole" between two filesystems. Essentially a bind mount for APFS volumes. However this is (currently) not user configurable and only available for paths like /Users. @@ -156,8 +157,10 @@ sudo rm /Library/LaunchDaemons/org.nixos.nix-daemon.plist For special cases like NFS mount points or package manager roots synthetic.conf(5) provides a mechanism for some limited, user-controlled file-creation at /. - This only applies on a reboot, but apfs.util can be used - to trigger the creation (not deletion) of new entries. + This only applies at boot time, however apfs.util can be used + to trigger the creation (not deletion) of new entries without a reboot. + It would be ideal if this could create firmlinks, however a symlink or mountpoint + are the only options. From 477d7c2d07e146c91950401b8b9d9380ce6787e5 Mon Sep 17 00:00:00 2001 From: Daiderd Jordan Date: Thu, 26 Mar 2020 21:51:13 +0100 Subject: [PATCH 42/56] installer: refuse apfs volume creation when FileVault is enabled --- doc/manual/installation/installing-binary.xml | 6 +++-- scripts/create-darwin-volume.sh | 22 +++++++++++++++++-- 2 files changed, 24 insertions(+), 4 deletions(-) diff --git a/doc/manual/installation/installing-binary.xml b/doc/manual/installation/installing-binary.xml index c11ba9cce..498248662 100644 --- a/doc/manual/installation/installing-binary.xml +++ b/doc/manual/installation/installing-binary.xml @@ -230,8 +230,10 @@ LABEL=Nix\040Store /nix apfs rw - This new volume also won't be encrypted by default, and enabling is - only possible interactively? + This new volume also won't be encrypted by default, and enabling it + requires extra setup. For machines with a T2 chip + all data is already entrypted at rest, older hardware won't even when + FileVault is enabled for the rest of the system. diff --git a/scripts/create-darwin-volume.sh b/scripts/create-darwin-volume.sh index ea4133444..a0da85f43 100755 --- a/scripts/create-darwin-volume.sh +++ b/scripts/create-darwin-volume.sh @@ -14,7 +14,12 @@ disk_identifier() { xpath "/plist/dict/key[text()='ParentWholeDisk']/following-sibling::string[1]/text()" 2>/dev/null } -volume_get() { +volume_list_true() { + key=$1 t=$2 + xpath "/plist/dict/array/dict/key[text()='Volumes']/following-sibling::array/dict/key[text()='$key']/following-sibling::true[1]" 2> /dev/null +} + +volume_get_string() { key=$1 i=$2 xpath "/plist/dict/array/dict/key[text()='Volumes']/following-sibling::array/dict[$i]/key[text()='$key']/following-sibling::string[1]/text()" 2> /dev/null } @@ -24,7 +29,7 @@ find_nix_volume() { i=1 volumes=$(apfs_volumes_for "$disk") while true; do - name=$(echo "$volumes" | volume_get "Name" "$i") + name=$(echo "$volumes" | volume_get_string "Name" "$i") if [ -z "$name" ]; then break fi @@ -54,6 +59,12 @@ test_nix() { test -d "/nix" } +test_filevault() { + disk=$1 + apfs_volumes_for "$disk" | volume_list_true FileVault | grep -q true || return + ! sudo xartutil --list >/dev/null 2>/dev/null +} + main() { ( echo "" @@ -99,6 +110,13 @@ main() { volume=$(find_nix_volume "$disk") if [ -z "$volume" ]; then echo "Creating a Nix Store volume..." >&2 + + if test_filevault "$disk"; then + echo "error: FileVault detected, refusing to create unencrypted volume" >&2 + echo "See https://nixos.org/nix/manual/#sect-apfs-volume-installation" >&2 + exit 1 + fi + sudo diskutil apfs addVolume "$disk" APFS 'Nix Store' -mountpoint /nix volume="Nix Store" else From 2b0a81d92d28994374465c44c79f020d5e044700 Mon Sep 17 00:00:00 2001 From: "Travis A. Everett" Date: Thu, 14 May 2020 21:59:10 -0500 Subject: [PATCH 43/56] focus on golden-path covering most scenarios This should handle installation scenarios we can handle with anything resembling confidence. Goal is approximating the existing setup--not enforcing a best-practice... Approaches (+ installer-handled, - manual) and configs each covers: + no change needed; /nix OK on boot volume: All pre-Catalina (regardless of T2 or FileVault use) + create new unencrypted volume: Catalina, pre-T2, no FileVault + create new encrypted-at-rest volume: Catalina, pre-T2, FileVault Catalina, T2, no FileVault - require user to pre-create encrypted volume Catalina, T2, FileVault --- doc/manual/installation/installing-binary.xml | 350 +++++++++++++----- scripts/create-darwin-volume.sh | 77 +++- scripts/install-nix-from-closure.sh | 19 +- 3 files changed, 331 insertions(+), 115 deletions(-) diff --git a/doc/manual/installation/installing-binary.xml b/doc/manual/installation/installing-binary.xml index 498248662..8d548f0ea 100644 --- a/doc/manual/installation/installing-binary.xml +++ b/doc/manual/installation/installing-binary.xml @@ -6,16 +6,30 @@ Installing a Binary Distribution -If you are using Linux or macOS, the easiest way to install Nix -is to run the following command: + + If you are using Linux or macOS versions up to 10.14 (Mojave), the + easiest way to install Nix is to run the following command: + $ sh <(curl https://nixos.org/nix/install) -As of Nix 2.1.0, the Nix installer will always default to creating a -single-user installation, however opting in to the multi-user -installation is highly recommended. + + If you're using macOS 10.15 (Catalina) or newer, consult + the macOS installation instructions + before installing. + + + + As of Nix 2.1.0, the Nix installer will always default to creating a + single-user installation, however opting in to the multi-user + installation is highly recommended. +
@@ -36,7 +50,7 @@ run this under your usual user account, not as root. The script will invoke sudo to create /nix if it doesn’t already exist. If you don’t have sudo, you should manually create -/nix first as root, e.g.: +/nix first as root, e.g.: $ mkdir /nix @@ -47,7 +61,7 @@ The install script will modify the first writable file from amongst .bash_profile, .bash_login and .profile to source ~/.nix-profile/etc/profile.d/nix.sh. You can set -the NIX_INSTALLER_NO_MODIFY_PROFILE environment +the NIX_INSTALLER_NO_MODIFY_PROFILE environment variable before executing the install script to disable this behaviour. @@ -81,12 +95,10 @@ $ rm -rf /nix You can instruct the installer to perform a multi-user installation on your system: - - - sh <(curl https://nixos.org/nix/install) --daemon - + sh <(curl https://nixos.org/nix/install) --daemon + The multi-user installation of Nix will create build users between the user IDs 30001 and 30032, and a group with the group ID 30000. @@ -136,82 +148,253 @@ sudo rm /Library/LaunchDaemons/org.nixos.nix-daemon.plist
-
- APFS Volume Installation +
+ macOS Installation - The root filesystem is read-only as of macOS 10.15 Catalina, all writable - paths were moved to a separate data volume. This means creating or writing - to /nix is not allowed. While changing the default prefix - would be possible, it's a very intrusive change that has side effects we want to - avoid for now. + Starting with macOS 10.15 (Catalina), the root filesystem is read-only. + This means /nix can no longer live on your system + volume, and that you'll need a workaround to install Nix. - For common writable locations firmlinks were introduced, - described by Apple as a "bi-directional wormhole" between two filesystems. - Essentially a bind mount for APFS volumes. However this is (currently) not - user configurable and only available for paths like /Users. + The recommended approach, which creates an unencrypted APFS volume + for your Nix store and a "synthetic" empty directory to mount it + over at /nix, is least likely to impair Nix + or your system. + + With all separate-volume approaches, it's possible something on + your system (particularly daemons/services and restored apps) may + need access to your Nix store before the volume is mounted. Adding + additional encryption makes this more likely. + + - For special cases like NFS mount points or package manager roots synthetic.conf(5) - provides a mechanism for some limited, user-controlled file-creation at /. - This only applies at boot time, however apfs.util can be used - to trigger the creation (not deletion) of new entries without a reboot. - It would be ideal if this could create firmlinks, however a symlink or mountpoint - are the only options. + If you're using a recent Mac with a + T2 chip, + your drive will still be encrypted at rest (in which case "unencrypted" + is a bit of a misnomer). To use this approach, just install Nix with: - -alice$ /System/Library/Filesystems/apfs.fs/Contents/Resources/apfs.util -B - + $ sh <(curl https://nixos.org/nix/install) --darwin-use-unencrypted-nix-store-volume - - - - The simplest solution is creating a symlink with /etc/synthetic.conf - to the data volume. (not recommended) - + + If you don't like the sound of this, you'll want to weigh the + other approaches and tradeoffs detailed in this section. + - -nix /System/Volumes/Data/nix - + + Eventual solutions? + + All of the known workarounds have drawbacks, but we hope + better solutions will be available in the future. Some that + we have our eye on are: + + + + + A true firmlink would enable the Nix store to live on the + primary data volume without the build problems caused by + the symlink approach. End users cannot currently + create true firmlinks. + + + + + If the Nix store volume shared FileVault encryption + with the primary data volume (probably by using the same + volume group and role), FileVault encryption could be + easily supported by the installer without requiring + manual setup by each user. + + + + - -alice$ ls -l / -lrwxr-xr-x 1 root wheel 25 Jan 1 2019 nix -> /System/Volumes/Data/nix - +
+ Change the Nix store path prefix + + Changing the default prefix for the Nix store is a simple + approach which enables you to leave it on your root volume, + where it can take full advantage of FileVault encryption if + enabled. Unfortunately, this approach also opts your device out + of some benefits that are enabled by using the same prefix + across systems: - - However builds that detect or resolve this symlink will leak the canonical - location or even fail in certain cases, making this approach undesirable. - - + + + + Your system won't be able to take advantage of the binary + cache (unless someone is able to stand up and support + duplicate caching infrastructure), which means you'll + spend more time waiting for builds. + + + + + It's harder to build and deploy packages to Linux systems. + + + + - - - An empty directory can also be created using /etc/synthetic.conf, - this won't be writable but can be used as a mount point. And with - APFS it's relatively easy to create an separate - volume for nix instead. - + - -nix - + It would also possible (and often requested) to just apply this + change ecosystem-wide, but it's an intrusive process that has + side effects we want to avoid for now. + + + + +
- -alice$ /System/Library/Filesystems/apfs.fs/Contents/Resources/apfs.util -B -alice$ sudo diskutil apfs addVolume diskX APFS 'Nix Store' -mountpoint /nix -alice$ mount -/dev/disk1s6 on /nix (apfs, local, journaled) - +
+ Use a separate encrypted volume + + If you like, you can also add encryption to the recommended + approach taken by the installer. You can do this by pre-creating + an encrypted volume before you run the installer--or you can + run the installer and encrypt the volume it creates later. + + + + In either case, adding encryption to a second volume isn't quite + as simple as enabling FileVault for your boot volume. Before you + dive in, there are a few things to weigh: + + + + + The additional volume won't be encrypted with your existing + FileVault key, so you'll need another mechanism to decrypt + the volume. + + + + + You can store the password in Keychain to automatically + decrypt the volume on boot--but it'll have to wait on Keychain + and may not mount before your GUI apps restore. If any of + your launchd agents or apps depend on Nix-installed software + (for example, if you use a Nix-installed login shell), the + restore may fail or break. + + + On a case-by-case basis, you may be able to work around this + problem by using wait4path to block + execution until your executable is available. + + + It's also possible to decrypt and mount the volume earlier + with a login hook--but this mechanism appears to be + deprecated and its future is unclear. + + + + + You can hard-code the password in the clear, so that your + store volume can be decrypted before Keychain is available. + + + + + If you are comfortable navigating these tradeoffs, you can encrypt the volume with + something along the lines of: + + + + alice$ diskutil apfs enableFileVault /nix -user disk + + +
+ +
+ + Symlink the Nix store to a custom location + + Another simple approach is using /etc/synthetic.conf + to symlink the Nix store to the data volume. This option also + enables your store to share any configured FileVault encryption. + Unfortunately, builds that resolve the symlink may leak the + canonical path or even fail. + + + Because of these downsides, we can't recommend this approach. + + +
+ +
+ Notes on the recommended approach + + This section goes into a little more detail on the recommended + approach. You don't need to understand it to run the installer, + but it can serve as a helpful reference if you run into trouble. + + + + + In order to compose user-writable locations into the new + read-only system root, Apple introduced a new concept called + firmlinks, which it describes as a + "bi-directional wormhole" between two filesystems. You can + see the current firmlinks in /usr/share/firmlinks. + Unfortunately, firmlinks aren't (currently?) user-configurable. + + + + For special cases like NFS mount points or package manager roots, + synthetic.conf(5) + supports limited user-controlled file-creation (of symlinks, + and synthetic empty directories) at /. + To create a synthetic empty directory for mounting at /nix, + add the following line to /etc/synthetic.conf + (create it if necessary): + + + nix + + + + + This configuration is applied at boot time, but you can use + apfs.util to trigger creation (not deletion) + of new entries without a reboot: + + + alice$ /System/Library/Filesystems/apfs.fs/Contents/Resources/apfs.util -B + + + + + Create the new APFS volume with diskutil: + + + alice$ sudo diskutil apfs addVolume diskX APFS 'Nix Store' -mountpoint /nix + + + + + Using vifs, add the new mount to + /etc/fstab. If it doesn't already have + other entries, it should look something like: + # @@ -219,29 +402,16 @@ alice$ mount # # Failure to do so is unsupported and may be destructive. # -LABEL=Nix\040Store /nix apfs rw +LABEL=Nix\040Store /nix apfs rw,nobrowse - - On macOS volumes are also mounted quite late, launchd services or other - things that start during login will start before our volume is mounted. - For these cases eg. wait4path must be used for - things that depend on /nix. - - - - This new volume also won't be encrypted by default, and enabling it - requires extra setup. For machines with a T2 chip - all data is already entrypted at rest, older hardware won't even when - FileVault is enabled for the rest of the system. - - - -alice$ diskutil apfs enableFileVault /nix -user disk - - - - + + The nobrowse setting will keep Spotlight from indexing this + volume, and keep it from showing up on your desktop. + + + +
diff --git a/scripts/create-darwin-volume.sh b/scripts/create-darwin-volume.sh index a0da85f43..47cc3e913 100755 --- a/scripts/create-darwin-volume.sh +++ b/scripts/create-darwin-volume.sh @@ -59,10 +59,45 @@ test_nix() { test -d "/nix" } -test_filevault() { +test_t2_chip_present(){ + # Use xartutil to see if system has a t2 chip. + # + # This isn't well-documented on its own; until it is, + # let's keep track of knowledge/assumptions. + # + # Warnings: + # - Don't search "xart" if porn will cause you trouble :) + # - Other xartutil flags do dangerous things. Don't run them + # naively. If you must, search "xartutil" first. + # + # Assumptions: + # - the "xART session seeds recovery utility" + # appears to interact with xartstorageremoted + # - `sudo xartutil --list` lists xART sessions + # and their seeds and exits 0 if successful. If + # not, it exits 1 and prints an error such as: + # xartutil: ERROR: No supported link to the SEP present + # - xART sessions/seeds are present when a T2 chip is + # (and not, otherwise) + # - the presence of a T2 chip means a newly-created + # volume on the primary drive will be + # encrypted at rest + # - all together: `sudo xartutil --list` + # should exit 0 if a new Nix Store volume will + # be encrypted at rest, and exit 1 if not. + sudo xartutil --list >/dev/null 2>/dev/null +} + +test_filevault_in_use() { disk=$1 - apfs_volumes_for "$disk" | volume_list_true FileVault | grep -q true || return - ! sudo xartutil --list >/dev/null 2>/dev/null + # list vols on disk | get value of Filevault key | value is true + apfs_volumes_for "$disk" | volume_list_true FileVault | grep -q true +} + +# use after error msg for conditions we don't understand +suggest_report_error(){ + # ex "error: something sad happened :(" >&2 + echo " please report this @ https://github.com/nixos/nix/issues" >&2 } main() { @@ -89,7 +124,8 @@ main() { echo "Configuring /etc/synthetic.conf..." >&2 echo nix | sudo tee /etc/synthetic.conf if ! test_synthetic_conf; then - echo "error: failed to configure synthetic.conf" >&2 + echo "error: failed to configure synthetic.conf;" >&2 + suggest_report_error exit 1 fi fi @@ -101,7 +137,8 @@ main() { sudo mkdir -p /nix 2>/dev/null || true fi if ! test_nix; then - echo "error: failed to bootstrap /nix, a reboot might be required" >&2 + echo "error: failed to bootstrap /nix; if a reboot doesn't help," >&2 + suggest_report_error exit 1 fi fi @@ -111,10 +148,25 @@ main() { if [ -z "$volume" ]; then echo "Creating a Nix Store volume..." >&2 - if test_filevault "$disk"; then - echo "error: FileVault detected, refusing to create unencrypted volume" >&2 - echo "See https://nixos.org/nix/manual/#sect-apfs-volume-installation" >&2 - exit 1 + if test_filevault_in_use "$disk"; then + # TODO: Not sure if it's in-scope now, but `diskutil apfs list` + # shows both filevault and encrypted at rest status, and it + # may be the more semantic way to test for this? It'll show + # `FileVault: No (Encrypted at rest)` + # `FileVault: No` + # `FileVault: Yes (Unlocked)` + # and so on. + if test_t2_chip_present; then + echo "warning: boot volume is FileVault-encrypted, but the Nix store volume" >&2 + echo " is only encrypted at rest." >&2 + echo " See https://nixos.org/nix/manual/#sect-macos-installation" >&2 + else + echo "error: refusing to create Nix store volume because the boot volume is" >&2 + echo " FileVault encrypted, but encryption-at-rest is not available." >&2 + echo " Manually create a volume for the store and re-run this script." >&2 + echo " See https://nixos.org/nix/manual/#sect-macos-installation" >&2 + exit 1 + fi fi sudo diskutil apfs addVolume "$disk" APFS 'Nix Store' -mountpoint /nix @@ -128,13 +180,6 @@ main() { label=$(echo "$volume" | sed 's/ /\\040/g') printf "\$a\nLABEL=%s /nix apfs rw,nobrowse\n.\nwq\n" "$label" | EDITOR=ed sudo vifs fi - - echo "" >&2 - echo "The following options can be enabled to disable spotlight indexing" >&2 - echo "of the volume, which might be desirable." >&2 - echo "" >&2 - echo " $ sudo mdutil -i off /nix" >&2 - echo "" >&2 } main "$@" diff --git a/scripts/install-nix-from-closure.sh b/scripts/install-nix-from-closure.sh index 2f291ed4c..72aa5abf5 100644 --- a/scripts/install-nix-from-closure.sh +++ b/scripts/install-nix-from-closure.sh @@ -52,7 +52,7 @@ while [ $# -gt 0 ]; do NIX_INSTALLER_NO_CHANNEL_ADD=1;; --no-modify-profile) NIX_INSTALLER_NO_MODIFY_PROFILE=1;; - --create-volume) + --darwin-use-unencrypted-nix-store-volume) CREATE_DARWIN_VOLUME=1;; *) ( @@ -77,12 +77,13 @@ while [ $# -gt 0 ]; do echo "" ) >&2 - if [ "$(uname -s)" = "Darwin" ]; then + # darwin and Catalina+ + if [ "$(uname -s)" = "Darwin" ] && [ "$macos_major" -gt 14 ]; then ( - echo " --create-volume: Create an APFS volume for the store and create the /nix" - echo " mountpoint for it using synthetic.conf." - echo " (required on macOS >=10.15)" - echo " See https://nixos.org/nix/manual/#sect-apfs-volume-installation" + echo " --darwin-use-unencrypted-nix-store-volume: Create an APFS volume for the Nix" + echo " store and mount it at /nix. This is the recommended way to create" + echo " /nix with a read-only / on macOS >=10.15." + echo " See: https://nixos.org/nix/manual/#sect-macos-installation" echo "" ) >&2 fi @@ -98,12 +99,12 @@ if [ "$(uname -s)" = "Darwin" ]; then fi info=$(diskutil info -plist / | xpath "/plist/dict/key[text()='Writable']/following-sibling::true[1]" 2> /dev/null) - if ! [ -e $dest ] && [ -n "$info" ]; then + if ! [ -e $dest ] && [ -n "$info" ] && [ "$macos_major" -gt 14 ]; then ( echo "" echo "Installing on macOS >=10.15 requires relocating the store to an apfs volume." - echo "Use sh <(curl https://nixos.org/nix/install) --create-volume or run the preparation steps manually." - echo "See https://nixos.org/nix/manual/#sect-apfs-volume-installation" + echo "Use sh <(curl https://nixos.org/nix/install) --darwin-use-unencrypted-nix-store-volume or run the preparation steps manually." + echo "See https://nixos.org/nix/manual/#sect-macos-installation" echo "" ) >&2 exit 1 From d3df1889a1f49b114a8d78270ea8a6d0523f5e35 Mon Sep 17 00:00:00 2001 From: Daiderd Jordan Date: Thu, 21 May 2020 20:03:09 +0200 Subject: [PATCH 44/56] installer: don't clobber synthetic.conf --- scripts/create-darwin-volume.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/create-darwin-volume.sh b/scripts/create-darwin-volume.sh index 47cc3e913..5214d1652 100755 --- a/scripts/create-darwin-volume.sh +++ b/scripts/create-darwin-volume.sh @@ -122,7 +122,7 @@ main() { if ! test_synthetic_conf; then echo "Configuring /etc/synthetic.conf..." >&2 - echo nix | sudo tee /etc/synthetic.conf + echo nix | sudo tee -a /etc/synthetic.conf if ! test_synthetic_conf; then echo "error: failed to configure synthetic.conf;" >&2 suggest_report_error From 909d8cb2934869c945ac1cc20dfb71df513042eb Mon Sep 17 00:00:00 2001 From: Suraj Barkale Date: Fri, 22 May 2020 11:05:25 +1000 Subject: [PATCH 45/56] Use /etc/zshenv instead of /etc/zshrc for profile As noted in https://github.com/NixOS/nix/issues/3456 the `/etc/zshenv` file provides a better place for sourcing the nix environment. --- scripts/install-multi-user.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/install-multi-user.sh b/scripts/install-multi-user.sh index a0f1deb98..838192733 100644 --- a/scripts/install-multi-user.sh +++ b/scripts/install-multi-user.sh @@ -28,7 +28,7 @@ readonly NIX_FIRST_BUILD_UID="30001" # default shell profile that comes with Nix doesn't support it. readonly NIX_ROOT="/nix" -readonly PROFILE_TARGETS=("/etc/bashrc" "/etc/profile.d/nix.sh" "/etc/zshrc") +readonly PROFILE_TARGETS=("/etc/bashrc" "/etc/profile.d/nix.sh" "/etc/zshenv") readonly PROFILE_BACKUP_SUFFIX=".backup-before-nix" readonly PROFILE_NIX_FILE="$NIX_ROOT/var/nix/profiles/default/etc/profile.d/nix-daemon.sh" From 2a7ea2eb6c54c82d5e858ea6ae9de929face5e55 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Domen=20Ko=C5=BEar?= Date: Sat, 23 May 2020 11:12:05 +0200 Subject: [PATCH 46/56] scripts/create-darwin-volume.sh: remove unused variable --- scripts/create-darwin-volume.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/create-darwin-volume.sh b/scripts/create-darwin-volume.sh index 5214d1652..dac30d72d 100755 --- a/scripts/create-darwin-volume.sh +++ b/scripts/create-darwin-volume.sh @@ -15,7 +15,7 @@ disk_identifier() { } volume_list_true() { - key=$1 t=$2 + key=$1 xpath "/plist/dict/array/dict/key[text()='Volumes']/following-sibling::array/dict/key[text()='$key']/following-sibling::true[1]" 2> /dev/null } From 6f6bdd63a0f6dae4ab91422645f548837029dee9 Mon Sep 17 00:00:00 2001 From: Daiderd Jordan Date: Sat, 23 May 2020 12:10:12 +0200 Subject: [PATCH 47/56] fix hydra build products Since the binary tarball was replaced none of the hydra builds include the manual. The dist phase isn't enabled by default the manual build products where not written. --- release.nix | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/release.nix b/release.nix index 2cc4bb4c0..2a320e1c3 100644 --- a/release.nix +++ b/release.nix @@ -115,17 +115,17 @@ let installFlags = "sysconfdir=$(out)/etc"; + postInstall = '' + mkdir -p $doc/nix-support + echo "doc manual $doc/share/doc/nix/manual" >> $doc/nix-support/hydra-build-products + ''; + doCheck = true; doInstallCheck = true; installCheckFlags = "sysconfdir=$(out)/etc"; separateDebugInfo = true; - - preDist = '' - mkdir -p $doc/nix-support - echo "doc manual $doc/share/doc/nix/manual" >> $doc/nix-support/hydra-build-products - ''; }); From e2af11ce071fc109ad3e517bf15a87024d0a9ae5 Mon Sep 17 00:00:00 2001 From: Joachim Breitner Date: Sat, 23 May 2020 15:26:59 +0200 Subject: [PATCH 48/56] Manpages: Do not refer to nixpkgs-channels Unless I am misinformed, using the `nixpkgs` repository directly is now preferred? --- doc/manual/command-ref/env-common.xml | 2 +- doc/manual/command-ref/nix-env.xml | 7 ++----- doc/manual/command-ref/nix-shell.xml | 6 +++--- doc/manual/expressions/builtins.xml | 4 ++-- 4 files changed, 8 insertions(+), 11 deletions(-) diff --git a/doc/manual/command-ref/env-common.xml b/doc/manual/command-ref/env-common.xml index 0217de7b2..8466cc463 100644 --- a/doc/manual/command-ref/env-common.xml +++ b/doc/manual/command-ref/env-common.xml @@ -53,7 +53,7 @@ nixpkgs=/home/eelco/Dev/nixpkgs-branch:/etc/nixos NIX_PATH to -nixpkgs=https://github.com/NixOS/nixpkgs-channels/archive/nixos-15.09.tar.gz +nixpkgs=https://github.com/NixOS/nixpkgs/archive/nixos-15.09.tar.gz tells Nix to download the latest revision in the Nixpkgs/NixOS 15.09 channel. diff --git a/doc/manual/command-ref/nix-env.xml b/doc/manual/command-ref/nix-env.xml index 9c03ccce1..2b95b6819 100644 --- a/doc/manual/command-ref/nix-env.xml +++ b/doc/manual/command-ref/nix-env.xml @@ -526,13 +526,10 @@ these paths will be fetched (0.04 MiB download, 0.19 MiB unpacked): 14.12 channel: -$ nix-env -f https://github.com/NixOS/nixpkgs-channels/archive/nixos-14.12.tar.gz -iA firefox +$ nix-env -f https://github.com/NixOS/nixpkgs/archive/nixos-14.12.tar.gz -iA firefox -(The GitHub repository nixpkgs-channels is updated -automatically from the main nixpkgs repository -after certain tests have succeeded and binaries have been built and -uploaded to the binary cache at cache.nixos.org.) + diff --git a/doc/manual/command-ref/nix-shell.xml b/doc/manual/command-ref/nix-shell.xml index 766482460..2fef323c5 100644 --- a/doc/manual/command-ref/nix-shell.xml +++ b/doc/manual/command-ref/nix-shell.xml @@ -258,7 +258,7 @@ path. You can override it by passing or setting containing the Pan package from a specific revision of Nixpkgs: -$ nix-shell -p pan -I nixpkgs=https://github.com/NixOS/nixpkgs-channels/archive/8a3eea054838b55aca962c3fbde9c83c102b8bf2.tar.gz +$ nix-shell -p pan -I nixpkgs=https://github.com/NixOS/nixpkgs/archive/8a3eea054838b55aca962c3fbde9c83c102b8bf2.tar.gz [nix-shell:~]$ pan --version Pan 0.139 @@ -352,7 +352,7 @@ following Haskell script uses a specific branch of Nixpkgs/NixOS (the -#! nix-shell -I nixpkgs=https://github.com/NixOS/nixpkgs-channels/archive/0672315759b3e15e2121365f067c1c8c56bb4722.tar.gz +#! nix-shell -I nixpkgs=https://github.com/NixOS/nixpkgs/archive/0672315759b3e15e2121365f067c1c8c56bb4722.tar.gz diff --git a/doc/manual/expressions/builtins.xml b/doc/manual/expressions/builtins.xml index 6709c4081..a18c5801a 100644 --- a/doc/manual/expressions/builtins.xml +++ b/doc/manual/expressions/builtins.xml @@ -324,7 +324,7 @@ if builtins ? getEnv then builtins.getEnv "PATH" else "" particular version of Nixpkgs, e.g. -with import (fetchTarball https://github.com/NixOS/nixpkgs-channels/archive/nixos-14.12.tar.gz) {}; +with import (fetchTarball https://github.com/NixOS/nixpkgs/archive/nixos-14.12.tar.gz) {}; stdenv.mkDerivation { … } @@ -349,7 +349,7 @@ stdenv.mkDerivation { … } with import (fetchTarball { - url = "https://github.com/NixOS/nixpkgs-channels/archive/nixos-14.12.tar.gz"; + url = "https://github.com/NixOS/nixpkgs/archive/nixos-14.12.tar.gz"; sha256 = "1jppksrfvbk5ypiqdz4cddxdl8z6zyzdb2srq8fcffr327ld5jj2"; }) {}; From ecc5c90dfc0e9877aef89bf0192203cd1a8f2b21 Mon Sep 17 00:00:00 2001 From: Tobias Pflug Date: Mon, 25 May 2020 11:50:41 +0200 Subject: [PATCH 49/56] Add unit tests for hashing functions --- src/libutil/tests/hash.cc | 80 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 80 insertions(+) create mode 100644 src/libutil/tests/hash.cc diff --git a/src/libutil/tests/hash.cc b/src/libutil/tests/hash.cc new file mode 100644 index 000000000..7cb439817 --- /dev/null +++ b/src/libutil/tests/hash.cc @@ -0,0 +1,80 @@ +#include "hash.hh" +#include + +namespace nix { + + /* ---------------------------------------------------------------------------- + * hashString + * --------------------------------------------------------------------------*/ + + TEST(hashString, testKnownMD5Hashes1) { + // values taken from: https://tools.ietf.org/html/rfc1321 + auto s1 = ""; + auto hash = hashString(HashType::htMD5, s1); + ASSERT_EQ(hash.to_string(Base::Base16), "md5:d41d8cd98f00b204e9800998ecf8427e"); + } + + TEST(hashString, testKnownMD5Hashes2) { + // values taken from: https://tools.ietf.org/html/rfc1321 + auto s2 = "abc"; + auto hash = hashString(HashType::htMD5, s2); + ASSERT_EQ(hash.to_string(Base::Base16), "md5:900150983cd24fb0d6963f7d28e17f72"); + } + + TEST(hashString, testKnownSHA1Hashes1) { + // values taken from: https://tools.ietf.org/html/rfc3174 + auto s = "abc"; + auto hash = hashString(HashType::htSHA1, s); + ASSERT_EQ(hash.to_string(Base::Base16),"sha1:a9993e364706816aba3e25717850c26c9cd0d89d"); + } + + TEST(hashString, testKnownSHA1Hashes2) { + // values taken from: https://tools.ietf.org/html/rfc3174 + auto s = "abcdbcdecdefdefgefghfghighijhijkijkljklmklmnlmnomnopnopq"; + auto hash = hashString(HashType::htSHA1, s); + ASSERT_EQ(hash.to_string(Base::Base16),"sha1:84983e441c3bd26ebaae4aa1f95129e5e54670f1"); + } + + TEST(hashString, testKnownSHA256Hashes1) { + // values taken from: https://tools.ietf.org/html/rfc4634 + auto s = "abc"; + + auto hash = hashString(HashType::htSHA256, s); + ASSERT_EQ(hash.to_string(Base::Base16), + "sha256:ba7816bf8f01cfea414140de5dae2223b00361a396177a9cb410ff61f20015ad"); + } + + TEST(hashString, testKnownSHA256Hashes2) { + // values taken from: https://tools.ietf.org/html/rfc4634 + auto s = "abcdbcdecdefdefgefghfghighijhijkijkljklmklmnlmnomnopnopq"; + auto hash = hashString(HashType::htSHA256, s); + ASSERT_EQ(hash.to_string(Base::Base16), + "sha256:248d6a61d20638b8e5c026930c3e6039a33ce45964ff2167f6ecedd419db06c1"); + } + + TEST(hashString, testKnownSHA512Hashes1) { + // values taken from: https://tools.ietf.org/html/rfc4634 + auto s = "abc"; + auto hash = hashString(HashType::htSHA512, s); + ASSERT_EQ(hash.to_string(Base::Base16), + "sha512:ddaf35a193617abacc417349ae20413112e6fa4e89a9" + "7ea20a9eeee64b55d39a2192992a274fc1a836ba3c23a3feebbd" + "454d4423643ce80e2a9ac94fa54ca49f"); + } + + TEST(hashString, testKnownSHA512Hashes2) { + // values taken from: https://tools.ietf.org/html/rfc4634 + auto s = "abcdefghbcdefghicdefghijdefghijkefghijklfghijklmghijklmnhijklmnoijklmnopjklmnopqklmnopqrlmnopqrsmnopqrstnopqrstu"; + + auto hash = hashString(HashType::htSHA512, s); + ASSERT_EQ(hash.to_string(Base::Base16), + "sha512:8e959b75dae313da8cf4f72814fc143f8f7779c6eb9f7fa1" + "7299aeadb6889018501d289e4900f7e4331b99dec4b5433a" + "c7d329eeb6dd26545e96e55b874be909"); + } + + TEST(hashString, hashingWithUnknownAlgoExits) { + auto s = "unknown"; + ASSERT_DEATH(hashString(HashType::htUnknown, s), ""); + } +} From c284700867ce09abee1f891a402271a42d207b4d Mon Sep 17 00:00:00 2001 From: Tobias Pflug Date: Mon, 25 May 2020 11:57:45 +0200 Subject: [PATCH 50/56] Add unit tests for "json.hh" --- src/libutil/tests/json.cc | 193 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 193 insertions(+) create mode 100644 src/libutil/tests/json.cc diff --git a/src/libutil/tests/json.cc b/src/libutil/tests/json.cc new file mode 100644 index 000000000..dea73f53a --- /dev/null +++ b/src/libutil/tests/json.cc @@ -0,0 +1,193 @@ +#include "json.hh" +#include +#include + +namespace nix { + + /* ---------------------------------------------------------------------------- + * toJSON + * --------------------------------------------------------------------------*/ + + TEST(toJSON, quotesCharPtr) { + const char* input = "test"; + std::stringstream out; + toJSON(out, input); + + ASSERT_EQ(out.str(), "\"test\""); + } + + TEST(toJSON, quotesStdString) { + std::string input = "test"; + std::stringstream out; + toJSON(out, input); + + ASSERT_EQ(out.str(), "\"test\""); + } + + TEST(toJSON, convertsNullptrtoNull) { + auto input = nullptr; + std::stringstream out; + toJSON(out, input); + + ASSERT_EQ(out.str(), "null"); + } + + TEST(toJSON, convertsNullToNull) { + const char* input = 0; + std::stringstream out; + toJSON(out, input); + + ASSERT_EQ(out.str(), "null"); + } + + + TEST(toJSON, convertsFloat) { + auto input = 1.024f; + std::stringstream out; + toJSON(out, input); + + ASSERT_EQ(out.str(), "1.024"); + } + + TEST(toJSON, convertsDouble) { + const double input = 1.024; + std::stringstream out; + toJSON(out, input); + + ASSERT_EQ(out.str(), "1.024"); + } + + TEST(toJSON, convertsBool) { + auto input = false; + std::stringstream out; + toJSON(out, input); + + ASSERT_EQ(out.str(), "false"); + } + + TEST(toJSON, quotesTab) { + std::stringstream out; + toJSON(out, "\t"); + + ASSERT_EQ(out.str(), "\"\\t\""); + } + + TEST(toJSON, quotesNewline) { + std::stringstream out; + toJSON(out, "\n"); + + ASSERT_EQ(out.str(), "\"\\n\""); + } + + TEST(toJSON, quotesCreturn) { + std::stringstream out; + toJSON(out, "\r"); + + ASSERT_EQ(out.str(), "\"\\r\""); + } + + TEST(toJSON, quotesCreturnNewLine) { + std::stringstream out; + toJSON(out, "\r\n"); + + ASSERT_EQ(out.str(), "\"\\r\\n\""); + } + + TEST(toJSON, quotesDoublequotes) { + std::stringstream out; + toJSON(out, "\""); + + ASSERT_EQ(out.str(), "\"\\\"\""); + } + + TEST(toJSON, substringEscape) { + std::stringstream out; + const char *s = "foo\t"; + toJSON(out, s+3, s + strlen(s)); + + ASSERT_EQ(out.str(), "\"\\t\""); + } + + /* ---------------------------------------------------------------------------- + * JSONObject + * --------------------------------------------------------------------------*/ + + TEST(JSONObject, emptyObject) { + std::stringstream out; + { + JSONObject t(out); + } + ASSERT_EQ(out.str(), "{}"); + } + + TEST(JSONObject, objectWithList) { + std::stringstream out; + { + JSONObject t(out); + auto l = t.list("list"); + l.elem("element"); + } + ASSERT_EQ(out.str(), R"#({"list":["element"]})#"); + } + + TEST(JSONObject, objectWithListIndent) { + std::stringstream out; + { + JSONObject t(out, true); + auto l = t.list("list"); + l.elem("element"); + } + ASSERT_EQ(out.str(), +R"#({ + "list": [ + "element" + ] +})#"); + } + + TEST(JSONObject, objectWithPlaceholderAndList) { + std::stringstream out; + { + JSONObject t(out); + auto l = t.placeholder("list"); + l.list().elem("element"); + } + + ASSERT_EQ(out.str(), R"#({"list":["element"]})#"); + } + + TEST(JSONObject, objectWithPlaceholderAndObject) { + std::stringstream out; + { + JSONObject t(out); + auto l = t.placeholder("object"); + l.object().attr("key", "value"); + } + + ASSERT_EQ(out.str(), R"#({"object":{"key":"value"}})#"); + } + + /* ---------------------------------------------------------------------------- + * JSONList + * --------------------------------------------------------------------------*/ + + TEST(JSONList, empty) { + std::stringstream out; + { + JSONList l(out); + } + ASSERT_EQ(out.str(), R"#([])#"); + } + + TEST(JSONList, withElements) { + std::stringstream out; + { + JSONList l(out); + l.elem("one"); + l.object(); + l.placeholder().write("three"); + } + ASSERT_EQ(out.str(), R"#(["one",{},"three"])#"); + } +} + From 90b0c630a0e9e66f69f1d24b538982c20b5486b9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Domen=20Ko=C5=BEar?= Date: Wed, 26 Feb 2020 10:50:40 +0100 Subject: [PATCH 51/56] install-multi-user: allow overriding user count --- scripts/install-multi-user.sh | 4 +++- scripts/install-nix-from-closure.sh | 3 +++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/scripts/install-multi-user.sh b/scripts/install-multi-user.sh index a0f1deb98..991cf1998 100644 --- a/scripts/install-multi-user.sh +++ b/scripts/install-multi-user.sh @@ -20,7 +20,9 @@ readonly GREEN='\033[32m' readonly GREEN_UL='\033[4;32m' readonly RED='\033[31m' -readonly NIX_USER_COUNT="32" +# installer allows overriding build user count to speed up installation +# as creating each user takes non-trivial amount of time on macos +readonly NIX_USER_COUNT=${NIX_USER_COUNT:-32} readonly NIX_BUILD_GROUP_ID="30000" readonly NIX_BUILD_GROUP_NAME="nixbld" readonly NIX_FIRST_BUILD_UID="30001" diff --git a/scripts/install-nix-from-closure.sh b/scripts/install-nix-from-closure.sh index 72aa5abf5..6ea23a44a 100644 --- a/scripts/install-nix-from-closure.sh +++ b/scripts/install-nix-from-closure.sh @@ -50,6 +50,9 @@ while [ $# -gt 0 ]; do INSTALL_MODE=no-daemon;; --no-channel-add) NIX_INSTALLER_NO_CHANNEL_ADD=1;; + --daemon-user-count) + NIX_USER_COUNT=$2 + shift;; --no-modify-profile) NIX_INSTALLER_NO_MODIFY_PROFILE=1;; --darwin-use-unencrypted-nix-store-volume) From 573ff8dfcaacc4495e7f7880d86f70145a074578 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Domen=20Ko=C5=BEar?= Date: Mon, 25 May 2020 17:31:46 +0200 Subject: [PATCH 52/56] Allow passing extra nix.conf to installer --- scripts/install-multi-user.sh | 1 + scripts/install-nix-from-closure.sh | 4 ++++ 2 files changed, 5 insertions(+) diff --git a/scripts/install-multi-user.sh b/scripts/install-multi-user.sh index a0f1deb98..b0cb51943 100644 --- a/scripts/install-multi-user.sh +++ b/scripts/install-multi-user.sh @@ -646,6 +646,7 @@ setup_default_profile() { place_nix_configuration() { cat < "$SCRATCH/nix.conf" +$NIX_EXTRA_CONF build-users-group = $NIX_BUILD_GROUP_NAME EOF _sudo "to place the default nix daemon configuration (part 2)" \ diff --git a/scripts/install-nix-from-closure.sh b/scripts/install-nix-from-closure.sh index 72aa5abf5..3e0312c78 100644 --- a/scripts/install-nix-from-closure.sh +++ b/scripts/install-nix-from-closure.sh @@ -41,6 +41,7 @@ fi INSTALL_MODE=no-daemon CREATE_DARWIN_VOLUME=0 +NIX_EXTRA_CONF= # handle the command line flags while [ $# -gt 0 ]; do case $1 in @@ -54,6 +55,9 @@ while [ $# -gt 0 ]; do NIX_INSTALLER_NO_MODIFY_PROFILE=1;; --darwin-use-unencrypted-nix-store-volume) CREATE_DARWIN_VOLUME=1;; + --nix-extra-conf-file) + NIX_EXTRA_CONF=$(cat $2) + shift;; *) ( echo "Nix Installer [--daemon|--no-daemon] [--no-channel-add] [--no-modify-profile]" From 4b388e84318722f970326967e1648675655740fe Mon Sep 17 00:00:00 2001 From: Tobias Pflug Date: Mon, 25 May 2020 18:34:55 +0200 Subject: [PATCH 53/56] Add unit tests for xml-writer --- src/libutil/tests/xml-writer.cc | 105 ++++++++++++++++++++++++++++++++ 1 file changed, 105 insertions(+) create mode 100644 src/libutil/tests/xml-writer.cc diff --git a/src/libutil/tests/xml-writer.cc b/src/libutil/tests/xml-writer.cc new file mode 100644 index 000000000..adcde25c9 --- /dev/null +++ b/src/libutil/tests/xml-writer.cc @@ -0,0 +1,105 @@ +#include "xml-writer.hh" +#include +#include + +namespace nix { + + /* ---------------------------------------------------------------------------- + * XMLWriter + * --------------------------------------------------------------------------*/ + + TEST(XMLWriter, emptyObject) { + std::stringstream out; + { + XMLWriter t(false, out); + } + + ASSERT_EQ(out.str(), "\n"); + } + + TEST(XMLWriter, objectWithEmptyElement) { + std::stringstream out; + { + XMLWriter t(false, out); + t.openElement("foobar"); + } + + ASSERT_EQ(out.str(), "\n"); + } + + TEST(XMLWriter, objectWithElementWithAttrs) { + std::stringstream out; + { + XMLWriter t(false, out); + XMLAttrs attrs = { + { "foo", "bar" } + }; + t.openElement("foobar", attrs); + } + + ASSERT_EQ(out.str(), "\n"); + } + + TEST(XMLWriter, objectWithElementWithEmptyAttrs) { + std::stringstream out; + { + XMLWriter t(false, out); + XMLAttrs attrs = {}; + t.openElement("foobar", attrs); + } + + ASSERT_EQ(out.str(), "\n"); + } + + TEST(XMLWriter, objectWithElementWithAttrsEscaping) { + std::stringstream out; + { + XMLWriter t(false, out); + XMLAttrs attrs = { + { "", "" } + }; + t.openElement("foobar", attrs); + } + + // XXX: While "" is escaped, "" isn't which I think is a bug. + ASSERT_EQ(out.str(), "\n=\"<value>\">"); + } + + TEST(XMLWriter, objectWithElementWithAttrsIndented) { + std::stringstream out; + { + XMLWriter t(true, out); + XMLAttrs attrs = { + { "foo", "bar" } + }; + t.openElement("foobar", attrs); + } + + ASSERT_EQ(out.str(), "\n\n\n"); + } + + TEST(XMLWriter, writeEmptyElement) { + std::stringstream out; + { + XMLWriter t(false, out); + t.writeEmptyElement("foobar"); + } + + ASSERT_EQ(out.str(), "\n"); + } + + TEST(XMLWriter, writeEmptyElementWithAttributes) { + std::stringstream out; + { + XMLWriter t(false, out); + XMLAttrs attrs = { + { "foo", "bar" } + }; + t.writeEmptyElement("foobar", attrs); + + } + + ASSERT_EQ(out.str(), "\n"); + } + +} From 1a5ac894e929cefceec22c0f1ca248ee6be445ff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Domen=20Ko=C5=BEar?= Date: Tue, 26 May 2020 15:49:26 +0200 Subject: [PATCH 54/56] Fix installer script bugs - --no-channel-add didn't have effect on multi-user installation - some new flags didn't work at all - document all installer flags --- scripts/install-multi-user.sh | 24 ++++++++++++++---------- scripts/install-nix-from-closure.sh | 13 ++++++++----- 2 files changed, 22 insertions(+), 15 deletions(-) diff --git a/scripts/install-multi-user.sh b/scripts/install-multi-user.sh index 74cc6a5b0..ab41e0242 100644 --- a/scripts/install-multi-user.sh +++ b/scripts/install-multi-user.sh @@ -29,6 +29,7 @@ readonly NIX_FIRST_BUILD_UID="30001" # Please don't change this. We don't support it, because the # default shell profile that comes with Nix doesn't support it. readonly NIX_ROOT="/nix" +readonly NIX_EXTRA_CONF=${NIX_EXTRA_CONF:-} readonly PROFILE_TARGETS=("/etc/bashrc" "/etc/profile.d/nix.sh" "/etc/zshrc") readonly PROFILE_BACKUP_SUFFIX=".backup-before-nix" @@ -452,9 +453,11 @@ create_directories() { } place_channel_configuration() { - echo "https://nixos.org/channels/nixpkgs-unstable nixpkgs" > "$SCRATCH/.nix-channels" - _sudo "to set up the default system channel (part 1)" \ - install -m 0664 "$SCRATCH/.nix-channels" "$ROOT_HOME/.nix-channels" + if [ -z "$NIX_INSTALLER_NO_CHANNEL_ADD" ]; then + echo "https://nixos.org/channels/nixpkgs-unstable nixpkgs" > "$SCRATCH/.nix-channels" + _sudo "to set up the default system channel (part 1)" \ + install -m 0664 "$SCRATCH/.nix-channels" "$ROOT_HOME/.nix-channels" + fi } welcome_to_nix() { @@ -636,13 +639,14 @@ setup_default_profile() { export NIX_SSL_CERT_FILE=/nix/var/nix/profiles/default/etc/ssl/certs/ca-bundle.crt fi - # Have to explicitly pass NIX_SSL_CERT_FILE as part of the sudo call, - # otherwise it will be lost in environments where sudo doesn't pass - # all the environment variables by default. - _sudo "to update the default channel in the default profile" \ - HOME="$ROOT_HOME" NIX_SSL_CERT_FILE="$NIX_SSL_CERT_FILE" "$NIX_INSTALLED_NIX/bin/nix-channel" --update nixpkgs \ - || channel_update_failed=1 - + if [ -z "$NIX_INSTALLER_NO_CHANNEL_ADD" ]; then + # Have to explicitly pass NIX_SSL_CERT_FILE as part of the sudo call, + # otherwise it will be lost in environments where sudo doesn't pass + # all the environment variables by default. + _sudo "to update the default channel in the default profile" \ + HOME="$ROOT_HOME" NIX_SSL_CERT_FILE="$NIX_SSL_CERT_FILE" "$NIX_INSTALLED_NIX/bin/nix-channel" --update nixpkgs \ + || channel_update_failed=1 + fi } diff --git a/scripts/install-nix-from-closure.sh b/scripts/install-nix-from-closure.sh index 635aaa16d..826ca8b8c 100644 --- a/scripts/install-nix-from-closure.sh +++ b/scripts/install-nix-from-closure.sh @@ -41,7 +41,6 @@ fi INSTALL_MODE=no-daemon CREATE_DARWIN_VOLUME=0 -NIX_EXTRA_CONF= # handle the command line flags while [ $# -gt 0 ]; do case $1 in @@ -50,20 +49,20 @@ while [ $# -gt 0 ]; do --no-daemon) INSTALL_MODE=no-daemon;; --no-channel-add) - NIX_INSTALLER_NO_CHANNEL_ADD=1;; + export NIX_INSTALLER_NO_CHANNEL_ADD=1;; --daemon-user-count) - NIX_USER_COUNT=$2 + export NIX_USER_COUNT=$2 shift;; --no-modify-profile) NIX_INSTALLER_NO_MODIFY_PROFILE=1;; --darwin-use-unencrypted-nix-store-volume) CREATE_DARWIN_VOLUME=1;; --nix-extra-conf-file) - NIX_EXTRA_CONF=$(cat $2) + export NIX_EXTRA_CONF="$(cat $2)" shift;; *) ( - echo "Nix Installer [--daemon|--no-daemon] [--no-channel-add] [--no-modify-profile]" + echo "Nix Installer [--daemon|--no-daemon] [--daemon-user-count INT] [--no-channel-add] [--no-modify-profile] [--darwin-use-unencrypted-nix-store-volume] [--nix-extra-conf-file FILE]" echo "Choose installation method." echo "" @@ -82,6 +81,10 @@ while [ $# -gt 0 ]; do echo " --no-modify-profile: Skip channel installation. When not provided nixpkgs-unstable" echo " is installed by default." echo "" + echo " --daemon-user-count: Number of build users to create. Defaults to 32." + echo "" + echo " --nix-extra-conf-file: Path to nix.conf to prepend when installing /etc/nix.conf" + echo "" ) >&2 # darwin and Catalina+ From 3d3c219d917525b0a131c4332dd65eadfc818f49 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Domen=20Ko=C5=BEar?= Date: Tue, 26 May 2020 16:23:03 +0200 Subject: [PATCH 55/56] installer: fix unused variable --- scripts/install-multi-user.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts/install-multi-user.sh b/scripts/install-multi-user.sh index ab41e0242..d04b4bbbf 100644 --- a/scripts/install-multi-user.sh +++ b/scripts/install-multi-user.sh @@ -453,7 +453,7 @@ create_directories() { } place_channel_configuration() { - if [ -z "$NIX_INSTALLER_NO_CHANNEL_ADD" ]; then + if [ -z "${NIX_INSTALLER_NO_CHANNEL_ADD:-}" ]; then echo "https://nixos.org/channels/nixpkgs-unstable nixpkgs" > "$SCRATCH/.nix-channels" _sudo "to set up the default system channel (part 1)" \ install -m 0664 "$SCRATCH/.nix-channels" "$ROOT_HOME/.nix-channels" @@ -639,7 +639,7 @@ setup_default_profile() { export NIX_SSL_CERT_FILE=/nix/var/nix/profiles/default/etc/ssl/certs/ca-bundle.crt fi - if [ -z "$NIX_INSTALLER_NO_CHANNEL_ADD" ]; then + if [ -z "${NIX_INSTALLER_NO_CHANNEL_ADD:-}" ]; then # Have to explicitly pass NIX_SSL_CERT_FILE as part of the sudo call, # otherwise it will be lost in environments where sudo doesn't pass # all the environment variables by default. From 4e6d7cb55a884ac19990b487c2020ea257e0f9c3 Mon Sep 17 00:00:00 2001 From: Daiderd Jordan Date: Wed, 27 May 2020 20:58:42 +0200 Subject: [PATCH 56/56] installer: don't require xz on darwin On macOS the system tar has builtin support for lzma while xz isn't available as a separate binary. There's no builtin package manager there available either so having to install lzma (without nix) would be rather painful. --- scripts/install.in | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/scripts/install.in b/scripts/install.in index 6709f00d4..1d26c4ff0 100644 --- a/scripts/install.in +++ b/scripts/install.in @@ -36,7 +36,9 @@ tarball="$tmpDir/$(basename "$tmpDir/nix-@nixVersion@-$system.tar.xz")" require_util curl "download the binary tarball" require_util tar "unpack the binary tarball" -require_util xz "unpack the binary tarball" +if [ "$(uname -s)" != "Darwin" ]; then + require_util xz "unpack the binary tarball" +fi echo "downloading Nix @nixVersion@ binary tarball for $system from '$url' to '$tmpDir'..." curl -L "$url" -o "$tarball" || oops "failed to download '$url'"