Skip to content

Commit

Permalink
fix lockfile-overrides-bug (#16235)
Browse files Browse the repository at this point in the history
* solved bad merge

* wip

* fix tests

* fix

* wip

* wip

* fixes

* review

* new test with replace_requires

* wip

* avoid duplicated depends in build-order
  • Loading branch information
memsharded authored May 16, 2024
1 parent 180f5a3 commit 95da083
Show file tree
Hide file tree
Showing 11 changed files with 434 additions and 29 deletions.
11 changes: 9 additions & 2 deletions conans/client/graph/graph.py
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,11 @@ def propagate_downstream(self, require, node, src_node=None):
# print(" +++++Runtime conflict!", require, "with", node.ref)
return True
require.aggregate(existing.require)
# An override can be overriden by a downstream force/override
if existing.require.override and existing.require.ref != require.ref:
# If it is an override, but other value, it has been overriden too
existing.require.overriden_ref = existing.require.ref
existing.require.override_ref = require.ref

assert not require.version_range # No ranges slip into transitive_deps definitions
# TODO: Might need to move to an update() for performance
Expand All @@ -118,6 +123,7 @@ def propagate_downstream(self, require, node, src_node=None):
if down_require is None:
return

down_require.defining_require = require.defining_require
return d.src.propagate_downstream(down_require, node)

def check_downstream_exists(self, require):
Expand Down Expand Up @@ -161,6 +167,7 @@ def check_downstream_exists(self, require):
# print(" No need to check downstream more")
return result

down_require.defining_require = require.defining_require
source_node = dependant.src
return source_node.check_downstream_exists(down_require) or result

Expand Down Expand Up @@ -275,9 +282,9 @@ def create(nodes):
overrides = {}
for n in nodes:
for r in n.conanfile.requires.values():
if r.override:
if r.override and not r.overriden_ref: # overrides are not real graph edges
continue
if r.overriden_ref and not r.force:
if r.overriden_ref:
overrides.setdefault(r.overriden_ref, set()).add(r.override_ref)
else:
overrides.setdefault(r.ref, set()).add(None)
Expand Down
12 changes: 9 additions & 3 deletions conans/client/graph/graph_builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -81,9 +81,13 @@ def _expand_require(self, require, node, graph, profile_host, profile_build, gra

prev_ref = prev_node.ref if prev_node else prev_require.ref
if prev_require.force or prev_require.override: # override
require.overriden_ref = require.ref # Store that the require has been overriden
require.override_ref = prev_ref
require.ref = prev_ref
if prev_require.defining_require is not require:
require.overriden_ref = require.overriden_ref or require.ref.copy() # Old one
# require.override_ref can be !=None if lockfile-overrides defined
require.override_ref = (require.override_ref or prev_require.override_ref
or prev_require.ref.copy()) # New one
require.defining_require = prev_require.defining_require # The overrider
require.ref = prev_ref # New one, maybe resolved with revision
else:
self._conflicting_version(require, node, prev_require, prev_node,
prev_ref, base_previous, self._resolve_prereleases)
Expand Down Expand Up @@ -195,6 +199,8 @@ def _initialize_requires(self, node, graph, graph_lock, profile_build, profile_h
if not resolved:
self._resolve_alias(node, require, alias, graph)
self._resolve_replace_requires(node, require, profile_build, profile_host, graph)
if graph_lock:
graph_lock.resolve_overrides(require)
node.transitive_deps[require] = TransitiveRequirement(require, node=None)

def _resolve_alias(self, node, require, alias, graph):
Expand Down
2 changes: 1 addition & 1 deletion conans/client/graph/install_graph.py
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ def add(self, node):
if dep.dst.binary != BINARY_SKIP:
if dep.dst.ref == node.ref: # If the node is itself, then it is internal dep
install_pkg_ref.depends.append(dep.dst.pref.package_id)
else:
elif dep.dst.ref not in self.depends:
self.depends.append(dep.dst.ref)

def _install_order(self):
Expand Down
21 changes: 13 additions & 8 deletions conans/model/graph_lock.py
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,6 @@ def resolve_locked(self, node, require, resolve_prereleases):
locked_refs = self._conf_requires.refs()
else:
locked_refs = self._requires.refs()
self._resolve_overrides(require)
try:
self._resolve(require, locked_refs, resolve_prereleases)
except ConanException:
Expand All @@ -273,13 +272,19 @@ def resolve_locked(self, node, require, resolve_prereleases):
ConanOutput().error(msg, error_type="exception")
raise

def _resolve_overrides(self, require):
existing = self._overrides.get(require.ref)
if existing is not None and len(existing) == 1:
require.overriden_ref = require.ref # Store that the require has been overriden
ref = next(iter(existing))
require.ref = ref
require.override_ref = ref
def resolve_overrides(self, require):
""" The lockfile contains the overrides to be able to inject them when the lockfile is
applied to upstream dependencies, that have the overrides downstream
"""
if not self._overrides:
return

overriden = self._overrides.get(require.ref)
if overriden and len(overriden) == 1:
override_ref = next(iter(overriden))
require.overriden_ref = require.overriden_ref or require.ref.copy()
require.override_ref = override_ref
require.ref = override_ref

def resolve_prev(self, node):
if node.context == CONTEXT_BUILD:
Expand Down
5 changes: 5 additions & 0 deletions conans/model/recipe_ref.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,11 @@ def __init__(self, name=None, version=None, user=None, channel=None, revision=No
self.revision = revision
self.timestamp = timestamp

def copy(self):
# Used for creating copy in lockfile-overrides mechanism
return RecipeReference(self.name, self.version, self.user, self.channel, self.revision,
self.timestamp)

def __repr__(self):
""" long repr like pkg/0.1@user/channel#rrev%timestamp """
result = self.repr_notime()
Expand Down
13 changes: 3 additions & 10 deletions conans/model/requires.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,9 @@ def __init__(self, ref, *, headers=None, libs=None, build=False, run=None, visib
self._direct = direct
self.options = options
# Meta and auxiliary information
# The "defining_require" is the require that defines the current value. If this require is
# overriden/forced, this attribute will point to the overriding/forcing requirement.
self.defining_require = self # if not overriden, it points to itself
self.overriden_ref = None # to store if the requirement has been overriden (store old ref)
self.override_ref = None # to store if the requirement has been overriden (store new ref)
self.is_test = test # to store that it was a test, even if used as regular requires too
Expand Down Expand Up @@ -518,16 +521,6 @@ def build_require(self, ref, raise_if_duplicated=True, package_id_mode=None, vis
raise ConanException("Duplicated requirement: {}".format(ref))
self._requires[req] = req

def override(self, ref):
req = Requirement(ref)
old_requirement = self._requires.get(req)
if old_requirement is not None:
req.force = True
self._requires[req] = req
else:
req.override = True
self._requires[req] = req

def test_require(self, ref, run=None, options=None, force=None):
"""
Represent a testing framework like gtest
Expand Down
4 changes: 2 additions & 2 deletions conans/test/integration/graph/conflict_diamond_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -54,15 +54,15 @@ def _game_conanfile(version, reverse=False):
c.save({"game/conanfile.py": _game_conanfile(v)})
c.run("install game")
c.assert_overrides({"math/1.0": [f"math/{v}"],
"math/1.0.1": [f"math/{v}#8e1a7a5ce869d8c54ae3d33468fd657c"]})
"math/1.0.1": [f"math/{v}"]})
c.assert_listed_require({f"math/{v}": "Cache"})

# Check that order of requirements doesn't affect
for v in ("1.0", "1.0.1", "1.0.2"):
c.save({"game/conanfile.py": _game_conanfile(v, reverse=True)})
c.run("install game")
c.assert_overrides({"math/1.0": [f"math/{v}"],
"math/1.0.1": [f"math/{v}#8e1a7a5ce869d8c54ae3d33468fd657c"]})
"math/1.0.1": [f"math/{v}"]})
c.assert_listed_require({f"math/{v}": "Cache"})

c.run("install --requires=engine/1.0 --requires=ai/1.0", assert_error=True)
Expand Down
2 changes: 1 addition & 1 deletion conans/test/integration/graph/test_test_requires.py
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ def requirements(self):
c.run("create rapidcheck")
c.run("install game")
c.assert_listed_require({"gtest/1.0": "Cache"}, test=True)
c.assert_overrides({"gtest/1.1": ["gtest/1.0#08dd14dd79315dd95c94b85d13bd1388"]})
c.assert_overrides({"gtest/1.1": ["gtest/1.0"]})


def test_require_options():
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ def test_override(self):
"ros_core/pr-53@3rdparty/snapshot"
],
"ros_core/1.1.4@3rdparty/unstable": [
"ros_core/pr-53@3rdparty/snapshot#4d670581ccb765839f2239cc8dff8fbd"
"ros_core/pr-53@3rdparty/snapshot"
]
}
assert info['graph']["overrides"] == expected_overrides
Expand Down
Loading

0 comments on commit 95da083

Please sign in to comment.