Add argument grouping support for dependencies in bdep-{sync,init}cluster-sync

14 files changed, 369 insertions, 51 deletions

diff --git a/bdep/argument-grouping.cli b/bdep/argument-grouping.cli
new file mode 100644
index 0000000..bd181f1
--- /dev/null
+++ b/bdep/argument-grouping.cli
@@ -0,0 +1,67 @@
+// file      : bdep/argument-grouping.cli
+// license   : MIT; see accompanying LICENSE file
+
+include <bdep/common-options.hxx>;
+
+"\section=1"
+"\name=bdep-argument-grouping"
+"\summary=argument grouping facility"
+
+// NOTE: the grouping documentation was copied from CLI.
+//
+"
+\h|SYNOPSIS|
+
+\c{\b{bdep} \b{{} \i{options} \b{\}+} \i{argument} \b{+{} \i{options} \b{\}}}
+
+\h|DESCRIPTION|
+
+For certain commands certain options and command line variables can be grouped
+to only apply to specific arguments. This help topic describes the argument
+grouping facility used for this purpose.
+
+Groups can be specified before (leading) and/or after (trailing) the argument
+they apply to. A leading group starts with '\cb{{}' and ends with '\cb{\}+}'
+while a trailing group starts with '\cb{+{}' and ends with '\cb{\}}'. For
+example:
+
+\
+{ --foo --bar }+ arg   # 'arg' with '--foo' '--bar'
+arg +{ fox=1 baz=2 }   # 'arg' with 'fox=1' 'baz=2'
+\
+
+Multiple leading and/or trailing groups can be specified for the same
+argument. For example:
+
+\
+{ -f }+ { -b }+ arg +{ f=1 } +{ b=2 } # 'arg' with '-f' 'b' 'f=1' 'b=2'
+\
+
+Note that the group applies to a single argument only. For example:
+
+\
+{ --foo }+ arg1  arg2 +{ --bar }  # 'arg1' with '--foo' and
+                                  # 'arg2' with '--bar'
+\
+
+The group separators ('\cb{{}', '\cb{\}+'}, etc) must be separate command line
+arguments. In particular, they must not be adjacent either to the arguments
+inside the group nor to the argument they apply to. All such cases will be
+treated as ordinary arguments. For example:
+
+\
+{--foo}+ arg   # '{--foo}+' ...
+arg+{ --foo }  # 'arg+{' ...
+\
+
+If one of the group separators needs to be specified as an argument verbatim,
+then it must be escaped with '\cb{\\}'. For example:
+
+\
+}             # error: unexpected group separator
+}x            # '}x'
+\}            # '}'
+{ \}+ }+ arg  # 'arg' with '}+'
+\
+
+"
diff --git a/bdep/bdep.cli b/bdep/bdep.cli
index 45163e4..624cc25 100644
--- a/bdep/bdep.cli
+++ b/bdep/bdep.cli
@@ -494,14 +494,19 @@ namespace bdep
       "\l{bdep-common-options(1)} \- details on common options"
     }
 
+    bool projects-configs
+    {
+      "\l{bdep-projects-configs(1)} \- specifying projects and configurations"
+    }
+
     bool default-options-files
     {
       "\l{bdep-default-options-files(1)} \- specifying default options"
     }
 
-    bool projects-configs
+    bool argument-grouping
     {
-      "\l{bdep-projects-configs(1)} \- specifying projects and configurations"
+      "\l{bdep-argument-grouping(1)} \- argument grouping facility"
     }
   };
 
diff --git a/bdep/bdep.cxx b/bdep/bdep.cxx
index eba356e..3e762e8 100644
--- a/bdep/bdep.cxx
+++ b/bdep/bdep.cxx
@@ -123,20 +123,19 @@ static const size_t args_pos (numeric_limits<size_t>::max () / 2);
 // Once this is done, use the "final" values of the common options to do
 // global initializations (verbosity level, etc).
 //
-// If O is-a configuration_name_options, then also handle the @<cfg-name>
+// If O is-a configuration_name_options, then also handle the [-]@<cfg-name>
 // arguments and place them into configuration_name_options::config_name.
 //
