From 63c5c91cc053cbc1fcb8d3fe71c41142c9f51bfa Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Fri, 31 May 2019 18:48:28 +0200 Subject: [PATCH 1/8] Show hash mismatch warnings in SRI format --- 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 004be8010..a69592219 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -3170,7 +3170,7 @@ void DerivationGoal::registerOutputs() valid. */ delayedException = std::make_exception_ptr( BuildError("hash mismatch in fixed-output derivation '%s':\n wanted: %s\n got: %s", - dest, h.to_string(), h2.to_string())); + dest, h.to_string(SRI), h2.to_string(SRI))); Path actualDest = worker.store.toRealPath(dest); From b971e406dee82486d53737a928b0bb3b482a6c49 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Fri, 31 May 2019 19:01:11 +0200 Subject: [PATCH 2/8] Support 'dir' and other parameters in path flakerefs --- src/libexpr/primops/flakeref.cc | 90 ++++++++++++++++++--------------- 1 file changed, 49 insertions(+), 41 deletions(-) diff --git a/src/libexpr/primops/flakeref.cc b/src/libexpr/primops/flakeref.cc index 3c805eff8..306da20fe 100644 --- a/src/libexpr/primops/flakeref.cc +++ b/src/libexpr/primops/flakeref.cc @@ -1,4 +1,5 @@ #include "flakeref.hh" +#include "store-api.hh" #include @@ -30,10 +31,6 @@ const static std::string schemeRegex = "(?:http|https|ssh|git|file)"; const static std::string authorityRegex = "[a-zA-Z0-9._~-]*"; const static std::string segmentRegex = "[a-zA-Z0-9._~-]+"; const static std::string pathRegex = "/?" + segmentRegex + "(?:/" + segmentRegex + ")*"; -// FIXME: support escaping in query string. -// Note: '/' is not a valid query parameter, but so what... -const static std::string paramRegex = "[a-z]+=[/a-zA-Z0-9._-]*"; -const static std::string paramsRegex = "(?:[?](" + paramRegex + "(?:&" + paramRegex + ")*))"; // 'dir' path elements cannot start with a '.'. We also reject // potentially dangerous characters like ';'. @@ -41,7 +38,7 @@ const static std::string subDirElemRegex = "(?:[a-zA-Z0-9_-]+[a-zA-Z0-9._-]*)"; const static std::string subDirRegex = subDirElemRegex + "(?:/" + subDirElemRegex + ")*"; -FlakeRef::FlakeRef(const std::string & uri, bool allowRelative) +FlakeRef::FlakeRef(const std::string & uri_, bool allowRelative) { // FIXME: could combine this into one regex. @@ -50,21 +47,46 @@ FlakeRef::FlakeRef(const std::string & uri, bool allowRelative) std::regex::ECMAScript); static std::regex githubRegex( - "github:(" + ownerRegex + ")/(" + repoRegex + ")(?:/" + revOrRefRegex + ")?" - + paramsRegex + "?", + "github:(" + ownerRegex + ")/(" + repoRegex + ")(?:/" + revOrRefRegex + ")?", std::regex::ECMAScript); static std::regex uriRegex( "((" + schemeRegex + "):" + "(?://(" + authorityRegex + "))?" + - "(" + pathRegex + "))" + - paramsRegex + "?", + "(" + pathRegex + "))", std::regex::ECMAScript); static std::regex refRegex2(refRegex, std::regex::ECMAScript); static std::regex subDirRegex2(subDirRegex, std::regex::ECMAScript); + auto [uri, params] = splitUriAndParams(uri_); + + auto handleSubdir = [&](const std::string & name, const std::string & value) { + if (name == "dir") { + if (value != "" && !std::regex_match(value, subDirRegex2)) + throw Error("flake '%s' has invalid subdirectory '%s'", uri, value); + subdir = value; + return true; + } else + return false; + }; + + auto handleGitParams = [&](const std::string & name, const std::string & value) { + if (name == "rev") { + if (!std::regex_match(value, revRegex)) + throw Error("invalid Git revision '%s'", value); + rev = Hash(value, htSHA1); + } else if (name == "ref") { + if (!std::regex_match(value, refRegex2)) + throw Error("invalid Git ref '%s'", value); + ref = value; + } else if (handleSubdir(name, value)) + ; + else return false; + return true; + }; + std::cmatch match; if (std::regex_match(uri.c_str(), match, flakeRegex)) { IsAlias d; @@ -88,17 +110,11 @@ FlakeRef::FlakeRef(const std::string & uri, bool allowRelative) else if (match[4].matched) { ref = match[4]; } - for (auto & param : tokenizeString(match[5], "&")) { - auto n = param.find('='); - assert(n != param.npos); - std::string name(param, 0, n); - std::string value(param, n + 1); - if (name == "dir") { - if (value != "" && !std::regex_match(value, subDirRegex2)) - throw Error("flake '%s' has invalid subdirectory '%s'", uri, value); - subdir = value; - } else - throw Error("invalid Git flake reference parameter '%s', in '%s'", name, uri); + for (auto & param : params) { + if (handleSubdir(param.first, param.second)) + ; + else + throw Error("invalid Git flakeref parameter '%s', in '%s'", param.first, uri); } data = d; } @@ -108,26 +124,12 @@ FlakeRef::FlakeRef(const std::string & uri, bool allowRelative) { IsGit d; d.uri = match[1]; - for (auto & param : tokenizeString(match[5], "&")) { - auto n = param.find('='); - assert(n != param.npos); - std::string name(param, 0, n); - std::string value(param, n + 1); - if (name == "rev") { - if (!std::regex_match(value, revRegex)) - throw Error("invalid Git revision '%s'", value); - rev = Hash(value, htSHA1); - } else if (name == "ref") { - if (!std::regex_match(value, refRegex2)) - throw Error("invalid Git ref '%s'", value); - ref = value; - } else if (name == "dir") { - if (value != "" && !std::regex_match(value, subDirRegex2)) - throw Error("flake '%s' has invalid subdirectory '%s'", uri, value); - subdir = value; - } else + for (auto & param : params) { + if (handleGitParams(param.first, param.second)) + ; + else // FIXME: should probably pass through unknown parameters - throw Error("invalid Git flake reference parameter '%s', in '%s'", name, uri); + throw Error("invalid Git flakeref parameter '%s', in '%s'", param.first, uri); } if (rev && !ref) throw Error("flake URI '%s' lacks a Git ref", uri); @@ -138,6 +140,12 @@ FlakeRef::FlakeRef(const std::string & uri, bool allowRelative) IsPath d; d.path = allowRelative ? absPath(uri) : canonPath(uri); data = d; + for (auto & param : params) { + if (handleGitParams(param.first, param.second)) + ; + else + throw Error("invalid Git flakeref parameter '%s', in '%s'", param.first, uri); + } } else @@ -165,10 +173,10 @@ std::string FlakeRef::to_string() const } else if (auto refData = std::get_if(&data)) { - assert(subdir == ""); + string = refData->path; if (ref) addParam("ref", *ref); if (rev) addParam("rev", rev->gitRev()); - return refData->path; + if (subdir != "") addParam("dir", subdir); } else if (auto refData = std::get_if(&data)) { From 9169046e64cbffd85a445ebe4313b172a6681646 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Fri, 31 May 2019 20:10:56 +0200 Subject: [PATCH 3/8] Add operator << for LockFile Useful for debugging. --- src/libexpr/primops/flake.cc | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/libexpr/primops/flake.cc b/src/libexpr/primops/flake.cc index fdbdc83bc..af7d51834 100644 --- a/src/libexpr/primops/flake.cc +++ b/src/libexpr/primops/flake.cc @@ -121,7 +121,7 @@ nlohmann::json flakeEntryToJson(const LockFile::FlakeEntry & entry) return json; } -void writeLockFile(const LockFile & lockFile, const Path & path) +std::ostream & operator <<(std::ostream & stream, const LockFile & lockFile) { nlohmann::json json; json["version"] = 1; @@ -133,8 +133,14 @@ void writeLockFile(const LockFile & lockFile, const Path & path) json["inputs"] = nlohmann::json::object(); for (auto & x : lockFile.flakeEntries) json["inputs"][x.first.to_string()] = flakeEntryToJson(x.second); + stream << json.dump(4); // '4' = indentation in json file + return stream; +} + +void writeLockFile(const LockFile & lockFile, const Path & path) +{ createDirs(dirOf(path)); - writeFile(path, json.dump(4) + "\n"); // '4' = indentation in json file + writeFile(path, fmt("%s\n", lockFile)); } Path getUserRegistryPath() From 7adb10d29b0041a93d1afeec197bf9af6e8b25b5 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Fri, 31 May 2019 20:12:59 +0200 Subject: [PATCH 4/8] Fix reading the lockfile of a flake in a subdirectory --- src/libexpr/primops/flake.cc | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/libexpr/primops/flake.cc b/src/libexpr/primops/flake.cc index af7d51834..235e10922 100644 --- a/src/libexpr/primops/flake.cc +++ b/src/libexpr/primops/flake.cc @@ -482,9 +482,12 @@ ResolvedFlake resolveFlake(EvalState & state, const FlakeRef & topRef, HandleLoc Flake flake = getFlake(state, topRef, allowedToUseRegistries(handleLockFile, true)); LockFile oldLockFile; - if (!recreateLockFile (handleLockFile)) { + if (!recreateLockFile(handleLockFile)) { // If recreateLockFile, start with an empty lockfile - oldLockFile = readLockFile(flake.sourceInfo.storePath + "/flake.lock"); // FIXME: symlink attack + // FIXME: symlink attack + oldLockFile = readLockFile( + state.store->toRealPath(flake.sourceInfo.storePath) + + "/" + flake.sourceInfo.resolvedRef.subdir + "/flake.lock"); } LockFile lockFile(oldLockFile); From ccb1bad612e060fc4397d340edc64d18231744b6 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Fri, 31 May 2019 20:53:23 +0200 Subject: [PATCH 5/8] Allow bare flakerefs as installables So now $ nix build blender-bin works and builds the default package from that flake. You don't need to add a colon at the end anymore. --- src/libexpr/primops/flakeref.cc | 31 ++++++++++++++++++++++--------- src/libexpr/primops/flakeref.hh | 5 +++++ src/nix/installables.cc | 4 ++++ tests/flakes.sh | 2 +- 4 files changed, 32 insertions(+), 10 deletions(-) diff --git a/src/libexpr/primops/flakeref.cc b/src/libexpr/primops/flakeref.cc index 306da20fe..4b6922295 100644 --- a/src/libexpr/primops/flakeref.cc +++ b/src/libexpr/primops/flakeref.cc @@ -65,7 +65,7 @@ FlakeRef::FlakeRef(const std::string & uri_, bool allowRelative) auto handleSubdir = [&](const std::string & name, const std::string & value) { if (name == "dir") { if (value != "" && !std::regex_match(value, subDirRegex2)) - throw Error("flake '%s' has invalid subdirectory '%s'", uri, value); + throw BadFlakeRef("flake '%s' has invalid subdirectory '%s'", uri, value); subdir = value; return true; } else @@ -75,11 +75,11 @@ FlakeRef::FlakeRef(const std::string & uri_, bool allowRelative) auto handleGitParams = [&](const std::string & name, const std::string & value) { if (name == "rev") { if (!std::regex_match(value, revRegex)) - throw Error("invalid Git revision '%s'", value); + throw BadFlakeRef("invalid Git revision '%s'", value); rev = Hash(value, htSHA1); } else if (name == "ref") { if (!std::regex_match(value, refRegex2)) - throw Error("invalid Git ref '%s'", value); + throw BadFlakeRef("invalid Git ref '%s'", value); ref = value; } else if (handleSubdir(name, value)) ; @@ -114,7 +114,7 @@ FlakeRef::FlakeRef(const std::string & uri_, bool allowRelative) if (handleSubdir(param.first, param.second)) ; else - throw Error("invalid Git flakeref parameter '%s', in '%s'", param.first, uri); + throw BadFlakeRef("invalid Git flakeref parameter '%s', in '%s'", param.first, uri); } data = d; } @@ -129,14 +129,16 @@ FlakeRef::FlakeRef(const std::string & uri_, bool allowRelative) ; else // FIXME: should probably pass through unknown parameters - throw Error("invalid Git flakeref parameter '%s', in '%s'", param.first, uri); + throw BadFlakeRef("invalid Git flakeref parameter '%s', in '%s'", param.first, uri); } if (rev && !ref) - throw Error("flake URI '%s' lacks a Git ref", uri); + throw BadFlakeRef("flake URI '%s' lacks a Git ref", uri); data = d; } - else if (hasPrefix(uri, "/") || (allowRelative && (hasPrefix(uri, "./") || hasPrefix(uri, "../") || uri == "."))) { + else if ((hasPrefix(uri, "/") || (allowRelative && (hasPrefix(uri, "./") || hasPrefix(uri, "../") || uri == "."))) + && uri.find(':') == std::string::npos) + { IsPath d; d.path = allowRelative ? absPath(uri) : canonPath(uri); data = d; @@ -144,12 +146,12 @@ FlakeRef::FlakeRef(const std::string & uri_, bool allowRelative) if (handleGitParams(param.first, param.second)) ; else - throw Error("invalid Git flakeref parameter '%s', in '%s'", param.first, uri); + throw BadFlakeRef("invalid Git flakeref parameter '%s', in '%s'", param.first, uri); } } else - throw Error("'%s' is not a valid flake reference", uri); + throw BadFlakeRef("'%s' is not a valid flake reference", uri); } std::string FlakeRef::to_string() const @@ -225,4 +227,15 @@ FlakeRef FlakeRef::baseRef() const // Removes the ref and rev from a FlakeRef. result.rev = std::nullopt; return result; } + +std::optional parseFlakeRef( + const std::string & uri, bool allowRelative) +{ + try { + return FlakeRef(uri, allowRelative); + } catch (BadFlakeRef & e) { + return {}; + } +} + } diff --git a/src/libexpr/primops/flakeref.hh b/src/libexpr/primops/flakeref.hh index 299094634..52bb82ddb 100644 --- a/src/libexpr/primops/flakeref.hh +++ b/src/libexpr/primops/flakeref.hh @@ -180,4 +180,9 @@ struct FlakeRef std::ostream & operator << (std::ostream & str, const FlakeRef & flakeRef); +MakeError(BadFlakeRef, Error); + +std::optional parseFlakeRef( + const std::string & uri, bool allowRelative = false); + } diff --git a/src/nix/installables.cc b/src/nix/installables.cc index fe89a6bb4..40248eb5d 100644 --- a/src/nix/installables.cc +++ b/src/nix/installables.cc @@ -332,6 +332,10 @@ std::vector> SourceExprCommand::parseInstallables( getDefaultFlakeAttrPaths())); } + else if (auto flakeRef = parseFlakeRef(s, true)) + result.push_back(std::make_shared(*this, s, + getDefaultFlakeAttrPaths())); + else result.push_back(std::make_shared(*this, FlakeRef("nixpkgs"), s)); } diff --git a/tests/flakes.sh b/tests/flakes.sh index 377f93c8e..8b9cb7260 100644 --- a/tests/flakes.sh +++ b/tests/flakes.sh @@ -131,7 +131,7 @@ nix build -o $TEST_ROOT/result --flake-registry $registry flake1:foo [[ -e $TEST_ROOT/result/hello ]] # Test defaultPackage. -nix build -o $TEST_ROOT/result --flake-registry $registry flake1: +nix build -o $TEST_ROOT/result --flake-registry $registry flake1 [[ -e $TEST_ROOT/result/hello ]] # Building a flake with an unlocked dependency should fail in pure mode. From 8abb8647a33c3516026cd8a2954d34633377b23c Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Fri, 31 May 2019 21:52:02 +0200 Subject: [PATCH 6/8] Automatically determine subdir for path flakes This means that in a flake in a subdirectory of a Git repo, you can now do $ nix build rather than the inconvenient $ nix build ../..?dir=foo/bar --- src/libexpr/primops/flakeref.cc | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/libexpr/primops/flakeref.cc b/src/libexpr/primops/flakeref.cc index 4b6922295..6c90c3b64 100644 --- a/src/libexpr/primops/flakeref.cc +++ b/src/libexpr/primops/flakeref.cc @@ -140,7 +140,17 @@ FlakeRef::FlakeRef(const std::string & uri_, bool allowRelative) && uri.find(':') == std::string::npos) { IsPath d; - d.path = allowRelative ? absPath(uri) : canonPath(uri); + if (allowRelative) { + d.path = absPath(uri); + while (true) { + if (pathExists(d.path + "/.git")) break; + subdir = baseNameOf(d.path) + (subdir.empty() ? "" : "/" + subdir); + d.path = dirOf(d.path); + if (d.path == "/") + throw BadFlakeRef("path '%s' does not reference a Git repository", uri); + } + } else + d.path = canonPath(uri); data = d; for (auto & param : params) { if (handleGitParams(param.first, param.second)) From 8cb3bbd5044b8fbfc65f13455d1619a78ccf33a5 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Fri, 31 May 2019 22:17:39 +0200 Subject: [PATCH 7/8] Fix handling of bare flakerefs containing a colon --- src/nix/installables.cc | 8 ++++---- tests/flakes.sh | 3 +++ 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/src/nix/installables.cc b/src/nix/installables.cc index 40248eb5d..38ae416e3 100644 --- a/src/nix/installables.cc +++ b/src/nix/installables.cc @@ -314,6 +314,10 @@ std::vector> SourceExprCommand::parseInstallables( Strings{"packages." + std::string(s, 8)})); } + else if (auto flakeRef = parseFlakeRef(s, true)) + result.push_back(std::make_shared(*this, s, + getDefaultFlakeAttrPaths())); + else if ((colon = s.rfind(':')) != std::string::npos) { auto flakeRef = std::string(s, 0, colon); auto attrPath = std::string(s, colon + 1); @@ -332,10 +336,6 @@ std::vector> SourceExprCommand::parseInstallables( getDefaultFlakeAttrPaths())); } - else if (auto flakeRef = parseFlakeRef(s, true)) - result.push_back(std::make_shared(*this, s, - getDefaultFlakeAttrPaths())); - else result.push_back(std::make_shared(*this, FlakeRef("nixpkgs"), s)); } diff --git a/tests/flakes.sh b/tests/flakes.sh index 8b9cb7260..c4dd8c333 100644 --- a/tests/flakes.sh +++ b/tests/flakes.sh @@ -134,6 +134,9 @@ nix build -o $TEST_ROOT/result --flake-registry $registry flake1:foo nix build -o $TEST_ROOT/result --flake-registry $registry flake1 [[ -e $TEST_ROOT/result/hello ]] +nix build -o $TEST_ROOT/result --flake-registry $registry $flake1Dir +nix build -o $TEST_ROOT/result --flake-registry $registry file://$flake1Dir + # Building a flake with an unlocked dependency should fail in pure mode. (! nix eval "(builtins.getFlake "$flake2Dir")") From 15f241775ace2bbd807e7222e822bd5bf0f42ff7 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Fri, 31 May 2019 23:21:53 +0200 Subject: [PATCH 8/8] Doh --- src/nix/installables.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/nix/installables.cc b/src/nix/installables.cc index 38ae416e3..eb3c27d6b 100644 --- a/src/nix/installables.cc +++ b/src/nix/installables.cc @@ -315,7 +315,7 @@ std::vector> SourceExprCommand::parseInstallables( } else if (auto flakeRef = parseFlakeRef(s, true)) - result.push_back(std::make_shared(*this, s, + result.push_back(std::make_shared(*this, std::move(*flakeRef), getDefaultFlakeAttrPaths())); else if ((colon = s.rfind(':')) != std::string::npos) {