Merge pull request #11178 from obsidiansystems/better-exe-lookup

Move `NIX_BIN_DIR` and all logic using it to the Nix executable itself

doc/manual/rl-next/build-hook-default.md

@@ -0,0 +1,22 @@
+---
+synopsis: |-
+  The `build-hook` setting's default is less useful when using `libnixstore` as a library
+prs:
+- 11178
+---
+
+*This is an obscure issue that only affects usage of the `libnixstore` library outside of the Nix executable.*
+
+As part the ongoing [rewrite of the build system](https://github.com/NixOS/nix/issues/2503) to use [Meson](https://mesonbuild.com/), we are also switching to packaging individual Nix components separately (and building them in separate derivations).
+This means that when building `libnixstore` we do not know where the Nix binaries will be installed --- `libnixstore` doesn't know about downstream consumers like the Nix binaries at all.
+
+*This is also unrelated to the _`post`_-`build-hook`*, which is often used for pushing to a cache.*
+
+This has a small adverse affect on remote building --- the `build-remote` executable that is specified from the [`build-hook`](@docroot@/command-ref/conf-file.md#conf-build-hook) setting will not be gotten from the (presumed) installation location, but instead looked up on the `PATH`.
+This means that other applications linking `libnixstore` that wish to use remote building must arrange for the `nix` command to be on the PATH (or manually overriding `build-hook`) in order for that to work.
+
+Long term we don't envision this being a downside, because we plan to [get rid of `build-remote` and the build hook setting entirely](https://github.com/NixOS/nix/issues/1221).
+There is simply no need to add a second layer of remote-procedure-calling when we want to connect to a remote builder.
+The build hook protocol did in principle support custom ways of remote building, but that can also be accomplished with a custom service for the ssh or daemon/ssh-ng protocols, or with a custom [store type](@docroot@/store/types/) i.e. `Store` subclass. <!-- we normally don't mention classes, but consider that this release note is about a library use case -->
+
+The Perl bindings no longer expose `getBinDir` either, since they libraries those bindings wrap no longer know the location of installed binaries as described above.

src/libcmd/repl.cc

@@ -7,7 +7,6 @@
 
 #include "ansicolor.hh"
 #include "shared.hh"
-#include "config-global.hh"
 #include "eval.hh"
 #include "eval-settings.hh"
 #include "attr-path.hh"
@@ -77,10 +76,14 @@ struct NixRepl
     int displ;
     StringSet varNames;
 
+    RunNix * runNixPtr;
+
+    void runNix(Path program, const Strings & args, const std::optional<std::string> & input = {});
+
     std::unique_ptr<ReplInteracter> interacter;
 
     NixRepl(const LookupPath & lookupPath, nix::ref<Store> store,ref<EvalState> state,
-            std::function<AnnotatedValues()> getValues);
+            std::function<AnnotatedValues()> getValues, RunNix * runNix);
     virtual ~NixRepl() = default;
 
     ReplExitStatus mainLoop() override;
@@ -125,32 +128,16 @@ std::string removeWhitespace(std::string s)
 
 
 NixRepl::NixRepl(const LookupPath & lookupPath, nix::ref<Store> store, ref<EvalState> state,
-            std::function<NixRepl::AnnotatedValues()> getValues)
+            std::function<NixRepl::AnnotatedValues()> getValues, RunNix * runNix = nullptr)
     : AbstractNixRepl(state)
     , debugTraceIndex(0)
     , getValues(getValues)
     , staticEnv(new StaticEnv(nullptr, state->staticBaseEnv.get()))
+    , runNixPtr{runNix}
     , interacter(make_unique<ReadlineLikeInteracter>(getDataDir() + "/nix/repl-history"))
 {
 }
 
-void runNix(Path program, const Strings & args,
-    const std::optional<std::string> & input = {})
-{
-    auto subprocessEnv = getEnv();
-    subprocessEnv["NIX_CONFIG"] = globalConfig.toKeyValue();
-    //isInteractive avoid grabling interactive commands
-    runProgram2(RunOptions {
-        .program = settings.nixBinDir+ "/" + program,
-        .args = args,
-        .environment = subprocessEnv,
-        .input = input,
-        .isInteractive = true,
-    });
-
-    return;
-}
-
 static std::ostream & showDebugTrace(std::ostream & out, const PosTable & positions, const DebugTrace & dt)
 {
     if (dt.isError)
@@ -833,9 +820,18 @@ void NixRepl::evalString(std::string s, Value & v)
 }
 
 