-static inline bool
+static inline void
 cfg_name (configuration_name_options* o, const char* a, size_t p)
 {
-  string n (a);
+  string n (a + (*a == '@' ? 1 : 2));
 
   if (n.empty ())
-    fail << "empty configuration name";
+    fail << "missing configuration name in '" << a << "'";
 
   o->config_name ().emplace_back (move (n), p);
   o->config_name_specified (true);
-  return true;
 }
 
 static inline bool
@@ -197,10 +196,9 @@ init (const common_options& co,
 
       // @<cfg-name> & -@<cfg-name>
       //
-      size_t p (scan.position ());
-      if ((*a == '@' &&                cfg_name (&o, a + 1, p)) ||
-          (*a == '-' && a[1] == '@' && cfg_name (&o, a + 2, p)))
+      if (*a == '@' || (*a == '-' && a[1] == '@'))
       {
+        cfg_name (&o, a, scan.position ());
         scan.next ();
         continue;
       }
@@ -537,6 +535,8 @@ catch (const failed&)
 {
   return 1; // Diagnostics has already been issued.
 }
+// Note that there commands that rely on this handler.
+//
 catch (const cli::exception& e)
 {
   error << e;
diff --git a/bdep/build.txx b/bdep/build.txx
index c546559..0c128b4 100644
--- a/bdep/build.txx
+++ b/bdep/build.txx
@@ -125,7 +125,7 @@ namespace bdep
       // Pre-sync the configuration to avoid triggering the build system hook
       // (see sync for details).
       //
-      cmd_sync (o, prj, c, strings () /* pkg_args */, true /* implicit */);
+      cmd_sync (o, prj, c, true /* implicit */);
 
       build (o, c, ps, cfg_vars);
     }
diff --git a/bdep/buildfile b/bdep/buildfile
index 2bbf1c3..98a03f1 100644
--- a/bdep/buildfile
+++ b/bdep/buildfile
@@ -35,7 +35,7 @@ test-options      \
 update-options    \
 clean-options
 
-help_topics = projects-configs default-options-files
+help_topics = projects-configs argument-grouping default-options-files
 
 ./: exe{bdep}: {hxx ixx txx cxx}{+bdep} libue{bdep}
 
@@ -139,6 +139,7 @@ if $cli.configured
   # Help topics.
   #
   cli.cxx{projects-configs}:      cli{projects-configs}
+  cli.cxx{argument-grouping}:     cli{argument-grouping}
   cli.cxx{default-options-files}: cli{default-options-files}
 
   # Option length must be the same to get commands/topics/options aligned.
@@ -168,6 +169,7 @@ if $cli.configured
   # Avoid generating CLI runtime and empty inline file for help topics.
   #
   cli.cxx{projects-configs}:      cli.options += --suppress-cli --suppress-inline
+  cli.cxx{argument-grouping}:     cli.options += --suppress-cli --suppress-inline
   cli.cxx{default-options-files}: cli.options += --suppress-cli --suppress-inline
 
   # Include the generated cli files into the distribution and don't remove
diff --git a/bdep/help.cxx b/bdep/help.cxx
index f9416fd..b5bef1f 100644
--- a/bdep/help.cxx
+++ b/bdep/help.cxx
@@ -11,6 +11,7 @@
 // Help topics.
 //
 #include <bdep/projects-configs.hxx>
+#include <bdep/argument-grouping.hxx>
 #include <bdep/default-options-files.hxx>
 
 using namespace std;
@@ -32,6 +33,8 @@ namespace bdep
         usage = &print_bdep_common_options_long_usage;
       else if (t == "projects-configs")
         usage = &print_bdep_projects_configs_usage;
+      else if (t == "argument-grouping")
+        usage = &print_bdep_argument_grouping_usage;
       else if (t == "default-options-files")
         usage = &print_bdep_default_options_files_usage;
       else
diff --git a/bdep/init.cli b/bdep/init.cli
index 10e6ab2..9ca9a44 100644
--- a/bdep/init.cli
+++ b/bdep/init.cli
@@ -29,7 +29,7 @@ namespace bdep
      \c{<cfg-spec> = (\b{@}<cfg-name> | \b{--config}|\b{-c} <cfg-dir>)... | \b{--all}|\b{-a}\n
         <pkg-spec> = (\b{--directory}|\b{-d} <pkg-dir>)... | <prj-spec>\n
         <prj-spec> = \b{--directory}|\b{-d} <prj-dir>\n
-        <pkg-args> = (<pkg> | <cfg-var>)...\n
+        <pkg-args> = (\b{?}<pkg> | <cfg-var>)...\n
         <cfg-args> = [\b{--} <bpkg-options>] [\b{--existing}|\b{-e} | (<module> | <cfg-var>)...]}
 
      \h|DESCRIPTION|
@@ -68,6 +68,18 @@ namespace bdep
      $ bdep init -C ../prj-gcc @gcc -- -- ?sys:libsqlite3/*
      \
 
+     Configuration variables can be specified to only apply to specific
+     packages in <pkg-args> using the argument grouping mechanism
+     (\l{bdep-argument-grouping(1)}). Additionally, such packages can be
+     placed into specific linked configurations by specifying the
+     configuration with one of the \cb{--config*} options (or \cb{@} notation)
+     using the same grouping mechanism. For example (assuming \cb{gcc} is
+     linked to \cb{common}):
+
+     \
+     $ bdep init @gcc { @common config.liblarge.extra=true }+ ?liblarge
+     \
+
      \h|EXAMPLES|
 
      As an example, consider project \cb{prj} with two packages, \cb{foo}
diff --git a/bdep/init.cxx b/bdep/init.cxx
index 067226b..7533b4d 100644
--- a/bdep/init.cxx
+++ b/bdep/init.cxx
@@ -263,8 +263,8 @@ namespace bdep
           cmd_sync (o,
                     prj,
                     c,
-                    pkg_args,
                     false     /* implicit */,
+                    pkg_args,
                     true      /* fetch */,
                     true      /* yes */,
                     false     /* name_cfg */,
diff --git a/bdep/publish.cxx b/bdep/publish.cxx
index ec0933f..3eb2d04 100644
--- a/bdep/publish.cxx
+++ b/bdep/publish.cxx
@@ -1065,7 +1065,7 @@ namespace bdep
       }
 
       for (const shared_ptr<configuration>& c: scs)
