Merge pull request #4486 from shlevy/command-plugins-v2

Command plugins

doc/manual/src/release-notes/rl-2.4.md

@@ -0,0 +1,8 @@
+# Release 2.4 (202X-XX-XX)
+
+  - It is now an error to modify the `plugin-files` setting via a
+    command-line flag that appears after the first non-flag argument
+    to any command, including a subcommand to `nix`. For example,
+    `nix-instantiate default.nix --plugin-files ""` must now become
+    `nix-instantiate --plugin-files "" default.nix`.
+  - Plugins that add new `nix` subcommands are now actually respected.

src/build-remote/build-remote.cc

@@ -53,6 +53,9 @@ static int main_build_remote(int argc, char * * argv)
         unsetenv("DISPLAY");
         unsetenv("SSH_ASKPASS");
 
+        /* If we ever use the common args framework, make sure to
+           remove initPlugins below and initialize settings first.
+        */
         if (argc != 2)
             throw UsageError("called without required arguments");
 

src/libmain/common-args.cc

@@ -79,4 +79,11 @@ MixCommonArgs::MixCommonArgs(const string & programName)
     hiddenCategories.insert(cat);
 }
 
+void MixCommonArgs::initialFlagsProcessed()
+{
+    initPlugins();
+    pluginsInited();
+}
+
+
 }

src/libmain/common-args.hh

@@ -7,10 +7,14 @@ namespace nix {
 //static constexpr auto commonArgsCategory = "Miscellaneous common options";
 static constexpr auto loggingCategory = "Logging-related options";
 
-struct MixCommonArgs : virtual Args
+class MixCommonArgs : public virtual Args
 {
+    void initialFlagsProcessed() override;
+public:
     string programName;
     MixCommonArgs(const string & programName);
+protected:
+    virtual void pluginsInited() {}
 };
 
 struct MixDryRun : virtual Args

src/libstore/globals.cc

@@ -243,8 +243,17 @@ void MaxBuildJobsSetting::set(const std::string & str, bool append)
 }
 
 
+void PluginFilesSetting::set(const std::string & str, bool append)
+{
+    if (pluginsLoaded)
+        throw UsageError("plugin-files set after plugins were loaded, you may need to move the flag before the subcommand");
+    BaseSetting<Paths>::set(str, append);
+}
+
+
 void initPlugins()
 {
+    assert(!settings.pluginFiles.pluginsLoaded);
     for (const auto & pluginFile : settings.pluginFiles.get()) {
         Paths pluginFiles;
         try {
@@ -270,6 +279,9 @@ void initPlugins()
        unknown settings. */
     globalConfig.reapplyUnknownSettings();
     globalConfig.warnUnknownSettings();
+
+    /* Tell the user if they try to set plugin-files after we've already loaded */
+    settings.pluginFiles.pluginsLoaded = true;
 }
 
 }

src/libstore/globals.hh

@@ -28,6 +28,23 @@ struct MaxBuildJobsSetting : public BaseSetting<unsigned int>
     void set(const std::string & str, bool append = false) override;
 };
 
+struct PluginFilesSetting : public BaseSetting<Paths>
+{
+    bool pluginsLoaded = false;
+
+    PluginFilesSetting(Config * options,
+        const Paths & def,
+        const std::string & name,
+        const std::string & description,
+        const std::set<std::string> & aliases = {})
+        : BaseSetting<Paths>(def, name, description, aliases)
+    {
+        options->addSetting(this);
+    }
+
+    void set(const std::string & str, bool append = false) override;
+};
+
 class Settings : public Config {
 
     unsigned int getDefaultCores();
@@ -819,7 +836,7 @@ public:
     Setting<uint64_t> minFreeCheckInterval{this, 5, "min-free-check-interval",
         "Number of seconds between checking free disk space."};
 
-    Setting<Paths> pluginFiles{
+    PluginFilesSetting pluginFiles{
         this, {}, "plugin-files",
         R"(
           A list of plugin files to be loaded by Nix. Each of these files will

src/libutil/args.cc

@@ -60,6 +60,7 @@ void Args::parseCmdline(const Strings & _cmdline)
         verbosity = lvlError;
     }
 
+    bool argsSeen = false;
     for (auto pos = cmdline.begin(); pos != cmdline.end(); ) {
 
         auto arg = *pos;
@@ -88,6 +89,10 @@ void Args::parseCmdline(const Strings & _cmdline)
                 throw UsageError("unrecognised flag '%1%'", arg);
         }
         else {
+            if (!argsSeen) {
+                argsSeen = true;
+                initialFlagsProcessed();
+            }
             pos = rewriteArgs(cmdline, pos);
             pendingArgs.push_back(*pos++);
             if (processArgs(pendingArgs, false))
@@ -96,6 +101,9 @@ void Args::parseCmdline(const Strings & _cmdline)
     }
 
     processArgs(pendingArgs, true);
+
+    if (!argsSeen)
+        initialFlagsProcessed();
 }
 
 bool Args::processFlag(Strings::iterator & pos, Strings::iterator end)
@@ -298,8 +306,8 @@ Strings argvToStrings(int argc, char * * argv)
     return args;
 }
 
