From 645450d27b340439878a073c67a477bb380c15af Mon Sep 17 00:00:00 2001
From: Karen Arutyunov <karen@codesynthesis.com>
Date: Mon, 15 Feb 2021 21:46:05 +0300
Subject: Regularize step ids and introduce step id fallbacks

---
 bbot/worker/worker.cxx       | 192 +++++++++++++++++++++++++++++++------------
 doc/manual.cli               |  52 ++++++------
 tests/integration/testscript |  16 +++-
 3 files changed, 181 insertions(+), 79 deletions(-)

diff --git a/bbot/worker/worker.cxx b/bbot/worker/worker.cxx
index 3f7b941..ef86dd4 100644
--- a/bbot/worker/worker.cxx
+++ b/bbot/worker/worker.cxx
@@ -511,33 +511,43 @@ build (size_t argc, const char* argv[])
     //
     enum class step_id
     {
-      bpkg_configure_create,
+      bpkg_create,
       bpkg_configure_add,
       bpkg_configure_fetch,
       bpkg_configure_build,
-      bpkg_update_update,
-      bpkg_test_test,
-      bpkg_install_install,
+      bpkg_update,
+      bpkg_test,
+      bpkg_install,
       b_test_installed_create,
       b_test_installed_configure,
       b_test_installed_test,
       bpkg_test_installed_create,
-      bpkg_uninstall_uninstall
+      bpkg_test_installed_configure_add,
+      bpkg_test_installed_configure_fetch,
+      bpkg_test_installed_configure_build,
+      bpkg_test_installed_update,
+      bpkg_test_installed_test,
+      bpkg_uninstall
     };
 
     const strings step_id_str {
-      "bpkg.configure.create",
+      "bpkg.create",
       "bpkg.configure.add",
       "bpkg.configure.fetch",
       "bpkg.configure.build",
-      "bpkg.update.update",
-      "bpkg.test.test",
-      "bpkg.install.install",
+      "bpkg.update",
+      "bpkg.test",
+      "bpkg.install",
       "b.test-installed.create",
       "b.test-installed.configure",
       "b.test-installed.test",
       "bpkg.test-installed.create",
-      "bpkg.uninstall.uninstall"};
+      "bpkg.test-installed.configure.add",
+      "bpkg.test-installed.configure.fetch",
+      "bpkg.test-installed.configure.build",
+      "bpkg.test-installed.update",
+      "bpkg.test-installed.test",
+      "bpkg.uninstall"};
 
     // Split the argument into prefix (empty if not present) and unquoted
     // value. Return nullopt if the prefix is invalid.
@@ -570,7 +580,7 @@ build (size_t argc, const char* argv[])
         args.emplace (move (a));
       else
       {
-        args.emplace ("bpkg.configure.create", a.second);
+        args.emplace ("bpkg.create", a.second);
         args.emplace ("b.test-installed.create", a.second);
         args.emplace ("bpkg.test-installed.create", move (a.second));
       }
@@ -620,9 +630,10 @@ build (size_t argc, const char* argv[])
 
       bool mod (v->second[0] != '-' && v->second.find ('=') == string::npos);
 
-      if (mod && !v->first.empty () &&
-          v->first != "bpkg.configure.create" &&
-          v->first != "b.test-installed.create")
+      if (mod && !v->first.empty ()             &&
+          v->first != "bpkg.create"             &&
+          v->first != "b.test-installed.create" &&
+          v->first != "bpkg.test-installed.create")
         fail << "invalid module prefix in '" << a << "'";
 
       add_arg (mod ? modules : env_args, move (*v));