-        cmd_sync (o, prj, c, strings () /* pkg_args */, true /* implicit */);
+        cmd_sync (o, prj, c, true /* implicit */);
     }
 
     return cmd_publish (o, prj, move (pkgs), move (dist_dirs));
diff --git a/bdep/sync.cli b/bdep/sync.cli
index fcc1417..c25cd7d 100644
--- a/bdep/sync.cli
+++ b/bdep/sync.cli
@@ -67,6 +67,13 @@ namespace bdep
      Note also that \c{\b{--immediate}|\b{-i}} or \c{\b{--recursive}|\b{-r}}
      can only be specified with an explicit \cb{--upgrade} or \cb{--patch}.
 
+     Configuration variables can be specified to only apply to specific
+     packages in <pkg-args> and <dep-spec> using the argument grouping
+     mechanism (\l{bdep-argument-grouping(1)}). Additionally, packages in
+     <pkg-args> can be placed into specific linked configurations by
+     specifying the configuration with one of the \cb{--config*} options
+     (or \cb{@} notation) using the same grouping mechanism.
+
      If during synchronization a build-time dependency is encountered and
      there is no build configuration of a suitable type associated with the
      project, then the user is prompted (unless the respective
@@ -241,6 +248,18 @@ namespace bdep
     uint16_t --hook = 0;
   };
 
+  // Options that can be specified in a group for ?<pkg> in pkg-args.
+  //
+  class cmd_sync_pkg_options
+  {
+    // Note that this is also used as storage for configuration names
+    // specified as @<name>.
+    //
+    vector<string> --config-name|-n;
+    vector<uint64_t> --config-id;
+    vector<dir_path> --config|-c;
+  };
+
   "
    \h|DEFAULT OPTIONS FILES|
 
diff --git a/bdep/sync.cxx b/bdep/sync.cxx
index b3ef500..5dc99a0 100644
--- a/bdep/sync.cxx
+++ b/bdep/sync.cxx
@@ -791,6 +791,8 @@ namespace bdep
   // configurations (as opposed to paths). Also upgrade can only be specified
   // with origin_prj.
   //
