Skip to content

Commit

Permalink
Make loggers pluggable
Browse files Browse the repository at this point in the history
Thanks to some precision guidance by regnat, we can remove libstore's
dependency on libmain by just making the logging mechanisms pluggable.

This:
* moves loggers.{hh,cc} from libmain to libutil
* removes the now-unnecessary (and incorrect) `toJSON` specialization
* moves LogFormat from types.hh to loggers.hh
* adds a completer (untested)

To add another logger, the following steps should be all that's
necessary:
1) Add enum variant in libutil/loggers.hh and name to the `std::set` in
loggers.cc
2) Modify the specialized `set` and `to_string` methods to include the
new variant
3) Use the `OnStartup` class to `registerLogger` the new logger method

Co-authored-by: regnat <rg@regnat.ovh>
  • Loading branch information
cole-h and thufschmitt committed Sep 18, 2020
1 parent 85f6568 commit 6efa529
Show file tree
Hide file tree
Showing 10 changed files with 132 additions and 77 deletions.
34 changes: 0 additions & 34 deletions src/libmain/loggers.cc

This file was deleted.

11 changes: 0 additions & 11 deletions src/libmain/loggers.hh

This file was deleted.

8 changes: 8 additions & 0 deletions src/libmain/progress-bar.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
#include "sync.hh"
#include "store-api.hh"
#include "names.hh"
#include "loggers.hh"

#include <atomic>
#include <map>
Expand All @@ -11,6 +12,13 @@

namespace nix {

static auto l1 = OnStartup([] {
registerLogger("bar", []() -> Logger* { return makeProgressBar(false); });
});
static auto l2 = OnStartup([] {
registerLogger("bar-with-logs", []() -> Logger* { return makeProgressBar(true); });
});

static std::string getS(const std::vector<Logger::Field> & fields, size_t n)
{
assert(n < fields.size());
Expand Down
70 changes: 52 additions & 18 deletions src/libstore/globals.cc
Original file line number Diff line number Diff line change
Expand Up @@ -185,18 +185,35 @@ template<> void BaseSetting<SandboxMode>::convertToArg(Args & args, const std::s
});
}

static void logFormatCompleter(size_t index, std::string_view prefix)
{
for (auto & builder : registeredLoggers)
if (hasPrefix(builder->name, prefix))
completions->insert(builder->name);
}

std::string listLogFormats()
{
std::string res;

for (auto format = logFormats.begin(); format != logFormats.end(); ++format) {
if (!res.empty()) res += ", ";
if (std::next(format) == logFormats.end()) res += "or ";
res += "'";
res += *format;
res += "'";
}

return res;
}

template<> void BaseSetting<LogFormat>::set(const std::string & str)
{
if (str == "raw")
value = LogFormat::raw;
else if (str == "raw-with-logs")
value = LogFormat::rawWithLogs;
else if (str == "internal-json")
value = LogFormat::internalJson;
else if (str == "bar")
value = LogFormat::bar;
else if (str == "bar-with-logs")
value = LogFormat::barWithLogs;
if (str == "raw") value = LogFormat::raw;
else if (str == "raw-with-logs") value = LogFormat::rawWithLogs;
else if (str == "internal-json") value = LogFormat::internalJson;
else if (str == "bar") value = LogFormat::bar;
else if (str == "bar-with-logs") value = LogFormat::barWithLogs;
else throw UsageError("option '%s' has an invalid value '%s'", name, str);

createDefaultLogger();
Expand All @@ -212,25 +229,42 @@ template<> std::string BaseSetting<LogFormat>::to_string() const
else abort();
}

template<> nlohmann::json BaseSetting<LogFormat>::toJSON()
{
return AbstractSetting::toJSON();
}

template<> void BaseSetting<LogFormat>::convertToArg(Args & args, const std::string & category)
{
args.addFlag({
.longName = name,
.description = "format of log output; `raw`, `raw-with-logs`, `internal-json`, `bar`, "
"or `bar-with-logs`",
.description = fmt("format of log output; %s", listLogFormats()),
.category = category,
.labels = {"format"},
.handler = {[&](std::string format) {
settings.logFormat.set(format);
}}
}},
.completer = logFormatCompleter
});
}

void setLogFormat(const LogFormat & logFormat)
{
settings.logFormat = logFormat;
createDefaultLogger();
}

Logger* makeDefaultLogger()
{
for (auto & builder : registeredLoggers) {
if (builder->name == settings.logFormat.to_string()) {
return builder->builder();
}
}

throw UsageError("Unknown logger '%s'", settings.logFormat.to_string());
}

void createDefaultLogger()
{
logger = makeDefaultLogger();
}

