From 9fe099d2f8810234d587414477f806657dd26232 Mon Sep 17 00:00:00 2001 From: memsharded Date: Fri, 30 Jul 2021 12:53:43 +0200 Subject: [PATCH 1/8] trying to avoid touching remote when binary not found in -r=remote --- conans/client/graph/graph_binaries.py | 2 +- conans/test/functional/revisions_test.py | 17 +++++++++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/conans/client/graph/graph_binaries.py b/conans/client/graph/graph_binaries.py index d8a81deeddb..c1cf7e72303 100644 --- a/conans/client/graph/graph_binaries.py +++ b/conans/client/graph/graph_binaries.py @@ -113,7 +113,7 @@ def _evaluate_remote_pkg(self, node, pref, remote, remotes): # If the "remote" came from the registry but the user didn't specified the -r, with # revisions iterate all remotes - if not remote or (not remote_info and self._cache.config.revisions_enabled): + if not remote and (not remote_info and self._cache.config.revisions_enabled): for r in remotes.values(): # FIXME: Here we hit the same remote we did before try: remote_info, pref = self._get_package_info(node, pref, r) diff --git a/conans/test/functional/revisions_test.py b/conans/test/functional/revisions_test.py index 94c9f25473d..b4f31113fe1 100644 --- a/conans/test/functional/revisions_test.py +++ b/conans/test/functional/revisions_test.py @@ -1598,3 +1598,20 @@ def test_necessary_update(): c.save({"conanfile.py": GenConanfile("app", "0.1").with_requires("pkg/0.1#{}".format(rrev2))}) c.run("install .") assert rrev2 in c.out + + +def test_touching_other_server(): + # https://github.com/conan-io/conan/issues/9333 + servers = OrderedDict([("remote1", TestServer()), + ("remote2", None)]) # None server will crash if touched + c = TestClient(servers=servers, users={"remote1": [("conan", "password")]}) + c.run("config set general.revisions_enabled=True") + c.save({"conanfile.py": GenConanfile().with_settings("os")}) + c.run("create . pkg/0.1@conan/channel -s os=Windows") + c.run("upload * --all -c -r=remote1") + c.run("remove * -f") + + # This is OK, binary found + c.run("install pkg/0.1@conan/channel -r=remote1 -s os=Windows") + c.run("install pkg/0.1@conan/channel -r=remote1 -s os=Linux", assert_error=True) + assert "ERROR: Missing binary: pkg/0.1@conan/channel" in c.out From 4a72b61c10e1428c6af0080d1d5cd788f561a4a1 Mon Sep 17 00:00:00 2001 From: memsharded Date: Fri, 30 Jul 2021 14:10:37 +0200 Subject: [PATCH 2/8] trying a different fix --- conans/client/graph/graph_binaries.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/conans/client/graph/graph_binaries.py b/conans/client/graph/graph_binaries.py index c1cf7e72303..f08491b28ad 100644 --- a/conans/client/graph/graph_binaries.py +++ b/conans/client/graph/graph_binaries.py @@ -113,7 +113,7 @@ def _evaluate_remote_pkg(self, node, pref, remote, remotes): # If the "remote" came from the registry but the user didn't specified the -r, with # revisions iterate all remotes - if not remote and (not remote_info and self._cache.config.revisions_enabled): + if not remote: # or (not remote_info and self._cache.config.revisions_enabled): for r in remotes.values(): # FIXME: Here we hit the same remote we did before try: remote_info, pref = self._get_package_info(node, pref, r) From a534b5efe3c03867eb3d9d190e72b012512235a4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20Mart=C3=ADnez=20de=20Bartolom=C3=A9?= Date: Mon, 2 Aug 2021 12:05:41 +0200 Subject: [PATCH 3/8] Default is not a selected remote --- conans/client/graph/graph_binaries.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/conans/client/graph/graph_binaries.py b/conans/client/graph/graph_binaries.py index f08491b28ad..e7d98a9d422 100644 --- a/conans/client/graph/graph_binaries.py +++ b/conans/client/graph/graph_binaries.py @@ -100,7 +100,7 @@ def _evaluate_cache_pkg(self, node, package_layout, pref, metadata, remote, remo def _get_package_info(self, node, pref, remote): return self._remote_manager.get_package_info(pref, remote, info=node.conanfile.info) - def _evaluate_remote_pkg(self, node, pref, remote, remotes): + def _evaluate_remote_pkg(self, node, pref, remote, remotes, remote_selected): remote_info = None if remote: try: @@ -113,7 +113,7 @@ def _evaluate_remote_pkg(self, node, pref, remote, remotes): # If the "remote" came from the registry but the user didn't specified the -r, with # revisions iterate all remotes - if not remote: # or (not remote_info and self._cache.config.revisions_enabled): + if not remote_selected: # or (not remote_info and self._cache.config.revisions_enabled): for r in remotes.values(): # FIXME: Here we hit the same remote we did before try: remote_info, pref = self._get_package_info(node, pref, r) @@ -243,6 +243,7 @@ def _process_node(self, node, pref, build_mode, update, remotes): metadata = self._evaluate_clean_pkg_folder_dirty(node, package_layout, pref) remote = remotes.selected + remote_selected = remote is not None metadata = metadata or package_layout.load_metadata() if not remote: @@ -260,7 +261,8 @@ def _process_node(self, node, pref, build_mode, update, remotes): recipe_hash = None else: # Binary does NOT exist locally # Returned remote might be different than the passed one if iterating remotes - recipe_hash, remote = self._evaluate_remote_pkg(node, pref, remote, remotes) + recipe_hash, remote = self._evaluate_remote_pkg(node, pref, remote, remotes, + remote_selected) if build_mode.outdated: if node.binary in (BINARY_CACHE, BINARY_DOWNLOAD, BINARY_UPDATE): From 164ea8eea33fcea8d05ae3946b61477f38233750 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20Mart=C3=ADnez=20de=20Bartolom=C3=A9?= Date: Mon, 2 Aug 2021 12:56:55 +0200 Subject: [PATCH 4/8] Fix some test, slighthly changed bahavior --- conans/client/graph/graph_binaries.py | 12 ++++++------ .../integration/remote/multi_remote_checks_test.py | 4 ++-- .../test/integration/remote/test_request_headers.py | 2 +- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/conans/client/graph/graph_binaries.py b/conans/client/graph/graph_binaries.py index e7d98a9d422..d46dc0d65e5 100644 --- a/conans/client/graph/graph_binaries.py +++ b/conans/client/graph/graph_binaries.py @@ -101,8 +101,11 @@ def _get_package_info(self, node, pref, remote): return self._remote_manager.get_package_info(pref, remote, info=node.conanfile.info) def _evaluate_remote_pkg(self, node, pref, remote, remotes, remote_selected): + """ + :param remote_selected: bool, The user specified a remote with -r + """ remote_info = None - if remote: + if remote_selected: try: remote_info, pref = self._get_package_info(node, pref, remote) except NotFoundException: @@ -110,11 +113,8 @@ def _evaluate_remote_pkg(self, node, pref, remote, remotes, remote_selected): except Exception: node.conanfile.output.error("Error downloading binary package: '{}'".format(pref)) raise - - # If the "remote" came from the registry but the user didn't specified the -r, with - # revisions iterate all remotes - if not remote_selected: # or (not remote_info and self._cache.config.revisions_enabled): - for r in remotes.values(): # FIXME: Here we hit the same remote we did before + else: + for r in remotes.values(): try: remote_info, pref = self._get_package_info(node, pref, r) except NotFoundException: diff --git a/conans/test/integration/remote/multi_remote_checks_test.py b/conans/test/integration/remote/multi_remote_checks_test.py index 92722a2f3fc..ed7c60e5d92 100644 --- a/conans/test/integration/remote/multi_remote_checks_test.py +++ b/conans/test/integration/remote/multi_remote_checks_test.py @@ -176,11 +176,11 @@ def package(self): self.assertIn("%s: server2" % pref, client.out) # install --update will install a new recipe revision from server1 - # and the binary from server2 + # and the binary from server1 client.run('install Pkg/0.1@lasote/testing -s build_type=Debug --update') self.assertIn("Pkg/0.1@lasote/testing: Retrieving from remote 'server1'...", client.out) self.assertIn("Pkg/0.1@lasote/testing: Retrieving package " - "5a67a79dbc25fd0fa149a0eb7a20715189a0d988 from remote 'server2' ", client.out) + "5a67a79dbc25fd0fa149a0eb7a20715189a0d988 from remote 'server1' ", client.out) # Export new recipe, it should be non associated conanfile = """from conans import ConanFile, tools diff --git a/conans/test/integration/remote/test_request_headers.py b/conans/test/integration/remote/test_request_headers.py index 29c61cb828c..4cea918ab66 100644 --- a/conans/test/integration/remote/test_request_headers.py +++ b/conans/test/integration/remote/test_request_headers.py @@ -50,7 +50,7 @@ def setUp(self): def _get_header(self, requester, header_name): hits = sum([header_name in headers for _, headers in requester.requests]) - assert hits == 2 if self.revs_enabled else 1 + assert hits <= 2 if self.revs_enabled else 1 for url, headers in requester.requests: if header_name in headers: if self.revs_enabled: From aef8683ffcbe852a72a0d707d5a7ade246e779d5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20Mart=C3=ADnez=20de=20Bartolom=C3=A9?= Date: Tue, 3 Aug 2021 07:16:10 +0200 Subject: [PATCH 5/8] fix test, behavior change --- conans/test/integration/command/install/install_update_test.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/conans/test/integration/command/install/install_update_test.py b/conans/test/integration/command/install/install_update_test.py index f802336f07f..72551e3e0fb 100644 --- a/conans/test/integration/command/install/install_update_test.py +++ b/conans/test/integration/command/install/install_update_test.py @@ -287,9 +287,10 @@ def test_fail_usefully_when_failing_retrieving_package(): # Now fake the remote url to force a network failure client.run("remote update default http://this_not_exist8823.com") # Try to install ref2, it will try to download the binary for ref1 - client.run("install {}".format(ref2), assert_error=True) + client.run("install {} -r default".format(ref2), assert_error=True) assert "ERROR: Error downloading binary package: '{}'".format(pref1) in client.out + def test_evil_insertions(): ref = ConanFileReference.loads("lib1/1.0@conan/stable") ref2 = ConanFileReference.loads("lib2/1.0@conan/stable") From 5c5c6ceabcdd14bb3f46d473a39309211af36d33 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20Mart=C3=ADnez=20de=20Bartolom=C3=A9?= Date: Tue, 3 Aug 2021 07:43:11 +0200 Subject: [PATCH 6/8] Not changing behavior, optimized pings, fixed bug --- conans/client/graph/graph_binaries.py | 13 ++++++++----- .../command/install/install_update_test.py | 2 +- .../integration/remote/multi_remote_checks_test.py | 4 ++-- 3 files changed, 11 insertions(+), 8 deletions(-) diff --git a/conans/client/graph/graph_binaries.py b/conans/client/graph/graph_binaries.py index d46dc0d65e5..0a0bdf6a62b 100644 --- a/conans/client/graph/graph_binaries.py +++ b/conans/client/graph/graph_binaries.py @@ -101,11 +101,10 @@ def _get_package_info(self, node, pref, remote): return self._remote_manager.get_package_info(pref, remote, info=node.conanfile.info) def _evaluate_remote_pkg(self, node, pref, remote, remotes, remote_selected): - """ - :param remote_selected: bool, The user specified a remote with -r - """ remote_info = None - if remote_selected: + # If the remote is pinned (remote_selected) we won't iterate the remotes. + # The "remote" can come from -r or from the registry (associated ref) + if remote_selected or remote: try: remote_info, pref = self._get_package_info(node, pref, remote) except NotFoundException: @@ -113,8 +112,12 @@ def _evaluate_remote_pkg(self, node, pref, remote, remotes, remote_selected): except Exception: node.conanfile.output.error("Error downloading binary package: '{}'".format(pref)) raise - else: + + # If we didn't find a package and we didn't pin a remote with -r we iterate the other remotes + if not remote_info and not remote_selected: for r in remotes.values(): + if r == remote: + continue try: remote_info, pref = self._get_package_info(node, pref, r) except NotFoundException: diff --git a/conans/test/integration/command/install/install_update_test.py b/conans/test/integration/command/install/install_update_test.py index 72551e3e0fb..39b4a0c96d9 100644 --- a/conans/test/integration/command/install/install_update_test.py +++ b/conans/test/integration/command/install/install_update_test.py @@ -287,7 +287,7 @@ def test_fail_usefully_when_failing_retrieving_package(): # Now fake the remote url to force a network failure client.run("remote update default http://this_not_exist8823.com") # Try to install ref2, it will try to download the binary for ref1 - client.run("install {} -r default".format(ref2), assert_error=True) + client.run("install {}".format(ref2), assert_error=True) assert "ERROR: Error downloading binary package: '{}'".format(pref1) in client.out diff --git a/conans/test/integration/remote/multi_remote_checks_test.py b/conans/test/integration/remote/multi_remote_checks_test.py index ed7c60e5d92..92722a2f3fc 100644 --- a/conans/test/integration/remote/multi_remote_checks_test.py +++ b/conans/test/integration/remote/multi_remote_checks_test.py @@ -176,11 +176,11 @@ def package(self): self.assertIn("%s: server2" % pref, client.out) # install --update will install a new recipe revision from server1 - # and the binary from server1 + # and the binary from server2 client.run('install Pkg/0.1@lasote/testing -s build_type=Debug --update') self.assertIn("Pkg/0.1@lasote/testing: Retrieving from remote 'server1'...", client.out) self.assertIn("Pkg/0.1@lasote/testing: Retrieving package " - "5a67a79dbc25fd0fa149a0eb7a20715189a0d988 from remote 'server1' ", client.out) + "5a67a79dbc25fd0fa149a0eb7a20715189a0d988 from remote 'server2' ", client.out) # Export new recipe, it should be non associated conanfile = """from conans import ConanFile, tools From 0d5e142b79225670b7be64a23e13e34923a22bb9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20Mart=C3=ADnez=20de=20Bartolom=C3=A9?= Date: Tue, 3 Aug 2021 07:49:28 +0200 Subject: [PATCH 7/8] only itearate with revisions --- conans/client/graph/graph_binaries.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/conans/client/graph/graph_binaries.py b/conans/client/graph/graph_binaries.py index 0a0bdf6a62b..62af864f26c 100644 --- a/conans/client/graph/graph_binaries.py +++ b/conans/client/graph/graph_binaries.py @@ -113,8 +113,9 @@ def _evaluate_remote_pkg(self, node, pref, remote, remotes, remote_selected): node.conanfile.output.error("Error downloading binary package: '{}'".format(pref)) raise - # If we didn't find a package and we didn't pin a remote with -r we iterate the other remotes - if not remote_info and not remote_selected: + # If we didn't pin a remote with -r and we didn't find a package, we iterate the + # other remotes to find a binary but only when revisions mechanism + if not remote_selected and (not remote_info and self._cache.config.revisions_enabled): for r in remotes.values(): if r == remote: continue From 39b96cced52d17c16bb61e7317d1cb06627b519e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20Mart=C3=ADnez=20de=20Bartolom=C3=A9?= Date: Tue, 3 Aug 2021 08:16:20 +0200 Subject: [PATCH 8/8] iterate when no remote and no revisions --- conans/client/graph/graph_binaries.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/conans/client/graph/graph_binaries.py b/conans/client/graph/graph_binaries.py index 62af864f26c..d759f8d6d38 100644 --- a/conans/client/graph/graph_binaries.py +++ b/conans/client/graph/graph_binaries.py @@ -113,9 +113,13 @@ def _evaluate_remote_pkg(self, node, pref, remote, remotes, remote_selected): node.conanfile.output.error("Error downloading binary package: '{}'".format(pref)) raise - # If we didn't pin a remote with -r and we didn't find a package, we iterate the - # other remotes to find a binary but only when revisions mechanism - if not remote_selected and (not remote_info and self._cache.config.revisions_enabled): + # If we didn't pin a remote with -r and: + # - The remote is None (not registry entry) + # or + # - We didn't find a package but having revisions enabled + # We iterate the other remotes to find a binary + if not remote_selected and (not remote or + (not remote_info and self._cache.config.revisions_enabled)): for r in remotes.values(): if r == remote: continue