Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[bug] _/_ doesn't work the same as usual user/channel #7846

Closed
alex-700 opened this issue Oct 7, 2020 · 6 comments
Closed

[bug] _/_ doesn't work the same as usual user/channel #7846

alex-700 opened this issue Oct 7, 2020 · 6 comments
Assignees
Milestone

Comments

@alex-700
Copy link

alex-700 commented Oct 7, 2020

  • The quick reproducer in unit-test is here.
  • This bug was introduced in Use patterns in server query when resolving version ranges  #7673.
  • The query packagename/version@_/_ is not the same as packagename/version, because the latter can return also packagename/version@someuser/somechannel and client side filtration was deleted in the PR mentioned above.
@memsharded
Copy link
Member

Hi @alex-700

Thanks for reporting this.

Just to clarify, the _/_ user and channel should never appear in the UI, not as a requires definition, not in any Conan command. This doesn't appear at all in the docs.

The correct code is requires = "pkg/version", or conan install pkg/version@

Does the same unit-test fails if you change that?

This could be a regression, lets have a look, if such, it should be fixed asap in 1.30.1

@alex-700
Copy link
Author

alex-700 commented Oct 7, 2020

@memsharded,

Thanks for the quick response.

Yeah, I know about _/_ in the UI. It was just a fast way to show the problem in the conan client-server interaction. This bug affects the UI too (in the critical manner), e.g. if you have bincrafters remote before CCI in the remotes list and try to resolve some dependency (I know about spdlog/[^1.2]) you will get spdlog/someversion/bincrafters/stable instead of spdlog/someversion (correct one from CCI).

No, the same unit-test doesn't fail. This test shows the situation when we need to resolve dependency from CCI, but conan-server get the query somepackage/someversion without @_/_.

@memsharded
Copy link
Member

This is weird. I am doing a variation of your test, removing the @_/_ and it seems green here:

diff --git a/conans/test/unittests/client/graph/version_ranges_graph_test.py b/conans/test/unittests/client/graph/version_ranges_graph_test.py
index 210a5d8c0..653c86c2d 100644
--- a/conans/test/unittests/client/graph/version_ranges_graph_test.py
+++ b/conans/test/unittests/client/graph/version_ranges_graph_test.py
@@ -45,6 +45,8 @@ class VersionRangesTest(GraphTest):
             say_content = GenConanfile().with_name("Say").with_version(v)
             say_ref = ConanFileReference.loads("Say/%s@myuser/testing" % v)
             self.retriever.save_recipe(say_ref, say_content)
+            say_ref = ConanFileReference.loads("Say/%s" % v)
+            self.retriever.save_recipe(say_ref, say_content)

     def build_graph(self, content, update=False):
         self.loader._cached_conanfile_classes = {}
@@ -118,6 +120,40 @@ class VersionRangesTest(GraphTest):
             say_ref = ConanFileReference.loads("Say/%s@myuser/testing" % solution)
             self.assertEqual(_clear_revs(conanfile.requires), Requirements(str(say_ref)))

+    def test_remote_basic_without_userchannel(self):
+        self.resolver._local_search = None
+        remote_packages = []
+        for v in ["0.1", "0.2", "0.3", "1.1", "1.1.2", "1.2.1", "2.1", "2.2.1"]:
+            say_ref = ConanFileReference.loads("Say/[%s]" % v)
+            remote_packages.append(say_ref)
+        self.remotes.add("myremote", "myurl")
+        self.remote_manager.packages = remote_packages
+        for expr, solution in [(">0.0", "2.2.1"),
+                               (">0.1,<1", "0.3"),
+                               (">0.1,<1||2.1", "2.1"),
+                               ("", "2.2.1"),
+                               ("~0", "0.3"),
+                               ("~=1", "1.2.1"),
+                               ("~1.1", "1.1.2"),
+                               ("~=2", "2.2.1"),
+                               ("~=2.1", "2.1"),
+                               ]:
+            req = ConanFileReference.loads("Say/[%s]" % expr)
+            deps_graph = self.build_graph(GenConanfile("Hello", "1.2").with_require(req),
+                                          update=True)
+            self.assertEqual(self.remote_manager.count, {'Say/*': 1})
+            self.assertEqual(2, len(deps_graph.nodes))
+            hello = _get_nodes(deps_graph, "Hello")[0]
+            say = _get_nodes(deps_graph, "Say")[0]
+            self.assertEqual(_get_edges(deps_graph), {Edge(hello, say)})
+
+            self.assertEqual(hello.ref, None)
+            conanfile = hello.conanfile
+            self.assertEqual(conanfile.version, "1.2")
+            self.assertEqual(conanfile.name, "Hello")
+            say_ref = ConanFileReference.loads("Say/%s@" % solution)
+            self.assertEqual(_clear_revs(conanfile.requires), Requirements(str(say_ref)))
+
     def test_remote_optimized(self):
         self.resolver._local_search = None
         remote_packages = []

I am going to try an integration test instead.

@memsharded memsharded added this to the 1.30.1 milestone Oct 7, 2020
@memsharded
Copy link
Member

Reproduced, trying to come up with a fix

@memsharded
Copy link
Member

Fixing regression in #7847, to be released asap in Conan 1.30.1

@memsharded
Copy link
Member

#7847 merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants