diff --git a/conans/model/graph_lock.py b/conans/model/graph_lock.py index 0603ee23343..eeb1055e8b7 100644 --- a/conans/model/graph_lock.py +++ b/conans/model/graph_lock.py @@ -49,16 +49,19 @@ def deserialize(data): return result def add(self, ref, package_ids=None): - # In case we have an existing, incomplete thing - pop_ref = RecipeReference.loads(str(ref)) - self._requires.pop(pop_ref, None) # Partial cannot have package_ids - old_package_ids = self._requires.pop(ref, None) - if old_package_ids is not None: - if package_ids is not None: - package_ids = old_package_ids.update(package_ids) - else: - package_ids = old_package_ids - self._requires[ref] = package_ids + if ref.revision is not None: + old_package_ids = self._requires.pop(ref, None) # Get existing one + if old_package_ids is not None: + if package_ids is not None: + package_ids = old_package_ids.update(package_ids) + else: + package_ids = old_package_ids + self._requires[ref] = package_ids + else: # Manual addition of something without revision + existing = {r: r for r in self._requires}.get(ref) + if existing and existing.revision is not None: + raise ConanException(f"Cannot add {ref} to lockfile, already exists") + self._requires[ref] = package_ids def sort(self): self._requires = OrderedDict(reversed(sorted(self._requires.items()))) diff --git a/conans/model/recipe_ref.py b/conans/model/recipe_ref.py index 62377e38438..f4965a68a0a 100644 --- a/conans/model/recipe_ref.py +++ b/conans/model/recipe_ref.py @@ -78,12 +78,16 @@ def __eq__(self, ref): # This is necessary for building an ordered list of UNIQUE recipe_references for Lockfile if ref is None: return False - return (self.name, self.version, self.user, self.channel, self.revision) == \ - (ref.name, ref.version, ref.user, ref.channel, ref.revision) + # If one revision is not defined, they are equal + if self.revision is not None and ref.revision is not None: + return (self.name, self.version, self.user, self.channel, self.revision) == \ + (ref.name, ref.version, ref.user, ref.channel, ref.revision) + return (self.name, self.version, self.user, self.channel) == \ + (ref.name, ref.version, ref.user, ref.channel) def __hash__(self): # This is necessary for building an ordered list of UNIQUE recipe_references for Lockfile - return hash((self.name, self.version, self.user, self.channel, self.revision)) + return hash((self.name, self.version, self.user, self.channel)) @staticmethod def loads(rref): # TODO: change this default to validate only on end points diff --git a/conans/test/integration/lockfile/test_lock_requires.py b/conans/test/integration/lockfile/test_lock_requires.py index 0c378d035e4..98e13912655 100644 --- a/conans/test/integration/lockfile/test_lock_requires.py +++ b/conans/test/integration/lockfile/test_lock_requires.py @@ -450,3 +450,82 @@ def test(self): assert "dep/1.0" in c.out assert "dep/2.0" not in c.out assert "package tested" in c.out + + +class TestErrorDuplicates: + def test_error_duplicates(self): + """ the problem is having 2 different, almost identical requires that will point to the same + thing, with different traits and not colliding. + Lockfiles do a ``require.ref`` update and that alters some dictionaries iteration, producing + an infinite loop and blocking + """ + c = TestClient() + pkg = textwrap.dedent(""" + from conan import ConanFile + class Pkg(ConanFile): + name = "pkg" + version = "0.1" + def requirements(self): + self.requires("dep/0.1#f8c2264d0b32a4c33f251fe2944bb642", headers=False, libs=False, + visible=False) + self.requires("dep/0.1", headers=True, libs=False, visible=False) + """) + c.save({"dep/conanfile.py": GenConanfile("dep", "0.1"), + "pkg/conanfile.py": pkg}) + c.run("create dep --lockfile-out=conan.lock") + c.run("create pkg", assert_error=True) + assert "Duplicated requirement: dep/0.1" in c.out + c.run("create pkg --lockfile=conan.lock", assert_error=True) + assert "Duplicated requirement: dep/0.1" in c.out + + def test_error_duplicates_reverse(self): + """ Same as above, but order requires changed + """ + c = TestClient() + pkg = textwrap.dedent(""" + from conan import ConanFile + class Pkg(ConanFile): + name = "pkg" + version = "0.1" + def requirements(self): + self.requires("dep/0.1", headers=True, libs=False, visible=False) + self.requires("dep/0.1#f8c2264d0b32a4c33f251fe2944bb642", headers=False, libs=False, + visible=False) + """) + c.save({"dep/conanfile.py": GenConanfile("dep", "0.1"), + "pkg/conanfile.py": pkg}) + c.run("create dep --lockfile-out=conan.lock") + c.run("create pkg", assert_error=True) + assert "Duplicated requirement: dep/0.1" in c.out + c.run("create pkg --lockfile=conan.lock", assert_error=True) + assert "Duplicated requirement: dep/0.1" in c.out + + def test_error_duplicates_revisions(self): + """ 2 different revisions can be added without conflict, if they are not visible and not + other conflicting traits + """ + c = TestClient() + pkg = textwrap.dedent(""" + from conan import ConanFile + class Pkg(ConanFile): + name = "pkg" + version = "0.1" + def requirements(self): + self.requires("dep/0.1#f8c2264d0b32a4c33f251fe2944bb642", headers=False, + libs=False, visible=False) + self.requires("dep/0.1#7b91e6100797b8b012eb3cdc5544800b", headers=True, + libs=False, visible=False) + """) + c.save({"dep/conanfile.py": GenConanfile("dep", "0.1"), + "dep2/conanfile.py": GenConanfile("dep", "0.1").with_class_attribute("potato=42"), + "pkg/conanfile.py": pkg}) + c.run("create dep --lockfile-out=conan.lock") + c.run("create dep2 --lockfile=conan.lock --lockfile-out=conan.lock") + + c.run("create pkg") + assert "dep/0.1#f8c2264d0b32a4c33f251fe2944bb642 - Cache" in c.out + assert "dep/0.1#7b91e6100797b8b012eb3cdc5544800b - Cache" in c.out + print(c.load("conan.lock")) + c.run("create pkg --lockfile=conan.lock") + assert "dep/0.1#f8c2264d0b32a4c33f251fe2944bb642 - Cache" in c.out + assert "dep/0.1#7b91e6100797b8b012eb3cdc5544800b - Cache" in c.out diff --git a/conans/test/integration/lockfile/test_user_overrides.py b/conans/test/integration/lockfile/test_user_overrides.py index e37a8223d4e..9061e15dafa 100644 --- a/conans/test/integration/lockfile/test_user_overrides.py +++ b/conans/test/integration/lockfile/test_user_overrides.py @@ -181,12 +181,11 @@ def test_add_multiple_revisions(): assert ['math/1.0#revx%0.0', 'math/1.0#rev2', 'math/1.0#rev1', 'math/1.0#rev0'] == \ new_lock["requires"] - # add without revision at all - # It is not that this makes a lot of sense, but it is up to the user, the important thing - # is that it doesn't crash - c.run("lock add --requires=math/1.0") + # add without revision at all, will give us an error, as it doesn't make sense + c.run("lock add --requires=math/1.0", assert_error=True) + assert "Cannot add math/1.0 to lockfile, already exists" in c.out new_lock = json.loads(c.load("conan.lock")) - assert ['math/1.0#revx%0.0', 'math/1.0#rev2', 'math/1.0#rev1', 'math/1.0#rev0', 'math/1.0'] == \ + assert ['math/1.0#revx%0.0', 'math/1.0#rev2', 'math/1.0#rev1', 'math/1.0#rev0'] == \ new_lock["requires"]