From c13cbd20ab97e46eeb39939494d137a2f831b5e1 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Thu, 2 Mar 2023 14:46:28 +0100 Subject: [PATCH 1/4] Logger::writeToStdout(): Use writeFull() This ensures that write errors do not get ignored. --- src/libutil/logging.cc | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/libutil/logging.cc b/src/libutil/logging.cc index 904ba6ebe..56bdaf87a 100644 --- a/src/libutil/logging.cc +++ b/src/libutil/logging.cc @@ -32,7 +32,8 @@ void Logger::warn(const std::string & msg) void Logger::writeToStdout(std::string_view s) { - std::cout << s << "\n"; + writeFull(STDOUT_FILENO, s); + writeFull(STDOUT_FILENO, "\n"); } class SimpleLogger : public Logger @@ -84,7 +85,7 @@ public: void startActivity(ActivityId act, Verbosity lvl, ActivityType type, const std::string & s, const Fields & fields, ActivityId parent) - override + override { if (lvl <= verbosity && !s.empty()) log(lvl, s + "..."); From 989b823ac5265a7738396566fa0792f3cb55160e Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Thu, 2 Mar 2023 14:51:14 +0100 Subject: [PATCH 2/4] nix store cat: Use writeFull() Fixes #7939. --- src/nix/cat.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/nix/cat.cc b/src/nix/cat.cc index 6420a0f79..60aa66ce0 100644 --- a/src/nix/cat.cc +++ b/src/nix/cat.cc @@ -17,7 +17,7 @@ struct MixCat : virtual Args if (st.type != FSAccessor::Type::tRegular) throw Error("path '%1%' is not a regular file", path); - std::cout << accessor->readFile(path); + writeFull(STDOUT_FILENO, accessor->readFile(path)); } }; From 09f5975c6a0d8706b22f15e4c5998018e56484ae Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Thu, 2 Mar 2023 14:52:37 +0100 Subject: [PATCH 3/4] Logger::cout: Use fmt() This ensures that in cout(s), 's' does not get interpreted as a format string. --- src/libutil/logging.hh | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/libutil/logging.hh b/src/libutil/logging.hh index 4642c49f7..a68edd15a 100644 --- a/src/libutil/logging.hh +++ b/src/libutil/logging.hh @@ -102,11 +102,9 @@ public: virtual void writeToStdout(std::string_view s); template - inline void cout(const std::string & fs, const Args & ... args) + inline void cout(const Args & ... args) { - boost::format f(fs); - formatHelper(f, args...); - writeToStdout(f.str()); + writeToStdout(fmt(args...)); } virtual std::optional ask(std::string_view s) From b69a73a2305aaee2a4da73da601dbde3f1ddd7a6 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Thu, 2 Mar 2023 15:02:24 +0100 Subject: [PATCH 4/4] Get rid of some unchecked calls to std::cout --- src/nix/build.cc | 4 ++-- src/nix/describe-stores.cc | 2 +- src/nix/diff-closures.cc | 2 +- src/nix/eval.cc | 4 ++-- src/nix/flake.cc | 2 +- src/nix/log.cc | 2 +- src/nix/ls.cc | 2 +- src/nix/main.cc | 12 ++++++------ src/nix/make-content-addressed.cc | 2 +- src/nix/prefetch.cc | 4 ++-- src/nix/profile.cc | 16 ++++++++-------- src/nix/realisation.cc | 12 +++++------- src/nix/search.cc | 5 ++--- src/nix/show-derivation.cc | 2 +- src/nix/sigs.cc | 4 ++-- 15 files changed, 36 insertions(+), 39 deletions(-) diff --git a/src/nix/build.cc b/src/nix/build.cc index 12b22d999..f4f2ec81d 100644 --- a/src/nix/build.cc +++ b/src/nix/build.cc @@ -139,11 +139,11 @@ struct CmdBuild : InstallablesCommand, MixDryRun, MixJSON, MixProfile for (auto & buildable : buildables) { std::visit(overloaded { [&](const BuiltPath::Opaque & bo) { - std::cout << store->printStorePath(bo.path) << std::endl; + logger->cout(store->printStorePath(bo.path)); }, [&](const BuiltPath::Built & bfd) { for (auto & output : bfd.outputs) { - std::cout << store->printStorePath(output.second) << std::endl; + logger->cout(store->printStorePath(output.second)); } }, }, buildable.path.raw()); diff --git a/src/nix/describe-stores.cc b/src/nix/describe-stores.cc index 1dd384c0e..eafcedd1f 100644 --- a/src/nix/describe-stores.cc +++ b/src/nix/describe-stores.cc @@ -25,7 +25,7 @@ struct CmdDescribeStores : Command, MixJSON res[storeName] = storeConfig->toJSON(); } if (json) { - std::cout << res; + logger->cout("%s", res); } else { for (auto & [storeName, storeConfig] : res.items()) { std::cout << "## " << storeName << std::endl << std::endl; diff --git a/src/nix/diff-closures.cc b/src/nix/diff-closures.cc index 3489cc132..c7c37b66f 100644 --- a/src/nix/diff-closures.cc +++ b/src/nix/diff-closures.cc @@ -97,7 +97,7 @@ void printClosureDiff( items.push_back(fmt("%s → %s", showVersions(removed), showVersions(added))); if (showDelta) items.push_back(fmt("%s%+.1f KiB" ANSI_NORMAL, sizeDelta > 0 ? ANSI_RED : ANSI_GREEN, sizeDelta / 1024.0)); - std::cout << fmt("%s%s: %s\n", indent, name, concatStringsSep(", ", items)); + logger->cout("%s%s: %s", indent, name, concatStringsSep(", ", items)); } } } diff --git a/src/nix/eval.cc b/src/nix/eval.cc index a579213fd..209fd3ed2 100644 --- a/src/nix/eval.cc +++ b/src/nix/eval.cc @@ -112,11 +112,11 @@ struct CmdEval : MixJSON, InstallableCommand, MixReadOnlyOption else if (raw) { stopProgressBar(); - std::cout << *state->coerceToString(noPos, *v, context, "while generating the eval command output"); + writeFull(STDOUT_FILENO, *state->coerceToString(noPos, *v, context, "while generating the eval command output")); } else if (json) { - std::cout << printValueAsJSON(*state, true, *v, pos, context, false).dump() << std::endl; + logger->cout("%s", printValueAsJSON(*state, true, *v, pos, context, false)); } else { diff --git a/src/nix/flake.cc b/src/nix/flake.cc index 053a9c9e1..3fe093fc7 100644 --- a/src/nix/flake.cc +++ b/src/nix/flake.cc @@ -952,7 +952,7 @@ struct CmdFlakeArchive : FlakeCommand, MixJSON, MixDryRun {"path", store->printStorePath(flake.flake.sourceInfo->storePath)}, {"inputs", traverse(*flake.lockFile.root)}, }; - std::cout << jsonRoot.dump() << std::endl; + logger->cout("%s", jsonRoot); } else { traverse(*flake.lockFile.root); } diff --git a/src/nix/log.cc b/src/nix/log.cc index a0598ca13..0c9f778f0 100644 --- a/src/nix/log.cc +++ b/src/nix/log.cc @@ -53,7 +53,7 @@ struct CmdLog : InstallableCommand if (!log) continue; stopProgressBar(); printInfo("got build log for '%s' from '%s'", installable->what(), logSub.getUri()); - std::cout << *log; + writeFull(STDOUT_FILENO, *log); return; } diff --git a/src/nix/ls.cc b/src/nix/ls.cc index e964b01b3..c990a303c 100644 --- a/src/nix/ls.cc +++ b/src/nix/ls.cc @@ -93,7 +93,7 @@ struct MixLs : virtual Args, MixJSON if (json) { if (showDirectory) throw UsageError("'--directory' is useless with '--json'"); - std::cout << listNar(accessor, path, recursive); + logger->cout("%s", listNar(accessor, path, recursive)); } else listText(accessor); } diff --git a/src/nix/main.cc b/src/nix/main.cc index d3d2f5b16..53bf649d4 100644 --- a/src/nix/main.cc +++ b/src/nix/main.cc @@ -292,7 +292,7 @@ void mainWrapped(int argc, char * * argv) NixArgs args; if (argc == 2 && std::string(argv[1]) == "__dump-args") { - std::cout << args.toJSON().dump() << "\n"; + logger->cout("%s", args.toJSON()); return; } @@ -312,7 +312,7 @@ void mainWrapped(int argc, char * * argv) b["doc"] = trim(stripIndentation(primOp->doc)); res[state.symbols[builtin.name]] = std::move(b); } - std::cout << res.dump() << "\n"; + logger->cout("%s", res); return; } @@ -321,14 +321,14 @@ void mainWrapped(int argc, char * * argv) if (completions) { switch (completionType) { case ctNormal: - std::cout << "normal\n"; break; + logger->cout("normal"); break; case ctFilenames: - std::cout << "filenames\n"; break; + logger->cout("filenames"); break; case ctAttrs: - std::cout << "attrs\n"; break; + logger->cout("attrs"); break; } for (auto & s : *completions) - std::cout << s.completion << "\t" << trim(s.description) << "\n"; + logger->cout(s.completion + "\t" + trim(s.description)); } }); diff --git a/src/nix/make-content-addressed.cc b/src/nix/make-content-addressed.cc index d86b90fc7..6693c55ac 100644 --- a/src/nix/make-content-addressed.cc +++ b/src/nix/make-content-addressed.cc @@ -45,7 +45,7 @@ struct CmdMakeContentAddressed : virtual CopyCommand, virtual StorePathsCommand, } auto json = json::object(); json["rewrites"] = jsonRewrites; - std::cout << json.dump(); + logger->cout("%s", json); } else { for (auto & path : storePaths) { auto i = remappings.find(path); diff --git a/src/nix/prefetch.cc b/src/nix/prefetch.cc index fc3823406..51c8a3319 100644 --- a/src/nix/prefetch.cc +++ b/src/nix/prefetch.cc @@ -234,9 +234,9 @@ static int main_nix_prefetch_url(int argc, char * * argv) if (!printPath) printInfo("path is '%s'", store->printStorePath(storePath)); - std::cout << printHash16or32(hash) << std::endl; + logger->cout(printHash16or32(hash)); if (printPath) - std::cout << store->printStorePath(storePath) << std::endl; + logger->cout(store->printStorePath(storePath)); return 0; } diff --git a/src/nix/profile.cc b/src/nix/profile.cc index 711fbe2f0..eef33b3d9 100644 --- a/src/nix/profile.cc +++ b/src/nix/profile.cc @@ -228,12 +228,12 @@ struct ProfileManifest while (i != prevElems.end() || j != curElems.end()) { if (j != curElems.end() && (i == prevElems.end() || i->describe() > j->describe())) { - std::cout << fmt("%s%s: ∅ -> %s\n", indent, j->describe(), j->versions()); + logger->cout("%s%s: ∅ -> %s", indent, j->describe(), j->versions()); changes = true; ++j; } else if (i != prevElems.end() && (j == curElems.end() || i->describe() < j->describe())) { - std::cout << fmt("%s%s: %s -> ∅\n", indent, i->describe(), i->versions()); + logger->cout("%s%s: %s -> ∅", indent, i->describe(), i->versions()); changes = true; ++i; } @@ -241,7 +241,7 @@ struct ProfileManifest auto v1 = i->versions(); auto v2 = j->versions(); if (v1 != v2) { - std::cout << fmt("%s%s: %s -> %s\n", indent, i->describe(), v1, v2); + logger->cout("%s%s: %s -> %s", indent, i->describe(), v1, v2); changes = true; } ++i; @@ -250,7 +250,7 @@ struct ProfileManifest } if (!changes) - std::cout << fmt("%sNo changes.\n", indent); + logger->cout("%sNo changes.", indent); } }; @@ -640,9 +640,9 @@ struct CmdProfileDiffClosures : virtual StoreCommand, MixDefaultProfile for (auto & gen : gens) { if (prevGen) { - if (!first) std::cout << "\n"; + if (!first) logger->cout(""); first = false; - std::cout << fmt("Version %d -> %d:\n", prevGen->number, gen.number); + logger->cout("Version %d -> %d:", prevGen->number, gen.number); printClosureDiff(store, store->followLinksToStorePath(prevGen->path), store->followLinksToStorePath(gen.path), @@ -678,10 +678,10 @@ struct CmdProfileHistory : virtual StoreCommand, EvalCommand, MixDefaultProfile for (auto & gen : gens) { ProfileManifest manifest(*getEvalState(), gen.path); - if (!first) std::cout << "\n"; + if (!first) logger->cout(""); first = false; - std::cout << fmt("Version %s%d" ANSI_NORMAL " (%s)%s:\n", + logger->cout("Version %s%d" ANSI_NORMAL " (%s)%s:", gen.number == curGen ? ANSI_GREEN : ANSI_BOLD, gen.number, std::put_time(std::gmtime(&gen.creationTime), "%Y-%m-%d"), diff --git a/src/nix/realisation.cc b/src/nix/realisation.cc index c9a7157cd..0d3466515 100644 --- a/src/nix/realisation.cc +++ b/src/nix/realisation.cc @@ -65,18 +65,16 @@ struct CmdRealisationInfo : BuiltPathsCommand, MixJSON res.push_back(currentPath); } - std::cout << res.dump(); + logger->cout("%s", res); } else { for (auto & path : realisations) { if (auto realisation = std::get_if(&path.raw)) { - std::cout << - realisation->id.to_string() << " " << - store->printStorePath(realisation->outPath); + logger->cout("%s %s", + realisation->id.to_string(), + store->printStorePath(realisation->outPath)); } else - std::cout << store->printStorePath(path.path()); - - std::cout << std::endl; + logger->cout("%s", store->printStorePath(path.path())); } } } diff --git a/src/nix/search.cc b/src/nix/search.cc index 4fa1e7837..2e38f7e4b 100644 --- a/src/nix/search.cc +++ b/src/nix/search.cc @@ -196,9 +196,8 @@ struct CmdSearch : InstallableCommand, MixJSON for (auto & cursor : installable->getCursors(*state)) visit(*cursor, cursor->getAttrPath(), true); - if (json) { - std::cout << jsonOut->dump() << std::endl; - } + if (json) + logger->cout("%s", *jsonOut); if (!json && !results) throw Error("no results for the given search term(s)!"); diff --git a/src/nix/show-derivation.cc b/src/nix/show-derivation.cc index d1a516cad..520e8b1ce 100644 --- a/src/nix/show-derivation.cc +++ b/src/nix/show-derivation.cc @@ -57,7 +57,7 @@ struct CmdShowDerivation : InstallablesCommand jsonRoot[store->printStorePath(drvPath)] = store->readDerivation(drvPath).toJSON(*store); } - std::cout << jsonRoot.dump(2) << std::endl; + logger->cout(jsonRoot.dump(2)); } }; diff --git a/src/nix/sigs.cc b/src/nix/sigs.cc index 3d659d6d2..ee27e3725 100644 --- a/src/nix/sigs.cc +++ b/src/nix/sigs.cc @@ -173,7 +173,7 @@ struct CmdKeyGenerateSecret : Command if (!keyName) throw UsageError("required argument '--key-name' is missing"); - std::cout << SecretKey::generate(*keyName).to_string(); + writeFull(STDOUT_FILENO, SecretKey::generate(*keyName).to_string()); } }; @@ -194,7 +194,7 @@ struct CmdKeyConvertSecretToPublic : Command void run() override { SecretKey secretKey(drainFD(STDIN_FILENO)); - std::cout << secretKey.toPublicKey().to_string(); + writeFull(STDOUT_FILENO, secretKey.toPublicKey().to_string()); } };