+void NixRepl::runNix(Path program, const Strings & args, const std::optional<std::string> & input)
+{
+    if (runNixPtr)
+        (*runNixPtr)(program, args, input);
+    else
+        throw Error("Cannot run '%s', no method of calling the Nix CLI provided", program);
+}
+
+
 std::unique_ptr<AbstractNixRepl> AbstractNixRepl::create(
    const LookupPath & lookupPath, nix::ref<Store> store, ref<EvalState> state,
-   std::function<AnnotatedValues()> getValues)
+   std::function<AnnotatedValues()> getValues, RunNix * runNix)
 {
     return std::make_unique<NixRepl>(
         lookupPath,

src/libcmd/repl.hh

@@ -19,9 +19,19 @@ struct AbstractNixRepl
 
     typedef std::vector<std::pair<Value*,std::string>> AnnotatedValues;
 
+    using RunNix = void(Path program, const Strings & args, const std::optional<std::string> & input);
+
+    /**
+     * @param runNix Function to run the nix CLI to support various
+     * `:<something>` commands. Optional; if not provided,
+     * everything else will still work fine, but those commands won't.
+     */
     static std::unique_ptr<AbstractNixRepl> create(
-        const LookupPath & lookupPath, nix::ref<Store> store, ref<EvalState> state,
-        std::function<AnnotatedValues()> getValues);
+        const LookupPath & lookupPath,
+        nix::ref<Store> store,
+        ref<EvalState> state,
+        std::function<AnnotatedValues()> getValues,
+        RunNix * runNix = nullptr);
 
     static ReplExitStatus runSimple(
         ref<EvalState> evalState,

src/libstore/globals.cc

@@ -64,7 +64,6 @@ Settings::Settings()
     , nixStateDir(canonPath(getEnvNonEmpty("NIX_STATE_DIR").value_or(NIX_STATE_DIR)))
     , nixConfDir(canonPath(getEnvNonEmpty("NIX_CONF_DIR").value_or(NIX_CONF_DIR)))
     , nixUserConfFiles(getUserConfigFiles())
-    , nixBinDir(canonPath(getEnvNonEmpty("NIX_BIN_DIR").value_or(NIX_BIN_DIR)))
     , nixManDir(canonPath(NIX_MAN_DIR))
     , nixDaemonSocketFile(canonPath(getEnvNonEmpty("NIX_DAEMON_SOCKET_PATH").value_or(nixStateDir + DEFAULT_SOCKET_PATH)))
 {
@@ -95,34 +94,6 @@ Settings::Settings()
     sandboxPaths = tokenizeString<StringSet>("/System/Library/Frameworks /System/Library/PrivateFrameworks /bin/sh /bin/bash /private/tmp /private/var/tmp /usr/lib");
     allowedImpureHostPrefixes = tokenizeString<StringSet>("/System/Library /usr/lib /dev /bin/sh");
 #endif
-
-    /* Set the build hook location
-
-       For builds we perform a self-invocation, so Nix has to be self-aware.
-       That is, it has to know where it is installed. We don't think it's sentient.
-
-       Normally, nix is installed according to `nixBinDir`, which is set at compile time,
-       but can be overridden. This makes for a great default that works even if this
-       code is linked as a library into some other program whose main is not aware
-       that it might need to be a build remote hook.
-
-       However, it may not have been installed at all. For example, if it's a static build,
-       there's a good chance that it has been moved out of its installation directory.
-       That makes `nixBinDir` useless. Instead, we'll query the OS for the path to the
-       current executable, using `getSelfExe()`.
-
-       As a last resort, we resort to `PATH`. Hopefully we find a `nix` there that's compatible.
-       If you're porting Nix to a new platform, that might be good enough for a while, but
-       you'll want to improve `getSelfExe()` to work on your platform.
-     */
-    std::string nixExePath = nixBinDir + "/nix";
-    if (!pathExists(nixExePath)) {
-        nixExePath = getSelfExe().value_or("nix");
-    }
-    buildHook = {
-        nixExePath,
-        "__build-remote",
-    };
 }
 
 void loadConfFile(AbstractConfig & config)

src/libstore/globals.hh

@@ -84,11 +84,6 @@ public:
      */
     std::vector<Path> nixUserConfFiles;
 
-    /**
-     * The directory where the main programs are stored.
-     */
-    Path nixBinDir;
-
     /**
      * The directory where the man pages are stored.
      */
@@ -246,7 +241,7 @@ public:
         )",
         {"build-timeout"}};
 