void MaxBuildJobsSetting::set(const std::string & str)
{
if (str == "auto") value = std::max(1U, std::thread::hardware_concurrency());
Expand Down
16 changes: 13 additions & 3 deletions src/libstore/globals.hh
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
#include "config.hh"
#include "abstractsettingtojson.hh"
#include "util.hh"
#include "loggers.hh"

#include <map>
#include <limits>
Expand All @@ -12,6 +13,8 @@

namespace nix {

std::string listLogFormats();

typedef enum { smEnabled, smRelaxed, smDisabled } SandboxMode;

struct MaxBuildJobsSetting : public BaseSetting<unsigned int>
Expand Down Expand Up @@ -866,9 +869,6 @@ public:
Setting<Strings> experimentalFeatures{this, {}, "experimental-features",
"Experimental Nix features to enable."};

Setting<LogFormat> logFormat{this, LogFormat::bar, "log-format",
"Default build output logging format; \"raw\", \"raw-with-logs\", \"internal-json\", \"bar\" or \"bar-with-logs\"."};

bool isExperimentalFeatureEnabled(const std::string & name);

void requireExperimentalFeature(const std::string & name);
Expand All @@ -884,8 +884,18 @@ public:

Setting<std::string> flakeRegistry{this, "https://github.com/NixOS/flake-registry/raw/master/flake-registry.json", "flake-registry",
"Path or URI of the global flake registry."};

// FIXME: default shows as "3", but should show as "bar", due to the default
// being an enum variant
Setting<LogFormat> logFormat{this, LogFormat::bar, "log-format",
fmt("Default build output logging format. Valid options are: %s.", listLogFormats())};

Logger* makeDefaultLogger();
};

void setLogFormat(const LogFormat & logFormat);

void createDefaultLogger();

// FIXME: don't use a global variable.
extern Settings settings;
Expand Down
4 changes: 1 addition & 3 deletions src/libstore/local.mk
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,6 @@ libstore_SOURCES := $(wildcard $(d)/*.cc $(d)/builtins/*.cc)

libstore_LIBS = libutil

libstore_ALLOW_UNDEFINED = 1

libstore_LDFLAGS = $(SQLITE3_LIBS) -lbz2 $(LIBCURL_LIBS) $(SODIUM_LIBS) -pthread
ifneq ($(OS), FreeBSD)
libstore_LDFLAGS += -ldl
Expand All @@ -34,7 +32,7 @@ ifeq ($(HAVE_SECCOMP), 1)
endif

libstore_CXXFLAGS += \
-I src/libmain -I src/libutil -I src/libstore \
-I src/libutil -I src/libstore \
-DNIX_PREFIX=\"$(prefix)\" \
-DNIX_STORE_DIR=\"$(storedir)\" \
-DNIX_DATA_DIR=\"$(datadir)\" \
Expand Down
21 changes: 21 additions & 0 deletions src/libutil/loggers.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
#include "loggers.hh"

namespace nix {

std::set<std::string> logFormats = {
"raw",
"raw-with-logs",
"internal-json",
"bar",
"bar-with-logs"
};

std::vector<std::shared_ptr<LoggerBuilder>> registeredLoggers;

void registerLogger(std::string name, std::function<Logger*()> builder)
{
LoggerBuilder lBuilder { .name = name, .builder = builder };
registeredLoggers.push_back(std::make_shared<LoggerBuilder>(lBuilder));
}

}
26 changes: 26 additions & 0 deletions src/libutil/loggers.hh
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
#pragma once

#include "logging.hh"

namespace nix {

enum class LogFormat {
raw,
rawWithLogs,
internalJson,
bar,
barWithLogs,
};

struct LoggerBuilder {
std::string name;
std::function<Logger*()> builder;
};

extern std::set<std::string> logFormats;

extern std::vector<std::shared_ptr<LoggerBuilder>> registeredLoggers;

void registerLogger(std::string name, std::function<Logger*()> builder);

}
11 changes: 11 additions & 0 deletions src/libutil/logging.cc
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#include "logging.hh"
#include "util.hh"
#include "config.hh"
#include "loggers.hh"

#include <atomic>
#include <nlohmann/json.hpp>
Expand All @@ -10,6 +11,16 @@ namespace nix {

LoggerSettings loggerSettings;

static auto l1 = OnStartup([] {
registerLogger("internal-json", []() -> Logger* { return makeJSONLogger(*makeSimpleLogger(true)); });
});
static auto l2 = OnStartup([] {
registerLogger("raw", []() -> Logger* { return makeSimpleLogger(false); });
});
static auto l3 = OnStartup([] {
registerLogger("raw-with-logs", []() -> Logger* { return makeSimpleLogger(true); });
});

static GlobalConfig::Register r1(&loggerSettings);

static thread_local ActivityId curActivity = 0;
Expand Down
8 changes: 0 additions & 8 deletions src/libutil/types.hh
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,6 @@ typedef list<string> Strings;
typedef set<string> StringSet;
typedef std::map<string, string> StringMap;

enum class LogFormat {
raw,
rawWithLogs,
internalJson,
bar,
barWithLogs,
};

/* Paths are just strings. */

typedef string Path;
Expand Down

0 comments on commit 6efa529

Please sign in to comment.