+  // Note that pkg_args and dep_pkgs may contain groups.
+  //
   static void
   cmd_sync (const common_options& co,
             const dir_path& origin_prj,
@@ -853,6 +855,9 @@ namespace bdep
     // Note that this may add more (implicit) configurations to origin_prj's
     // entry.
     //
+    // @@ We may end up openning the database (in load_implicit()) for each
+    //    project multiple times.
+    //
     for (const linked_config& cfg: linked_cfgs)
       load_implicit (co, cfg.path, prjs, origin_prj, origin_tr);
 
@@ -867,8 +872,10 @@ namespace bdep
         {
           auto& pkgs (cfg->packages);
 
-          for (const string& n: dep_pkgs)
+          for (cli::vector_group_scanner s (dep_pkgs); s.more (); )
           {
+            const char* n (s.next ());
+
             if (find_if (pkgs.begin (), pkgs.end (),
                          [&n] (const package_state& ps)
                          {
@@ -876,6 +883,8 @@ namespace bdep
                          }) != pkgs.end ())
               fail << "initialized package " << n << " specified as dependency" <<
                 info << "package initialized in project " << prj.path;
+
+            s.skip_group ();
           }
         }
       }
@@ -959,23 +968,30 @@ namespace bdep
     {
       if (origin_only)
       {
-        for (const string& a: pkg_args)
+        for (cli::vector_group_scanner s (pkg_args); s.more (); )
         {
-          if (a.find ('=') == string::npos)
+          if (strchr (s.next (), '=') == nullptr)
           {
             origin_only = false;
             break;
           }
+
+          s.skip_group ();
         }
       }
 
-      for (const string& a: pkg_args)
+      for (cli::vector_group_scanner s (pkg_args); s.more (); )
       {
-        size_t p (a.find ('='));
-        if (p == string::npos)
+        const char* a (s.next ());
+        const char* p (strchr (a, '='));
+
+        if (p == nullptr)
+        {
+          s.skip_group ();
           continue;
+        }
 
-        if (a.front () != '!')
+        if (*a != '!')
         {
           if (!dep_pkgs.empty ())
             dep_vars = true;
@@ -989,13 +1005,16 @@ namespace bdep
           }
           else
             fail << "non-global configuration variable " <<
-              string (a, 0, p) << " without packages or dependencies";
+              string (a, 0, p - a) << " without packages or dependencies";
 
           if (dep_vars || origin_vars)
             continue;
         }
 
         args.push_back (a);
+
+        // Note: we let diagnostics for unhandled groups cover groups for
+        //       configuration variables.
       }
 
       if (!args.empty ())
@@ -1074,9 +1093,17 @@ namespace bdep
           //
           if (vars)
           {
-            for (const string& a: pkg_args)
-              if (a.find ('=') != string::npos && a.front () != '!')
-                args.push_back (a);
+            for (cli::vector_group_scanner s (pkg_args); s.more (); )
+            {
+              const char* a (s.next ());
+              if (strchr (a, '=') != nullptr)
+              {
+                if (*a != '!')
+                  args.push_back (a);
+              }
+              else
+                s.skip_group ();
+            }
           }
 
           if (g)
@@ -1121,8 +1148,13 @@ namespace bdep
     //
     if (upgrade)
     {
-      for (const string& n: dep_pkgs)
+      for (cli::vector_group_scanner s (dep_pkgs); s.more (); )
       {
+        const char* n (s.next ());
+
+        // Note that we are using the leading group for our options since
+        // we pass the user's group as trailing below.
+        //
         bool g (multi_cfg || dep_vars);
         if (g)
           args.push_back ("{");
@@ -1152,9 +1184,17 @@ namespace bdep
         //
         if (dep_vars)
         {
-          for (const string& a: pkg_args)
-            if (a.find ('=') != string::npos && a.front () != '!')
-              args.push_back (a);
+          for (cli::vector_group_scanner s (pkg_args); s.more (); )
+          {
+            const char* a (s.next ());
+            if (strchr (a, '=') != nullptr)
+            {
+              if (*a != '!')
+                args.push_back (a);
+            }
+            else
+              s.skip_group ();
+          }
         }
 
         if (g)
@@ -1162,44 +1202,192 @@ namespace bdep
 
         // Make sure it is treated as a dependency.
         //
-        args.push_back ('?' + n);
+        args.push_back (string ("?") + n);
+
+        // Note that bpkg expects options first and configuration variables
+        // last. Which mean that if we have dep_vars above and an option
+        // below, then things will blow up. Though it's unclear what option
+        // someone may want to pass here.
+        //
+        cli::scanner& gs (s.group ());
+        if (gs.more ())
+        {
+          args.push_back ("+{");
+          for (; gs.more (); args.push_back (gs.next ())) ;
+          args.push_back ("}");
+        }
       }
     }
 
     // Finally, add packages (?<pkg>) from pkg_args, if any.
     //
     // Similar to the dep_pkgs case above, we restrict this to the origin
