Fix JSON output issues (#1600)

* Fixed: `--json` throwing exceptions in some situations (lockfiles)
* Refactored Console pimpl handling
* Automatic json printing instead of explicit.
* added a way to cancel the print of json log from libmamba, used in mamba which already output the right json;
From now on the json output will be automatically printed at the end of the program
(after `main()` call) instead of explicitly invoked.

.pre-commit-config.yaml

@@ -32,7 +32,7 @@ repos:
         -   id: isort
             exclude: tests/data
 -   repo: https://gitlab.com/pycqa/flake8
-    rev: 4.0.1
+    rev: 3.9.2
     hooks:
     -   id: flake8
         language_version: python3

libmamba/include/mamba/core/output.hpp

@@ -125,7 +125,6 @@ namespace mamba
 
         static std::string hide_secrets(const std::string_view& str);
 
-        void json_print();
         void json_write(const nlohmann::json& j);
         void json_append(const std::string& value);
         void json_append(const nlohmann::json& j);
@@ -134,13 +133,16 @@ namespace mamba
 
         static void print_buffer(std::ostream& ostream);
 
+        void cancel_json_print();
+
     private:
         Console();
         ~Console();
 
+        void json_print();
         void deactivate_progress_bar(std::size_t idx, const std::string_view& msg = "");
 
-        ConsoleData* p_data;
+        std::unique_ptr<ConsoleData> p_data;
 
         friend class ProgressProxy;
     };

libmamba/src/api/info.cpp

@@ -85,7 +85,6 @@ namespace mamba
                 items_map.insert({ key, val });
 
             Console::instance().json_write(items_map);
-            Console::instance().json_print();
         }
 
         void print_info()

libmamba/src/api/install.cpp

@@ -467,7 +467,6 @@ namespace mamba
             {
                 Console::instance().json_write(
                     { { "success", false }, { "solver_problems", solver.all_problems() } });
-                Console::instance().json_print();
             }
 
             Console::stream() << "The environment can't be solved, aborting the operation";
@@ -541,8 +540,6 @@ namespace mamba
             Console::print(join(
                 "", std::vector<std::string>({ "Empty environment created at prefix: ", prefix })));
             Console::instance().json_write({ { "success", true } });
-
-            Console::instance().json_print();
         }
 
         void create_target_directory(const fs::path prefix)

libmamba/src/api/shell.cpp

@@ -106,7 +106,6 @@ namespace mamba
                       { "operation", "shell_hook" },
                       { "context", { { "shell_type", shell_type } } },
                       { "actions", { { "print", { activator->hook() } } } } });
-                Console::instance().json_print();
             }
             else
             {

libmamba/src/core/output.cpp

@@ -263,6 +263,7 @@ namespace mamba
         std::string json_hier;
         unsigned int json_index;
         nlohmann::json json_log;
+        bool is_json_print_cancelled = false;
 
         std::vector<std::string> m_buffer;
     };
@@ -280,8 +281,12 @@ namespace mamba
 
     Console::~Console()
     {
-        delete p_data;
-        p_data = nullptr;
+        if (!p_data->is_json_print_cancelled
+            && !p_data->json_log.is_null())  // Note: we cannot rely on Context::instance() to still
+                                             // be valid at this point.
+        {
+            this->json_print();
+        }
     }
 
     Console& Console::instance()
@@ -295,6 +300,11 @@ namespace mamba
         return ConsoleStream();
     }
 
