improve output upload (#14984)

* improve output upload

* wip

* Update conans/client/cmd/uploader.py

Co-authored-by: Rubén Rincón Blanco <git@rinconblanco.es>

* fix test

* some subtitles

* review

* more output

---------

Co-authored-by: Rubén Rincón Blanco <git@rinconblanco.es>

conan/api/subapi/upload.py

@@ -64,8 +64,15 @@ def upload_backup_sources(self, package_list):
         download_cache_path = download_cache_path or HomePaths(self.conan_api.cache_folder).default_sources_backup_folder
         excluded_urls = config.get("core.sources:exclude_urls", check_type=list, default=[])
 
+        output = ConanOutput()
+        output.subtitle("Uploading backup sources")
+        output.info("Gathering files to upload")
         files = DownloadCache(download_cache_path).get_backup_sources_files_to_upload(package_list,
                                                                                       excluded_urls)
+        if not files:
+            output.info("No backup sources files to upload")
+            return files
+
         # TODO: verify might need a config to force it to False
         uploader = FileUploader(app.requester, verify=True, config=config)
         # TODO: For Artifactory, we can list all files once and check from there instead
@@ -76,12 +83,14 @@ def upload_backup_sources(self, package_list):
             try:
                 # Always upload summary .json but only upload blob if it does not already exist
                 if file.endswith(".json") or not uploader.exists(full_url, auth=None):
-                    ConanOutput().info(f"Uploading file '{basename}' to backup sources server")
+                    output.info(f"Uploading file '{basename}' to backup sources server")
                     uploader.upload(full_url, file, dedup=False, auth=None)
                 else:
-                    ConanOutput().info(f"File '{basename}' already in backup sources server, "
-                                       "skipping upload")
+                    output.info(f"File '{basename}' already in backup sources server, "
+                                "skipping upload")
             except (AuthenticationException, ForbiddenException) as e:
                 raise ConanException(f"The source backup server '{url}' needs authentication"
                                      f"/permissions, please provide 'source_credentials.json': {e}")
+
+        output.success("Upload backup sources complete\n")
         return files

conan/cli/commands/upload.py

@@ -1,6 +1,6 @@
 from conan.api.conan_api import ConanAPI
 from conan.api.model import ListPattern, MultiPackagesList
-from conan.api.output import cli_out_write, ConanOutput
+from conan.api.output import ConanOutput
 from conan.cli import make_abs_path
 from conan.cli.command import conan_command, OnceArgument
 from conan.cli.commands.list import print_list_json, print_serial
@@ -12,7 +12,7 @@ def summary_upload_list(results):
     """ Do litte format modification to serialized
     list bundle so it looks prettier on text output
     """
-    cli_out_write("Upload summary:")
+    ConanOutput().subtitle("Upload summary")
     info = results["results"]
 
     def format_upload(item):

conans/client/cmd/uploader.py

@@ -23,16 +23,17 @@ class UploadUpstreamChecker:
     """
     def __init__(self, app: ConanApp):
         self._app = app
-        self._output = ConanOutput()
 
     def check(self, upload_bundle, remote, force):
+        ConanOutput().subtitle("Checking server existing packages")
         for ref, recipe_bundle in upload_bundle.refs().items():
             self._check_upstream_recipe(ref, recipe_bundle, remote, force)
             for pref, prev_bundle in upload_bundle.prefs(ref, recipe_bundle).items():
                 self._check_upstream_package(pref, prev_bundle, remote, force)
 
     def _check_upstream_recipe(self, ref, ref_bundle, remote, force):
-        self._output.info("Checking which revisions exist in the remote server")
+        output = ConanOutput(scope=str(ref))
+        output.info("Checking which revisions exist in the remote server")
         try:
             assert ref.revision
             # TODO: It is a bit ugly, interface-wise to ask for revisions to check existence
@@ -43,11 +44,11 @@ def _check_upstream_recipe(self, ref, ref_bundle, remote, force):
             ref_bundle["upload"] = True
         else:
             if force:
-                self._output.info("Recipe '{}' already in server, forcing upload".format(ref.repr_notime()))
+                output.info(f"Recipe '{ref.repr_notime()}' already in server, forcing upload")
                 ref_bundle["force_upload"] = True
                 ref_bundle["upload"] = True
             else:
-                self._output.info("Recipe '{}' already in server, skipping upload".format(ref.repr_notime()))
+                output.info(f"Recipe '{ref.repr_notime()}' already in server, skipping upload")
                 ref_bundle["upload"] = False
                 ref_bundle["force_upload"] = False
 
@@ -63,23 +64,23 @@ def _check_upstream_package(self, pref, prev_bundle, remote, force):
             prev_bundle["force_upload"] = False
             prev_bundle["upload"] = True
         else:
+            output = ConanOutput(scope=str(pref.ref))
             if force:
-                self._output.info("Package '{}' already in server, forcing upload".format(pref.repr_notime()))
+                output.info(f"Package '{pref.repr_notime()}' already in server, forcing upload")
                 prev_bundle["force_upload"] = True
                 prev_bundle["upload"] = True
             else:
-                self._output.info("Package '{}' already in server, skipping upload".format(pref.repr_notime()))
+                output.info(f"Package '{pref.repr_notime()}' already in server, skipping upload")
                 prev_bundle["force_upload"] = False
                 prev_bundle["upload"] = False
 
 
 class PackagePreparator:
     def __init__(self, app: ConanApp):
         self._app = app
-        self._output = ConanOutput()
 
     def prepare(self, upload_bundle, enabled_remotes):
-        self._output.info("Preparing artifacts to upload")
+        ConanOutput().subtitle("Preparing artifacts for upload")
         for ref, bundle in upload_bundle.refs().items():
             layout = self._app.cache.recipe_layout(ref)
             conanfile_path = layout.conanfile()
@@ -108,10 +109,11 @@ def _prepare_recipe(self, ref, ref_bundle, conanfile, remotes):
     def _compress_recipe_files(self, layout, ref):
         download_export_folder = layout.download_export()
 
+        output = ConanOutput(scope=str(ref))
         for f in (EXPORT_TGZ_NAME, EXPORT_SOURCES_TGZ_NAME):
             tgz_path = os.path.join(download_export_folder, f)
             if is_dirty(tgz_path):
-                self._output.warning("%s: Removing %s, marked as dirty" % (str(ref), f))
+                output.warning("Removing %s, marked as dirty" % f)
                 os.remove(tgz_path)
                 clean_dirty(tgz_path)
 
@@ -138,21 +140,19 @@ def _compress_recipe_files(self, layout, ref):
         files.pop(CONANFILE)
         files.pop(CONAN_MANIFEST)
 
-        def add_tgz(tgz_name, tgz_files, msg):
+        def add_tgz(tgz_name, tgz_files):
             tgz = os.path.join(download_export_folder, tgz_name)
             if os.path.isfile(tgz):
                 result[tgz_name] = tgz
             elif tgz_files:
-                if self._output and not self._output.is_terminal:
-                    self._output.info(msg)
                 compresslevel = self._app.cache.new_config.get("core.gzip:compresslevel",
                                                                check_type=int)
                 tgz = compress_files(tgz_files, tgz_name, download_export_folder,
-                                     compresslevel=compresslevel)
+                                     compresslevel=compresslevel, ref=ref)
                 result[tgz_name] = tgz
 
-        add_tgz(EXPORT_TGZ_NAME, files, "Compressing recipe...")
-        add_tgz(EXPORT_SOURCES_TGZ_NAME, src_files, "Compressing recipe sources...")
+        add_tgz(EXPORT_TGZ_NAME, files)
+        add_tgz(EXPORT_SOURCES_TGZ_NAME, src_files)
         return result
 
     def _prepare_package(self, pref, prev_bundle):
@@ -164,10 +164,11 @@ def _prepare_package(self, pref, prev_bundle):
         prev_bundle["files"] = cache_files
 
     def _compress_package_files(self, layout, pref):
+        output = ConanOutput(scope=str(pref))
         download_pkg_folder = layout.download_package()
         package_tgz = os.path.join(download_pkg_folder, PACKAGE_TGZ_NAME)
         if is_dirty(package_tgz):
-            self._output.warning("%s: Removing %s, marked as dirty" % (str(pref), PACKAGE_TGZ_NAME))
+            output.warning("Removing %s, marked as dirty" % PACKAGE_TGZ_NAME)
             os.remove(package_tgz)
             clean_dirty(package_tgz)
 
@@ -191,12 +192,10 @@ def _compress_package_files(self, layout, pref):
         files.pop(CONAN_MANIFEST)
 
         if not os.path.isfile(package_tgz):
-            if self._output and not self._output.is_terminal:
-                self._output.info("Compressing package...")
             tgz_files = {f: path for f, path in files.items()}
             compresslevel = self._app.cache.new_config.get("core.gzip:compresslevel", check_type=int)
             tgz_path = compress_files(tgz_files, PACKAGE_TGZ_NAME, download_pkg_folder,
-                                      compresslevel=compresslevel)
+                                      compresslevel=compresslevel, ref=pref)
             assert tgz_path == package_tgz
             assert os.path.exists(package_tgz)
 
@@ -215,13 +214,14 @@ def __init__(self, app: ConanApp):
         self._output = ConanOutput()
 
     def upload(self, upload_data, remote):
-        self._output.info("Uploading artifacts")
+        ConanOutput().subtitle("Uploading artifacts")
         for ref, bundle in upload_data.refs().items():
             if bundle.get("upload"):
                 self.upload_recipe(ref, bundle, remote)
             for pref, prev_bundle in upload_data.prefs(ref, bundle).items():
                 if prev_bundle.get("upload"):
                     self.upload_package(pref, prev_bundle, remote)
+        ConanOutput().success("Upload complete\n")
 
     def upload_recipe(self, ref, bundle, remote):
         self._output.info(f"Uploading recipe '{ref.repr_notime()}'")
@@ -250,9 +250,7 @@ def compress_files(files, name, dest_dir, compresslevel=None, ref=None):
     t1 = time.time()
     # FIXME, better write to disk sequentially and not keep tgz contents in memory
     tgz_path = os.path.join(dest_dir, name)
-    if name in (PACKAGE_TGZ_NAME, EXPORT_SOURCES_TGZ_NAME) and len(files) > 100:
-        ref_name = f"{ref}:" or ""
-        ConanOutput().info(f"Compressing {ref_name}{name}")
+    ConanOutput(scope=str(ref)).info(f"Compressing {name}")
     with set_dirty_context_manager(tgz_path), open(tgz_path, "wb") as tgz_handle:
         tgz = gzopen_without_timestamps(name, mode="w", fileobj=tgz_handle,
                                         compresslevel=compresslevel)

conans/client/source.py

@@ -1,5 +1,6 @@
 import os
 
+from conan.api.output import ConanOutput
 from conans.errors import ConanException, conanfile_exception_formatter, NotFoundException, \
     conanfile_remove_attr
 from conans.util.files import (is_dirty, mkdir, rmdir, set_dirty_context_manager,
@@ -41,8 +42,7 @@ def retrieve_exports_sources(remote_manager, recipe_layout, conanfile, ref, remo
                % str(ref))
         raise ConanException(msg)
 
-    # FIXME: this output is scoped but without reference, check if we want this
-    conanfile.output.info("Sources downloaded from '{}'".format(sources_remote.name))
+    ConanOutput(scope=str(ref)).info("Sources downloaded from '{}'".format(sources_remote.name))
 
 
 def config_source(export_source_folder, conanfile, hook_manager):

conans/test/integration/command/upload/upload_complete_test.py

@@ -60,6 +60,7 @@ def test_upload_with_pattern():
         client.run("export . --user=frodo --channel=stable")
 
     client.run("upload hello* --confirm -r default")
+    print(client.out)
     for num in range(3):
         assert "Uploading recipe 'hello%s/1.2.1@frodo/stable" % num in client.out
 
@@ -234,13 +235,13 @@ def test_upload_same_package_dont_compress(self):
         client.run("create . --name foo --version 1.0")
 
         client.run("upload foo/1.0 -r default")
-        self.assertIn("Compressing recipe", client.out)
-        self.assertIn("Compressing package", str(client.out))
+        self.assertIn("foo/1.0: Compressing conan_sources.tgz", client.out)
+        self.assertIn("foo/1.0:da39a3ee5e6b4b0d3255bfef95601890afd80709: "
+                      "Compressing conan_package.tgz", client.out)
 
         client.run("upload foo/1.0 -r default")
-        self.assertNotIn("Compressing recipe", client.out)
-        self.assertNotIn("Compressing package", str(client.out))
-        self.assertIn("already in server, skipping upload", str(client.out))
+        self.assertNotIn("Compressing", client.out)
+        self.assertIn("already in server, skipping upload", client.out)
 
     @pytest.mark.xfail(reason="No output json available yet for upload. Also old test, has to be "
                               "upgraded")

conans/test/integration/command/upload/upload_compression_test.py

@@ -18,8 +18,8 @@ def test_reuse_uploaded_tgz():
     client.save(files)
     client.run("create . --user=user --channel=stable")
     client.run("upload %s -r default" % str(ref))
-    assert "Compressing recipe" in client.out
-    assert "Compressing package" in client.out
+    assert "Compressing conan_export.tgz" in client.out
+    assert "Compressing conan_package.tgz" in client.out
 
 
 def test_reuse_downloaded_tgz():
@@ -32,17 +32,17 @@ def test_reuse_downloaded_tgz():
     client.save(files)
     client.run("create . --user=user --channel=stable")
     client.run("upload hello0/0.1@user/stable -r default")
-    assert "Compressing recipe" in client.out
-    assert "Compressing package" in client.out
+    assert "Compressing conan_export.tgz" in client.out
+    assert "Compressing conan_package.tgz" in client.out
 
     # Other user downloads the package
     # THEN A NEW USER DOWNLOADS THE PACKAGES AND UPLOADS COMPRESSING AGAIN
     # BECAUSE ONLY TGZ IS KEPT WHEN UPLOADING
     other_client = TestClient(servers=client.servers, inputs=["admin", "password"])
     other_client.run("download hello0/0.1@user/stable -r default")
     other_client.run("upload hello0/0.1@user/stable -r default")
-    assert "Compressing recipe" in client.out
-    assert "Compressing package" in client.out
+    assert "Compressing conan_export.tgz" in client.out
+    assert "Compressing conan_package.tgz" in client.out
 
 
 def test_upload_only_tgz_if_needed():
@@ -56,11 +56,11 @@ def test_upload_only_tgz_if_needed():
 
     # Upload conans
     client.run("upload %s -r default --only-recipe" % str(ref))
-    assert "Compressing recipe" in client.out
+    assert "Compressing conan_export.tgz" in client.out
 
     # Not needed to tgz again
     client.run("upload %s -r default --only-recipe" % str(ref))
-    assert "Compressing recipe" not in client.out
+    assert "Compressing" not in client.out
 
     # Check that conans exists on server
     server_paths = client.servers["default"].server_store
@@ -73,17 +73,17 @@ def test_upload_only_tgz_if_needed():
 
     # Upload package
     client.run("upload %s#*:%s -r default -c" % (str(ref), str(pref.package_id)))
-    assert "Compressing package" in client.out
+    assert "Compressing conan_package.tgz" in client.out
 
     # Not needed to tgz again
     client.run("upload %s#*:%s -r default -c" % (str(ref), str(pref.package_id)))
-    assert "Compressing package" not in client.out
+    assert "Compressing" not in client.out
 
     # If we install the package again will be removed and re tgz
     client.run("install --requires=%s --build missing" % str(ref))
     # Upload package
     client.run("upload %s#*:%s -r default -c" % (str(ref), str(pref.package_id)))
-    assert "Compressing package" not in client.out
+    assert "Compressing" not in client.out
 
     # Check library on server
     folder = uncompress_packaged_files(server_paths, pref)

conans/test/integration/command/upload/upload_test.py

@@ -126,8 +126,7 @@ def gzopen_patched(name, mode="r", fileobj=None, **kwargs):
             self.assertTrue(is_dirty(tgz))
 
         client.run("upload * --confirm -r default --only-recipe")
-        self.assertIn("WARN: hello0/1.2.1@user/testing: Removing conan_sources.tgz, "
-                      "marked as dirty", client.out)
+        self.assertIn("Removing conan_sources.tgz, marked as dirty", client.out)
         self.assertTrue(os.path.exists(tgz))
         self.assertFalse(is_dirty(tgz))
 
@@ -154,9 +153,7 @@ def gzopen_patched(name, mode="r", fileobj=None, **kwargs):
             self.assertTrue(is_dirty(tgz))
 
         client.run("upload * --confirm -r default")
-        self.assertIn("WARN: hello0/1.2.1@user/testing:%s: "
-                      "Removing conan_package.tgz, marked as dirty" % NO_SETTINGS_PACKAGE_ID,
-                      client.out)
+        self.assertIn("WARN: Removing conan_package.tgz, marked as dirty", client.out)
         self.assertTrue(os.path.exists(tgz))
         self.assertFalse(is_dirty(tgz))