From b76346651a0e27e8824af4bb59224792df8dd4d0 Mon Sep 17 00:00:00 2001
From: Karen Arutyunov <karen@codesynthesis.com>
Date: Wed, 30 Mar 2022 17:13:57 +0300
Subject: Fix bdep-status output in JSON format not to fail for non-initialized
 packages

---
 bdep/project.cxx        |  13 ++-
 bdep/project.hxx        |  17 +++-
 bdep/status.cli         |   7 +-
 bdep/status.cxx         | 146 ++++++++++++++++++++++++++------
 bdep/sync.cxx           |   2 +-
 tests/status.testscript | 217 +++++++++++++++++++++++++++++++++++++++++++-----
 6 files changed, 344 insertions(+), 58 deletions(-)

diff --git a/bdep/project.cxx b/bdep/project.cxx
index 5650dea..975454f 100644
--- a/bdep/project.cxx
+++ b/bdep/project.cxx
@@ -21,7 +21,8 @@ namespace bdep
                        const dir_path& prj,
                        transaction& t,
                        bool fallback_default,
-                       bool validate)
+                       bool validate,
+                       bool allow_none)
   {
     configurations r;
     bool fallback (false);
@@ -126,7 +127,12 @@ namespace bdep
         add (move (c));
 
       if (r.empty ())
+      {
+        if (allow_none)
+          return make_pair (move (r), false /* fallback */);
+
         fail << "no existing configurations";
+      }
     }
 
     // default
@@ -340,7 +346,8 @@ namespace bdep
   project_packages
   find_project_packages (const dir_paths& dirs,
                          bool ignore_packages,
-                         bool load_packages)
+                         bool load_packages,
+                         bool allow_empty)
   {
     project_packages r;
 
@@ -408,7 +415,7 @@ namespace bdep
       // packages are in it or, if nothing was discovered, use it as the
       // source for the package list.
       //
-      package_locations pls (load_package_locations (r.project));
+      package_locations pls (load_package_locations (r.project, allow_empty));
 
       if (!r.packages.empty ())
       {
diff --git a/bdep/project.hxx b/bdep/project.hxx
index 9265945..a26dffd 100644
--- a/bdep/project.hxx
+++ b/bdep/project.hxx
@@ -176,6 +176,10 @@ namespace bdep
   // default configurations if none were mentioned. Unless validate is false,
   // also validate that the configuration directories still exist.
   //
+  // Fail if --all|-a is specified and the project has no associated
+  // configurations, unless allow_none is true in which case return an empty
+  // list of configurations.
+  //
   // Besides configurations, also return an indication if they are retrieved
   // as a fallback to default configurations (true if that's the case).
   //
@@ -186,7 +190,8 @@ namespace bdep
                        const dir_path& prj,
                        transaction&,
                        bool fallback_default = true,
-                       bool validate = true);
+                       bool validate = true,
+                       bool allow_none = false);
 
   // Given a directory which can be a project root, a package root, or, if
   // requested, one of their subdirectories, return the absolute project
@@ -245,12 +250,16 @@ namespace bdep
   project_packages
   find_project_packages (const dir_paths&,
                          bool ignore_packages,
-                         bool load_packages = true);
+                         bool load_packages = true,
+                         bool allow_empty = false);
 
   inline project_packages
-  find_project_packages (const project_options& po, bool ip, bool lp = true)
+  find_project_packages (const project_options& po,
+                         bool ip,
+                         bool lp = true,
+                         bool ae = false)
   {
-    return find_project_packages (po.directory (), ip, lp);
+    return find_project_packages (po.directory (), ip, lp, ae);
   }
 
   inline dir_path
diff --git a/bdep/status.cli b/bdep/status.cli
index f046369..1360606 100644
--- a/bdep/status.cli
+++ b/bdep/status.cli
@@ -111,8 +111,11 @@ namespace bdep
      Refer to the \cb{list} subcommand of \l{bdep-config(1)} for details on
      the \cb{struct} \cb{configuration} members. Refer to
      \l{bpkg-pkg-status(1)} for the definition of \cb{struct}
-     \cb{package_status}.
-     "
+     \cb{package_status}. Note also that in the \cb{json} output format
+     certain conditions (no associated configurations, no initialized
+     packages, etc) are not treated as errors but instead result in valid
+     output. The uninitialized packages have the special \cb{uninitialized}
+     \cb{status} value."
   }
 
   class cmd_status_options: project_options
