From 21d72035370ec68622ec05c4a88f5de854ab47f8 Mon Sep 17 00:00:00 2001 From: Rebecca Turner Date: Sat, 3 Feb 2024 16:17:19 -0800 Subject: [PATCH 1/2] Merge `libmain/loggers` into `libutil/logging` --- src/libmain/common-args.cc | 1 - src/libmain/loggers.cc | 56 ---------------------- src/libmain/loggers.hh | 21 --------- src/libmain/shared.cc | 1 - src/libutil/local.mk | 11 ++++- src/libutil/logging.cc | 97 ++++++++++++++++++++++++++++++++++++++ src/libutil/logging.hh | 37 +++++++++++++++ src/nix/main.cc | 1 - 8 files changed, 144 insertions(+), 81 deletions(-) delete mode 100644 src/libmain/loggers.cc delete mode 100644 src/libmain/loggers.hh diff --git a/src/libmain/common-args.cc b/src/libmain/common-args.cc index 5b49aaabcef..db6a5de3860 100644 --- a/src/libmain/common-args.cc +++ b/src/libmain/common-args.cc @@ -2,7 +2,6 @@ #include "args/root.hh" #include "globals.hh" #include "logging.hh" -#include "loggers.hh" #include "util.hh" namespace nix { diff --git a/src/libmain/loggers.cc b/src/libmain/loggers.cc deleted file mode 100644 index 9829859de32..00000000000 --- a/src/libmain/loggers.cc +++ /dev/null @@ -1,56 +0,0 @@ -#include "loggers.hh" -#include "environment-variables.hh" -#include "progress-bar.hh" - -namespace nix { - -LogFormat defaultLogFormat = LogFormat::raw; - -LogFormat parseLogFormat(const std::string & logFormatStr) { - if (logFormatStr == "raw" || getEnv("NIX_GET_COMPLETIONS")) - return LogFormat::raw; - else if (logFormatStr == "raw-with-logs") - return LogFormat::rawWithLogs; - else if (logFormatStr == "internal-json") - return LogFormat::internalJSON; - else if (logFormatStr == "bar") - return LogFormat::bar; - else if (logFormatStr == "bar-with-logs") - return LogFormat::barWithLogs; - throw Error("option 'log-format' has an invalid value '%s'", logFormatStr); -} - -Logger * makeDefaultLogger() { - switch (defaultLogFormat) { - case LogFormat::raw: - return makeSimpleLogger(false); - case LogFormat::rawWithLogs: - return makeSimpleLogger(true); - case LogFormat::internalJSON: - return makeJSONLogger(*makeSimpleLogger(true)); - case LogFormat::bar: - return makeProgressBar(); - case LogFormat::barWithLogs: { - auto logger = makeProgressBar(); - logger->setPrintBuildLogs(true); - return logger; - } - default: - abort(); - } -} - -void setLogFormat(const std::string & logFormatStr) { - setLogFormat(parseLogFormat(logFormatStr)); -} - -void setLogFormat(const LogFormat & logFormat) { - defaultLogFormat = logFormat; - createDefaultLogger(); -} - -void createDefaultLogger() { - logger = makeDefaultLogger(); -} - -} diff --git a/src/libmain/loggers.hh b/src/libmain/loggers.hh deleted file mode 100644 index e5721420cb6..00000000000 --- a/src/libmain/loggers.hh +++ /dev/null @@ -1,21 +0,0 @@ -#pragma once -///@file - -#include "types.hh" - -namespace nix { - -enum class LogFormat { - raw, - rawWithLogs, - internalJSON, - bar, - barWithLogs, -}; - -void setLogFormat(const std::string & logFormatStr); -void setLogFormat(const LogFormat & logFormat); - -void createDefaultLogger(); - -} diff --git a/src/libmain/shared.cc b/src/libmain/shared.cc index 862ef355b95..5467ba26738 100644 --- a/src/libmain/shared.cc +++ b/src/libmain/shared.cc @@ -3,7 +3,6 @@ #include "shared.hh" #include "store-api.hh" #include "gc-store.hh" -#include "loggers.hh" #include "progress-bar.hh" #include "signals.hh" diff --git a/src/libutil/local.mk b/src/libutil/local.mk index 200026c1e06..6f7236ee9e8 100644 --- a/src/libutil/local.mk +++ b/src/libutil/local.mk @@ -8,7 +8,16 @@ libutil_SOURCES := $(wildcard $(d)/*.cc $(d)/signature/*.cc) libutil_CXXFLAGS += -I src/libutil -libutil_LDFLAGS += $(THREAD_LDFLAGS) $(LIBCURL_LIBS) $(SODIUM_LIBS) $(OPENSSL_LIBS) $(LIBBROTLI_LIBS) $(LIBARCHIVE_LIBS) $(BOOST_LDFLAGS) -lboost_context +libutil_LDFLAGS += \ + $(THREAD_LDFLAGS) \ + $(LIBCURL_LIBS) \ + $(SODIUM_LIBS) \ + $(OPENSSL_LIBS) \ + $(LIBBROTLI_LIBS) \ + $(LIBARCHIVE_LIBS) \ + $(BOOST_LDFLAGS) \ + -lboost_context \ + $(libmain_LDFLAGS) $(foreach i, $(wildcard $(d)/args/*.hh), \ $(eval $(call install-file-in, $(i), $(includedir)/nix/args, 0644))) diff --git a/src/libutil/logging.cc b/src/libutil/logging.cc index d68ddacc0c1..886400a0a1c 100644 --- a/src/libutil/logging.cc +++ b/src/libutil/logging.cc @@ -333,4 +333,101 @@ Activity::~Activity() } } +LogFormat defaultLogFormat = LogFormat::raw; + +std::optional parseLogFormat(const std::string & str) +{ + if (str == "raw" || getEnv("NIX_GET_COMPLETIONS")) + return LogFormat::raw; + else if (str == "raw-with-logs") + return LogFormat::rawWithLogs; + else if (str == "internal-json") + return LogFormat::internalJSON; + else if (str == "bar") + return LogFormat::bar; + else if (str == "bar-with-logs") + return LogFormat::barWithLogs; + else + return std::nullopt; +} + +std::ostream & operator<<(std::ostream & output, const LogFormat & format) +{ + switch (format) { + case LogFormat::raw: + return output << "raw"; + case LogFormat::rawWithLogs: + return output << "raw-with-logs"; + case LogFormat::internalJSON: + return output << "internal-json"; + case LogFormat::bar: + return output << "bar"; + case LogFormat::barWithLogs: + return output << "bar-with-logs"; + default: + abort(); + } + return output; +} + +std::string logFormatToString(const LogFormat & format) +{ + std::ostringstream stream; + stream << format; + return stream.str(); +} + +void to_json(nlohmann::json &j, const LogFormat &format) +{ + j = logFormatToString(format); +} + +void from_json(const nlohmann::json &j, LogFormat &format) +{ + auto parsed = parseLogFormat(j.template get()); + if (parsed.has_value()) { + format = parsed.value(); + } else { + // Error ID 302 seems to fit best: + // "During implicit or explicit value conversion, the JSON type + // must be compatible to the target type." + throw nlohmann::detail::type_error::create(302, "string is not a valid log-format", &j); + } +} + +Logger * makeDefaultLogger() +{ + switch (defaultLogFormat) { + case LogFormat::raw: + return makeSimpleLogger(false); + case LogFormat::rawWithLogs: + return makeSimpleLogger(true); + case LogFormat::internalJSON: + return makeJSONLogger(*makeSimpleLogger(true)); + case LogFormat::bar: + return makeProgressBar(); + case LogFormat::barWithLogs: { + auto logger = makeProgressBar(); + logger->setPrintBuildLogs(true); + return logger; + } + default: + abort(); + } +} + +void setLogFormat(const LogFormat & logFormat) +{ + if (defaultLogFormat == logFormat) { + return; + } + defaultLogFormat = logFormat; + createDefaultLogger(); +} + +void createDefaultLogger() +{ + logger = makeDefaultLogger(); +} + } diff --git a/src/libutil/logging.hh b/src/libutil/logging.hh index 183f2d8e13e..54d7fa83911 100644 --- a/src/libutil/logging.hh +++ b/src/libutil/logging.hh @@ -6,9 +6,44 @@ #include "config.hh" #include +#include namespace nix { +enum class LogFormat { + /** + * The format used by old-style commands like `nix-build`, but without build + * logs. + */ + raw, + /** + * The format used by old-style commands like `nix-build`. + */ + rawWithLogs, + /** + * A JSON format for internal use. + */ + internalJSON, + /** + * The progress bar format used by `nix` (v3) commands. + */ + bar, + /** + * The progress bar format used by `nix` (v3) commands, but with build logs. + */ + barWithLogs, +}; + +std::ostream & operator<<(std::ostream & output, const LogFormat & format); +std::string logFormatToString(const LogFormat & format); +std::optional parseLogFormat(const std::string & str); + +void to_json(nlohmann::json & j, const LogFormat & format); +void from_json(const nlohmann::json & j, LogFormat & format); + +void setLogFormat(const LogFormat & logFormat); +void createDefaultLogger(); + typedef enum { actUnknown = 0, actCopyPath = 100, @@ -175,6 +210,8 @@ Logger * makeSimpleLogger(bool printBuildLogs = true); Logger * makeJSONLogger(Logger & prevLogger); +Logger * makeProgressBar(); + std::optional parseJSONMessage(const std::string & msg); bool handleJSONLogMessage(nlohmann::json & json, diff --git a/src/nix/main.cc b/src/nix/main.cc index 39c04069be6..9cac407716d 100644 --- a/src/nix/main.cc +++ b/src/nix/main.cc @@ -13,7 +13,6 @@ #include "store-api.hh" #include "filetransfer.hh" #include "finally.hh" -#include "loggers.hh" #include "markdown.hh" #include "memory-input-accessor.hh" From 4c35ede5479254ea3776876d40d3182fca03971d Mon Sep 17 00:00:00 2001 From: Rebecca Turner Date: Sat, 3 Feb 2024 16:18:00 -0800 Subject: [PATCH 2/2] Convert `--log-format` CLI argument to setting --- src/libmain/common-args.cc | 8 ------ src/libmain/shared.cc | 2 +- src/libutil/config-impl.hh | 56 ++++++++++++++++++++++++++++++++++++++ src/libutil/config.cc | 2 ++ src/libutil/json-utils.hh | 4 +++ src/libutil/logging.hh | 20 ++++++++++++++ src/nix/main.cc | 37 +++++++++++++++++-------- 7 files changed, 108 insertions(+), 21 deletions(-) diff --git a/src/libmain/common-args.cc b/src/libmain/common-args.cc index db6a5de3860..ff258112adf 100644 --- a/src/libmain/common-args.cc +++ b/src/libmain/common-args.cc @@ -55,14 +55,6 @@ 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`.", - .category = loggingCategory, - .labels = {"format"}, - .handler = {[](std::string format) { setLogFormat(format); }}, - }); - addFlag({ .longName = "max-jobs", .shortName = 'j', diff --git a/src/libmain/shared.cc b/src/libmain/shared.cc index 5467ba26738..9cf9d8c4ad7 100644 --- a/src/libmain/shared.cc +++ b/src/libmain/shared.cc @@ -182,7 +182,7 @@ LegacyArgs::LegacyArgs(const std::string & programName, .longName = "no-build-output", .shortName = 'Q', .description = "Do not show build output.", - .handler = {[&]() {setLogFormat(LogFormat::raw); }}, + .handler = {[&]() {loggerSettings.logFormat.assign(LogFormat::raw); }}, }); addFlag({ diff --git a/src/libutil/config-impl.hh b/src/libutil/config-impl.hh index 6431ec1e0f2..aac7d40dff8 100644 --- a/src/libutil/config-impl.hh +++ b/src/libutil/config-impl.hh @@ -132,4 +132,60 @@ std::string Setting::to_string() const return std::to_string(value); } +template<> +LogFormat Setting::parse(const std::string & str) const +{ + auto format = parseLogFormat(str); + if (format.has_value()) { + return format.value(); + } else { + throw UsageError("setting '%s' has invalid value '%s'", name, str); + } +} + +template<> +std::string Setting::to_string() const +{ + return logFormatToString(value); +} + +template<> +void Setting::convertToArg(Args &args, const std::string &category) +{ + args.addFlag({ + .longName = name, + .description = fmt("Set the `%s` setting.", name), + .category = category, + .labels = {"format"}, + .handler = {[this](std::string format) { override(parse(format)); }}, + .experimentalFeature = experimentalFeature, + }); +} + +template<> +std::optional Setting>::parse(const std::string & str) const +{ + if (str.empty()) { + return std::nullopt; + } + + auto format = parseLogFormat(str); + if (format.has_value()) { + return format.value(); + } else { + throw UsageError("setting '%s' has invalid value '%s'", name, str); + } +} + +template<> +std::string Setting>::to_string() const +{ + if (value.has_value()) { + return logFormatToString(value.value()); + } else { + // TODO: Will returning the empty string here cause problems? + return ""; + } +} + } diff --git a/src/libutil/config.cc b/src/libutil/config.cc index 338b6ff0994..18acaf65ad9 100644 --- a/src/libutil/config.cc +++ b/src/libutil/config.cc @@ -387,6 +387,8 @@ template class Setting; template class Setting; template class Setting; template class Setting>; +template class Setting; +template class Setting>; static Path parsePath(const AbstractSetting & s, const std::string & str) { diff --git a/src/libutil/json-utils.hh b/src/libutil/json-utils.hh index 06dd80cf7d0..73f84cc5cf0 100644 --- a/src/libutil/json-utils.hh +++ b/src/libutil/json-utils.hh @@ -64,6 +64,10 @@ struct json_avoids_null> : std::true_type {}; template struct json_avoids_null> : std::true_type {}; +enum class LogFormat; +template<> +struct json_avoids_null : std::true_type {}; + } namespace nlohmann { diff --git a/src/libutil/logging.hh b/src/libutil/logging.hh index 54d7fa83911..97fcdb59744 100644 --- a/src/libutil/logging.hh +++ b/src/libutil/logging.hh @@ -83,6 +83,26 @@ struct LoggerSettings : Config Whether Nix should print out a stack trace in case of Nix expression evaluation errors. )"}; + + Setting logFormat{ + this, LogFormat::raw, "log-format", + R"( + The log format to use. + One of `raw`, `raw-with-logs`, `internal-json`, `bar`, or `bar-with-logs`. + )", + {}, + false, + std::nullopt, + {[](const LogFormat & value) { + setLogFormat(value); + }}}; + + Setting> logFormatLegacy{ + this, std::nullopt, "log-format-legacy", + R"( + The log format to use for legacy commands like `nix-build` and `nix-shell`. + One of `raw`, `raw-with-logs`, `internal-json`, `bar`, or `bar-with-logs`. + )"}; }; extern LoggerSettings loggerSettings; diff --git a/src/nix/main.cc b/src/nix/main.cc index 9cac407716d..46226acd2ad 100644 --- a/src/nix/main.cc +++ b/src/nix/main.cc @@ -322,6 +322,22 @@ void mainWrapped(int argc, char * * argv) return; } + programPath = argv[0]; + auto programName = std::string(baseNameOf(programPath)); + auto legacy = (*RegisterLegacyCommand::commands)[programName]; + + if (!legacy) { + // New-style commands default to `bar` logs. + // `initNix()` reads configuration files and will override this setting + // if it's set. + loggerSettings.logFormat.assign(LogFormat::bar); + } + + if (argc > 1 && std::string_view(argv[1]) == "__build-remote") { + programName = "build-remote"; + argv++; argc--; + } + initNix(); initGC(); @@ -337,22 +353,19 @@ void mainWrapped(int argc, char * * argv) Finally f([] { logger->stop(); }); - programPath = argv[0]; - auto programName = std::string(baseNameOf(programPath)); - - if (argc > 1 && std::string_view(argv[1]) == "__build-remote") { - programName = "build-remote"; - argv++; argc--; - } - - { - auto legacy = (*RegisterLegacyCommand::commands)[programName]; - if (legacy) return legacy(argc, argv); + if (legacy) { + // If we're in a legacy command and `logFormatLegacy` has a value, use + // that for the log format setting. + if (loggerSettings.logFormatLegacy.get().has_value()) { + loggerSettings.logFormat.assign( + loggerSettings.logFormatLegacy.get().value() + ); + } + return legacy(argc, argv); } evalSettings.pureEval = true; - setLogFormat("bar"); settings.verboseBuild = false; if (isatty(STDERR_FILENO)) { verbosity = lvlNotice;