-    // configurations.
+    // configurations unless configuration(s) were explicitly specified by the
+    // user.
     //
-    for (const string& a: pkg_args)
+    for (cli::vector_group_scanner s (pkg_args); s.more (); )
     {
-      if (a.find ('=') != string::npos)
+      const char* ca (s.next ());
+
+      if (strchr (ca, '=') != nullptr)
         continue;
 
-      if (multi_cfg)
+      string a (ca); // Not guaranteed to be valid after group processing.
+
+      cli::scanner& gs (s.group ());
+
+      if (gs.more () || multi_cfg)
       {
         args.push_back ("{");
 
-        // Note that here (unlike the dep_pkgs case above), we have to make
-        // sure the configuration is actually involved.
-        //
-        for (const sync_config& ocfg: origin_cfgs)
+        cmd_sync_pkg_options po;
+        try
         {
-          if (find_if (cfgs.begin (), cfgs.end (),
-                       [&ocfg] (const config& cfg)
-                       {
-                         return ocfg.path () == cfg.path.get ();
-                       }) == cfgs.end ())
-            continue;
+          while (gs.more ())
+          {
+            const char* a (gs.peek ());
+
+            // Handle @<cfg-name> & -@<cfg-name>.
+            //
+            if (*a == '@' || (*a == '-' && a[1] == '@'))
+            {
+              string n (a + (*a == '@' ? 1 : 2));
+
+              if (n.empty ())
+                fail << "missing configuration name in '" << a << "'";
+
+              po.config_name ().emplace_back (move (n), gs.position ());
+              po.config_name_specified (true);
+
+              gs.next ();
+              continue;
+            }
+
+            if (!po.parse (gs))
+              break;
+          }
+        }
+        catch (const cli::exception& e)
+        {
+          fail << e << " grouped for package " << a;
+        }
 
-          args.push_back ("--config-uuid=" +
-                          linked_cfgs.find (ocfg.path ())->uuid.string ());
+        if (po.config_specified ()    ||
+            po.config_id_specified () ||
+            po.config_name_specified ())
+        {
+          auto append = [&linked_cfgs, &args, &a] (const dir_path& d)
+          {
+            if (const linked_config* cfg = linked_cfgs.find (d))
+            {
+              args.push_back ("--config-uuid=" + cfg->uuid.string ());
+            }
+            else
+              fail << "configuration " << d << " is not part of linked "
+                   << "configuration cluster being synchronized" <<
+                info << "specified for package " << a;
+          };
+
+          // We will be a bit lax and allow specifying with --config any
+          // configuration in the cluster, not necessarily associated with the
+          // origin project (which we may not have).
+          //
+          for (dir_path d: po.config ())
+            append (normalize (d, "configuration"));
+
+          if (const char* o = (po.config_id_specified ()  ? "--config-id" :
+                               po.config_name_specified () ? "--config-name|-n" :
+                               nullptr))
+          {
+            if (!origin)
+              fail << o << "specified without project" <<
+                info << "specified for package " << a;
+
+            // Origin project is first.
+            //
+            const dir_path& pd (prjs.front ().path);
+
+            auto lookup = [&append, &po, &pd] (database& db)
+            {
+              // Similar code to find_configurations().
+              //
+              using query = bdep::query<configuration>;
+
+              for (uint64_t id: po.config_id ())
+              {
+                if (auto cfg = db.find<configuration> (id))
+                  append (cfg->path);
+                else
+                  fail << "no configuration id " << id << " in project " << pd;
+              }
+
+              for (const string& n: po.config_name ())
+              {
+                if (auto cfg = db.query_one<configuration> (query::name == n))
+                  append (cfg->path);
+                else
+                  fail << "no configuration name '" << n << "' in project "
+                       << pd;
+              }
+            };
+
+            // Reuse the transaction, if any (similar to load_implicit()).
+            //
+            if (origin_tr != nullptr)
+            {
+              lookup (origin_tr->database ());
+            }
+            else
+            {
+              // Save and restore the current transaction, if any.
+              //
+              transaction* ct (nullptr);
+              if (transaction::has_current ())
+              {
+                ct = &transaction::current ();
+                transaction::reset_current ();
+              }
+
+              auto tg (make_guard ([ct] ()
+                                   {
+                                     if (ct != nullptr)
+                                       transaction::current (*ct);
+                                   }));
+
+              database db (open (pd, trace));
+              transaction t (db.begin ());
+              lookup (db);
+              t.commit ();
+            }
+          }
+        }
+        else
+        {
+          // Note that here (unlike the dep_pkgs case above), we have to make
+          // sure the configuration is actually involved.
+          //
+          for (const sync_config& ocfg: origin_cfgs)
+          {
+            if (find_if (cfgs.begin (), cfgs.end (),
+                         [&ocfg] (const config& cfg)
+                         {
+                           return ocfg.path () == cfg.path.get ();
+                         }) == cfgs.end ())
+              continue;
+
+            args.push_back ("--config-uuid=" +
+                            linked_cfgs.find (ocfg.path ())->uuid.string ());
+          }
         }
 
+        // Add the rest of group arguments (e.g., configuration variables).
+        //
+        for (; gs.more (); args.push_back (gs.next ())) ;
+
         args.push_back ("}+");
       }
 
-      args.push_back (a);
+      args.push_back (move (a));
     }
 
     // We do a separate fetch instead of letting pkg-build do it. This way we
