From 7de2b5973a57313558f74e7a6bdb6c3203e78a3e Mon Sep 17 00:00:00 2001 From: Amaan Qureshi Date: Wed, 25 Feb 2026 19:33:06 -0500 Subject: [PATCH] configuration: make `AbsolutePath` a wrapper type Public inheritance from `std::filesystem::path` lets `AbsolutePath` silently slice down to a plain path when passed by value, so this commit changes `AbsolutePath` to use a `path` field instead, which is easier to reason about and prevents that. --- src/libstore-tests/local-overlay-store.cc | 4 +- src/libstore-tests/local-store.cc | 4 +- src/libstore/filetransfer.cc | 2 +- .../include/nix/store/local-settings.hh | 2 +- src/libutil/include/nix/util/configuration.hh | 99 +++++++++++++++++-- 5 files changed, 98 insertions(+), 13 deletions(-) diff --git a/src/libstore-tests/local-overlay-store.cc b/src/libstore-tests/local-overlay-store.cc index 4c12f6256ab..033f39c475a 100644 --- a/src/libstore-tests/local-overlay-store.cc +++ b/src/libstore-tests/local-overlay-store.cc @@ -21,7 +21,7 @@ TEST(LocalOverlayStore, constructConfig_rootQueryParam) }, }; - EXPECT_EQ(config.rootDir.get(), std::optional{std::string{root}}); + EXPECT_EQ(config.rootDir.get(), std::optional{std::string{root}}); } TEST(LocalOverlayStore, constructConfig_rootPath) @@ -33,7 +33,7 @@ TEST(LocalOverlayStore, constructConfig_rootPath) #endif LocalOverlayStoreConfig config{std::string{root}, {}}; - EXPECT_EQ(config.rootDir.get(), std::optional{std::string{root}}); + EXPECT_EQ(config.rootDir.get(), std::optional{std::string{root}}); } } // namespace nix diff --git a/src/libstore-tests/local-store.cc b/src/libstore-tests/local-store.cc index a094774f8fc..170af52838d 100644 --- a/src/libstore-tests/local-store.cc +++ b/src/libstore-tests/local-store.cc @@ -27,7 +27,7 @@ TEST(LocalStore, constructConfig_rootQueryParam) }, }; - EXPECT_EQ(config.rootDir.get(), std::optional{std::string{root}}); + EXPECT_EQ(config.rootDir.get(), std::optional{std::string{root}}); } TEST(LocalStore, constructConfig_rootPath) @@ -39,7 +39,7 @@ TEST(LocalStore, constructConfig_rootPath) #endif LocalStoreConfig config{std::string{root}, {}}; - EXPECT_EQ(config.rootDir.get(), std::optional{std::string{root}}); + EXPECT_EQ(config.rootDir.get(), std::optional{std::string{root}}); } TEST(LocalStore, constructConfig_to_string) diff --git a/src/libstore/filetransfer.cc b/src/libstore/filetransfer.cc index 099f436ae65..c68dda54df4 100644 --- a/src/libstore/filetransfer.cc +++ b/src/libstore/filetransfer.cc @@ -47,7 +47,7 @@ FileTransferSettings::FileTransferSettings() { auto sslOverride = getEnv("NIX_SSL_CERT_FILE").value_or(getEnv("SSL_CERT_FILE").value_or("")); if (sslOverride != "") - caFile = sslOverride; + caFile = std::optional{sslOverride}; } FileTransferSettings fileTransferSettings; diff --git a/src/libstore/include/nix/store/local-settings.hh b/src/libstore/include/nix/store/local-settings.hh index af7eccdf44f..a320c52293c 100644 --- a/src/libstore/include/nix/store/local-settings.hh +++ b/src/libstore/include/nix/store/local-settings.hh @@ -528,7 +528,7 @@ public: * Get the diff hook path if run-diff-hook is enabled. * @return Pointer to path if enabled, nullptr otherwise. */ - const std::filesystem::path * getDiffHook() const + const AbsolutePath * getDiffHook() const { if (!runDiffHook.get()) { return nullptr; diff --git a/src/libutil/include/nix/util/configuration.hh b/src/libutil/include/nix/util/configuration.hh index 446b68f3d33..cbc102a24e9 100644 --- a/src/libutil/include/nix/util/configuration.hh +++ b/src/libutil/include/nix/util/configuration.hh @@ -222,19 +222,73 @@ protected: * For `Setting`. `parse()` calls `canonPath`, * rejecting empty and relative paths. */ -struct AbsolutePath : std::filesystem::path +struct AbsolutePath { - using path::path; - using path::operator=; + std::filesystem::path path; - AbsolutePath(const std::filesystem::path & p) - : path(p) + AbsolutePath(std::filesystem::path p) + : path(std::move(p)) { } - AbsolutePath(std::filesystem::path && p) - : path(std::move(p)) + AbsolutePath(const char * s) + : path(s) + { + } + + operator const std::filesystem::path &() const + { + return path; + } + + std::string string() const + { + return path.string(); + } + + const auto & native() const + { + return path.native(); + } + + const char * c_str() const + { + return path.c_str(); + } + + bool empty() const + { + return path.empty(); + } + + std::filesystem::path operator/(const std::filesystem::path & rhs) const + { + return path / rhs; + } + + bool operator==(const AbsolutePath & rhs) const + { + return path == rhs.path; + } + + bool operator==(const std::filesystem::path & rhs) const + { + return path == rhs; + } + + bool operator==(const std::string & rhs) const { + return path == rhs; + } + + auto operator<=>(const AbsolutePath & rhs) const + { + return path <=> rhs.path; + } + + friend std::ostream & operator<<(std::ostream & os, const AbsolutePath & p) + { + return os << p.path.string(); } }; @@ -405,6 +459,37 @@ public: } }; +template<> +class Setting : public BaseSetting +{ +public: + using BaseSetting::BaseSetting; + using BaseSetting::operator=; + + Setting( + Config * options, + const AbsolutePath & def, + const std::string & name, + const std::string & description, + const StringSet & aliases = {}, + const bool documentDefault = true, + std::optional experimentalFeature = std::nullopt) + : BaseSetting(def, documentDefault, name, description, aliases, std::move(experimentalFeature)) + { + options->addSetting(this); + } + + void operator=(const AbsolutePath & v) + { + this->assign(v); + } + + operator const std::filesystem::path &() const + { + return this->value.path; + } +}; + template<> void BaseSetting>::appendOrSet(std::set newValue, bool append);