@@ -632,10 +643,19 @@ build (size_t argc, const char* argv[])
     // specific prefixes come last.
     //
     auto step_args = [&step_id_str] (const std::multimap<string, string>& args,
-                                     step_id step) -> strings
+                                     step_id step,
+                                     optional<step_id> fallback = nullopt)
+                     -> strings
     {
       strings r;
-      const string& s (step_id_str[static_cast<size_t> (step)]);
+      const string& sid (step_id_str[static_cast<size_t> (step)]);
+
+      // If no arguments found for the step id, then use the fallback step id,
+      // if specified.
+      //
+      const string& s (args.find (sid) == args.end () && fallback
+                       ? step_id_str[static_cast<size_t> (*fallback)]
+                       : sid);
 
       for (size_t n (0);; ++n)
       {
@@ -656,12 +676,12 @@ build (size_t argc, const char* argv[])
 
     // Search for config.install.root variable. If it is present and has a
     // non-empty value, then test the package installation and uninstall. Note
-    // that passing [null] value would be meaningless, so we don't recognize it
-    // as a special one. While at it, cache the bpkg.configure.create args for
-    // later use.
+    // that passing [null] value would be meaningless, so we don't recognize
+    // it as a special one. While at it, cache the bpkg.create args for later
+    // use.
     //
     dir_path install_root;
-    strings cargs (step_args (config_args, step_id::bpkg_configure_create));
+    strings cargs (step_args (config_args, step_id::bpkg_create));
     {
       size_t n (19);
       auto space = [] (char c) {return c == ' ' || c == '\t';};
@@ -814,6 +834,8 @@ build (size_t argc, const char* argv[])
       return true;
     };
 
+    // The main phase.
+    //
     // If this is a build system module, perform a "pre-step" by building it
     // in a separate configuration reproducing the one used to build build2
     // itself. Note that the configuration and the environment options and
@@ -1043,7 +1065,7 @@ build (size_t argc, const char* argv[])
 
       // bpkg create <env-modules> <env-config-args> <config-args>
       //
-      // bpkg.configure.create
+      // bpkg.create
       //
       {
         // If the package is a build system module, then make sure it is
@@ -1055,8 +1077,8 @@ build (size_t argc, const char* argv[])
           "create",
           "-d", build_dir.string (),
           "--wipe",
-          step_args (modules,  step_id::bpkg_configure_create),
-          step_args (env_args, step_id::bpkg_configure_create),
+          step_args (modules,  step_id::bpkg_create),
+          step_args (env_args, step_id::bpkg_create),
           cargs,
           module && !bootstrap ? module_import.c_str () : nullptr);
 
@@ -1143,14 +1165,14 @@ build (size_t argc, const char* argv[])
 
       // bpkg update <env-config-args> <config-args> <package-name>
       //
-      // bpkg.update.update
+      // bpkg.update
       //
       r.status |= run_bpkg (
         trace, r.log, wre,
         "-v",
         "update",
-        step_args (env_args,    step_id::bpkg_update_update),
-        step_args (config_args, step_id::bpkg_update_update),
+        step_args (env_args,    step_id::bpkg_update),
+        step_args (config_args, step_id::bpkg_update),
         pkg);
 
       if (!r.status)
@@ -1211,6 +1233,11 @@ build (size_t argc, const char* argv[])
     // override and/or set the environment variables for bpkg processes.
     // Return true if all operations for all packages succeed.
     //
+    // Pass true as the installed argument to use the test installed phase
+    // step ids (bpkg.test-installed.*), falling back to the main phase step
+    // ids (bpkg.*) when no environment/configuration arguments are specified
+    // for them.
+    //
     // Pass true as the sys_dep argument to configure the dependent package as
     // a system dependency, which is normally required for testing modules and
     // installed dependents. Note that bpkg configures the dependent package
@@ -1222,10 +1249,22 @@ build (size_t argc, const char* argv[])
                  &redist]
                 (operation_result& r,
                  const dir_path& dist_root,
+                 bool installed,
                  bool sys_dep,
                  const char* import = nullptr,
                  const small_vector<string, 1>& envvars = {})
     {
+      auto args = [installed, &step_args] (
+        const std::multimap<string, string>& args,
+        step_id main_step,
+        step_id test_installed_step)
+      {
+        return installed
+               ? step_args (args, test_installed_step, main_step)
+               : step_args (args, main_step);
+
+      };
+
       for (const test_dependency& td: pm.tests)
       {
         const string& pkg (td.name.string ());
@@ -1245,8 +1284,15 @@ build (size_t argc, const char* argv[])
           "--configure-only",
           "--checkout-root", dist_root,
           "--yes",
-          step_args (env_args,    step_id::bpkg_configure_build),
-          step_args (config_args, step_id::bpkg_configure_build),
+
+          args (env_args,
+                step_id::bpkg_configure_build,
+                step_id::bpkg_test_installed_configure_build),
+
+          args (config_args,
+                step_id::bpkg_configure_build,
+                step_id::bpkg_test_installed_configure_build),
+
           import,
           "--",
           td.string (),
@@ -1297,8 +1343,15 @@ build (size_t argc, const char* argv[])
           trace, r.log, wre,
           "-v",
           "update",
-          step_args (env_args,    step_id::bpkg_update_update),
-          step_args (config_args, step_id::bpkg_update_update),
+
+          args (env_args,
+                step_id::bpkg_update,
+                step_id::bpkg_test_installed_update),
+
+          args (config_args,
+                step_id::bpkg_update,
+                step_id::bpkg_test_installed_update),
+
           import,
           pkg);
 
@@ -1320,8 +1373,15 @@ build (size_t argc, const char* argv[])
           "-v",
           "test",
           "--package-cwd", // See above for details.
-          step_args (env_args,    step_id::bpkg_test_test),
-          step_args (config_args, step_id::bpkg_test_test),
+
+          args (env_args,
+                step_id::bpkg_test,
+                step_id::bpkg_test_installed_test),
+
+          args (config_args,
+                step_id::bpkg_test,
+                step_id::bpkg_test_installed_test),
+
           import,
           pkg);
 
@@ -1351,15 +1411,15 @@ build (size_t argc, const char* argv[])
       {
         // bpkg test <env-config-args> <config-args> <package-name>
         //
-        // bpkg.test.test
+        // bpkg.test
         //
         r.status |= run_bpkg (
           trace, r.log, wre,
           "-v",
           "test",
           "--package-cwd", // See above for details.
-          step_args (env_args,    step_id::bpkg_test_test),
-          step_args (config_args, step_id::bpkg_test_test),
+          step_args (env_args,    step_id::bpkg_test),
+          step_args (config_args, step_id::bpkg_test),
           pkg);
 
         if (!r.status)
@@ -1376,6 +1436,7 @@ build (size_t argc, const char* argv[])
       if (external_tests &&
           !test (r,
                  dist_root,
+                 false /* installed */,
                  module,
                  bootstrap ? module_import.c_str () : nullptr))
         break;
@@ -1416,14 +1477,14 @@ build (size_t argc, const char* argv[])
 
       // bpkg install <env-config-args> <config-args> <package-name>
       //
-      // bpkg.install.install
+      // bpkg.install
       //
       r.status |= run_bpkg (
         trace, r.log, wre,
         "-v",
         "install",
-        step_args (env_args,    step_id::bpkg_install_install),
-        step_args (config_args, step_id::bpkg_install_install),
+        step_args (env_args,    step_id::bpkg_install),
+        step_args (config_args, step_id::bpkg_install),
         pkg);
 
       if (!r.status)
@@ -1432,7 +1493,7 @@ build (size_t argc, const char* argv[])
       rm.status |= r.status;
     }
 
-    // Test installed.
+    // The test installed phase.
     //
     // Make sure that the installed package executables are properly imported
     // when configuring and running tests, unless we are testing the build
@@ -1585,7 +1646,7 @@ build (size_t argc, const char* argv[])
         //
         // bpkg create <env-modules> <env-config-args> <config-args>
         //
-        // bpkg.test-installed.create
+        // bpkg.test-installed.create (fallback to bpkg.create)
         //
         dir_path config_dir ("build-installed-bpkg");
 
@@ -1595,9 +1656,18 @@ build (size_t argc, const char* argv[])
           "create",
           "-d", config_dir.string (),
           "--wipe",
-          step_args (modules,     step_id::bpkg_test_installed_create),
-          step_args (env_args,    step_id::bpkg_test_installed_create),
-          step_args (config_args, step_id::bpkg_test_installed_create));
+
+          step_args (modules,
+                     step_id::bpkg_test_installed_create,
+                     step_id::bpkg_create),
+
+          step_args (env_args,
+                     step_id::bpkg_test_installed_create,
+                     step_id::bpkg_create),
+
+          step_args (config_args,
+                     step_id::bpkg_test_installed_create,
+                     step_id::bpkg_create));
 
         if (!r.status)
           break;