@@ -1751,8 +1939,8 @@ namespace bdep
   cmd_sync (const common_options& co,
             const dir_path& prj,
             const shared_ptr<configuration>& c,
-            const strings& pkg_args,
             bool implicit,
+            const strings& pkg_args,
             bool fetch,
             bool yes,
             bool name_cfg,
@@ -1842,6 +2030,8 @@ namespace bdep
     // starts with '?' (dependency flag) or contains '=' (config variable),
     // then we assume it is pkg-args.
     //
+    // Note: scan_argument() passes through groups.
+    //
     strings pkg_args;
     strings dep_pkgs;
     while (args.more ())
diff --git a/bdep/sync.hxx b/bdep/sync.hxx
index d3f5429..4e092dc 100644
--- a/bdep/sync.hxx
+++ b/bdep/sync.hxx
@@ -39,8 +39,8 @@ namespace bdep
   cmd_sync (const common_options&,
             const dir_path& prj,
             const shared_ptr<configuration>&,
-            const strings& pkg_args,
             bool implicit,
+            const strings& pkg_args = strings (),
             bool fetch = true,
             bool yes = true,
             bool name_cfg = false,
diff --git a/bdep/utility.hxx b/bdep/utility.hxx
index d5e98c8..8e3ca3c 100644
--- a/bdep/utility.hxx
+++ b/bdep/utility.hxx
@@ -303,6 +303,26 @@ namespace bdep
     return r;
   }
 
+  namespace cli
+  {
+    class vector_group_scanner: public group_scanner
+    {
+    public:
+      explicit
+      vector_group_scanner (const std::vector<std::string>& args)
+          : group_scanner (scan_), scan_ (args) {}
+
+      void
+      skip_group ()
+      {
+        for (scanner& g (group ()); g.more (); g.skip ()) ;
+      }
+
+    private:
+      vector_scanner scan_;
+    };
+  }
+
   // Verify that a string is a valid UTF-8 byte sequence encoding only the
   // graphic Unicode codepoints. Issue diagnostics (including a suggestion to
   // use option opt, if specified) and fail if that's not the case.
diff --git a/doc/cli.sh b/doc/cli.sh
index dc44239..b6405c5 100755
--- a/doc/cli.sh
+++ b/doc/cli.sh
@@ -83,7 +83,7 @@ compile "bdep" $o --output-prefix "" --class-doc bdep::commands=short --class-do
 # the help topics sections in bdep/buildfile and help.cxx.
 #
 pages="new help init sync fetch status ci release publish deinit config test \
-update clean projects-configs default-options-files"
+update clean projects-configs argument-grouping default-options-files"
 
 for p in $pages; do
   compile $p $o