+    void Console::cancel_json_print()
+    {
+        p_data->is_json_print_cancelled = true;
+    }
+
     std::string Console::hide_secrets(const std::string_view& str)
     {
         return mamba::hide_secrets(str);
@@ -302,9 +312,9 @@ namespace mamba
 
     void Console::print(const std::string_view& str, bool force_print)
     {
-        if (!(Context::instance().quiet || Context::instance().json) || force_print)
+        if (force_print || !(Context::instance().quiet || Context::instance().json))
         {
-            ConsoleData* data = instance().p_data;
+            auto& data = instance().p_data;
             const std::lock_guard<std::mutex> lock(data->m_mutex);
 
             if (data->p_progress_bar_manager && data->p_progress_bar_manager->started())
@@ -318,11 +328,9 @@ namespace mamba
         }
     }
 
-    // std::vector<std::string> Console::m_buffer({});
-
     void Console::print_buffer(std::ostream& ostream)
     {
-        ConsoleData* data = instance().p_data;
+        auto& data = instance().p_data;
         for (auto& message : data->m_buffer)
             ostream << message << "\n";
 
@@ -425,8 +433,7 @@ namespace mamba
 
     void Console::json_print()
     {
-        if (Context::instance().json)
-            print(p_data->json_log.unflatten().dump(4), true);
+        print(p_data->json_log.unflatten().dump(4), true);
     }
 
     // write all the key/value pairs of a JSON object into the current entry, which
@@ -478,7 +485,7 @@ namespace mamba
     // go up in the hierarchy
     void Console::json_up()
     {
-        if (Context::instance().json)
+        if (Context::instance().json && !p_data->json_hier.empty())
             p_data->json_hier.erase(p_data->json_hier.rfind('/'));
     }
 

libmamba/src/core/transaction.cpp

@@ -853,8 +853,6 @@ namespace mamba
         if (empty())
             Console::instance().json_write(
                 { { "message", "All requested packages already installed" } });
-        // finally, print the JSON
-        Console::instance().json_print();
 
         if (ctx.dry_run)
         {

libmambapy/src/main.cpp

@@ -27,6 +27,7 @@
 #include "mamba/core/util.hpp"
 #include "mamba/core/validate.hpp"
 #include "mamba/core/virtual_packages.hpp"
+#include "mamba/core/output.hpp"
 
 #include <stdexcept>
 
@@ -509,6 +510,8 @@ PYBIND11_MODULE(bindings, m)
 
     m.def("get_virtual_packages", &get_virtual_packages);
 
+    m.def("cancel_json_output", [] { Console::instance().cancel_json_print(); });
+
     m.attr("SOLVER_SOLVABLE") = SOLVER_SOLVABLE;
     m.attr("SOLVER_SOLVABLE_NAME") = SOLVER_SOLVABLE_NAME;
     m.attr("SOLVER_SOLVABLE_PROVIDES") = SOLVER_SOLVABLE_PROVIDES;

mamba/mamba/linking.py

@@ -10,6 +10,8 @@ from conda.exceptions import (
     PackagesNotFoundError,
 )
 
+from libmambapy import cancel_json_output as cancel_mamba_json_output
+
 
 def handle_txn(unlink_link_transaction, prefix, args, newenv, remove_op=False):
     if unlink_link_transaction.nothing_to_do:
@@ -18,6 +20,7 @@ def handle_txn(unlink_link_transaction, prefix, args, newenv, remove_op=False):
             raise PackagesNotFoundError(args.package_names)
         elif not newenv:
             if context.json:
+                cancel_mamba_json_output()
                 cli_common.stdout_json_success(
                     message="All requested packages already installed."
                 )
@@ -26,6 +29,7 @@ def handle_txn(unlink_link_transaction, prefix, args, newenv, remove_op=False):
     if context.dry_run:
         actions = unlink_link_transaction._make_legacy_action_groups()[0]
         if context.json:
+            cancel_mamba_json_output()
             cli_common.stdout_json_success(prefix=prefix, actions=actions, dry_run=True)
         raise DryRunExit()
 
@@ -42,5 +46,6 @@ def handle_txn(unlink_link_transaction, prefix, args, newenv, remove_op=False):
         raise CondaSystemExit("Exiting", e)
 
     if context.json:
+        cancel_mamba_json_output()
         actions = unlink_link_transaction._make_legacy_action_groups()[0]
         cli_common.stdout_json_success(prefix=prefix, actions=actions)

mamba/tests/test_all.py

@@ -1,6 +1,9 @@
 import json
 import os
 import platform
+import random
+import shutil
+import string
 import subprocess
 import uuid
 from distutils.version import StrictVersion
@@ -16,6 +19,10 @@ from utils import (
 )
 
 
+def random_string(N=10):
+    return "".join(random.choices(string.ascii_uppercase + string.digits, k=N))
+
+
 def test_install():
     add_glibc_virtual_package()
     copy_channels_osx()
@@ -186,8 +193,10 @@ multichannel_config = {
 
 
 @pytest.mark.parametrize("config_file", [multichannel_config], indirect=["config_file"])
-def test_multi_channels(config_file):
+def test_multi_channels(config_file, tmpdir):
     # we need to create a config file first
+    call_env = os.environ.copy()
+    call_env["CONDA_PKGS_DIRS"] = str(tmpdir / random_string())
     output = subprocess.check_output(
         [
             "mamba",
@@ -197,9 +206,12 @@ def test_multi_channels(config_file):
             "conda-forge2::xtensor",
             "--dry-run",
             "--json",
-        ]
+        ],
+        env=call_env,
     )
-    res = json.loads(output.decode())
+    result = output.decode()
+    print(result)
+    res = json.loads(result)
     for pkg in res["actions"]["FETCH"]:
         assert pkg["channel"].startswith("https://conda.anaconda.org/conda-forge")
     for pkg in res["actions"]["LINK"]:
@@ -234,7 +246,7 @@ def test_unicode(tmpdir):
         ["mamba", "create", "-p", str(tmpdir / uc), "--json", "xtensor"]
     )
     output = json.loads(output)
-    assert output["prefix"] == str(tmpdir / uc)
+    assert output["actions"]["PREFIX"] == str(tmpdir / uc)
 
     import libmambapy
 

micromamba/tests/test_create.py

@@ -117,6 +117,9 @@ class TestCreate:
         assert res["env_name"] == ""
         assert res["specs"] == specs
 
+        json_res = create(*cmd, "--json")
+        assert json_res["success"] == True
+
     @pytest.mark.parametrize("root_prefix", (None, "env_var", "cli"))
     @pytest.mark.parametrize("target_is_root", (False, True))
     @pytest.mark.parametrize("cli_prefix", (False, True))