-MultiCommand::MultiCommand(const Commands & commands)
-    : commands(commands)
+MultiCommand::MultiCommand(const Commands & commands_)
+    : commands(commands_)
 {
     expectArgs({
         .label = "subcommand",

src/libutil/args.hh

@@ -132,6 +132,10 @@ protected:
 
     std::set<std::string> hiddenCategories;
 
+    /* Called after all command line flags before the first non-flag
+       argument (if any) have been processed. */
+    virtual void initialFlagsProcessed() {}
+
 public:
 
     void addFlag(Flag && flag);

src/nix-build/nix-build.cc

@@ -240,8 +240,6 @@ static void main_nix_build(int argc, char * * argv)
 
     myArgs.parseCmdline(args);
 
-    initPlugins();
-
     if (packages && fromArgs)
         throw UsageError("'-p' and '-E' are mutually exclusive");
 

src/nix-channel/nix-channel.cc

@@ -196,8 +196,6 @@ static int main_nix_channel(int argc, char ** argv)
             return true;
         });
 
-        initPlugins();
-
         switch (cmd) {
             case cNone:
                 throw UsageError("no command specified");

src/nix-collect-garbage/nix-collect-garbage.cc

@@ -74,8 +74,6 @@ static int main_nix_collect_garbage(int argc, char * * argv)
             return true;
         });
 
-        initPlugins();
-
         auto profilesDir = settings.nixStateDir + "/profiles";
         if (removeOld) removeOldGenerations(profilesDir);
 

src/nix-copy-closure/nix-copy-closure.cc

@@ -43,8 +43,6 @@ static int main_nix_copy_closure(int argc, char ** argv)
             return true;
         });
 
-        initPlugins();
-
         if (sshHost.empty())
             throw UsageError("no host name specified");
 

src/nix-env/nix-env.cc

@@ -1420,8 +1420,6 @@ static int main_nix_env(int argc, char * * argv)
 
         myArgs.parseCmdline(argvToStrings(argc, argv));
 
-        initPlugins();
-
         if (!op) throw UsageError("no operation specified");
 
         auto store = openStore();

src/nix-instantiate/nix-instantiate.cc

@@ -149,8 +149,6 @@ static int main_nix_instantiate(int argc, char * * argv)
 
         myArgs.parseCmdline(argvToStrings(argc, argv));
 
-        initPlugins();
-
         if (evalOnly && !wantsReadWrite)
             settings.readOnlyMode = true;
 

src/nix-store/nix-store.cc

@@ -1067,8 +1067,6 @@ static int main_nix_store(int argc, char * * argv)
             return true;
         });
 
-        initPlugins();
-
         if (!op) throw UsageError("no operation specified");
 
         if (op != opDump && op != opRestore) /* !!! hack */

src/nix/daemon.cc

@@ -326,8 +326,6 @@ static int main_nix_daemon(int argc, char * * argv)
             return true;
         });
 
-        initPlugins();
-
         runDaemon(stdio);
 
         return 0;

src/nix/main.cc

@@ -159,6 +159,12 @@ struct NixArgs : virtual MultiCommand, virtual MixCommonArgs
           #include "nix.md"
           ;
     }
+
+    // Plugins may add new subcommands.
+    void pluginsInited() override
+    {
+        commands = RegisterCommand::getCommandsFor({});
+    }
 };
 
 static void showHelp(std::vector<std::string> subcommand)
@@ -283,8 +289,6 @@ void mainWrapped(int argc, char * * argv)
 
     if (completions) return;
 
-    initPlugins();
-
     if (args.showVersion) {
         printVersion(programName);
         return;

src/nix/prefetch.cc

@@ -171,8 +171,6 @@ static int main_nix_prefetch_url(int argc, char * * argv)
 
         myArgs.parseCmdline(argvToStrings(argc, argv));
 
-        initPlugins();
-
         if (args.size() > 2)
             throw UsageError("too many arguments");
 

tests/plugins.sh

@@ -2,6 +2,6 @@ source common.sh
 
 set -o pipefail
 
-res=$(nix eval --expr builtins.anotherNull --option setting-set true --option plugin-files $PWD/plugins/libplugintest*)
+res=$(nix --option setting-set true --option plugin-files $PWD/plugins/libplugintest* eval --expr builtins.anotherNull)
 
 [ "$res"x = "nullx" ]