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

fix lockfile-overrides-bug #16235

Merged
merged 12 commits into from
May 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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:
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was a not related small bug fix.

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):
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was dead code.

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