diff --git a/bdep/status.cxx b/bdep/status.cxx
index 82eac25..c8b74b4 100644
--- a/bdep/status.cxx
+++ b/bdep/status.cxx
@@ -173,14 +173,6 @@ namespace bdep
     //
     for (const shared_ptr<configuration>& c: cfgs)
     {
-      // Collect the packages to print, unless the dependency packages are
-      // specified.
-      //
-      strings pkgs;
-
-      if (dep_pkgs.empty ())
-        pkgs = config_packages (*c, prj_pkgs.packages);
-
       ss.begin_object ();
       ss.member_name ("configuration", false /* check */);
       ss.begin_object ();
@@ -192,7 +184,27 @@ namespace bdep
 
       ss.end_object ();
 
-      if (!c->packages.empty () && (!pkgs.empty () || !dep_pkgs.empty ()))
+      // Collect the initialized packages to print, unless the dependency
+      // packages are specified.
+      //
+      strings pkgs;
+
+      if (dep_pkgs.empty ())
+        pkgs = config_packages (*c, prj_pkgs.packages);
+
+      // Statuses of the project packages or their dependencies represented as
+      // a JSON array. Will be used as a pre-serialized value for the packages
+      // member of the configuration packages status object.
+      //
+      string ps;
+
+      // If we print statuses of the dependency packages, then retrieve them
+      // all as bpkg-status' stdout. Otherwise, retrieve the initialized
+      // packages statuses, if any, as bpkg-status' stdout and append statuses
+      // of uninitialized packages, if any.
+      //
+      const vector<package_state>& cpkgs (c->packages);
+      if (!cpkgs.empty () && (!pkgs.empty () || !dep_pkgs.empty ()))
       {
         const dir_path& prj (prj_pkgs.project);
 
@@ -206,12 +218,6 @@ namespace bdep
         //
         assert (pkgs.empty () == !dep_pkgs.empty ());
 
-        // Save the JSON representation of package statuses into a string from
-        // bpkg-status' stdout and use it as a pre-serialized value for the
-        // packages member of the configuration packages status object.
-        //
-        string ps;
-
         fdpipe pipe (open_pipe ()); // Text mode seems appropriate.
 
         process pr (start_bpkg_status (o,
@@ -249,13 +255,82 @@ namespace bdep
         if (!ps.empty () && ps.back () == '\n')
           ps.pop_back ();
 
-        ss.member_name ("packages", false /* check */);
-        ss.value_json_text (ps);
+        // While at it, let's verify that the output looks like a JSON array,
+        // since we will rely on the presence of the framing brackets below.
+        //
+        if (ps.empty () || ps.front () != '[' || ps.back () != ']')
+          fail << "invalid bpkg-status output:\n" << ps;
       }
-      else
+
+      // Append the uninitialized packages statuses to the JSON array, unless
+      // we are printing the dependency packages.
+      //
+      if (dep_pkgs.empty ())
+      {
+        // If the initialized packages are present and so the array
+        // representation is not empty, then unwrap the statuses removing the
+        // framing brackets before appending the uninitialized packages
+        // statuses and wrap them all back afterwards. Let's do it lazily.
+        //
+        bool first (true);
+
+        for (const package_location& l: prj_pkgs.packages)
+        {
+          if (find_if (cpkgs.begin (),
+                       cpkgs.end (),
+                       [&l] (const package_state& p)
+                       {
+                         return p.name == l.name;
+                       }) == cpkgs.end ()) // Unintialized?
+          {
+            // Unwrap the array representation, if present, on the first
+            // uninitialized package.
+            //
+            if (first)
+            {
+              first = false;
+
+              if (!ps.empty ())
+              {
+                size_t b (ps.find_first_not_of ("[\n"));
+                size_t e (ps.find_last_not_of ("\n]"));
+
+                // We would fail earlier otherwise.
+                //
+                assert (b != string::npos && e != string::npos);
+
+                ps = string (ps, b, e + 1 - b);
+              }
+            }
+
+            if (!ps.empty ())
+              ps += ",\n";
+
+            // Note: here we rely on the fact that the package name doesn't
+            // need escaping.
+            //
+            ps += "  {\n"
+                  "    \"name\": \"" + l.name.string () + "\",\n"
+                  "    \"status\": \"uninitialized\"\n"
+                  "  }";
+          }
+        }
+
+        // Wrap the array if any uninitialized packages statuses have been
+        // added.
+        //
+        if (!first)
+          ps = "[\n" + ps + "\n]";
+      }
+
+      // Note that we can end up with an empty statuses representation (for
+      // example, when query the status of a dependency package in the empty
+      // configuration).
+      //
+      if (!ps.empty ())
       {
-        // Not that unlike in the lines output we don't tell the user that
-        // there are no packages.
+        ss.member_name ("packages", false /* check */);
+        ss.value_json_text (ps);
       }
 
       ss.end_object ();
@@ -279,12 +354,20 @@ namespace bdep
     strings dep_pkgs;
     for (; args.more (); dep_pkgs.push_back (args.next ())) ;
 
-    // The same ignore/load story as in sync.
+    bool json (o.stdout_format () == stdout_format::json);
+
+    // The same ignore/load story as in sync for the lines format.
+    //
+    // For the JSON format we load all the project packages, unless they are
+    // specified explicitly, and allow the empty projects. Note that in
+    // contrast to the lines format we print statuses of uninitialized
+    // packages and empty configurations.
     //
     project_packages pp (
       find_project_packages (o,
                              !dep_pkgs.empty () /* ignore_packages */,
-                             false              /* load_packages   */));
+                             json               /* load_packages */,
+                             json               /* allow_empty */));
 
     const dir_path& prj (pp.project);
 
@@ -293,13 +376,26 @@ namespace bdep
     configurations cfgs;
     {
       transaction t (db.begin ());
-      pair<configurations, bool> cs (find_configurations (o, prj, t));
+
+      // If --all|-a is specified and the project has no associated
+      // configurations let's print an empty array of configurations for the
+      // JSON format instead of failing (as we do for the lines format).
+      //
+      pair<configurations, bool> cs (
+        find_configurations (o,
+                             prj,
+                             t,
+                             true /* fallback_default */,
+                             true /* validate */,
+                             json /* allow_none */));
+
       t.commit ();
 
       // If specified, verify packages are present in at least one
-      // configuration.
+      // configuration. But not for the JSON format where we also print
+      // statuses of uninitialized packages.
       //
-      if (!pp.packages.empty ())
+      if (!pp.packages.empty () && !json)
         verify_project_packages (pp, cs);
 
       cfgs = move (cs.first);
diff --git a/bdep/sync.cxx b/bdep/sync.cxx
index 113cb04..c253220 100644
--- a/bdep/sync.cxx
+++ b/bdep/sync.cxx
@@ -1716,7 +1716,7 @@ namespace bdep
           diag_record dr (fail);
           dr << "unexpected " << name_bpkg (co) << " output:";
           for (const string& s: dep_chain)
-            dr << '\n' << s ;
+            dr << '\n' << s;
         }
 
         break;
diff --git a/tests/status.testscript b/tests/status.testscript
index e5e111b..4209969 100644
--- a/tests/status.testscript
+++ b/tests/status.testscript
@@ -9,51 +9,115 @@ new    += 2>!
 init   += $config_cxx -d prj 2>!
 sync   += -d prj 2>!
 deinit += -d prj
+config += -d prj 2>!
 
 : no-cfg
 :
 {
-  $clone_prj;
+  : default
+  :
+  {
+    $clone_root_prj;
 
-  $* 2>>/"EOE" != 0
-    error: no default configuration in project $~/prj/
-      info: use \(@<cfg-name> | --config|-c <cfg-dir> | --all|-a\) to specify configuration explicitly
-    EOE
+    $* 2>>/"EOE" != 0;
+      error: no default configuration in project $~/prj/
+        info: use \(@<cfg-name> | --config|-c <cfg-dir> | --all|-a\) to specify configuration explicitly
+      EOE
+
+    $* --stdout-format 'json' 2>>/"EOE" != 0
+      error: no default configuration in project $~/prj/
+        info: use \(@<cfg-name> | --config|-c <cfg-dir> | --all|-a\) to specify configuration explicitly
+      EOE
+  }
+
+  : all
+  :
+  {
+    $clone_root_prj;
+
+    $* -a 2>>EOE != 0;
+      error: no existing configurations
+      EOE
+
+    $* -a --stdout-format 'json' >'[]'
+  }
 }
 
 : single-cfg
 :
 {
-  $clone_prj;
+  : single-project
+  :
+  {
+    $clone_root_prj;
 
-  $init -C @cfg &prj-cfg/***;
+    $init -C @cfg &prj-cfg/***;
 
-  $* >'prj configured 0.1.0-a.0.19700101000000';
+    $* >'prj configured 0.1.0-a.0.19700101000000';
 
-  # While at it, test printing the status in the JSON format.
-  #
-  $* --stdout-format 'json' >>~%EOO%;
+    $* --stdout-format 'json' >>~%EOO%;
+      [
+        {
+          "configuration": {
+            "id": 1,
+      %      "path": ".+prj-cfg",%
+            "name": "cfg"
+          },
+          "packages": [
+        {
+      %.+
+        }
+      ]
+        }
+      ]
+      EOO
+
+    $deinit 2>>/"EOE"
+      deinitializing in project $~/prj/
+      synchronizing:
+        drop prj
+      EOE
+  }
+
+  : no-projects
+  :
+  {
+    $new -t empty prj &prj/***;
+
+    $config create @cfg &prj-cfg/***;
+
+    $* 2>>~%EOE% != 0;
+      %error: no packages in project .+%
+      EOE
+
+    $* --stdout-format 'json' >>~%EOO%;
     [
       {
         "configuration": {
           "id": 1,
     %      "path": ".+prj-cfg",%
           "name": "cfg"
-        },
-        "packages": [
-      {
-    %.+
+        }
       }
     ]
+    EOO
+
+    $* libbar 2>>EOE;
+      info: no packages initialized in configuration @cfg, skipping
+      EOE
+
+    $* --stdout-format 'json' libbar >>~%EOO%
+    [
+      {
+        "configuration": {
+          "id": 1,
+    %      "path": ".+prj-cfg",%
+          "name": "cfg"
+        }
       }
     ]
     EOO
-
-  $deinit 2>>/"EOE"
-    deinitializing in project $~/prj/
-    synchronizing:
-      drop prj
-    EOE
+  }
 }
 
 : multi-cfg
@@ -74,8 +138,6 @@ deinit += -d prj
     prj configured 0.1.0-a.0.19700101000000
     EOO
 
-  # While at it, test printing the status in the JSON format.
-  #
   $* --all --stdout-format 'json' >>~%EOO%;
     [
       {
@@ -112,6 +174,98 @@ deinit += -d prj
     EOE
 }
 
+: multi-prj
+:
+{
+  $new -t empty prj &prj/***;
+  $new -d prj --package pkg1;
+  $new -d prj --package pkg2;
+
+  $init -d prj/pkg1 -C @cfg &prj-cfg/***;
+
+  # Initialized package is specified.
+  #
+  $* -d prj/pkg1 >>EOE;
+    pkg1 configured 0.1.0-a.0.19700101000000
+    EOE
+
+  $* -d prj/pkg1 --stdout-format 'json' >>~%EOO%;
+    [
+      {
+        "configuration": {
+          "id": 1,
+    %      "path": ".+prj-cfg",%
+          "name": "cfg"
+        },
+        "packages": [
+      {
+        "name": "pkg1",
+        "status": "configured",
+        "version": "0.1.0-a.0.19700101000000",
+        "hold_package": true,
+        "hold_version": true
+      }
+    ]
+      }
+    ]
+    EOO
+
+  # Uninitialized package is specified.
+  #
+  $* -d prj/pkg2 2>>EOE != 0;
+    error: package pkg2 is not initialized in any default configuration(s)
+    EOE
+
+  $* -d prj/pkg2 --stdout-format 'json' >>~%EOO%;
+    [
+      {
+        "configuration": {
+          "id": 1,
+    %      "path": ".+prj-cfg",%
+          "name": "cfg"
+        },
+        "packages": [
+      {
+        "name": "pkg2",
+        "status": "uninitialized"
+      }
+    ]
+      }
+    ]
+    EOO
+
+  # No package is specified.
+  #
+  $* >>EOE;
+    pkg1 configured 0.1.0-a.0.19700101000000
+    EOE
+
+  $* --stdout-format 'json' >>~%EOO%
+    [
+      {
+        "configuration": {
+          "id": 1,
+    %      "path": ".+prj-cfg",%
+          "name": "cfg"
+        },
+        "packages": [
+      {
+        "name": "pkg1",
+        "status": "configured",
+        "version": "0.1.0-a.0.19700101000000",
+        "hold_package": true,
+        "hold_version": true
+      },
+      {
+        "name": "pkg2",
+        "status": "uninitialized"
+      }
+    ]
+      }
+    ]
+    EOO
+}
+
 : dependency
 :
 {
@@ -145,6 +299,23 @@ deinit += -d prj
     %  libprj configured 0.+%
     EOO
 
+  $* --recursive --stdout-format 'json' >>~%EOO%;
+    [
+      {
+        "configuration": {
+          "id": 1,
+    %      "path": ".+prj-cfg",%
+          "name": "cfg"
+        },
+        "packages": [
+      {
+    %.+
+      }
+    ]
+      }
+    ]
+    EOO
+
   $deinit 2>>/"EOE"
     deinitializing in project $~/prj/
     synchronizing:
-- 
cgit v1.1