@@ -1606,14 +1676,21 @@ build (size_t argc, const char* argv[])
 
         // bpkg add <env-config-args> <config-args> <repository-url>
         //
-        // bpkg.configure.add
+        // bpkg.test-installed.configure.add (fallback to bpkg.configure.add)
         //
         r.status |= run_bpkg (
           trace, r.log, wre,
           "-v",
           "add",
-          step_args (env_args,    step_id::bpkg_configure_add),
-          step_args (config_args, step_id::bpkg_configure_add),
+
+          step_args (env_args,
+                     step_id::bpkg_test_installed_configure_add,
+                     step_id::bpkg_configure_add),
+
+          step_args (config_args,
+                     step_id::bpkg_test_installed_configure_add,
+                     step_id::bpkg_configure_add),
+
           repo);
 
         if (!r.status)
@@ -1621,14 +1698,22 @@ build (size_t argc, const char* argv[])
 
         // bpkg fetch <env-config-args> <config-args> <trust-options>
         //
-        // bpkg.configure.fetch
+        // bpkg.test-installed.configure.fetch (fallback to
+        // bpkg.configure.fetch)
         //
         r.status |= run_bpkg (
           trace, r.log, wre,
           "-v",
           "fetch",
-          step_args (env_args,    step_id::bpkg_configure_fetch),
-          step_args (config_args, step_id::bpkg_configure_fetch),
+
+          step_args (env_args,
+                     step_id::bpkg_test_installed_configure_fetch,
+                     step_id::bpkg_configure_fetch),
+
+          step_args (config_args,
+                     step_id::bpkg_test_installed_configure_fetch,
+                     step_id::bpkg_configure_fetch),
+
           trust_ops);
 
         if (!r.status)