-    Setting<Strings> buildHook{this, {}, "build-hook",
+    Setting<Strings> buildHook{this, {"nix", "__build-remote"}, "build-hook",
         R"(
           The path to the helper program that executes remote builds.
 

src/libstore/local.mk

@@ -71,7 +71,6 @@ libstore_CXXFLAGS += \
  -DNIX_STATE_DIR=\"$(NIX_ROOT)$(localstatedir)/nix\" \
  -DNIX_LOG_DIR=\"$(NIX_ROOT)$(localstatedir)/log/nix\" \
  -DNIX_CONF_DIR=\"$(NIX_ROOT)$(sysconfdir)/nix\" \
- -DNIX_BIN_DIR=\"$(NIX_ROOT)$(bindir)\" \
  -DNIX_MAN_DIR=\"$(NIX_ROOT)$(mandir)\" \
  -DLSOF=\"$(NIX_ROOT)$(lsof)\"
 

src/libstore/meson.build

@@ -328,7 +328,6 @@ prefix = get_option('prefix')
 path_opts = [
   # Meson built-ins.
   'datadir',
-  'bindir',
   'mandir',
   'libdir',
   'includedir',
@@ -373,7 +372,6 @@ cpp_str_defines = {
   'NIX_STATE_DIR': state_dir / 'nix',
   'NIX_LOG_DIR':   log_dir,
   'NIX_CONF_DIR':  sysconfdir / 'nix',
-  'NIX_BIN_DIR':   bindir,
   'NIX_MAN_DIR':   mandir,
 }
 

src/libstore/unix/build/hook-instance.cc

@@ -4,6 +4,7 @@
 #include "file-system.hh"
 #include "child.hh"
 #include "strings.hh"
+#include "executable-path.hh"
 
 namespace nix {
 
@@ -16,11 +17,18 @@ HookInstance::HookInstance()
     if (buildHookArgs.empty())
         throw Error("'build-hook' setting is empty");
 
-    auto buildHook = canonPath(buildHookArgs.front());
+    std::filesystem::path buildHook = buildHookArgs.front();
     buildHookArgs.pop_front();
 
+    try {
+        buildHook = ExecutablePath::load().findPath(buildHook);
+    } catch (ExecutableLookupError & e) {
+        e.addTrace(nullptr, "while resolving the 'build-hook' setting'");
+        throw;
+    }
+
     Strings args;
-    args.push_back(std::string(baseNameOf(buildHook)));
+    args.push_back(buildHook.filename().string());
 
     for (auto & arg : buildHookArgs)
         args.push_back(arg);
@@ -59,7 +67,7 @@ HookInstance::HookInstance()
         if (dup2(builderOut.readSide.get(), 5) == -1)
             throw SysError("dupping builder's stdout/stderr");
 
-        execv(buildHook.c_str(), stringsToCharPtrs(args).data());
+        execv(buildHook.native().c_str(), stringsToCharPtrs(args).data());
 
         throw SysError("executing '%s'", buildHook);
     });

src/libutil/executable-path.cc

@@ -60,7 +60,7 @@ OsString ExecutablePath::render() const
 }
 
 std::optional<fs::path>
-ExecutablePath::find(const OsString & exe, std::function<bool(const fs::path &)> isExecutable) const
+ExecutablePath::findName(const OsString & exe, std::function<bool(const fs::path &)> isExecutable) const
 {
     // "If the pathname being sought contains a <slash>, the search
     // through the path prefixes shall not be performed."
@@ -76,4 +76,20 @@ ExecutablePath::find(const OsString & exe, std::function<bool(const fs::path &)>
     return std::nullopt;
 }
 
+fs::path ExecutablePath::findPath(const fs::path & exe, std::function<bool(const fs::path &)> isExecutable) const
+{
+    // "If the pathname being sought contains a <slash>, the search
+    // through the path prefixes shall not be performed."
+    // https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap08.html#tag_08_03
+    if (exe.filename() == exe) {
+        auto resOpt = findName(exe, isExecutable);
+        if (resOpt)
+            return *resOpt;
+        else
+            throw ExecutableLookupError("Could not find executable '%s'", exe.native());
+    } else {
+        return exe;
+    }
+}
+
 } // namespace nix

src/libutil/executable-path.hh

@@ -5,6 +5,8 @@
 
 namespace nix {
 
+MakeError(ExecutableLookupError, Error);
+
 struct ExecutablePath
 {
     std::vector<std::filesystem::path> directories;
@@ -54,10 +56,21 @@ struct ExecutablePath
      *
      * @return path to a resolved executable
      */
-    std::optional<std::filesystem::path> find(
+    std::optional<std::filesystem::path> findName(
         const OsString & exe,
         std::function<bool(const std::filesystem::path &)> isExecutableFile = isExecutableFileAmbient) const;
 
+    /**
+     * Like the `findName` but also allows a file path as input.
+     *
+     * This implements the full POSIX spec: if the path is just a name,
+     * it searches like the above. Otherwise, it returns the path as is.
+     * If (in the name case) the search fails, an exception is thrown.
+     */
+    std::filesystem::path findPath(
+        const std::filesystem::path & exe,
+        std::function<bool(const std::filesystem::path &)> isExecutable = isExecutableFileAmbient) const;
+
     bool operator==(const ExecutablePath &) const = default;
 };
 

src/nix-channel/nix-channel.cc

@@ -7,6 +7,7 @@
 #include "eval-settings.hh" // for defexpr
 #include "users.hh"
 #include "tarball.hh"
+#include "self-exe.hh"
 
 #include <fcntl.h>
 #include <regex>
@@ -67,7 +68,7 @@ static void removeChannel(const std::string & name)
     channels.erase(name);
     writeChannels();
 
-    runProgram(settings.nixBinDir + "/nix-env", true, { "--profile", profile, "--uninstall", name });
+    runProgram(getNixBin("nix-env").string(), true, { "--profile", profile, "--uninstall", name });
 }
 
 static Path nixDefExpr;
@@ -118,7 +119,7 @@ static void update(const StringSet & channelNames)
 
             bool unpacked = false;
             if (std::regex_search(filename, std::regex("\\.tar\\.(gz|bz2|xz)$"))) {
-                runProgram(settings.nixBinDir + "/nix-build", false, { "--no-out-link", "--expr", "import " + unpackChannelPath +
+                runProgram(getNixBin("nix-build").string(), false, { "--no-out-link", "--expr", "import " + unpackChannelPath +
                             "{ name = \"" + cname + "\"; channelName = \"" + name + "\"; src = builtins.storePath \"" + filename + "\"; }" });
                 unpacked = true;
             }
@@ -143,7 +144,7 @@ static void update(const StringSet & channelNames)
     for (auto & expr : exprs)
         envArgs.push_back(std::move(expr));
     envArgs.push_back("--quiet");
-    runProgram(settings.nixBinDir + "/nix-env", false, envArgs);
+    runProgram(getNixBin("nix-env").string(), false, envArgs);
 
     // Make the channels appear in nix-env.
     struct stat st;
@@ -244,7 +245,7 @@ static int main_nix_channel(int argc, char ** argv)
             case cListGenerations:
                 if (!args.empty())
                     throw UsageError("'--list-generations' expects no arguments");
-                std::cout << runProgram(settings.nixBinDir + "/nix-env", false, {"--profile", profile, "--list-generations"}) << std::flush;
+                std::cout << runProgram(getNixBin("nix-env").string(), false, {"--profile", profile, "--list-generations"}) << std::flush;
                 break;
             case cRollback:
                 if (args.size() > 1)
@@ -256,7 +257,7 @@ static int main_nix_channel(int argc, char ** argv)
                 } else {
                     envArgs.push_back("--rollback");
                 }
-                runProgram(settings.nixBinDir + "/nix-env", false, envArgs);
+                runProgram(getNixBin("nix-env").string(), false, envArgs);
                 break;
         }
 

src/nix/local.mk

@@ -26,6 +26,8 @@ endif
 
 nix_CXXFLAGS += $(INCLUDE_libutil) $(INCLUDE_libstore) $(INCLUDE_libfetchers) $(INCLUDE_libexpr) $(INCLUDE_libflake) $(INCLUDE_libmain) -I src/libcmd -I doc/manual $(INCLUDE_nix)
 
+nix_CXXFLAGS += -DNIX_BIN_DIR=\"$(NIX_ROOT)$(bindir)\"
+
 nix_LIBS = libexpr libmain libfetchers libflake libstore libutil libcmd
 
 nix_LDFLAGS = $(THREAD_LDFLAGS) $(SODIUM_LIBS) $(EDITLINE_LIBS) $(BOOST_LDFLAGS) $(LOWDOWN_LIBS)

src/nix/main.cc

@@ -19,6 +19,7 @@
 #include "network-proxy.hh"
 #include "eval-cache.hh"
 #include "flake/flake.hh"
+#include "self-exe.hh"
 
 #include <sys/types.h>
 #include <regex>
@@ -366,6 +367,17 @@ void mainWrapped(int argc, char * * argv)
     initGC();
     flake::initLib(flakeSettings);
 
+    /* Set the build hook location
+
+       For builds we perform a self-invocation, so Nix has to be
+       self-aware. That is, it has to know where it is installed. We
+       don't think it's sentient.
+     */
+    settings.buildHook.setDefault(Strings {
+        getNixBin({}).string(),
+        "__build-remote",
+    });
+
     #if __linux__
     if (isRootUser()) {
         try {