From 48342cafeb79370ca5ab139d5cf7b0a987d4fc74 Mon Sep 17 00:00:00 2001 From: Uilian Ries Date: Tue, 24 Oct 2023 15:58:09 +0200 Subject: [PATCH 1/8] Validate upload when ignoring metadata Signed-off-by: Uilian Ries --- .../metadata/test_metadata_commands.py | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/conans/test/integration/metadata/test_metadata_commands.py b/conans/test/integration/metadata/test_metadata_commands.py index aacea1d7c70..d5474d58e53 100644 --- a/conans/test/integration/metadata/test_metadata_commands.py +++ b/conans/test/integration/metadata/test_metadata_commands.py @@ -198,3 +198,25 @@ def test_no_download_cached(self): assert "mylogs2!!!!" in mylogs mybuildlogs = load(os.path.join(c2_pkg_metadata_path, "logs", "mybuildlogs.txt")) assert "mybuildlogs2!!!!" in mybuildlogs + + def test_upload_ignored_metadata(self): + """ + Upload command should ignore metadata files when passing --metadata="" + """ + client = TestClient(default_server_user=True) + client.save({"conanfile.py": GenConanfile("pkg", "0.1")}) + client.run("create .") + pid = client.created_package_id("pkg/0.1") + + def _save_metadata(client, ref): + client.run(f"cache path {ref} --folder=metadata") + metadata_path = str(client.stdout).strip() + myfile = os.path.join(metadata_path, "logs", "somefile.log") + save(myfile, ref) + + _save_metadata(client, "pkg/0.1") + _save_metadata(client, f"pkg/0.1:{pid}") + + client.run('upload * --confirm --remote=default --metadata=""') + assert "Recipe metadata" not in client.out + assert "Package metadata" not in client.out From 9502e2a5b52647dc065bdcd2fe10a6639b72426f Mon Sep 17 00:00:00 2001 From: Uilian Ries Date: Tue, 24 Oct 2023 16:14:23 +0200 Subject: [PATCH 2/8] ignore metadata upload in case passing empty string Signed-off-by: Uilian Ries --- conan/internal/upload_metadata.py | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/conan/internal/upload_metadata.py b/conan/internal/upload_metadata.py index 0dc8c2319cc..1d6f80226c7 100644 --- a/conan/internal/upload_metadata.py +++ b/conan/internal/upload_metadata.py @@ -18,7 +18,21 @@ def _metadata_files(folder, metadata): return result -def gather_metadata(package_list, cache, metadata): +def gather_metadata(package_list, cache, metadata: list): + """ + List and configure the metadata files to be uploaded. + The metadata supports patterns, so it can be used to upload only a subset of the metadata files. + When metadata is not specified, all metadata files are uploaded. + But, when metadata is empty string (""), it means that no metadata files should be uploaded. + + :param package_list: Package to be uploaded + :param cache: Conan client cache + :param metadata: A list of patterns of metadata that should be uploaded. Default None + """ + + if metadata == ['']: + return + for rref, recipe_bundle in package_list.refs().items(): if metadata or recipe_bundle["upload"]: metadata_folder = cache.recipe_layout(rref).metadata() From 6f0ea6f374140f01c3a0e8b6f72358da5579a990 Mon Sep 17 00:00:00 2001 From: Uilian Ries Date: Tue, 24 Oct 2023 17:11:55 +0200 Subject: [PATCH 3/8] Do not accept mixed metadata pattern Signed-off-by: Uilian Ries --- conan/internal/upload_metadata.py | 3 ++ .../metadata/test_metadata_commands.py | 32 ++++++++++++++----- 2 files changed, 27 insertions(+), 8 deletions(-) diff --git a/conan/internal/upload_metadata.py b/conan/internal/upload_metadata.py index 1d6f80226c7..2ed319ba752 100644 --- a/conan/internal/upload_metadata.py +++ b/conan/internal/upload_metadata.py @@ -2,6 +2,7 @@ import os from conan.api.output import ConanOutput +from conan.errors import ConanException def _metadata_files(folder, metadata): @@ -32,6 +33,8 @@ def gather_metadata(package_list, cache, metadata: list): if metadata == ['']: return + elif metadata and '' in metadata: + raise ConanException("Empty string is not a valid pattern for metadata upload") for rref, recipe_bundle in package_list.refs().items(): if metadata or recipe_bundle["upload"]: diff --git a/conans/test/integration/metadata/test_metadata_commands.py b/conans/test/integration/metadata/test_metadata_commands.py index d5474d58e53..784c87dbcbc 100644 --- a/conans/test/integration/metadata/test_metadata_commands.py +++ b/conans/test/integration/metadata/test_metadata_commands.py @@ -8,6 +8,12 @@ class TestMetadataCommands: + def _save_metadata_file(self, client, pkg_ref, filename="somefile.log"): + client.run(f"cache path {pkg_ref} --folder=metadata") + metadata_path = str(client.stdout).strip() + myfile = os.path.join(metadata_path, "logs", filename) + save(myfile, pkg_ref) + def test_upload(self): c = TestClient(default_server_user=True) c.save({"conanfile.py": GenConanfile("pkg", "0.1")}) @@ -208,15 +214,25 @@ def test_upload_ignored_metadata(self): client.run("create .") pid = client.created_package_id("pkg/0.1") - def _save_metadata(client, ref): - client.run(f"cache path {ref} --folder=metadata") - metadata_path = str(client.stdout).strip() - myfile = os.path.join(metadata_path, "logs", "somefile.log") - save(myfile, ref) - - _save_metadata(client, "pkg/0.1") - _save_metadata(client, f"pkg/0.1:{pid}") + self._save_metadata_file(client, "pkg/0.1") + self._save_metadata_file(client, f"pkg/0.1:{pid}") client.run('upload * --confirm --remote=default --metadata=""') assert "Recipe metadata" not in client.out assert "Package metadata" not in client.out + + def test_upload_ignored_metadata_with_pattern(self): + """ + Upload command should ignore metadata files when passing --metadata="". + Even if a pattern is passed to the upload command. + """ + client = TestClient(default_server_user=True) + client.save({"conanfile.py": GenConanfile("pkg", "0.1")}) + client.run("create .") + pid = client.created_package_id("pkg/0.1") + + self._save_metadata_file(client, "pkg/0.1") + self._save_metadata_file(client, f"pkg/0.1:{pid}") + + client.run('upload * --confirm --remote=default --metadata="" --metadata="logs/*"', assert_error=True) + assert "ERROR: Empty string is not a valid pattern for metadata upload" in client.out From 356cab14c2c344fb63308d79890c29454e285df1 Mon Sep 17 00:00:00 2001 From: Uilian Ries Date: Wed, 25 Oct 2023 08:45:51 +0200 Subject: [PATCH 4/8] Move validation to upload sub api Signed-off-by: Uilian Ries --- conan/api/subapi/upload.py | 8 ++++++-- conan/internal/upload_metadata.py | 7 ------- .../test/integration/metadata/test_metadata_commands.py | 2 +- 3 files changed, 7 insertions(+), 10 deletions(-) diff --git a/conan/api/subapi/upload.py b/conan/api/subapi/upload.py index 75fc7756762..eb437a9d999 100644 --- a/conan/api/subapi/upload.py +++ b/conan/api/subapi/upload.py @@ -38,11 +38,15 @@ def prepare(self, package_list, enabled_remotes, metadata=None): :param package_list: :param enabled_remotes: :param metadata: A list of patterns of metadata that should be uploaded. Default None - means all metadata will be uploaded together with the pkg artifacts""" + means all metadata will be uploaded together with the pkg artifacts. If metadata is empty + string (""), it means that no metadata files should be uploaded.""" + if metadata and metadata != [''] and '' in metadata: + raise ConanException("Empty string and patterns can not be mixed for metadata.") app = ConanApp(self.conan_api.cache_folder) preparator = PackagePreparator(app) preparator.prepare(package_list, enabled_remotes) - gather_metadata(package_list, app.cache, metadata) + if metadata != ['']: + gather_metadata(package_list, app.cache, metadata) signer = PkgSignaturesPlugin(app.cache) # This might add files entries to package_list with signatures signer.sign(package_list) diff --git a/conan/internal/upload_metadata.py b/conan/internal/upload_metadata.py index 2ed319ba752..23ca6ae8f9a 100644 --- a/conan/internal/upload_metadata.py +++ b/conan/internal/upload_metadata.py @@ -24,18 +24,11 @@ def gather_metadata(package_list, cache, metadata: list): List and configure the metadata files to be uploaded. The metadata supports patterns, so it can be used to upload only a subset of the metadata files. When metadata is not specified, all metadata files are uploaded. - But, when metadata is empty string (""), it means that no metadata files should be uploaded. :param package_list: Package to be uploaded :param cache: Conan client cache :param metadata: A list of patterns of metadata that should be uploaded. Default None """ - - if metadata == ['']: - return - elif metadata and '' in metadata: - raise ConanException("Empty string is not a valid pattern for metadata upload") - for rref, recipe_bundle in package_list.refs().items(): if metadata or recipe_bundle["upload"]: metadata_folder = cache.recipe_layout(rref).metadata() diff --git a/conans/test/integration/metadata/test_metadata_commands.py b/conans/test/integration/metadata/test_metadata_commands.py index 784c87dbcbc..d450dd6e01b 100644 --- a/conans/test/integration/metadata/test_metadata_commands.py +++ b/conans/test/integration/metadata/test_metadata_commands.py @@ -235,4 +235,4 @@ def test_upload_ignored_metadata_with_pattern(self): self._save_metadata_file(client, f"pkg/0.1:{pid}") client.run('upload * --confirm --remote=default --metadata="" --metadata="logs/*"', assert_error=True) - assert "ERROR: Empty string is not a valid pattern for metadata upload" in client.out + assert "ERROR: Empty string and patterns can not be mixed for metadata." in client.out From 97a70b922382539af91adb169ab27f99b93abdbb Mon Sep 17 00:00:00 2001 From: Uilian Ries Date: Wed, 25 Oct 2023 08:47:11 +0200 Subject: [PATCH 5/8] Revert gather metadata Signed-off-by: Uilian Ries --- conan/internal/upload_metadata.py | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/conan/internal/upload_metadata.py b/conan/internal/upload_metadata.py index 23ca6ae8f9a..0dc8c2319cc 100644 --- a/conan/internal/upload_metadata.py +++ b/conan/internal/upload_metadata.py @@ -2,7 +2,6 @@ import os from conan.api.output import ConanOutput -from conan.errors import ConanException def _metadata_files(folder, metadata): @@ -19,16 +18,7 @@ def _metadata_files(folder, metadata): return result -def gather_metadata(package_list, cache, metadata: list): - """ - List and configure the metadata files to be uploaded. - The metadata supports patterns, so it can be used to upload only a subset of the metadata files. - When metadata is not specified, all metadata files are uploaded. - - :param package_list: Package to be uploaded - :param cache: Conan client cache - :param metadata: A list of patterns of metadata that should be uploaded. Default None - """ +def gather_metadata(package_list, cache, metadata): for rref, recipe_bundle in package_list.refs().items(): if metadata or recipe_bundle["upload"]: metadata_folder = cache.recipe_layout(rref).metadata() From 4b86f387f26475dd7cdcedd0ec92ec669bb266f0 Mon Sep 17 00:00:00 2001 From: Uilian Ries Date: Wed, 25 Oct 2023 09:15:21 +0200 Subject: [PATCH 6/8] Improve metadata upload test Signed-off-by: Uilian Ries --- .../metadata/test_metadata_commands.py | 111 ++++++------------ 1 file changed, 39 insertions(+), 72 deletions(-) diff --git a/conans/test/integration/metadata/test_metadata_commands.py b/conans/test/integration/metadata/test_metadata_commands.py index d450dd6e01b..682140270b5 100644 --- a/conans/test/integration/metadata/test_metadata_commands.py +++ b/conans/test/integration/metadata/test_metadata_commands.py @@ -1,4 +1,5 @@ import os +import pytest from conans.test.assets.genconanfile import GenConanfile from conans.test.utils.test_files import temp_folder @@ -8,28 +9,27 @@ class TestMetadataCommands: - def _save_metadata_file(self, client, pkg_ref, filename="somefile.log"): + @pytest.fixture + def create_conan_pkg(self): + client = TestClient(default_server_user=True) + client.save({"conanfile.py": GenConanfile("pkg", "0.1")}) + client.run("create .") + pid = client.created_package_id("pkg/0.1") + return client, pid + + def save_metadata_file(self, client, pkg_ref, filename="somefile.log"): client.run(f"cache path {pkg_ref} --folder=metadata") metadata_path = str(client.stdout).strip() myfile = os.path.join(metadata_path, "logs", filename) - save(myfile, pkg_ref) + save(myfile, f"{pkg_ref}!!!!") + return metadata_path, myfile - def test_upload(self): - c = TestClient(default_server_user=True) - c.save({"conanfile.py": GenConanfile("pkg", "0.1")}) - c.run("create .") - pid = c.created_package_id("pkg/0.1") + def test_upload(self, create_conan_pkg): + c, pid = create_conan_pkg # Add some metadata - c.run("cache path pkg/0.1 --folder=metadata") - metadata_path = str(c.stdout).strip() - myfile = os.path.join(metadata_path, "logs", "mylogs.txt") - save(myfile, "mylogs!!!!") - - c.run(f"cache path pkg/0.1:{pid} --folder=metadata") - pkg_metadata_path = str(c.stdout).strip() - myfile = os.path.join(pkg_metadata_path, "logs", "mybuildlogs.txt") - save(myfile, "mybuildlogs!!!!") + self.save_metadata_file(c, "pkg/0.1", "mylogs.txt") + self.save_metadata_file(c, f"pkg/0.1:{pid}", "mybuildlogs.txt") # Now upload everything c.run("upload * -c -r=default") @@ -37,10 +37,8 @@ def test_upload(self): assert "pkg/0.1:da39a3ee5e6b4b0d3255bfef95601890afd80709: Package metadata: 1 files" in c.out # Add new files to the metadata - myfile = os.path.join(metadata_path, "logs", "mylogs2.txt") - save(myfile, "mylogs2!!!!") - myfile = os.path.join(pkg_metadata_path, "logs", "mybuildlogs2.txt") - save(myfile, "mybuildlogs2!!!!") + self.save_metadata_file(c, "pkg/0.1", "mylogs2.txt") + self.save_metadata_file(c, f"pkg/0.1:{pid}", "mybuildlogs2.txt") # Upload the metadata, even if the revisions exist in the server # adding the new metadata logs files c.run("upload * -c -r=default --metadata=*") @@ -69,10 +67,7 @@ def test_update_contents(self): c.run("export .") # Add some metadata - c.run("cache path pkg/0.1 --folder=metadata") - metadata_path = str(c.stdout).strip() - myfile = os.path.join(metadata_path, "logs", "mylogs.txt") - save(myfile, "mylogs!!!!") + _, myfile = self.save_metadata_file(c, "pkg/0.1", "mylogs.txt") # Now upload everything c.run("upload * -c -r=default") @@ -93,13 +88,11 @@ def test_update_contents(self): content = load(os.path.join(metadata_path, "logs", "mylogs.txt")) assert "mylogs2!!!!" in content - def test_folder_exist(self): + def test_folder_exist(self, create_conan_pkg): """ so we can cp -R to the metadata folder, having to create the folder in the cache is weird """ - c = TestClient(default_server_user=True) - c.save({"conanfile.py": GenConanfile("pkg", "0.1")}) - c.run("create .") + c, _ = create_conan_pkg c.run("cache path pkg/0.1 --folder=metadata") metadata_path = str(c.stdout).strip() assert os.path.isdir(metadata_path) @@ -107,27 +100,17 @@ def test_folder_exist(self): pkg_metadata_path = str(c.stdout).strip() assert os.path.isdir(pkg_metadata_path) - def test_direct_download_redownload(self): + def test_direct_download_redownload(self, create_conan_pkg): """ When we directly download things, without "conan install" first, it is also able to fetch the requested metadata Also, re-downloading same thing shouldn't fail """ - c = TestClient(default_server_user=True) - c.save({"conanfile.py": GenConanfile("pkg", "0.1")}) - c.run("create .") - pid = c.created_package_id("pkg/0.1") + c, pid = create_conan_pkg # Add some metadata - c.run("cache path pkg/0.1 --folder=metadata") - metadata_path = str(c.stdout).strip() - myfile = os.path.join(metadata_path, "logs", "mylogs.txt") - save(myfile, "mylogs!!!!") - - c.run(f"cache path pkg/0.1:{pid} --folder=metadata") - pkg_metadata_path = str(c.stdout).strip() - myfile = os.path.join(pkg_metadata_path, "logs", "mybuildlogs.txt") - save(myfile, "mybuildlogs!!!!") + metadata_path, _ = self.save_metadata_file(c, "pkg/0.1", "mylogs.txt") + self.save_metadata_file(c, f"pkg/0.1:{pid}", "mybuildlogs.txt") # Now upload everything c.run("upload * -c -r=default") @@ -150,24 +133,14 @@ def test_direct_download_redownload(self): pkg_metadata_path = str(c.stdout).strip() assert os.path.isfile(os.path.join(pkg_metadata_path, "logs", "mybuildlogs.txt")) - def test_no_download_cached(self): + def test_no_download_cached(self, create_conan_pkg): """ as the metadata can change, no checksum, no revision, cannot be cached """ - c = TestClient(default_server_user=True) - c.save({"conanfile.py": GenConanfile("pkg", "0.1")}) - c.run("create .") - pid = c.created_package_id("pkg/0.1") + c, pid = create_conan_pkg # Add some metadata - c.run("cache path pkg/0.1 --folder=metadata") - metadata_path = str(c.stdout).strip() - myrecipefile = os.path.join(metadata_path, "logs", "mylogs.txt") - save(myrecipefile, "mylogs!!!!") - - c.run(f"cache path pkg/0.1:{pid} --folder=metadata") - pkg_metadata_path = str(c.stdout).strip() - mypkgfile = os.path.join(pkg_metadata_path, "logs", "mybuildlogs.txt") - save(mypkgfile, "mybuildlogs!!!!") + _, myrecipefile = self.save_metadata_file(c, "pkg/0.1", "mylogs.txt") + _, mypkgfile = self.save_metadata_file(c, f"pkg/0.1:{pid}", "mybuildlogs.txt") # Now upload everything c.run("upload * -c -r=default") @@ -184,11 +157,11 @@ def test_no_download_cached(self): c2.run("cache path pkg/0.1 --folder=metadata") c2_metadata_path = str(c2.stdout).strip() mylogs = load(os.path.join(c2_metadata_path, "logs", "mylogs.txt")) - assert "mylogs!!!!" in mylogs + assert "pkg/0.1!!!!" in mylogs c2.run(f"cache path pkg/0.1:{pid} --folder=metadata") c2_pkg_metadata_path = str(c2.stdout).strip() mybuildlogs = load(os.path.join(c2_pkg_metadata_path, "logs", "mybuildlogs.txt")) - assert "mybuildlogs!!!!" in mybuildlogs + assert f"pkg/0.1:{pid}!!!!" in mybuildlogs # Now the other client will update the metadata save(myrecipefile, "mylogs2!!!!") @@ -205,34 +178,28 @@ def test_no_download_cached(self): mybuildlogs = load(os.path.join(c2_pkg_metadata_path, "logs", "mybuildlogs.txt")) assert "mybuildlogs2!!!!" in mybuildlogs - def test_upload_ignored_metadata(self): + def test_upload_ignored_metadata(self, create_conan_pkg): """ Upload command should ignore metadata files when passing --metadata="" """ - client = TestClient(default_server_user=True) - client.save({"conanfile.py": GenConanfile("pkg", "0.1")}) - client.run("create .") - pid = client.created_package_id("pkg/0.1") + client, pid = create_conan_pkg - self._save_metadata_file(client, "pkg/0.1") - self._save_metadata_file(client, f"pkg/0.1:{pid}") + self.save_metadata_file(client, "pkg/0.1") + self.save_metadata_file(client, f"pkg/0.1:{pid}") client.run('upload * --confirm --remote=default --metadata=""') assert "Recipe metadata" not in client.out assert "Package metadata" not in client.out - def test_upload_ignored_metadata_with_pattern(self): + def test_upload_ignored_metadata_with_pattern(self, create_conan_pkg): """ Upload command should ignore metadata files when passing --metadata="". Even if a pattern is passed to the upload command. """ - client = TestClient(default_server_user=True) - client.save({"conanfile.py": GenConanfile("pkg", "0.1")}) - client.run("create .") - pid = client.created_package_id("pkg/0.1") + client, pid = create_conan_pkg - self._save_metadata_file(client, "pkg/0.1") - self._save_metadata_file(client, f"pkg/0.1:{pid}") + self.save_metadata_file(client, "pkg/0.1") + self.save_metadata_file(client, f"pkg/0.1:{pid}") client.run('upload * --confirm --remote=default --metadata="" --metadata="logs/*"', assert_error=True) assert "ERROR: Empty string and patterns can not be mixed for metadata." in client.out From c5810cdde4ef899672357521984224ff304d3839 Mon Sep 17 00:00:00 2001 From: Uilian Ries Date: Wed, 25 Oct 2023 09:21:33 +0200 Subject: [PATCH 7/8] Update test description Signed-off-by: Uilian Ries --- conans/test/integration/metadata/test_metadata_commands.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/conans/test/integration/metadata/test_metadata_commands.py b/conans/test/integration/metadata/test_metadata_commands.py index 682140270b5..4201638dee9 100644 --- a/conans/test/integration/metadata/test_metadata_commands.py +++ b/conans/test/integration/metadata/test_metadata_commands.py @@ -193,8 +193,7 @@ def test_upload_ignored_metadata(self, create_conan_pkg): def test_upload_ignored_metadata_with_pattern(self, create_conan_pkg): """ - Upload command should ignore metadata files when passing --metadata="". - Even if a pattern is passed to the upload command. + Upload command should fail when passing --metadata="" and a pattern """ client, pid = create_conan_pkg From 8999a0bc6be51b2fa60076f9544eeaa759135691 Mon Sep 17 00:00:00 2001 From: Uilian Ries Date: Wed, 25 Oct 2023 09:42:27 +0200 Subject: [PATCH 8/8] Simplify test for mixed metadata args Signed-off-by: Uilian Ries --- conans/test/integration/metadata/test_metadata_commands.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/conans/test/integration/metadata/test_metadata_commands.py b/conans/test/integration/metadata/test_metadata_commands.py index 4201638dee9..32278907f31 100644 --- a/conans/test/integration/metadata/test_metadata_commands.py +++ b/conans/test/integration/metadata/test_metadata_commands.py @@ -195,10 +195,9 @@ def test_upload_ignored_metadata_with_pattern(self, create_conan_pkg): """ Upload command should fail when passing --metadata="" and a pattern """ - client, pid = create_conan_pkg - - self.save_metadata_file(client, "pkg/0.1") - self.save_metadata_file(client, f"pkg/0.1:{pid}") + client = TestClient(default_server_user=True) + client.save({"conanfile.py": GenConanfile("pkg", "0.1")}) + client.run("export .") client.run('upload * --confirm --remote=default --metadata="" --metadata="logs/*"', assert_error=True) assert "ERROR: Empty string and patterns can not be mixed for metadata." in client.out