Skip to content

Commit

Permalink
Make sure that options are fully locked when creating lockfiles based…
Browse files Browse the repository at this point in the history
… on an existing lockfile (#7513)

* tests

* fixing tests

* docstring

* fixed tests

* changing behavior to raise if option conflicts

* recurse graph_build only when relaxed

* Update conans/test/functional/graph_lock/dynamic_test.py

Co-authored-by: Javier G. Sogo <jgsogo@gmail.com>

* Update conans/test/functional/graph_lock/dynamic_test.py

Co-authored-by: Javier G. Sogo <jgsogo@gmail.com>

* Update conans/test/functional/graph_lock/dynamic_test.py

Co-authored-by: Javier G. Sogo <jgsogo@gmail.com>

* remove question

Co-authored-by: Javier G. Sogo <jgsogo@gmail.com>
  • Loading branch information
memsharded and jgsogo committed Sep 30, 2020
1 parent d880bda commit 2541915
Show file tree
Hide file tree
Showing 4 changed files with 130 additions and 6 deletions.
6 changes: 4 additions & 2 deletions conans/client/graph/graph_builder.py
Expand Up @@ -269,8 +269,10 @@ def _expand_require(self, require, node, graph, check_updates, update, remotes,

# Recursion is only necessary if the inputs conflict with the current "previous"
# configuration of upstream versions and options
if not graph_lock and self._recurse(previous.public_closure, new_reqs, new_options,
previous.context):
# recursion can stop if there is a graph_lock not relaxed
lock_recurse = not (graph_lock and not graph_lock.relaxed)
if lock_recurse and self._recurse(previous.public_closure, new_reqs, new_options,
previous.context):
self._expand_node(previous, graph, new_reqs, node.ref, new_options, check_updates,
update, remotes, profile_host, profile_build, graph_lock)

Expand Down
5 changes: 5 additions & 0 deletions conans/model/graph_lock.py
Expand Up @@ -331,6 +331,10 @@ def relax(self):
for n in self._nodes.values():
n.relax()

@property
def relaxed(self):
return self._relaxed

def clean_modified(self):
for n in self._nodes.values():
n.clean_modified()
Expand Down Expand Up @@ -497,6 +501,7 @@ def pre_lock_node(self, node):
else:
node.graph_lock_node = locked_node
node.conanfile.options.values = locked_node.options
node.conanfile.options.freeze()

def lock_node(self, node, requires, build_requires=False):
""" apply options and constraints on requirements of a node, given the information from
Expand Down
108 changes: 108 additions & 0 deletions conans/test/functional/graph_lock/dynamic_test.py
@@ -1,4 +1,5 @@
import json
import textwrap
import unittest

from conans.test.utils.tools import TestClient, GenConanfile
Expand Down Expand Up @@ -254,3 +255,110 @@ def augment_test_package_requires(self):
else:
self.assertEqual(dep["ref"], "dep/0.1")
self.assertEqual(dep["prev"], "0")


class PartialOptionsTest(unittest.TestCase):
"""
When an option is locked in an existing lockfile, and we are using that lockfile to
create a new one, and somehow the option is changed there are 2 options:
- Allow the non-locked packages to change the value, according to the dependency resolution
algorithm. That will produce a different package-id that will be detected and raise
as incompatible to the locked one
- Force the locked options, that will result in the same package-id. The package attempting
to change that option, will not have it set, and can fail later (build time, link time)
This test implements the 2nd approach, let the lockfile define the options values, not the
package recipes
"""
def setUp(self):
client = TestClient()
client.run("config set general.default_package_id_mode=full_package_mode")
client.save({"conanfile.py": GenConanfile().with_option("myoption", [True, False])})
client.run("create . LibA/1.0@ -o LibA:myoption=True")
self.assertIn("LibA/1.0:d2560ba1787c188a1d7fabeb5f8e012ac53301bb - Build", client.out)
client.run("create . LibA/1.0@ -o LibA:myoption=False")
self.assertIn("LibA/1.0:5ab84d6acfe1f23c4fae0ab88f26e3a396351ac9 - Build", client.out)
self.client = client

def partial_lock_option_command_line_test(self):
# When 'LibA:myoption' is set in command line, the option value is saved in the libb.lock and it is applied to all
# graph, overriding LibC.
client = self.client
client.save({"conanfile.py": GenConanfile().with_require("LibA/1.0")})
client.run("create . LibB/1.0@ -o LibA:myoption=True")
client.run("lock create --reference=LibB/1.0 --lockfile-out=libb.lock -o LibA:myoption=True")

client.save({"conanfile.py": GenConanfile().with_require("LibA/1.0")
.with_default_option("LibA:myoption", False)})
client.run("create . LibC/1.0@")
self.assertIn("LibA/1.0:5ab84d6acfe1f23c4fae0ab88f26e3a396351ac9 - Cache", client.out)

client.save({"conanfile.py": GenConanfile().with_require("LibB/1.0")
.with_require("LibC/1.0")})
client.run("lock create conanfile.py --name=LibD --version=1.0 --lockfile=libb.lock "
"--lockfile-out=libd.lock")
self.assertIn("LibA/1.0:d2560ba1787c188a1d7fabeb5f8e012ac53301bb - Cache", client.out)
self.assertIn("LibB/1.0:777a7717c781c687b6d0fecc05d3818d0a031f92 - Cache", client.out)
self.assertIn("LibC/1.0:777a7717c781c687b6d0fecc05d3818d0a031f92 - Missing", client.out)

# Order of LibC, LibB doesn't matter
client.save({"conanfile.py": GenConanfile().with_require("LibC/1.0")
.with_require("LibB/1.0")})
client.run("lock create conanfile.py --name=LibD --version=1.0 --lockfile=libb.lock "
"--lockfile-out=libd.lock")
self.assertIn("LibA/1.0:d2560ba1787c188a1d7fabeb5f8e012ac53301bb - Cache", client.out)
self.assertIn("LibB/1.0:777a7717c781c687b6d0fecc05d3818d0a031f92 - Cache", client.out)
self.assertIn("LibC/1.0:777a7717c781c687b6d0fecc05d3818d0a031f92 - Missing", client.out)

def partial_lock_option_conanfile_default_test(self):
# when 'LibA:myoption' is locked, it is used, even if other packages define it.
client = self.client
client.save({"conanfile.py": GenConanfile().with_require("LibA/1.0")
.with_default_option("LibA:myoption", True)})
client.run("create . LibB/1.0@")
client.run("lock create --reference=LibB/1.0 --lockfile-out=libb.lock")

client.save({"conanfile.py": GenConanfile().with_require("LibA/1.0")
.with_default_option("LibA:myoption", False)})
client.run("create . LibC/1.0@")
self.assertIn("LibA/1.0:5ab84d6acfe1f23c4fae0ab88f26e3a396351ac9 - Cache", client.out)
self._check()

def partial_lock_option_conanfile_configure_test(self):
# when 'LibA:myoption' is locked, it is used, even if other packages define it.
client = self.client
client.save({"conanfile.py": GenConanfile().with_require("LibA/1.0")
.with_default_option("LibA:myoption", True)})
client.run("create . LibB/1.0@")
client.run("lock create --reference=LibB/1.0 --lockfile-out=libb.lock")

libc = textwrap.dedent("""
from conans import ConanFile
class LibC(ConanFile):
requires = "LibA/1.0"
def configure(self):
self.options["LibA"].myoption = False
""")
client.save({"conanfile.py": libc})
client.run("create . LibC/1.0@")
self.assertIn("LibA/1.0:5ab84d6acfe1f23c4fae0ab88f26e3a396351ac9 - Cache", client.out)
self._check()

def _check(self):
client = self.client

def _validate():
client.run("lock create conanfile.py --name=LibD --version=1.0 --lockfile=libb.lock "
"--lockfile-out=libd.lock", assert_error=True)
expected = ("LibA/1.0: LibC/1.0 tried to change LibA/1.0 option myoption to False\n"
"but it was already defined as True")
self.assertIn(expected, client.out)

client.save({"conanfile.py": GenConanfile().with_require("LibB/1.0")
.with_require("LibC/1.0")})
_validate()

# Order of LibC, LibB does matter
client.save({"conanfile.py": GenConanfile().with_require("LibC/1.0")
.with_require("LibB/1.0")})
_validate()
17 changes: 13 additions & 4 deletions conans/test/functional/graph_lock/lock_recipe_test.py
Expand Up @@ -31,16 +31,16 @@ def lock_recipe_test(self):
client.out)

client.save({"conanfile.py": GenConanfile().with_require("pkg/0.1")})
client.run("lock create conanfile.py --base --lockfile-out=conan.lock")
lock = json.loads(client.load("conan.lock"))
client.run("lock create conanfile.py --base --lockfile-out=base.lock")
lock = json.loads(client.load("base.lock"))
self.assertEqual(2, len(lock["graph_lock"]["nodes"]))
pkg_node = lock["graph_lock"]["nodes"]["1"]
if client.cache.config.revisions_enabled:
self.assertEqual(pkg_node["ref"], "pkg/0.1#f096d7d54098b7ad7012f9435d9c33f3")
else:
self.assertEqual(pkg_node["ref"], "pkg/0.1")
client.run("lock create conanfile.py -s os=Linux "
"--lockfile-out=linux.lock --lockfile=conan.lock")
"--lockfile-out=linux.lock --lockfile=base.lock")
lock = json.loads(client.load("linux.lock"))
pkg_node = lock["graph_lock"]["nodes"]["1"]
if client.cache.config.revisions_enabled:
Expand All @@ -55,7 +55,7 @@ def lock_recipe_test(self):
self.assertIsNone(pkg_node.get("modified"))

client.run("lock create conanfile.py -s os=Windows "
"--lockfile-out=windows.lock --lockfile=conan.lock")
"--lockfile-out=windows.lock --lockfile=base.lock")
lock = json.loads(client.load("windows.lock"))
pkg_node = lock["graph_lock"]["nodes"]["1"]
if client.cache.config.revisions_enabled:
Expand All @@ -68,6 +68,15 @@ def lock_recipe_test(self):
self.assertEqual(pkg_node["prev"], "0")
self.assertEqual(pkg_node["options"], "")

# Now it is possible to obtain the base one again from the full ones
client.run("lock create conanfile.py --base "
"--lockfile-out=windows_base.lock --lockfile=windows.lock")
self.assertEqual(client.load("windows_base.lock"), client.load("base.lock"))
# Now it is possible to obtain the base one again from the full ones
client.run("lock create conanfile.py --base "
"--lockfile-out=linux_base.lock --lockfile=linux.lock")
self.assertEqual(client.load("linux_base.lock"), client.load("base.lock"))

def lock_recipe_from_partial_test(self):
client = TestClient()
client.save({"conanfile.py": GenConanfile()})
Expand Down

0 comments on commit 2541915

Please sign in to comment.