@@ -1638,6 +1723,7 @@ build (size_t argc, const char* argv[])
         //
         if (!test (r,
                    rwd / dir_path ("dist-installed"),
+                   true    /* installed */,
                    true    /* sys_dep */,
                    nullptr /* import */,
                    envvars))
@@ -1647,6 +1733,8 @@ build (size_t argc, const char* argv[])
       rm.status |= r.status;
     }
 
+    // Back to the main phase.
+    //
     // Uninstall.
     //
     {
@@ -1656,14 +1744,14 @@ build (size_t argc, const char* argv[])
 
       // bpkg uninstall <env-config-args> <config-args> <package-name>
       //
-      // bpkg.uninstall.uninstall
+      // bpkg.uninstall
       //
       r.status |= run_bpkg (
         trace, r.log, wre,
         "-v",
         "uninstall",
-        step_args (env_args,    step_id::bpkg_uninstall_uninstall),
-        step_args (config_args, step_id::bpkg_uninstall_uninstall),
+        step_args (env_args,    step_id::bpkg_uninstall),
+        step_args (config_args, step_id::bpkg_uninstall),
         pkg);
 
       if (!r.status)
diff --git a/doc/manual.cli b/doc/manual.cli
index c5781ae..4337627 100644
--- a/doc/manual.cli
+++ b/doc/manual.cli
@@ -784,7 +784,7 @@ values as discussed in \l{#arch-controller Controller Logic}. The
 \c{<>}-values are from the task manifest and the environment:
 
 \
-# bpkg.configure.create
+# bpkg.create
 #
 bpkg -V create <env-modules> <env-config-args> <config-args>
 
@@ -800,13 +800,13 @@ bpkg -v fetch --trust <repository-fp>
 #
 bpkg -v build --yes --configure-only <package-name>/<package-version>
 
-# bpkg.update.update
+# bpkg.update
 #
 bpkg -v update <package-name>
 
 # if the test operation is supported by the package:
 #
-# bpkg.test.test
+# bpkg.test
 #
 bpkg -v test <package-name>
 
@@ -820,11 +820,11 @@ bpkg -v test <package-name>
   bpkg -v build --yes --configure-only \\
        '<package-name> [<version-constraint>]'
 
-  # bpkg.update.update
+  # bpkg.update
   #
   bpkg -v update <package-name>
 
-  # bpkg.test.test
+  # bpkg.test
   #
   bpkg -v test <package-name>
 }
@@ -832,7 +832,7 @@ bpkg -v test <package-name>
 # if config.install.root is specified:
 #
 {
-  # bpkg.install.install
+  # bpkg.install
   #
   bpkg -v install <package-name>
 
@@ -858,15 +858,17 @@ bpkg -v test <package-name>
   # task manifest values:
   #
   {
-    # bpkg.test-installed.create
+    # bpkg.test-installed.create (fallback to bpkg.create if absent)
     #
     bpkg -V create <env-modules> <env-config-args> <config-args>
 
-    # bpkg.configure.add
+    # bpkg.test-installed.configure.add (fallback to
+    # bpkg.configure.add if absent)
     #
     bpkg -v add <repository-url>
 
-    # bpkg.configure.fetch
+    # bpkg.test-installed.configure.fetch (fallback to
+    # bpkg.configure.fetch if absent)
     #
     bpkg -v fetch --trust <repository-fp>
 
@@ -875,22 +877,24 @@ bpkg -v test <package-name>
     # test-exclude task manifest values:
     #
     {
-      # bpkg.configure.build
+      # bpkg.test-installed.configure.build (fallback to
+      # bpkg.configure.build if absent)
       #
       bpkg -v build --yes --configure-only \\
            '<package-name> [<version-constraint>]'
 
-      # bpkg.update.update
+      # bpkg.test-installed.update (fallback to bpkg.update if
+      # absent)
       #
       bpkg -v update <package-name>
 
-      # bpkg.test.test
+      # bpkg.test-installed.test (fallback to bpkg.test if absent)
       #
       bpkg -v test <package-name>
     }
   }
 
-  # bpkg.uninstall.uninstall
+  # bpkg.uninstall
   #
   bpkg -v uninstall <package-name>
 }
@@ -981,7 +985,7 @@ are ignored. All other lines in this file have the following format:
 <machine-pattern> <config> <target>[/<environment>] <classes> [<config-arg>]* [<warning-regex>]*
 
 <config-arg> = [<prefix>:](<variable>|<option>)
-<prefix> = <tool>[.<operation>[.<command>]]
+<prefix> = <tool>[.phase][.<operation>[.<command>]]
 \
 
 Where \c{<machine-pattern>} is filesystem wildcard pattern that is matched
@@ -1012,8 +1016,8 @@ manifest.
 
 Values in the \c{<config-arg>} list can be opionally prefixed with the \i{step
 id} or a leading portion thereof to restrict it to a specific step, operation,
-or tool in the \i{worked script} (see \l{#arch-worker Worker
-Logic}). Unprefixed values only apply to the \c{bpkg.configure.create},
+phase, or tool in the \i{worker script} (see \l{#arch-worker Worker
+Logic}). Unprefixed values only apply to the \c{bpkg.create},
 \c{b.test-installed.create}, and \c{bpkg.test-installed.create} steps. Note
 that options with values can only be specified using the single argument
 notation. For example:
@@ -1082,14 +1086,14 @@ linux*-gcc_8 linux-gcc_8 x86_64-linux-gnu \"all gcc-8+:gcc-7+\"
 \
 
 If the \c{<config-arg>} list contains the \c{config.install.root} variable
-that applies to the \c{bpkg.configure.create} step, then in addition to
-building and possibly running tests, the \c{bbot} worker will also test
-installing and uninstalling each package. Furthermore, if the package contains
-subprojects that support the test operation and/or refers to other packages
-via the \c{tests}, \c{examples}, or \c{benchmarks} manifest values which are
-not excluded by the \c{test-exclude} task manifest values, then the worker
-will additionally build such subprojects/packages against the installation and
-run their tests.
+that applies to the \c{bpkg.create} step, then in addition to building and
+possibly running tests, the \c{bbot} worker will also test installing and
+uninstalling each package. Furthermore, if the package contains subprojects
+that support the test operation and/or refers to other packages via the
+\c{tests}, \c{examples}, or \c{benchmarks} manifest values which are not
+excluded by the \c{test-exclude} task manifest values, then the worker will
+additionally build such subprojects/packages against the installation and run
+their tests (test installed phase).
 
 Two types of installations can be tested: \i{system} and \i{private}. A system
 installation uses a well-known location, such as \c{/usr} or \c{/usr/local},
diff --git a/tests/integration/testscript b/tests/integration/testscript
index 078a4ae..fe1b578 100644
--- a/tests/integration/testscript
+++ b/tests/integration/testscript
@@ -66,7 +66,7 @@ rfp = yes
 
 #\
 pkg = libstudxml
-ver = 1.1.0-b.7.20190619110825.0d1dcfe61f15
+ver =  	1.1.0-b.9.20210202082911.e729667b0f34
 rep_url = https://stage.build2.org/1
 rep_type = pkg
 rfp = yes
@@ -77,7 +77,7 @@ rfp = yes
 # installed into ~/install/bin.
 #
 pkg = libbuild2-hello
-ver = 0.1.0-a.0.20200810114133.0dba3a3bd624
+ver =  	0.1.0-a.0.20201019074759.bba32abb6d3d
 rep_url = "https://github.com/build2/libbuild2-hello.git#master"
 rep_type = git
 #rep_url = https://stage.build2.org/1
@@ -89,7 +89,7 @@ rfp = yes
 # Use the build2 driver installed into ~/install/bin (see above).
 #
 pkg = libbuild2-kconfig
-ver = 0.1.0-a.0.20201020073414.55e1dcf947d5
+ver = 0.1.0-a.0.20210108084836.3687e4b95226
 rep_url = "https://github.com/build2/libbuild2-kconfig.git#master"
 rep_type = git
 #ver = 0.1.0-a.0.20200910053253.a71aa3f3938b
@@ -116,6 +116,16 @@ rep_type = git
 rfp = yes
 #\
 
+#\
+pkg = libxsd
+ver = 4.2.0-b.1.20210215122732.91a2243739fd
+rep_url = "https://git.codesynthesis.com/xsd/xsd.git#build2"
+rep_type = git
+#rep_url = https://stage.build2.org/1
+#rep_type = pkg
+rfp = yes
+#\
+
 # Note that we also need to make sure that the installed package libraries are
 # properly imported when configuring and running tests, and that the installed
 # executables are runnable.
-- 
cgit v1.1