diff options
50 files changed, 990 insertions, 370 deletions
diff --git a/doc/manual/change-authors.yml b/doc/manual/change-authors.yml index c56f588ca..5a7b8117e 100644 --- a/doc/manual/change-authors.yml +++ b/doc/manual/change-authors.yml @@ -60,6 +60,10 @@ jade: forgejo: jade github: lf- +kloenk: + forgejo: kloenk + github: kloenk + lovesegfault: github: lovesegfault diff --git a/doc/manual/rl-next/multiline-log-format.md b/doc/manual/rl-next/multiline-log-format.md new file mode 100644 index 000000000..62bb9f1e7 --- /dev/null +++ b/doc/manual/rl-next/multiline-log-format.md @@ -0,0 +1,14 @@ +--- +synopsis: Add log formats `multiline` and `multiline-with-logs` +cls: [1369] +credits: [kloenk] +category: Improvements +--- + +Added two new log formats (`multiline` and `multiline-with-logs`) that display +current activities below each other for better visibility. + +These formats attempt to use the maximum available lines +(defaulting to 25 if unable to determine) and print up to that many lines. +The status bar is displayed as the first line, with each subsequent +activity on its own line. diff --git a/doc/manual/src/command-ref/opt-common.md b/doc/manual/src/command-ref/opt-common.md index cd289b7ea..7740da953 100644 --- a/doc/manual/src/command-ref/opt-common.md +++ b/doc/manual/src/command-ref/opt-common.md @@ -78,6 +78,16 @@ Most commands in Lix accept the following command-line options: Display the raw logs, with the progress bar at the bottom. + - `multiline` + + Display a progress bar during the builds and in the lines below that one line per activity. + + + - `multiline-with-logs` + + Displayes the raw logs, with a progress bar and activities each in a new line at the bottom. + + - <span id="opt-no-build-output">[`--no-build-output`](#opt-no-build-output)</span> / `-Q` By default, output written by builders to standard output and standard error is echoed to the Lix command's standard error. diff --git a/src/libcmd/repl.cc b/src/libcmd/repl.cc index 99abbe44b..28341259c 100644 --- a/src/libcmd/repl.cc +++ b/src/libcmd/repl.cc @@ -1,3 +1,4 @@ +#include <cstdio> #include <editline.h> #include <iostream> #include <cstdlib> @@ -242,8 +243,7 @@ NixRepl::NixRepl(const SearchPath & searchPath, nix::ref<Store> store, ref<EvalS { } -void runNix(Path program, const Strings & args, - const std::optional<std::string> & input = {}) +void runNix(Path program, const Strings & args) { auto subprocessEnv = getEnv(); subprocessEnv["NIX_CONFIG"] = globalConfig.toKeyValue(); @@ -252,7 +252,6 @@ void runNix(Path program, const Strings & args, .program = settings.nixBinDir+ "/" + program, .args = args, .environment = subprocessEnv, - .input = input, }); return; diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index 9bd27e22d..afee89420 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -4,6 +4,7 @@ #include "primops.hh" #include "print-options.hh" #include "shared.hh" +#include "suggestions.hh" #include "types.hh" #include "store-api.hh" #include "derivations.hh" @@ -1426,11 +1427,13 @@ static std::string showAttrPath(EvalState & state, Env & env, const AttrPath & a void ExprSelect::eval(EvalState & state, Env & env, Value & v) { - Value vTmp; - PosIdx pos2; - Value * vAttrs = &vTmp; + Value vFirst; - e->eval(state, env, vTmp); + // Pointer to the current attrset Value in this select chain. + Value * vCurrent = &vFirst; + // Position for the current attrset Value in this select chain. + PosIdx posCurrent; + e->eval(state, env, vFirst); try { auto dts = state.debugRepl @@ -1443,48 +1446,75 @@ void ExprSelect::eval(EvalState & state, Env & env, Value & v) showAttrPath(state, env, attrPath)) : nullptr; - for (auto & i : attrPath) { + for (auto const & currentAttrName : attrPath) { state.nrLookups++; - Bindings::iterator j; - auto name = getName(i, state, env); - if (def) { - state.forceValue(*vAttrs, pos); - if (vAttrs->type() != nAttrs || - (j = vAttrs->attrs->find(name)) == vAttrs->attrs->end()) - { - def->eval(state, env, v); + + Symbol const name = getName(currentAttrName, state, env); + + state.forceValue(*vCurrent, pos); + + if (vCurrent->type() != nAttrs) { + + // If we have an `or` provided default, + // then this is allowed to not be an attrset. + if (def != nullptr) { + this->def->eval(state, env, v); return; } - } else { - state.forceAttrs(*vAttrs, pos, "while selecting an attribute"); - if ((j = vAttrs->attrs->find(name)) == vAttrs->attrs->end()) { - std::set<std::string> allAttrNames; - for (auto & attr : *vAttrs->attrs) - allAttrNames.insert(state.symbols[attr.name]); - auto suggestions = Suggestions::bestMatches(allAttrNames, state.symbols[name]); - state.error<EvalError>("attribute '%1%' missing", state.symbols[name]) - .atPos(pos).withSuggestions(suggestions).withFrame(env, *this).debugThrow(); + + // Otherwise, we must type error. + state.error<TypeError>( + "expected a set but found %s: %s", + showType(*vCurrent), + ValuePrinter(state, *vCurrent, errorPrintOptions) + ).withTrace(pos, "while selecting an attribute").debugThrow(); + } + + // Now that we know this is actually an attrset, try to find an attr + // with the selected name. + Bindings::iterator attrIt = vCurrent->attrs->find(name); + if (attrIt == vCurrent->attrs->end()) { + + // If we have an `or` provided default, then we'll use that. + if (def != nullptr) { + this->def->eval(state, env, v); + return; } + + // Otherwise, missing attr error. + std::set<std::string> allAttrNames; + for (auto const & attr : *vCurrent->attrs) { + allAttrNames.insert(state.symbols[attr.name]); + } + auto suggestions = Suggestions::bestMatches(allAttrNames, state.symbols[name]); + state.error<EvalError>("attribute '%s' missing", state.symbols[name]) + .atPos(pos) + .withSuggestions(suggestions) + .withFrame(env, *this) + .debugThrow(); } - vAttrs = j->value; - pos2 = j->pos; - if (state.countCalls) state.attrSelects[pos2]++; + + // If we're here, then we successfully found the attribute. + // Set our currently operated-on attrset to this one, and keep going. + vCurrent = attrIt->value; + posCurrent = attrIt->pos; + if (state.countCalls) state.attrSelects[posCurrent]++; } - state.forceValue(*vAttrs, (pos2 ? pos2 : this->pos ) ); + state.forceValue(*vCurrent, (posCurrent ? posCurrent : this->pos)); } catch (Error & e) { - if (pos2) { - auto pos2r = state.positions[pos2]; + if (posCurrent) { + auto pos2r = state.positions[posCurrent]; auto origin = std::get_if<SourcePath>(&pos2r.origin); if (!(origin && *origin == state.derivationInternal)) - state.addErrorTrace(e, pos2, "while evaluating the attribute '%1%'", + state.addErrorTrace(e, posCurrent, "while evaluating the attribute '%1%'", showAttrPath(state, env, attrPath)); } throw; } - v = *vAttrs; + v = *vCurrent; } diff --git a/src/libexpr/flake/flakeref.cc b/src/libexpr/flake/flakeref.cc index 1c90bfc43..8668961fe 100644 --- a/src/libexpr/flake/flakeref.cc +++ b/src/libexpr/flake/flakeref.cc @@ -204,7 +204,13 @@ std::pair<FlakeRef, std::string> parseFlakeRefWithFragment( std::string fragment; std::swap(fragment, parsedURL.fragment); - auto input = Input::fromURL(parsedURL, isFlake); + // This has a special meaning for flakes and must not be passed to libfetchers. + // Of course this means that libfetchers cannot have fetchers + // expecting an argument `dir` 🫠+ ParsedURL urlForFetchers(parsedURL); + urlForFetchers.query.erase("dir"); + + auto input = Input::fromURL(urlForFetchers, isFlake); input.parent = baseDir; return std::make_pair( diff --git a/src/libexpr/nixexpr.hh b/src/libexpr/nixexpr.hh index 45d44d1d1..418f888b3 100644 --- a/src/libexpr/nixexpr.hh +++ b/src/libexpr/nixexpr.hh @@ -157,8 +157,18 @@ struct ExprInheritFrom : ExprVar struct ExprSelect : Expr { PosIdx pos; - std::unique_ptr<Expr> e, def; + + /** The expression attributes are being selected on. e.g. `foo` in `foo.bar.baz`. */ + std::unique_ptr<Expr> e; + + /** A default value specified with `or`, if the selected attr doesn't exist. + * e.g. `bix` in `foo.bar.baz or bix` + */ + std::unique_ptr<Expr> def; + + /** The path of attributes being selected. e.g. `bar.baz` in `foo.bar.baz.` */ AttrPath attrPath; + ExprSelect(const PosIdx & pos, std::unique_ptr<Expr> e, AttrPath attrPath, std::unique_ptr<Expr> def) : pos(pos), e(std::move(e)), def(std::move(def)), attrPath(std::move(attrPath)) { }; ExprSelect(const PosIdx & pos, std::unique_ptr<Expr> e, Symbol name) : pos(pos), e(std::move(e)) { attrPath.push_back(AttrName(name)); }; PosIdx getPos() const override { return pos; } diff --git a/src/libfetchers/fetchers.hh b/src/libfetchers/fetchers.hh index 2bb4248be..40f2b6294 100644 --- a/src/libfetchers/fetchers.hh +++ b/src/libfetchers/fetchers.hh @@ -159,6 +159,37 @@ struct InputScheme std::optional<std::string> commitMsg) const; virtual std::pair<StorePath, Input> fetch(ref<Store> store, const Input & input) = 0; + +protected: + void emplaceURLQueryIntoAttrs( + const ParsedURL & parsedURL, + Attrs & attrs, + const StringSet & numericParams, + const StringSet & booleanParams) const + { + for (auto &[name, value] : parsedURL.query) { + if (name == "url") { + throw BadURL( + "URL '%s' must not override url via query param!", + parsedURL.to_string() + ); + } else if (numericParams.count(name) != 0) { + if (auto n = string2Int<uint64_t>(value)) { + attrs.insert_or_assign(name, *n); + } else { + throw BadURL( + "URL '%s' has non-numeric parameter '%s'", + parsedURL.to_string(), + name + ); + } + } else if (booleanParams.count(name) != 0) { + attrs.emplace(name, Explicit<bool> { value == "1" }); + } else { + attrs.emplace(name, value); + } + } + } }; void registerInputScheme(std::shared_ptr<InputScheme> && fetcher); diff --git a/src/libfetchers/git.cc b/src/libfetchers/git.cc index 2817fde23..a06bc86b2 100644 --- a/src/libfetchers/git.cc +++ b/src/libfetchers/git.cc @@ -273,18 +273,15 @@ struct GitInputScheme : InputScheme Attrs attrs; attrs.emplace("type", "git"); - - for (auto & [name, value] : url.query) { - if (name == "rev" || name == "ref") - attrs.emplace(name, value); - else if (name == "shallow" || name == "submodules" || name == "allRefs") - attrs.emplace(name, Explicit<bool> { value == "1" }); - else - url2.query.emplace(name, value); - } - attrs.emplace("url", url2.to_string()); + emplaceURLQueryIntoAttrs( + url, + attrs, + {"lastModified", "revCount"}, + {"shallow", "submodules", "allRefs"} + ); + return inputFromAttrs(attrs); } @@ -361,7 +358,7 @@ struct GitInputScheme : InputScheme args.push_back(destDir); - runProgram("git", true, args, {}, true); + runProgram("git", true, args, true); } std::optional<Path> getSourcePath(const Input & input) const override @@ -399,12 +396,15 @@ struct GitInputScheme : InputScheme if (commitMsg) { + auto [_fd, msgPath] = createTempFile("nix-msg"); + AutoDelete const _delete{msgPath}; + writeFile(msgPath, *commitMsg); + // Pause the logger to allow for user input (such as a gpg passphrase) in `git commit` logger->pause(); Finally restoreLogger([]() { logger->resume(); }); runProgram("git", true, - { "-C", *root, "--git-dir", gitDir, "commit", std::string(path.rel()), "-F", "-" }, - *commitMsg); + { "-C", *root, "--git-dir", gitDir, "commit", std::string(path.rel()), "-F", msgPath }); } } } @@ -589,7 +589,7 @@ struct GitInputScheme : InputScheme : ref == "HEAD" ? *ref : "refs/heads/" + *ref; - runProgram("git", true, { "-C", repoDir, "--git-dir", gitDir, "fetch", "--quiet", "--force", "--", actualUrl, fmt("%s:%s", fetchRef, fetchRef) }, {}, true); + runProgram("git", true, { "-C", repoDir, "--git-dir", gitDir, "fetch", "--quiet", "--force", "--", actualUrl, fmt("%s:%s", fetchRef, fetchRef) }, true); } catch (Error & e) { if (!pathExists(localRefFile)) throw; warn("could not update local clone of Git repository '%s'; continuing with the most recent version", actualUrl); @@ -656,7 +656,7 @@ struct GitInputScheme : InputScheme // everything to ensure we get the rev. Activity act(*logger, lvlTalkative, actUnknown, fmt("making temporary clone of '%s'", repoDir)); runProgram("git", true, { "-C", tmpDir, "fetch", "--quiet", "--force", - "--update-head-ok", "--", repoDir, "refs/*:refs/*" }, {}, true); + "--update-head-ok", "--", repoDir, "refs/*:refs/*" }, true); } runProgram("git", true, { "-C", tmpDir, "checkout", "--quiet", input.getRev()->gitRev() }); @@ -683,7 +683,7 @@ struct GitInputScheme : InputScheme { Activity act(*logger, lvlTalkative, actUnknown, fmt("fetching submodules of '%s'", actualUrl)); - runProgram("git", true, { "-C", tmpDir, "submodule", "--quiet", "update", "--init", "--recursive" }, {}, true); + runProgram("git", true, { "-C", tmpDir, "submodule", "--quiet", "update", "--init", "--recursive" }, true); } filter = isNotDotGitDirectory; diff --git a/src/libfetchers/github.cc b/src/libfetchers/github.cc index 60fefd1f3..b971781ae 100644 --- a/src/libfetchers/github.cc +++ b/src/libfetchers/github.cc @@ -1,3 +1,4 @@ +#include "attrs.hh" #include "filetransfer.hh" #include "cache.hh" #include "globals.hh" @@ -36,18 +37,11 @@ struct GitArchiveInputScheme : InputScheme auto path = tokenizeString<std::vector<std::string>>(url.path, "/"); - std::optional<Hash> rev; - std::optional<std::string> ref; - std::optional<std::string> host_url; + std::optional<std::string> refOrRev; auto size = path.size(); if (size == 3) { - if (std::regex_match(path[2], revRegex)) - rev = Hash::parseAny(path[2], htSHA1); - else if (std::regex_match(path[2], refRegex)) - ref = path[2]; - else - throw BadURL("in URL '%s', '%s' is not a commit hash or branch/tag name", url.url, path[2]); + refOrRev = path[2]; } else if (size > 3) { std::string rs; for (auto i = std::next(path.begin(), 2); i != path.end(); i++) { @@ -58,61 +52,91 @@ struct GitArchiveInputScheme : InputScheme } if (std::regex_match(rs, refRegex)) { - ref = rs; + refOrRev = rs; } else { throw BadURL("in URL '%s', '%s' is not a branch/tag name", url.url, rs); } } else if (size < 2) throw BadURL("URL '%s' is invalid", url.url); + Attrs attrs; + attrs.emplace("type", type()); + attrs.emplace("owner", path[0]); + attrs.emplace("repo", path[1]); + for (auto &[name, value] : url.query) { - if (name == "rev") { - if (rev) - throw BadURL("URL '%s' contains multiple commit hashes", url.url); - rev = Hash::parseAny(value, htSHA1); - } - else if (name == "ref") { - if (!std::regex_match(value, refRegex)) - throw BadURL("URL '%s' contains an invalid branch/tag name", url.url); - if (ref) - throw BadURL("URL '%s' contains multiple branch/tag names", url.url); - ref = value; - } - else if (name == "host") { - if (!std::regex_match(value, hostRegex)) - throw BadURL("URL '%s' contains an invalid instance host", url.url); - host_url = value; + if (name == "rev" || name == "ref") { + if (refOrRev) { + throw BadURL("URL '%s' already contains a ref or rev", url.url); + } else { + refOrRev = value; + } + } else if (name == "lastModified") { + if (auto n = string2Int<uint64_t>(value)) { + attrs.emplace(name, *n); + } else { + throw Error( + "Attribute 'lastModified' in URL '%s' must be an integer", + url.to_string() + ); + } + } else { + attrs.emplace(name, value); } - // FIXME: barf on unsupported attributes } - if (ref && rev) - throw BadURL("URL '%s' contains both a commit hash and a branch/tag name %s %s", url.url, *ref, rev->gitRev()); + if (refOrRev) attrs.emplace("refOrRev", *refOrRev); - Input input; - input.attrs.insert_or_assign("type", type()); - input.attrs.insert_or_assign("owner", path[0]); - input.attrs.insert_or_assign("repo", path[1]); - if (rev) input.attrs.insert_or_assign("rev", rev->gitRev()); - if (ref) input.attrs.insert_or_assign("ref", *ref); - if (host_url) input.attrs.insert_or_assign("host", *host_url); - - return input; + return inputFromAttrs(attrs); } std::optional<Input> inputFromAttrs(const Attrs & attrs) const override { - if (maybeGetStrAttr(attrs, "type") != type()) return {}; + // Attributes can contain refOrRev and it needs to be figured out + // which one it is (see inputFromURL for when that may happen). + // The correct one (ref or rev) will be written into finalAttrs and + // it needs to be mutable for that. + Attrs finalAttrs(attrs); + auto type_ = maybeGetStrAttr(finalAttrs, "type"); + if (type_ != type()) return {}; + + auto owner = getStrAttr(finalAttrs, "owner"); + auto repo = getStrAttr(finalAttrs, "repo"); + + auto url = fmt("%s:%s/%s", *type_, owner, repo); + if (auto host = maybeGetStrAttr(finalAttrs, "host")) { + if (!std::regex_match(*host, hostRegex)) { + throw BadURL("URL '%s' contains an invalid instance host", url); + } + } - for (auto & [name, value] : attrs) - if (name != "type" && name != "owner" && name != "repo" && name != "ref" && name != "rev" && name != "narHash" && name != "lastModified" && name != "host") - throw Error("unsupported input attribute '%s'", name); + if (auto refOrRev = maybeGetStrAttr(finalAttrs, "refOrRev")) { + finalAttrs.erase("refOrRev"); + if (std::regex_match(*refOrRev, revRegex)) { + finalAttrs.emplace("rev", *refOrRev); + } else if (std::regex_match(*refOrRev, refRegex)) { + finalAttrs.emplace("ref", *refOrRev); + } else { + throw Error( + "in URL '%s', '%s' is not a commit hash or a branch/tag name", + url, + *refOrRev + ); + } + } else if (auto ref = maybeGetStrAttr(finalAttrs, "ref")) { + if (!std::regex_match(*ref, refRegex)) { + throw BadURL("URL '%s' contains an invalid branch/tag name", url); + } + } - getStrAttr(attrs, "owner"); - getStrAttr(attrs, "repo"); + for (auto & [name, value] : finalAttrs) { + if (name != "type" && name != "owner" && name != "repo" && name != "ref" && name != "rev" && name != "narHash" && name != "lastModified" && name != "host") { + throw Error("unsupported input attribute '%s'", name); + } + } Input input; - input.attrs = attrs; + input.attrs = finalAttrs; return input; } diff --git a/src/libfetchers/indirect.cc b/src/libfetchers/indirect.cc index c73505b31..8c0176e84 100644 --- a/src/libfetchers/indirect.cc +++ b/src/libfetchers/indirect.cc @@ -17,6 +17,8 @@ struct IndirectInputScheme : InputScheme std::optional<Hash> rev; std::optional<std::string> ref; + Attrs attrs; + if (path.size() == 1) { } else if (path.size() == 2) { if (std::regex_match(path[1], revRegex)) @@ -26,29 +28,21 @@ struct IndirectInputScheme : InputScheme else throw BadURL("in flake URL '%s', '%s' is not a commit hash or branch/tag name", url.url, path[1]); } else if (path.size() == 3) { - if (!std::regex_match(path[1], refRegex)) - throw BadURL("in flake URL '%s', '%s' is not a branch/tag name", url.url, path[1]); ref = path[1]; - if (!std::regex_match(path[2], revRegex)) - throw BadURL("in flake URL '%s', '%s' is not a commit hash", url.url, path[2]); rev = Hash::parseAny(path[2], htSHA1); } else throw BadURL("GitHub URL '%s' is invalid", url.url); std::string id = path[0]; - if (!std::regex_match(id, flakeRegex)) - throw BadURL("'%s' is not a valid flake ID", id); - // FIXME: forbid query params? + attrs.emplace("type", "indirect"); + attrs.emplace("id", id); + if (rev) attrs.emplace("rev", rev->gitRev()); + if (ref) attrs.emplace("ref", *ref); - Input input; - input.direct = false; - input.attrs.insert_or_assign("type", "indirect"); - input.attrs.insert_or_assign("id", id); - if (rev) input.attrs.insert_or_assign("rev", rev->gitRev()); - if (ref) input.attrs.insert_or_assign("ref", *ref); + emplaceURLQueryIntoAttrs(url, attrs, {}, {}); - return input; + return inputFromAttrs(attrs); } std::optional<Input> inputFromAttrs(const Attrs & attrs) const override @@ -63,6 +57,18 @@ struct IndirectInputScheme : InputScheme if (!std::regex_match(id, flakeRegex)) throw BadURL("'%s' is not a valid flake ID", id); + // TODO come up with a nicer error message for those two. + if (auto rev = maybeGetStrAttr(attrs, "rev")) { + if (!std::regex_match(*rev, revRegex)) { + throw BadURL("in flake '%s', '%s' is not a commit hash", id, *rev); + } + } + if (auto ref = maybeGetStrAttr(attrs, "ref")) { + if (!std::regex_match(*ref, refRegex)) { + throw BadURL("in flake '%s', '%s' is not a valid branch/tag name", id, *ref); + } + } + Input input; input.direct = false; input.attrs = attrs; diff --git a/src/libfetchers/mercurial.cc b/src/libfetchers/mercurial.cc index 4fffa71d3..b4150e9df 100644 --- a/src/libfetchers/mercurial.cc +++ b/src/libfetchers/mercurial.cc @@ -28,10 +28,9 @@ static RunOptions hgOptions(const Strings & args) } // runProgram wrapper that uses hgOptions instead of stock RunOptions. -static std::string runHg(const Strings & args, const std::optional<std::string> & input = {}) +static std::string runHg(const Strings & args) { RunOptions opts = hgOptions(args); - opts.input = input; auto res = runProgram(std::move(opts)); @@ -57,12 +56,7 @@ struct MercurialInputScheme : InputScheme Attrs attrs; attrs.emplace("type", "hg"); - for (auto &[name, value] : url.query) { - if (name == "rev" || name == "ref") - attrs.emplace(name, value); - else - url2.query.emplace(name, value); - } + emplaceURLQueryIntoAttrs(url, attrs, {"revCount"}, {}); attrs.emplace("url", url2.to_string()); diff --git a/src/libfetchers/tarball.cc b/src/libfetchers/tarball.cc index c903895e2..8dfdecda8 100644 --- a/src/libfetchers/tarball.cc +++ b/src/libfetchers/tarball.cc @@ -201,29 +201,17 @@ struct CurlInputScheme : InputScheme if (!isValidURL(_url, requireTree)) return std::nullopt; - Input input; - auto url = _url; - url.scheme = parseUrlScheme(url.scheme).transport; - - auto narHash = url.query.find("narHash"); - if (narHash != url.query.end()) - input.attrs.insert_or_assign("narHash", narHash->second); + Attrs attrs; + attrs.emplace("type", inputType()); - if (auto i = get(url.query, "rev")) - input.attrs.insert_or_assign("rev", *i); - - if (auto i = get(url.query, "revCount")) - if (auto n = string2Int<uint64_t>(*i)) - input.attrs.insert_or_assign("revCount", *n); + url.scheme = parseUrlScheme(url.scheme).transport; - url.query.erase("rev"); - url.query.erase("revCount"); + emplaceURLQueryIntoAttrs(url, attrs, {"revCount"}, {}); - input.attrs.insert_or_assign("type", inputType()); - input.attrs.insert_or_assign("url", url.to_string()); - return input; + attrs.emplace("url", url.to_string()); + return inputFromAttrs(attrs); } std::optional<Input> inputFromAttrs(const Attrs & attrs) const override @@ -235,7 +223,7 @@ struct CurlInputScheme : InputScheme std::set<std::string> allowedNames = {"type", "url", "narHash", "name", "unpack", "rev", "revCount", "lastModified"}; for (auto & [name, value] : attrs) if (!allowedNames.count(name)) - throw Error("unsupported %s input attribute '%s'", *type, name); + throw Error("unsupported %s input attribute '%s'. If you wanted to fetch a tarball with a query parameter, please use '{ type = \"tarball\"; url = \"...\"; }'", *type, name); Input input; input.attrs = attrs; diff --git a/src/libmain/common-args.cc b/src/libmain/common-args.cc index 8fcb10325..0b756301c 100644 --- a/src/libmain/common-args.cc +++ b/src/libmain/common-args.cc @@ -58,7 +58,7 @@ MixCommonArgs::MixCommonArgs(const std::string & programName) addFlag({ .longName = "log-format", - .description = "Set the format of log output; one of `raw`, `internal-json`, `bar` or `bar-with-logs`.", + .description = "Set the format of log output; one of `raw`, `internal-json`, `bar`, `bar-with-logs`, `multiline` or `multiline-with-logs`.", .category = loggingCategory, .labels = {"format"}, .handler = {[](std::string format) { setLogFormat(format); }}, diff --git a/src/libmain/loggers.cc b/src/libmain/loggers.cc index 80080d616..8c3c4e355 100644 --- a/src/libmain/loggers.cc +++ b/src/libmain/loggers.cc @@ -17,6 +17,10 @@ LogFormat parseLogFormat(const std::string & logFormatStr) { return LogFormat::bar; else if (logFormatStr == "bar-with-logs") return LogFormat::barWithLogs; + else if (logFormatStr == "multiline") + return LogFormat::multiline; + else if (logFormatStr == "multiline-with-logs") + return LogFormat::multilineWithLogs; throw Error("option 'log-format' has an invalid value '%s'", logFormatStr); } @@ -35,6 +39,17 @@ Logger * makeDefaultLogger() { logger->setPrintBuildLogs(true); return logger; } + case LogFormat::multiline: { + auto logger = makeProgressBar(); + logger->setPrintMultiline(true); + return logger; + } + case LogFormat::multilineWithLogs: { + auto logger = makeProgressBar(); + logger->setPrintMultiline(true); + logger->setPrintBuildLogs(true); + return logger; + } default: abort(); } diff --git a/src/libmain/loggers.hh b/src/libmain/loggers.hh index e5721420c..6064b6160 100644 --- a/src/libmain/loggers.hh +++ b/src/libmain/loggers.hh @@ -11,6 +11,8 @@ enum class LogFormat { internalJSON, bar, barWithLogs, + multiline, + multilineWithLogs, }; void setLogFormat(const std::string & logFormatStr); diff --git a/src/libmain/progress-bar.cc b/src/libmain/progress-bar.cc index b3466a860..28bb14863 100644 --- a/src/libmain/progress-bar.cc +++ b/src/libmain/progress-bar.cc @@ -95,8 +95,7 @@ void ProgressBar::logEI(const ErrorInfo & ei) void ProgressBar::log(State & state, Verbosity lvl, std::string_view s) { if (state.active) { - writeToStderr("\r\e[K" + filterANSIEscapes(s, !isTTY) + ANSI_NORMAL "\n"); - draw(state); + draw(state, s); } else { auto s2 = s + ANSI_NORMAL "\n"; if (!isTTY) s2 = filterANSIEscapes(s2, true); @@ -234,10 +233,14 @@ void ProgressBar::result(ActivityId act, ResultType type, const std::vector<Fiel } log(*state, lvlInfo, ANSI_FAINT + info.name.value_or("unnamed") + suffix + ANSI_NORMAL + lastLine); } else { - state->activities.erase(i->second); - info.lastLine = lastLine; - state->activities.emplace_back(info); - i->second = std::prev(state->activities.end()); + if (!printMultiline) { + state->activities.erase(i->second); + info.lastLine = lastLine; + state->activities.emplace_back(info); + i->second = std::prev(state->activities.end()); + } else { + i->second->lastLine = lastLine; + } update(*state); } } @@ -290,60 +293,93 @@ void ProgressBar::update(State & state) updateCV.notify_one(); } -std::chrono::milliseconds ProgressBar::draw(State & state) +std::chrono::milliseconds ProgressBar::draw(State & state, const std::optional<std::string_view> & s) { auto nextWakeup = A_LONG_TIME; state.haveUpdate = false; if (state.paused || !state.active) return nextWakeup; - std::string line; + auto windowSize = getWindowSize(); + auto width = windowSize.second; + if (width <= 0) { + width = std::numeric_limits<decltype(width)>::max(); + } + + if (printMultiline && (state.lastLines >= 1)) { + // FIXME: make sure this works on windows + writeToStderr(fmt("\e[G\e[%dF\e[J", state.lastLines)); + } + + state.lastLines = 0; + if (s != std::nullopt) + writeToStderr("\r\e[K" + filterANSIEscapes(s.value(), !isTTY) + ANSI_NORMAL "\n"); + + std::string line; std::string status = getStatus(state); if (!status.empty()) { line += '['; line += status; line += "]"; } + if (printMultiline && !line.empty()) { + writeToStderr(filterANSIEscapes(line, false, width) + "\n"); + state.lastLines++; + } + auto height = windowSize.first > 0 ? windowSize.first : 25; + auto moreActivities = 0; auto now = std::chrono::steady_clock::now(); + std::string activity_line; if (!state.activities.empty()) { - if (!status.empty()) line += " "; - auto i = state.activities.rbegin(); - - while (i != state.activities.rend()) { - if (i->visible && (!i->s.empty() || !i->lastLine.empty())) { - /* Don't show activities until some time has - passed, to avoid displaying very short - activities. */ - auto delay = std::chrono::milliseconds(10); - if (i->startTime + delay < now) - break; - else - nextWakeup = std::min(nextWakeup, std::chrono::duration_cast<std::chrono::milliseconds>(delay - (now - i->startTime))); + for (auto i = state.activities.begin(); i != state.activities.end(); ++i) { + if (!(i->visible && (!i->s.empty() || !i->lastLine.empty()))) { + continue; } - ++i; - } + /* Don't show activities until some time has + passed, to avoid displaying very short + activities. */ + auto delay = std::chrono::milliseconds(10); + if (i->startTime + delay >= now) { + nextWakeup = std::min( + nextWakeup, + std::chrono::duration_cast<std::chrono::milliseconds>( + delay - (now - i->startTime) + ) + ); + } + + activity_line = i->s; - if (i != state.activities.rend()) { - line += i->s; if (!i->phase.empty()) { - line += " ("; - line += i->phase; - line += ")"; + activity_line += " ("; + activity_line += i->phase; + activity_line += ")"; } if (!i->lastLine.empty()) { - if (!i->s.empty()) line += ": "; - line += i->lastLine; + if (!i->s.empty()) + activity_line += ": "; + activity_line += i->lastLine; + } + + if (printMultiline) { + if (state.lastLines < (height -1)) { + writeToStderr(filterANSIEscapes(activity_line, false, width) + "\n"); + state.lastLines++; + } else moreActivities++; } } } - auto width = getWindowSize().second; - if (width <= 0) width = std::numeric_limits<decltype(width)>::max(); + if (printMultiline && moreActivities) + writeToStderr(fmt("And %d more...", moreActivities)); - writeToStderr("\r" + filterANSIEscapes(line, false, width) + ANSI_NORMAL + "\e[K"); + if (!printMultiline) { + line += " " + activity_line; + writeToStderr("\r" + filterANSIEscapes(line, false, width) + ANSI_NORMAL + "\e[K"); + } return nextWakeup; } @@ -366,31 +402,59 @@ std::string ProgressBar::getStatus(State & state) expected = std::max(expected, act.expected); - std::string s; + std::string rendered; if (running || done || expected || failed) { - if (running) - if (expected != 0) - s = fmt(ANSI_BLUE + numberFmt + ANSI_NORMAL "/" ANSI_GREEN + numberFmt + ANSI_NORMAL "/" + numberFmt, - running / unit, done / unit, expected / unit); - else - s = fmt(ANSI_BLUE + numberFmt + ANSI_NORMAL "/" ANSI_GREEN + numberFmt + ANSI_NORMAL, - running / unit, done / unit); - else if (expected != done) - if (expected != 0) - s = fmt(ANSI_GREEN + numberFmt + ANSI_NORMAL "/" + numberFmt, - done / unit, expected / unit); - else - s = fmt(ANSI_GREEN + numberFmt + ANSI_NORMAL, done / unit); - else - s = fmt(done ? ANSI_GREEN + numberFmt + ANSI_NORMAL : numberFmt, done / unit); - s = fmt(itemFmt, s); + if (running) { + if (expected != 0) { + auto const runningPart = fmt(numberFmt, running / unit); + auto const donePart = fmt(numberFmt, done / unit); + auto const expectedPart = fmt(numberFmt, expected / unit); + rendered = fmt( + ANSI_BLUE "%s" ANSI_NORMAL "/" ANSI_GREEN "%s" ANSI_NORMAL "/%s", + runningPart, + donePart, + expectedPart + ); + } else { + auto const runningPart = fmt(numberFmt, running / unit); + auto const donePart = fmt(numberFmt, done / unit); + rendered = fmt( + ANSI_BLUE "%s" ANSI_NORMAL "/" ANSI_GREEN "%s" ANSI_NORMAL, + runningPart, + donePart + ); + } + } else if (expected != done) { + if (expected != 0) { + auto const donePart = fmt(numberFmt, done / unit); + auto const expectedPart = fmt(numberFmt, expected / unit); + rendered = fmt( + ANSI_GREEN "%s" ANSI_NORMAL "/%s", + donePart, + expectedPart + ); + } else { + auto const donePart = fmt(numberFmt, done / unit); + rendered = fmt(ANSI_GREEN "%s" ANSI_NORMAL, donePart); + } + } else { + auto const donePart = fmt(numberFmt, done / unit); + + // We only color if `done` is non-zero. + if (done) { + rendered = concatStrings(ANSI_GREEN, donePart, ANSI_NORMAL); + } else { + rendered = donePart; + } + } + rendered = fmt(itemFmt, rendered); if (failed) - s += fmt(" (" ANSI_RED "%d failed" ANSI_NORMAL ")", failed / unit); + rendered += fmt(" (" ANSI_RED "%d failed" ANSI_NORMAL ")", failed / unit); } - return s; + return rendered; }; auto showActivity = [&](ActivityType type, const std::string & itemFmt, const std::string & numberFmt = "%d", double unit = 1) { @@ -442,9 +506,8 @@ void ProgressBar::writeToStdout(std::string_view s) { auto state(state_.lock()); if (state->active) { - std::cerr << "\r\e[K"; Logger::writeToStdout(s); - draw(*state); + draw(*state, {}); } else { Logger::writeToStdout(s); } @@ -457,7 +520,7 @@ std::optional<char> ProgressBar::ask(std::string_view msg) std::cerr << fmt("\r\e[K%s ", msg); auto s = trim(readLine(STDIN_FILENO)); if (s.size() != 1) return {}; - draw(*state); + draw(*state, {}); return s[0]; } @@ -466,6 +529,11 @@ void ProgressBar::setPrintBuildLogs(bool printBuildLogs) this->printBuildLogs = printBuildLogs; } +void ProgressBar::setPrintMultiline(bool printMultiline) +{ + this->printMultiline = printMultiline; +} + Logger * makeProgressBar() { return new ProgressBar(shouldANSI()); diff --git a/src/libmain/progress-bar.hh b/src/libmain/progress-bar.hh index 76e2ed4ff..176e941e8 100644 --- a/src/libmain/progress-bar.hh +++ b/src/libmain/progress-bar.hh @@ -47,6 +47,8 @@ struct ProgressBar : public Logger std::map<ActivityType, ActivitiesByType> activitiesByType; + int lastLines = 0; + uint64_t filesLinked = 0, bytesLinked = 0; uint64_t corruptedPaths = 0, untrustedPaths = 0; @@ -63,6 +65,7 @@ struct ProgressBar : public Logger std::condition_variable quitCV, updateCV; bool printBuildLogs = false; + bool printMultiline = false; bool isTTY; ProgressBar(bool isTTY) @@ -75,7 +78,7 @@ struct ProgressBar : public Logger while (state->active) { if (!state->haveUpdate) state.wait_for(updateCV, nextWakeup); - nextWakeup = draw(*state); + nextWakeup = draw(*state, {}); state.wait_for(quitCV, std::chrono::milliseconds(50)); } }); @@ -114,7 +117,7 @@ struct ProgressBar : public Logger void update(State & state); - std::chrono::milliseconds draw(State & state); + std::chrono::milliseconds draw(State & state, const std::optional<std::string_view> & s); std::string getStatus(State & state); @@ -123,6 +126,8 @@ struct ProgressBar : public Logger std::optional<char> ask(std::string_view msg) override; void setPrintBuildLogs(bool printBuildLogs) override; + + void setPrintMultiline(bool printMultiline) override; }; Logger * makeProgressBar(); diff --git a/src/libmain/shared.cc b/src/libmain/shared.cc index f99777a20..29538a9ca 100644 --- a/src/libmain/shared.cc +++ b/src/libmain/shared.cc @@ -378,7 +378,7 @@ RunPager::RunPager() RunPager::~RunPager() { try { - if (pid != -1) { + if (pid) { std::cout.flush(); dup2(std_out, STDOUT_FILENO); pid.wait(); diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc index 46399b0a8..c0cd523e6 100644 --- a/src/libstore/build/derivation-goal.cc +++ b/src/libstore/build/derivation-goal.cc @@ -760,7 +760,7 @@ void DerivationGoal::tryToBuild() /* Not now; wait until at least one child finishes or the wake-up timeout expires. */ if (!actLock) - actLock = std::make_unique<Activity>(*logger, lvlWarn, actBuildWaiting, + actLock = std::make_unique<Activity>(*logger, lvlTalkative, actBuildWaiting, fmt("waiting for a machine to build '%s'", Magenta(worker.store.printStorePath(drvPath)))); worker.waitForAWhile(shared_from_this()); outputLocks.unlock(); diff --git a/src/libstore/build/hook-instance.cc b/src/libstore/build/hook-instance.cc index 86f72486e..9a93646f4 100644 --- a/src/libstore/build/hook-instance.cc +++ b/src/libstore/build/hook-instance.cc @@ -79,7 +79,7 @@ HookInstance::~HookInstance() { try { toHook.writeSide.reset(); - if (pid != -1) pid.kill(); + if (pid) pid.kill(); } catch (...) { ignoreException(); } diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc index bae821266..efba648a4 100644 --- a/src/libstore/build/local-derivation-goal.cc +++ b/src/libstore/build/local-derivation-goal.cc @@ -51,9 +51,6 @@ #endif #if __APPLE__ -#include <spawn.h> -#include <sys/sysctl.h> - /* This definition is undocumented but depended upon by all major browsers. */ extern "C" int sandbox_init_with_parameters(const char *profile, uint64_t flags, const char *const parameters[], char **errorbuf); #endif @@ -140,7 +137,7 @@ LocalStore & LocalDerivationGoal::getLocalStore() void LocalDerivationGoal::killChild() { - if (pid != -1) { + if (pid) { worker.childTerminated(this); /* If we're using a build user, then there is a tricky race @@ -148,7 +145,7 @@ void LocalDerivationGoal::killChild() done its setuid() to the build user uid, then it won't be killed, and we'll potentially lock up in pid.wait(). So also send a conventional kill to the child. */ - ::kill(-pid, SIGKILL); /* ignore the result */ + ::kill(-pid.get(), SIGKILL); /* ignore the result */ killSandbox(true); @@ -161,19 +158,7 @@ void LocalDerivationGoal::killChild() void LocalDerivationGoal::killSandbox(bool getStats) { - if (cgroup) { - #if __linux__ - auto stats = destroyCgroup(*cgroup); - if (getStats) { - buildResult.cpuUser = stats.cpuUser; - buildResult.cpuSystem = stats.cpuSystem; - } - #else - abort(); - #endif - } - - else if (buildUser) { + if (buildUser) { auto uid = buildUser->getUID(); assert(uid != 0); killUser(uid); @@ -945,7 +930,7 @@ void LocalDerivationGoal::startBuilder() if (usingUserNamespace) options.cloneFlags |= CLONE_NEWUSER; - pid_t child = startProcess([&]() { runChild(); }, options); + pid_t child = startProcess([&]() { runChild(); }, options).release(); writeFull(sendPid.writeSide.get(), fmt("%d\n", child)); _exit(0); @@ -966,7 +951,7 @@ void LocalDerivationGoal::startBuilder() auto ss = tokenizeString<std::vector<std::string>>(readLine(sendPid.readSide.get())); assert(ss.size() == 1); - pid = string2Int<pid_t>(ss[0]).value(); + pid = Pid{string2Int<pid_t>(ss[0]).value()}; if (usingUserNamespace) { /* Set the UID/GID mapping of the builder's user namespace @@ -976,13 +961,13 @@ void LocalDerivationGoal::startBuilder() uid_t hostGid = buildUser ? buildUser->getGID() : getgid(); uid_t nrIds = buildUser ? buildUser->getUIDCount() : 1; - writeFile("/proc/" + std::to_string(pid) + "/uid_map", + writeFile("/proc/" + std::to_string(pid.get()) + "/uid_map", fmt("%d %d %d", sandboxUid(), hostUid, nrIds)); if (!buildUser || buildUser->getUIDCount() == 1) - writeFile("/proc/" + std::to_string(pid) + "/setgroups", "deny"); + writeFile("/proc/" + std::to_string(pid.get()) + "/setgroups", "deny"); - writeFile("/proc/" + std::to_string(pid) + "/gid_map", + writeFile("/proc/" + std::to_string(pid.get()) + "/gid_map", fmt("%d %d %d", sandboxGid(), hostGid, nrIds)); } else { debug("note: not using a user namespace"); @@ -1005,19 +990,19 @@ void LocalDerivationGoal::startBuilder() /* Save the mount- and user namespace of the child. We have to do this *before* the child does a chroot. */ - sandboxMountNamespace = AutoCloseFD{open(fmt("/proc/%d/ns/mnt", (pid_t) pid).c_str(), O_RDONLY)}; + sandboxMountNamespace = AutoCloseFD{open(fmt("/proc/%d/ns/mnt", pid.get()).c_str(), O_RDONLY)}; if (sandboxMountNamespace.get() == -1) throw SysError("getting sandbox mount namespace"); if (usingUserNamespace) { - sandboxUserNamespace = AutoCloseFD{open(fmt("/proc/%d/ns/user", (pid_t) pid).c_str(), O_RDONLY)}; + sandboxUserNamespace = AutoCloseFD{open(fmt("/proc/%d/ns/user", pid.get()).c_str(), O_RDONLY)}; if (sandboxUserNamespace.get() == -1) throw SysError("getting sandbox user namespace"); } /* Move the child into its own cgroup. */ if (cgroup) - writeFile(*cgroup + "/cgroup.procs", fmt("%d", (pid_t) pid)); + writeFile(*cgroup + "/cgroup.procs", fmt("%d", pid.get())); /* Signal the builder that we've updated its user namespace. */ writeFull(userNamespaceSync.writeSide.get(), "1"); @@ -1585,7 +1570,7 @@ void LocalDerivationGoal::addDependency(const StorePath & path) entering its mount namespace, which is not possible in multithreaded programs. So we do this in a child process.*/ - Pid child(startProcess([&]() { + Pid child = startProcess([&]() { if (usingUserNamespace && (setns(sandboxUserNamespace.get(), 0) == -1)) throw SysError("entering sandbox user namespace"); @@ -1596,7 +1581,7 @@ void LocalDerivationGoal::addDependency(const StorePath & path) doBind(source, target); _exit(0); - })); + }); int status = child.wait(); if (status != 0) @@ -2177,31 +2162,8 @@ void LocalDerivationGoal::runChild() } } -#if __APPLE__ - posix_spawnattr_t attrp; - - if (posix_spawnattr_init(&attrp)) - throw SysError("failed to initialize builder"); - - if (posix_spawnattr_setflags(&attrp, POSIX_SPAWN_SETEXEC)) - throw SysError("failed to initialize builder"); - - if (drv->platform == "aarch64-darwin") { - // Unset kern.curproc_arch_affinity so we can escape Rosetta - int affinity = 0; - sysctlbyname("kern.curproc_arch_affinity", NULL, NULL, &affinity, sizeof(affinity)); - - cpu_type_t cpu = CPU_TYPE_ARM64; - posix_spawnattr_setbinpref_np(&attrp, 1, &cpu, NULL); - } else if (drv->platform == "x86_64-darwin") { - cpu_type_t cpu = CPU_TYPE_X86_64; - posix_spawnattr_setbinpref_np(&attrp, 1, &cpu, NULL); - } - - posix_spawn(NULL, drv->builder.c_str(), NULL, &attrp, stringsToCharPtrs(args).data(), stringsToCharPtrs(envStrs).data()); -#else - execve(drv->builder.c_str(), stringsToCharPtrs(args).data(), stringsToCharPtrs(envStrs).data()); -#endif + execBuilder(drv->builder, args, envStrs); + // execBuilder should not return throw SysError("executing '%1%'", drv->builder); @@ -2217,6 +2179,11 @@ void LocalDerivationGoal::runChild() } } +void LocalDerivationGoal::execBuilder(std::string builder, Strings args, Strings envStrs) +{ + execve(builder.c_str(), stringsToCharPtrs(args).data(), stringsToCharPtrs(envStrs).data()); +} + SingleDrvOutputs LocalDerivationGoal::registerOutputs() { diff --git a/src/libstore/build/local-derivation-goal.hh b/src/libstore/build/local-derivation-goal.hh index f3a83d42f..91329ca35 100644 --- a/src/libstore/build/local-derivation-goal.hh +++ b/src/libstore/build/local-derivation-goal.hh @@ -178,7 +178,28 @@ struct LocalDerivationGoal : public DerivationGoal friend struct RestrictedStore; - using DerivationGoal::DerivationGoal; + /** + * Create a LocalDerivationGoal without an on-disk .drv file, + * possibly a platform-specific subclass + */ + static std::shared_ptr<LocalDerivationGoal> makeLocalDerivationGoal( + const StorePath & drvPath, + const OutputsSpec & wantedOutputs, + Worker & worker, + BuildMode buildMode + ); + + /** + * Create a LocalDerivationGoal for an on-disk .drv file, + * possibly a platform-specific subclass + */ + static std::shared_ptr<LocalDerivationGoal> makeLocalDerivationGoal( + const StorePath & drvPath, + const BasicDerivation & drv, + const OutputsSpec & wantedOutputs, + Worker & worker, + BuildMode buildMode + ); virtual ~LocalDerivationGoal() noexcept(false) override; @@ -282,7 +303,7 @@ struct LocalDerivationGoal : public DerivationGoal * Kill any processes running under the build user UID or in the * cgroup of the build. */ - void killSandbox(bool getStats); + virtual void killSandbox(bool getStats); /** * Create alternative path calculated from but distinct from the @@ -299,6 +320,16 @@ struct LocalDerivationGoal : public DerivationGoal * rewrites caught everything */ StorePath makeFallbackPath(OutputNameView outputName); + +protected: + using DerivationGoal::DerivationGoal; + + /** + * Execute the builder, replacing the current process. + * Generally this means an `execve` call. + */ + virtual void execBuilder(std::string builder, Strings args, Strings envStrs); + }; } diff --git a/src/libstore/build/substitution-goal.cc b/src/libstore/build/substitution-goal.cc index d8d9ba283..cc4cb3c8c 100644 --- a/src/libstore/build/substitution-goal.cc +++ b/src/libstore/build/substitution-goal.cc @@ -217,6 +217,7 @@ void PathSubstitutionGoal::tryToRun() promise = std::promise<void>(); thr = std::thread([this]() { + auto & fetchPath = subPath ? *subPath : storePath; try { ReceiveInterrupts receiveInterrupts; @@ -226,10 +227,17 @@ void PathSubstitutionGoal::tryToRun() Activity act(*logger, actSubstitute, Logger::Fields{worker.store.printStorePath(storePath), sub->getUri()}); PushActivity pact(act.id); - copyStorePath(*sub, worker.store, - subPath ? *subPath : storePath, repair, sub->isTrusted ? NoCheckSigs : CheckSigs); + copyStorePath( + *sub, worker.store, fetchPath, repair, sub->isTrusted ? NoCheckSigs : CheckSigs + ); promise.set_value(); + } catch (const EndOfFile &) { + promise.set_exception(std::make_exception_ptr(EndOfFile( + "NAR for '%s' fetched from '%s' is incomplete", + sub->printStorePath(fetchPath), + sub->getUri() + ))); } catch (...) { promise.set_exception(std::current_exception()); } diff --git a/src/libstore/build/worker.cc b/src/libstore/build/worker.cc index 04be0da99..a7a298c34 100644 --- a/src/libstore/build/worker.cc +++ b/src/libstore/build/worker.cc @@ -65,8 +65,8 @@ std::shared_ptr<DerivationGoal> Worker::makeDerivationGoal(const StorePath & drv { return makeDerivationGoalCommon(drvPath, wantedOutputs, [&]() -> std::shared_ptr<DerivationGoal> { return !dynamic_cast<LocalStore *>(&store) - ? std::make_shared</* */DerivationGoal>(drvPath, wantedOutputs, *this, buildMode) - : std::make_shared<LocalDerivationGoal>(drvPath, wantedOutputs, *this, buildMode); + ? std::make_shared<DerivationGoal>(drvPath, wantedOutputs, *this, buildMode) + : LocalDerivationGoal::makeLocalDerivationGoal(drvPath, wantedOutputs, *this, buildMode); }); } @@ -76,8 +76,8 @@ std::shared_ptr<DerivationGoal> Worker::makeBasicDerivationGoal(const StorePath { return makeDerivationGoalCommon(drvPath, wantedOutputs, [&]() -> std::shared_ptr<DerivationGoal> { return !dynamic_cast<LocalStore *>(&store) - ? std::make_shared</* */DerivationGoal>(drvPath, drv, wantedOutputs, *this, buildMode) - : std::make_shared<LocalDerivationGoal>(drvPath, drv, wantedOutputs, *this, buildMode); + ? std::make_shared<DerivationGoal>(drvPath, drv, wantedOutputs, *this, buildMode) + : LocalDerivationGoal::makeLocalDerivationGoal(drvPath, drv, wantedOutputs, *this, buildMode); }); } diff --git a/src/libstore/platform.cc b/src/libstore/platform.cc index acdedab99..d10d33f0e 100644 --- a/src/libstore/platform.cc +++ b/src/libstore/platform.cc @@ -1,4 +1,5 @@ #include "local-store.hh" +#include "build/local-derivation-goal.hh" #if __linux__ #include "platform/linux.hh" @@ -19,4 +20,43 @@ std::shared_ptr<LocalStore> LocalStore::makeLocalStore(const Params & params) return std::shared_ptr<LocalStore>(new FallbackLocalStore(params)); #endif } + +std::shared_ptr<LocalDerivationGoal> LocalDerivationGoal::makeLocalDerivationGoal( + const StorePath & drvPath, + const OutputsSpec & wantedOutputs, + Worker & worker, + BuildMode buildMode +) +{ +#if __linux__ + return std::make_shared<LinuxLocalDerivationGoal>(drvPath, wantedOutputs, worker, buildMode); +#elif __APPLE__ + return std::make_shared<DarwinLocalDerivationGoal>(drvPath, wantedOutputs, worker, buildMode); +#else + return std::make_shared<FallbackLocalDerivationGoal>(drvPath, wantedOutputs, worker, buildMode); +#endif +} + +std::shared_ptr<LocalDerivationGoal> LocalDerivationGoal::makeLocalDerivationGoal( + const StorePath & drvPath, + const BasicDerivation & drv, + const OutputsSpec & wantedOutputs, + Worker & worker, + BuildMode buildMode +) +{ +#if __linux__ + return std::make_shared<LinuxLocalDerivationGoal>( + drvPath, drv, wantedOutputs, worker, buildMode + ); +#elif __APPLE__ + return std::make_shared<DarwinLocalDerivationGoal>( + drvPath, drv, wantedOutputs, worker, buildMode + ); +#else + return std::make_shared<FallbackLocalDerivationGoal>( + drvPath, drv, wantedOutputs, worker, buildMode + ); +#endif +} } diff --git a/src/libstore/platform/darwin.cc b/src/libstore/platform/darwin.cc index bbb81784c..83b4b4183 100644 --- a/src/libstore/platform/darwin.cc +++ b/src/libstore/platform/darwin.cc @@ -6,6 +6,7 @@ #include <sys/proc_info.h> #include <sys/sysctl.h> #include <libproc.h> +#include <spawn.h> #include <regex> @@ -220,4 +221,29 @@ void DarwinLocalStore::findPlatformRoots(UncheckedRoots & unchecked) } } } + +void DarwinLocalDerivationGoal::execBuilder(std::string builder, Strings args, Strings envStrs) +{ + posix_spawnattr_t attrp; + + if (posix_spawnattr_init(&attrp)) + throw SysError("failed to initialize builder"); + + if (posix_spawnattr_setflags(&attrp, POSIX_SPAWN_SETEXEC)) + throw SysError("failed to initialize builder"); + + if (drv->platform == "aarch64-darwin") { + // Unset kern.curproc_arch_affinity so we can escape Rosetta + int affinity = 0; + sysctlbyname("kern.curproc_arch_affinity", NULL, NULL, &affinity, sizeof(affinity)); + + cpu_type_t cpu = CPU_TYPE_ARM64; + posix_spawnattr_setbinpref_np(&attrp, 1, &cpu, NULL); + } else if (drv->platform == "x86_64-darwin") { + cpu_type_t cpu = CPU_TYPE_X86_64; + posix_spawnattr_setbinpref_np(&attrp, 1, &cpu, NULL); + } + + posix_spawn(NULL, builder.c_str(), NULL, &attrp, stringsToCharPtrs(args).data(), stringsToCharPtrs(envStrs).data()); +} } diff --git a/src/libstore/platform/darwin.hh b/src/libstore/platform/darwin.hh index b7170aa05..0ac7077fb 100644 --- a/src/libstore/platform/darwin.hh +++ b/src/libstore/platform/darwin.hh @@ -1,6 +1,7 @@ #pragma once ///@file +#include "build/local-derivation-goal.hh" #include "gc-store.hh" #include "local-store.hh" @@ -32,4 +33,19 @@ private: void findPlatformRoots(UncheckedRoots & unchecked) override; }; +/** + * Darwin-specific implementation of LocalDerivationGoal + */ +class DarwinLocalDerivationGoal : public LocalDerivationGoal +{ +public: + using LocalDerivationGoal::LocalDerivationGoal; + +private: + /** + * Set process flags to enter or leave rosetta, then execute the builder + */ + void execBuilder(std::string builder, Strings args, Strings envStrs) override; +}; + } diff --git a/src/libstore/platform/fallback.hh b/src/libstore/platform/fallback.hh index fd27edbe6..3b77536bb 100644 --- a/src/libstore/platform/fallback.hh +++ b/src/libstore/platform/fallback.hh @@ -1,6 +1,7 @@ #pragma once ///@file +#include "build/local-derivation-goal.hh" #include "local-store.hh" namespace nix { @@ -28,4 +29,14 @@ public: } }; +/** + * Fallback platform implementation of LocalDerivationGoal + * Exists so we can make LocalDerivationGoal constructor protected + */ +class FallbackLocalDerivationGoal : public LocalDerivationGoal +{ +public: + using LocalDerivationGoal::LocalDerivationGoal; +}; + } diff --git a/src/libstore/platform/linux.cc b/src/libstore/platform/linux.cc index a34608894..6b94c01cc 100644 --- a/src/libstore/platform/linux.cc +++ b/src/libstore/platform/linux.cc @@ -1,3 +1,4 @@ +#include "cgroup.hh" #include "gc-store.hh" #include "signals.hh" #include "platform/linux.hh" @@ -114,4 +115,17 @@ void LinuxLocalStore::findPlatformRoots(UncheckedRoots & unchecked) readFileRoots("/proc/sys/kernel/fbsplash", unchecked); readFileRoots("/proc/sys/kernel/poweroff_cmd", unchecked); } + +void LinuxLocalDerivationGoal::killSandbox(bool getStats) +{ + if (cgroup) { + auto stats = destroyCgroup(*cgroup); + if (getStats) { + buildResult.cpuUser = stats.cpuUser; + buildResult.cpuSystem = stats.cpuSystem; + } + } else { + LocalDerivationGoal::killSandbox(getStats); + } +} } diff --git a/src/libstore/platform/linux.hh b/src/libstore/platform/linux.hh index 8b97e17c5..2cad001ea 100644 --- a/src/libstore/platform/linux.hh +++ b/src/libstore/platform/linux.hh @@ -1,6 +1,7 @@ #pragma once ///@file +#include "build/local-derivation-goal.hh" #include "gc-store.hh" #include "local-store.hh" @@ -32,4 +33,16 @@ private: void findPlatformRoots(UncheckedRoots & unchecked) override; }; +/** + * Linux-specific implementation of LocalDerivationGoal + */ +class LinuxLocalDerivationGoal : public LocalDerivationGoal +{ +public: + using LocalDerivationGoal::LocalDerivationGoal; + +private: + void killSandbox(bool getStatus) override; +}; + } diff --git a/src/libstore/ssh.cc b/src/libstore/ssh.cc index 932ebaa52..0d7bfa01d 100644 --- a/src/libstore/ssh.cc +++ b/src/libstore/ssh.cc @@ -131,7 +131,7 @@ Path SSHMaster::startMaster() auto state(state_.lock()); - if (state->sshMaster != -1) return state->socketPath; + if (state->sshMaster) return state->socketPath; state->socketPath = (Path) *state->tmpDir + "/ssh.sock"; diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index 244ecf256..7c0902978 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -1072,8 +1072,6 @@ void copyStorePath( }); TeeSink tee { sink, progressSink }; srcStore.narFromPath(storePath, tee); - }, [&]() { - throw EndOfFile("NAR for '%s' fetched from '%s' is incomplete", srcStore.printStorePath(storePath), srcStore.getUri()); }); dstStore.addToStore(*info, *source, repair, checkSigs); diff --git a/src/libutil/logging.hh b/src/libutil/logging.hh index 64be8bc62..3cead4296 100644 --- a/src/libutil/logging.hh +++ b/src/libutil/logging.hh @@ -14,23 +14,71 @@ typedef enum { actRealise = 102, actCopyPaths = 103, actBuilds = 104, + + /** Fields: + * 0: string: path to store derivation being built. + * 1: string: representing the machine this is being built on. Empty string if local machine. + * 2: int: curRound, not used anymore, always 1? + * 3: int: nrRounds, not used anymore always 1? + */ actBuild = 105, actOptimiseStore = 106, actVerifyPaths = 107, + + /** Fields: + * 0: string: store path + * 1: string: substituter + */ actSubstitute = 108, + + /** Fields: + * 0: string: store path + * 1: string: substituter + */ actQueryPathInfo = 109, + + /** Fields: + * 0: string: store path + */ actPostBuildHook = 110, actBuildWaiting = 111, } ActivityType; typedef enum { + /** Fields: + * 0: int: bytes linked + */ resFileLinked = 100, + + /** Fields: + * 0: string: last line + */ resBuildLogLine = 101, resUntrustedPath = 102, resCorruptedPath = 103, + + /** Fields: + * 0: string: phase name + */ resSetPhase = 104, + + /** Fields: + * 0: int: done + * 1: int: expected + * 2: int: running + * 3: int: failed + */ resProgress = 105, + + /** Fields: + * 0: int: ActivityType + * 1: int: expected + */ resSetExpected = 106, + + /** Fields: + * 0: string: last line + */ resPostBuildLogLine = 107, } ResultType; @@ -114,6 +162,9 @@ public: virtual void setPrintBuildLogs(bool printBuildLogs) { } + + virtual void setPrintMultiline(bool printMultiline) + { } }; /** diff --git a/src/libutil/namespaces.cc b/src/libutil/namespaces.cc index 98d3cd306..247fba2c4 100644 --- a/src/libutil/namespaces.cc +++ b/src/libutil/namespaces.cc @@ -94,12 +94,7 @@ bool userNamespacesSupported() static auto res = [&]() -> bool { try { - Pid pid = startProcess([&]() - { - _exit(0); - }, { - .cloneFlags = CLONE_NEWUSER - }); + Pid pid = startProcess([&]() { _exit(0); }, {.cloneFlags = CLONE_NEWUSER}); auto r = pid.wait(); assert(!r); @@ -120,8 +115,7 @@ bool mountAndPidNamespacesSupported() { try { - Pid pid = startProcess([&]() - { + Pid pid = startProcess([&]() { /* Make sure we don't remount the parent's /proc. */ if (mount(0, "/", 0, MS_PRIVATE | MS_REC, 0) == -1) _exit(1); diff --git a/src/libutil/processes.cc b/src/libutil/processes.cc index 8639a77f8..250092393 100644 --- a/src/libutil/processes.cc +++ b/src/libutil/processes.cc @@ -35,29 +35,25 @@ Pid::Pid() } -Pid::Pid(pid_t pid) - : pid(pid) +Pid::Pid(Pid && other) : pid(other.pid), separatePG(other.separatePG), killSignal(other.killSignal) { + other.pid = -1; } -Pid::~Pid() noexcept(false) -{ - if (pid != -1) kill(); -} - - -void Pid::operator =(pid_t pid) +Pid & Pid::operator=(Pid && other) { - if (this->pid != -1 && this->pid != pid) kill(); - this->pid = pid; - killSignal = SIGKILL; // reset signal to default + Pid tmp(std::move(other)); + std::swap(pid, tmp.pid); + std::swap(separatePG, tmp.separatePG); + std::swap(killSignal, tmp.killSignal); + return *this; } -Pid::operator pid_t() +Pid::~Pid() noexcept(false) { - return pid; + if (pid != -1) kill(); } @@ -131,7 +127,7 @@ void killUser(uid_t uid) users to which the current process can send signals. So we fork a process, switch to uid, and send a mass kill. */ - Pid pid = startProcess([&]() { + Pid pid{startProcess([&]() { if (setuid(uid) == -1) throw SysError("setting uid"); @@ -153,7 +149,7 @@ void killUser(uid_t uid) } _exit(0); - }); + })}; int status = pid.wait(); if (status != 0) @@ -187,7 +183,7 @@ static int childEntry(void * arg) #endif -pid_t startProcess(std::function<void()> fun, const ProcessOptions & options) +Pid startProcess(std::function<void()> fun, const ProcessOptions & options) { std::function<void()> wrapper = [&]() { logger = makeSimpleLogger(); @@ -231,13 +227,12 @@ pid_t startProcess(std::function<void()> fun, const ProcessOptions & options) if (pid == -1) throw SysError("unable to fork"); - return pid; + return Pid{pid}; } -std::string runProgram(Path program, bool searchPath, const Strings & args, - const std::optional<std::string> & input, bool isInteractive) +std::string runProgram(Path program, bool searchPath, const Strings & args, bool isInteractive) { - auto res = runProgram(RunOptions {.program = program, .searchPath = searchPath, .args = args, .input = input, .isInteractive = isInteractive}); + auto res = runProgram(RunOptions {.program = program, .searchPath = searchPath, .args = args, .isInteractive = isInteractive}); if (!statusOk(res.first)) throw ExecError(res.first, "program '%1%' %2%", program, statusToString(res.first)); @@ -266,20 +261,9 @@ void runProgram2(const RunOptions & options) { checkInterrupt(); - assert(!(options.standardIn && options.input)); - - std::unique_ptr<Source> source_; - Source * source = options.standardIn; - - if (options.input) { - source_ = std::make_unique<StringSource>(*options.input); - source = source_.get(); - } - /* Create a pipe. */ - Pipe out, in; + Pipe out; if (options.standardOut) out.create(); - if (source) in.create(); ProcessOptions processOptions; @@ -294,7 +278,7 @@ void runProgram2(const RunOptions & options) } /* Fork. */ - Pid pid = startProcess([&]() { + Pid pid{startProcess([&]() { if (options.environment) replaceEnv(*options.environment); if (options.standardOut && dup2(out.writeSide.get(), STDOUT_FILENO) == -1) @@ -302,8 +286,6 @@ void runProgram2(const RunOptions & options) if (options.mergeStderrToStdout) if (dup2(STDOUT_FILENO, STDERR_FILENO) == -1) throw SysError("cannot dup stdout into stderr"); - if (source && dup2(in.readSide.get(), STDIN_FILENO) == -1) - throw SysError("dupping stdin"); if (options.chdir && chdir((*options.chdir).c_str()) == -1) throw SysError("chdir failed"); @@ -328,51 +310,16 @@ void runProgram2(const RunOptions & options) execv(options.program.c_str(), stringsToCharPtrs(args_).data()); throw SysError("executing '%1%'", options.program); - }, processOptions); + }, processOptions)}; out.writeSide.close(); - std::thread writerThread; - - std::promise<void> promise; - - Finally doJoin([&]() { - if (writerThread.joinable()) - writerThread.join(); - }); - - - if (source) { - in.readSide.close(); - writerThread = std::thread([&]() { - try { - std::vector<char> buf(8 * 1024); - while (true) { - size_t n; - try { - n = source->read(buf.data(), buf.size()); - } catch (EndOfFile &) { - break; - } - writeFull(in.writeSide.get(), {buf.data(), n}); - } - promise.set_value(); - } catch (...) { - promise.set_exception(std::current_exception()); - } - in.writeSide.close(); - }); - } - if (options.standardOut) drainFD(out.readSide.get(), *options.standardOut); /* Wait for the child to finish. */ int status = pid.wait(); - /* Wait for the writer thread to finish. */ - if (source) promise.get_future().get(); - if (status) throw ExecError(status, "program '%1%' %2%", options.program, statusToString(status)); } diff --git a/src/libutil/processes.hh b/src/libutil/processes.hh index b84fc7c4b..48a195481 100644 --- a/src/libutil/processes.hh +++ b/src/libutil/processes.hh @@ -26,16 +26,18 @@ class Pid int killSignal = SIGKILL; public: Pid(); - Pid(pid_t pid); + explicit Pid(pid_t pid): pid(pid) {} + Pid(Pid && other); + Pid & operator=(Pid && other); ~Pid() noexcept(false); - void operator =(pid_t pid); - operator pid_t(); + explicit operator bool() const { return pid != -1; } int kill(); int wait(); void setSeparatePG(bool separatePG); void setKillSignal(int signal); pid_t release(); + pid_t get() const { return pid; } }; /** @@ -60,7 +62,8 @@ struct ProcessOptions int cloneFlags = 0; }; -pid_t startProcess(std::function<void()> fun, const ProcessOptions & options = ProcessOptions()); +[[nodiscard]] +Pid startProcess(std::function<void()> fun, const ProcessOptions & options = ProcessOptions()); /** @@ -68,8 +71,7 @@ pid_t startProcess(std::function<void()> fun, const ProcessOptions & options = P * shell backtick operator). */ std::string runProgram(Path program, bool searchPath = false, - const Strings & args = Strings(), - const std::optional<std::string> & input = {}, bool isInteractive = false); + const Strings & args = Strings(), bool isInteractive = false); struct RunOptions { @@ -80,8 +82,6 @@ struct RunOptions std::optional<uid_t> gid; std::optional<Path> chdir; std::optional<std::map<std::string, std::string>> environment; - std::optional<std::string> input; - Source * standardIn = nullptr; Sink * standardOut = nullptr; bool mergeStderrToStdout = false; bool isInteractive = false; diff --git a/src/libutil/serialise.cc b/src/libutil/serialise.cc index 3a8a01f16..80b111f08 100644 --- a/src/libutil/serialise.cc +++ b/src/libutil/serialise.cc @@ -266,20 +266,17 @@ std::unique_ptr<FinishSink> sourceToSink(std::function<void(Source &)> fun) } -std::unique_ptr<Source> sinkToSource( - std::function<void(Sink &)> fun, - std::function<void()> eof) +std::unique_ptr<Source> sinkToSource(std::function<void(Sink &)> fun) { struct SinkToSource : Source { typedef boost::coroutines2::coroutine<std::string> coro_t; std::function<void(Sink &)> fun; - std::function<void()> eof; std::optional<coro_t::pull_type> coro; - SinkToSource(std::function<void(Sink &)> fun, std::function<void()> eof) - : fun(fun), eof(eof) + SinkToSource(std::function<void(Sink &)> fun) + : fun(fun) { } @@ -298,7 +295,9 @@ std::unique_ptr<Source> sinkToSource( }); } - if (!*coro) { eof(); abort(); } + if (!*coro) { + throw EndOfFile("coroutine has finished"); + } if (pos == cur.size()) { if (!cur.empty()) { @@ -317,7 +316,7 @@ std::unique_ptr<Source> sinkToSource( } }; - return std::make_unique<SinkToSource>(fun, eof); + return std::make_unique<SinkToSource>(fun); } diff --git a/src/libutil/serialise.hh b/src/libutil/serialise.hh index c9294ba2d..0632e3109 100644 --- a/src/libutil/serialise.hh +++ b/src/libutil/serialise.hh @@ -338,11 +338,7 @@ std::unique_ptr<FinishSink> sourceToSink(std::function<void(Source &)> fun); * Convert a function that feeds data into a Sink into a Source. The * Source executes the function as a coroutine. */ -std::unique_ptr<Source> sinkToSource( - std::function<void(Sink &)> fun, - std::function<void()> eof = []() { - throw EndOfFile("coroutine has finished"); - }); +std::unique_ptr<Source> sinkToSource(std::function<void(Sink &)> fun); void writePadding(size_t len, Sink & sink); diff --git a/src/libutil/unix-domain-socket.cc b/src/libutil/unix-domain-socket.cc index a6e46ca50..d4fc37fab 100644 --- a/src/libutil/unix-domain-socket.cc +++ b/src/libutil/unix-domain-socket.cc @@ -65,7 +65,7 @@ static void bindConnectProcHelper( if (path.size() + 1 >= sizeof(addr.sun_path)) { Pipe pipe; pipe.create(); - Pid pid = startProcess([&] { + Pid pid{startProcess([&] { try { pipe.readSide.close(); Path dir = dirOf(path); @@ -83,7 +83,7 @@ static void bindConnectProcHelper( } catch (...) { writeFull(pipe.writeSide.get(), "-1\n"); } - }); + })}; pipe.writeSide.close(); auto errNo = string2Int<int>(chomp(drainFD(pipe.readSide.get()))); if (!errNo || *errNo == -1) diff --git a/src/nix/daemon.cc b/src/nix/daemon.cc index f1cc1ee9c..a9211d64a 100644 --- a/src/nix/daemon.cc +++ b/src/nix/daemon.cc @@ -369,7 +369,7 @@ static void daemonLoop(std::optional<TrustedFlag> forceTrustClientOpt) processConnection(openUncachedStore(), from, to, trusted, NotRecursive); exit(0); - }, options); + }, options).release(); } catch (Interrupted & e) { return; diff --git a/tests/functional/fetchers.sh b/tests/functional/fetchers.sh new file mode 100644 index 000000000..0f888dc33 --- /dev/null +++ b/tests/functional/fetchers.sh @@ -0,0 +1,91 @@ +source common.sh + +requireGit + +clearStore + +testFetchTreeError() { + rawFetchTreeArg="${1?fetchTree arg missing}" + messageSubstring="${2?messageSubstring missing}" + + output="$(nix eval --impure --raw --expr "(builtins.fetchTree $rawFetchTreeArg).outPath" 2>&1)" && status=0 || status=$? + grepQuiet "$messageSubstring" <<<"$output" + test "$status" -ne 0 +} + +# github/gitlab/sourcehut fetcher input validation +for provider in github gitlab sourcehut; do + # ref/rev validation + testFetchTreeError \ + "{ type = \"$provider\"; owner = \"foo\"; repo = \"bar\"; ref = \",\"; }" \ + "URL '$provider:foo/bar' contains an invalid branch/tag name" + + testFetchTreeError \ + "\"$provider://host/foo/bar/,\"" \ + "URL '$provider:foo/bar', ',' is not a commit hash or a branch/tag name" + + testFetchTreeError \ + "\"$provider://host/foo/bar/f16d8f43dd0998cdb315a2cccf2e4d10027e7ca4?rev=abc\"" \ + "URL '$provider://host/foo/bar/f16d8f43dd0998cdb315a2cccf2e4d10027e7ca4?rev=abc' already contains a ref or rev" + + testFetchTreeError \ + "\"$provider://host/foo/bar/ref?ref=ref2\"" \ + "URL '$provider://host/foo/bar/ref?ref=ref2' already contains a ref or rev" + + # host validation + testFetchTreeError \ + "{ type = \"$provider\"; owner = \"foo\"; repo = \"bar\"; host = \"git_hub.com\"; }" \ + "URL '$provider:foo/bar' contains an invalid instance host" + + testFetchTreeError \ + "\"$provider://host/foo/bar/ref?host=git_hub.com\"" \ + "URL '$provider:foo/bar' contains an invalid instance host" + + # invalid attributes + testFetchTreeError \ + "{ type = \"$provider\"; owner = \"foo\"; repo = \"bar\"; wrong = true; }" \ + "unsupported input attribute 'wrong'" + + testFetchTreeError \ + "\"$provider://host/foo/bar/ref?wrong=1\"" \ + "unsupported input attribute 'wrong'" +done + +# unsupported attributes w/ tarball fetcher +testFetchTreeError \ + "\"https://host/foo?wrong=1\"" \ + "unsupported tarball input attribute 'wrong'. If you wanted to fetch a tarball with a query parameter, please use '{ type = \"tarball\"; url = \"...\"; }" + +# test for unsupported attributes / validation in git fetcher +testFetchTreeError \ + "\"git+https://github.com/owner/repo?invalid=1\"" \ + "unsupported Git input attribute 'invalid'" + +testFetchTreeError \ + "\"git+https://github.com/owner/repo?url=foo\"" \ + "URL 'git+https://github.com/owner/repo?url=foo' must not override url via query param!" + +testFetchTreeError \ + "\"git+https://github.com/owner/repo?ref=foo.lock\"" \ + "invalid Git branch/tag name 'foo.lock'" + +testFetchTreeError \ + "{ type = \"git\"; url =\"https://github.com/owner/repo\"; ref = \"foo.lock\"; }" \ + "invalid Git branch/tag name 'foo.lock'" + +# same for mercurial +testFetchTreeError \ + "\"hg+https://forge.tld/owner/repo?invalid=1\"" \ + "unsupported Mercurial input attribute 'invalid'" + +testFetchTreeError \ + "{ type = \"hg\"; url = \"https://forge.tld/owner/repo\"; invalid = 1; }" \ + "unsupported Mercurial input attribute 'invalid'" + +testFetchTreeError \ + "\"hg+https://forge.tld/owner/repo?ref=,\"" \ + "invalid Mercurial branch/tag name ','" + +testFetchTreeError \ + "{ type = \"hg\"; url = \"https://forge.tld/owner/repo\"; ref = \",\"; }" \ + "invalid Mercurial branch/tag name ','" diff --git a/tests/functional/flakes/flakes.sh b/tests/functional/flakes/flakes.sh index 3ef518b23..68a2fd2ce 100644 --- a/tests/functional/flakes/flakes.sh +++ b/tests/functional/flakes/flakes.sh @@ -278,6 +278,19 @@ git -C $flake3Dir commit -m 'Add nonFlakeInputs' # Check whether `nix build` works with a lockfile which is missing a # nonFlakeInputs. nix build -o $TEST_ROOT/result $flake3Dir#sth --commit-lock-file +# check that the commit message is broadly correct. we can't check for +# exact contents of the message becase the build dirs change too much. +[[ "$(git -C $flake3Dir show -s --format=format:%B)" = \ +"flake.lock: Update + +Flake lock file updates: + +"?" Added input 'nonFlake': + 'git+file://"*"/flakes/flakes/nonFlake?ref=refs/heads/master&rev="*"' "*" +"?" Added input 'nonFlakeFile': + 'path:"*"/flakes/flakes/nonFlake/README.md?lastModified="*"&narHash=sha256-cPh6hp48IOdRxVV3xGd0PDgSxgzj5N/2cK0rMPNaR4o%3D' "*" +"?" Added input 'nonFlakeFile2': + 'path:"*"/flakes/flakes/nonFlake/README.md?lastModified="*"&narHash=sha256-cPh6hp48IOdRxVV3xGd0PDgSxgzj5N/2cK0rMPNaR4o%3D' "* ]] nix build -o $TEST_ROOT/result flake3#fnord [[ $(cat $TEST_ROOT/result) = FNORD ]] diff --git a/tests/functional/meson.build b/tests/functional/meson.build index a13dee001..f1c97f996 100644 --- a/tests/functional/meson.build +++ b/tests/functional/meson.build @@ -93,6 +93,7 @@ functional_tests_scripts = [ 'fetchGitRefs.sh', 'gc-runtime.sh', 'tarball.sh', + 'fetchers.sh', 'fetchGit.sh', 'fetchurl.sh', 'fetchPath.sh', @@ -182,6 +183,7 @@ functional_tests_scripts = [ 'debugger.sh', 'test-libstoreconsumer.sh', 'extra-sandbox-profile.sh', + 'substitute-truncated-nar.sh', ] # Plugin tests require shared libraries support. diff --git a/tests/functional/repl_characterization/test-session.cc b/tests/functional/repl_characterization/test-session.cc index e59064fc5..96f101cdd 100644 --- a/tests/functional/repl_characterization/test-session.cc +++ b/tests/functional/repl_characterization/test-session.cc @@ -22,7 +22,7 @@ RunningProcess RunningProcess::start(std::string executable, Strings args) procStdout.create(); // This is separate from runProgram2 because we have different IO requirements - pid_t pid = startProcess([&]() { + auto pid = startProcess([&]() { if (dup2(procStdout.writeSide.get(), STDOUT_FILENO) == -1) { throw SysError("dupping stdout"); } @@ -42,7 +42,7 @@ RunningProcess RunningProcess::start(std::string executable, Strings args) procStdin.readSide.close(); return RunningProcess{ - .pid = pid, + .pid = std::move(pid), .procStdin = std::move(procStdin), .procStdout = std::move(procStdout), }; diff --git a/tests/functional/repl_characterization/test-session.hh b/tests/functional/repl_characterization/test-session.hh index c77cce6d5..a9ba9cf0c 100644 --- a/tests/functional/repl_characterization/test-session.hh +++ b/tests/functional/repl_characterization/test-session.hh @@ -7,13 +7,14 @@ #include <string> #include "file-descriptor.hh" +#include "processes.hh" #include "tests/terminal-code-eater.hh" namespace nix { struct RunningProcess { - pid_t pid; + Pid pid; Pipe procStdin; Pipe procStdout; diff --git a/tests/functional/substitute-truncated-nar.sh b/tests/functional/substitute-truncated-nar.sh new file mode 100644 index 000000000..1ac7efaf6 --- /dev/null +++ b/tests/functional/substitute-truncated-nar.sh @@ -0,0 +1,29 @@ +source common.sh + +BINARY_CACHE=file://$cacheDir + +build() { + nix-build --no-out-link "$@" --expr 'derivation { + name = "text"; + system = builtins.currentSystem; + builder = "/bin/sh"; + args = [ "-c" "echo some text to make the nar less empty > $out" ]; + }' +} + +path=$(build) +nix copy --to "$BINARY_CACHE" "$path" +nix-collect-garbage >/dev/null 2>&1 + +nar=0bylmx35yjy2b1b4k7gjsl7i4vc03cpmryb41grfb1mp40n3hifl.nar.xz + +[ -e $cacheDir/nar/$nar ] || fail "long nar missing?" + +xzcat $cacheDir/nar/$nar > $TEST_HOME/tmp +truncate -s $(( $(stat -c %s $TEST_HOME/tmp) - 10 )) $TEST_HOME/tmp +xz < $TEST_HOME/tmp > $cacheDir/nar/$nar + +# Copying back '$path' from the binary cache. This should fail as it is truncated +if build --option substituters "$BINARY_CACHE" --option require-sigs false -j0; then + fail "Importing a truncated nar should fail" +fi diff --git a/tests/nixos/tarball-flakes.nix b/tests/nixos/tarball-flakes.nix index ca7627bd1..5deba4a12 100644 --- a/tests/nixos/tarball-flakes.nix +++ b/tests/nixos/tarball-flakes.nix @@ -69,7 +69,7 @@ in # Check that we got redirected to the immutable URL. locked_url = info["locked"]["url"] - assert locked_url == "http://localhost/stable/${nixpkgs.rev}.tar.gz", f"{locked_url=} != http://localhost/stable/${nixpkgs.rev}.tar.gz" + assert locked_url == "http://localhost/stable/${nixpkgs.rev}.tar.gz?rev=${nixpkgs.rev}&revCount=1234", f"{locked_url=} != http://localhost/stable/${nixpkgs.rev}.tar.gz" # Check that we got the rev and revCount attributes. revision = info["revision"] diff --git a/tests/unit/libutil/serialise.cc b/tests/unit/libutil/serialise.cc new file mode 100644 index 000000000..95ae43115 --- /dev/null +++ b/tests/unit/libutil/serialise.cc @@ -0,0 +1,166 @@ +#include "serialise.hh" +#include "error.hh" +#include "fmt.hh" +#include "pos-table.hh" +#include "ref.hh" +#include "types.hh" + +#include <limits.h> +#include <gtest/gtest.h> + +#include <numeric> + +namespace nix { + +TEST(Sink, uint64_t) +{ + StringSink s; + s << 42; + ASSERT_EQ(s.s, std::string({42, 0, 0, 0, 0, 0, 0, 0})); +} + +TEST(Sink, string_view) +{ + StringSink s; + s << ""; + // clang-format off + ASSERT_EQ( + s.s, + std::string({ + // length + 0, 0, 0, 0, 0, 0, 0, 0, + // data (omitted) + }) + ); + // clang-format on + + s = {}; + s << "test"; + // clang-format off + ASSERT_EQ( + s.s, + std::string({ + // length + 4, 0, 0, 0, 0, 0, 0, 0, + // data + 't', 'e', 's', 't', + // padding + 0, 0, 0, 0, + }) + ); + // clang-format on + + s = {}; + s << "longer string"; + // clang-format off + ASSERT_EQ( + s.s, + std::string({ + // length + 13, 0, 0, 0, 0, 0, 0, 0, + // data + 'l', 'o', 'n', 'g', 'e', 'r', ' ', 's', 't', 'r', 'i', 'n', 'g', + // padding + 0, 0, 0, + }) + ); + // clang-format on +} + +TEST(Sink, StringSet) +{ + StringSink s; + s << StringSet{}; + // clang-format off + ASSERT_EQ( + s.s, + std::string({ + // length + 0, 0, 0, 0, 0, 0, 0, 0, + // data (omitted) + }) + ); + // clang-format on + + s = {}; + s << StringSet{"a", ""}; + // clang-format off + ASSERT_EQ( + s.s, + std::string({ + // length + 2, 0, 0, 0, 0, 0, 0, 0, + // data "" + 0, 0, 0, 0, 0, 0, 0, 0, + // data "a" + 1, 0, 0, 0, 0, 0, 0, 0, 'a', 0, 0, 0, 0, 0, 0, 0, + }) + ); + // clang-format on +} + +TEST(Sink, Strings) +{ + StringSink s; + s << Strings{}; + // clang-format off + ASSERT_EQ( + s.s, + std::string({ + // length + 0, 0, 0, 0, 0, 0, 0, 0, + // data (omitted) + }) + ); + // clang-format on + + s = {}; + s << Strings{"a", ""}; + // clang-format off + ASSERT_EQ( + s.s, + std::string({ + // length + 2, 0, 0, 0, 0, 0, 0, 0, + // data "a" + 1, 0, 0, 0, 0, 0, 0, 0, 'a', 0, 0, 0, 0, 0, 0, 0, + // data "" + 0, 0, 0, 0, 0, 0, 0, 0, + }) + ); + // clang-format on +} + +TEST(Sink, Error) +{ + PosTable pt; + auto o = pt.addOrigin(Pos::String{make_ref<std::string>("test")}, 4); + + StringSink s; + s << Error{ErrorInfo{ + .level = lvlInfo, + .msg = HintFmt("foo"), + .pos = pt[pt.add(o, 1)], + .traces = {{.pos = pt[pt.add(o, 2)], .hint = HintFmt("b %1%", "foo")}}, + }}; + // NOTE position of the error and all traces are ignored + // by the wire format + // clang-format off + ASSERT_EQ( + s.s, + std::string({ + 5, 0, 0, 0, 0, 0, 0, 0, 'E', 'r', 'r', 'o', 'r', 0, 0, 0, + 3, 0, 0, 0, 0, 0, 0, 0, + 5, 0, 0, 0, 0, 0, 0, 0, 'E', 'r', 'r', 'o', 'r', 0, 0, 0, + 3, 0, 0, 0, 0, 0, 0, 0, 'f', 'o', 'o', 0, 0, 0, 0, 0, + 0, 0, 0, 0, 0, 0, 0, 0, + 1, 0, 0, 0, 0, 0, 0, 0, + 0, 0, 0, 0, 0, 0, 0, 0, + 16, 0, 0, 0, 0, 0, 0, 0, + 'b', ' ', '\x1b', '[', '3', '5', ';', '1', 'm', 'f', 'o', 'o', '\x1b', '[', '0', 'm', + }) + ); + // clang-format on +} + +} diff --git a/tests/unit/meson.build b/tests/unit/meson.build index 9f19e5fd9..2b5471526 100644 --- a/tests/unit/meson.build +++ b/tests/unit/meson.build @@ -49,6 +49,7 @@ libutil_tests_sources = files( 'libutil/paths-setting.cc', 'libutil/pool.cc', 'libutil/references.cc', + 'libutil/serialise.cc', 'libutil/suggestions.cc', 'libutil/tests.cc', 'libutil/url.cc', |