From c15fde40cd9d91af513be5514aaad92287365df1 Mon Sep 17 00:00:00 2001 From: memsharded Date: Tue, 18 Apr 2023 00:18:53 +0200 Subject: [PATCH 1/4] build_requires visible can be overriden --- conans/model/requires.py | 9 ++++--- .../build_requires/build_requires_test.py | 26 +++++++++++++++++++ 2 files changed, 31 insertions(+), 4 deletions(-) diff --git a/conans/model/requires.py b/conans/model/requires.py index d2222ffc76b..2f72c19417c 100644 --- a/conans/model/requires.py +++ b/conans/model/requires.py @@ -386,10 +386,11 @@ class BuildRequirements: def __init__(self, requires): self._requires = requires - def __call__(self, ref, package_id_mode=None, visible=False, run=None, options=None): + def __call__(self, ref, package_id_mode=None, visible=False, run=None, options=None, + override=None): # TODO: Check which arguments could be user-defined self._requires.build_require(ref, package_id_mode=package_id_mode, visible=visible, run=run, - options=options) + options=options, override=override) class ToolRequirements: @@ -479,7 +480,7 @@ def __call__(self, str_ref, **kwargs): self._requires[req] = req def build_require(self, ref, raise_if_duplicated=True, package_id_mode=None, visible=False, - run=None, options=None): + run=None, options=None, override=None): """ Represent a generic build require, could be a tool, like "cmake" or a bundle of build scripts. @@ -495,7 +496,7 @@ def build_require(self, ref, raise_if_duplicated=True, package_id_mode=None, vis # FIXME: This raise_if_duplicated is ugly, possibly remove ref = RecipeReference.loads(ref) req = Requirement(ref, headers=False, libs=False, build=True, run=run, visible=visible, - package_id_mode=package_id_mode, options=options) + package_id_mode=package_id_mode, options=options, override=override) if raise_if_duplicated and self._requires.get(req): raise ConanException("Duplicated requirement: {}".format(ref)) diff --git a/conans/test/integration/build_requires/build_requires_test.py b/conans/test/integration/build_requires/build_requires_test.py index 206da0c84d9..dd05e109b3f 100644 --- a/conans/test/integration/build_requires/build_requires_test.py +++ b/conans/test/integration/build_requires/build_requires_test.py @@ -747,3 +747,29 @@ def requirements(self): c.assert_listed_require({"dep/1.0": "Cache"}) c.run("create consumer --build-require") assert "dep/1.0" not in c.out + + +def test_overriden_host_but_not_build(): + c = TestClient() + pkg = textwrap.dedent(""" + from conan import ConanFile + class ProtoBuf(ConanFile): + name = "pkg" + version = "0.1" + def requirements(self): + self.requires("protobuf/1.0") + def build_requirements(self): + self.tool_requires("protobuf/1.0", visible=True) + """) + c.save({"protobuf/conanfile.py": GenConanfile("protobuf"), + "pkg/conanfile.py": pkg, + "app/conanfile.py": GenConanfile().with_requires("pkg/0.1") + .with_requirement("protobuf/1.1", override=True) + .with_build_requirement("protobuf/1.1", + override=True)}) + c.run("create protobuf --version=1.0") + c.run("create protobuf --version=1.1") + c.run("create pkg") + c.run("install app") + c.assert_listed_require({"protobuf/1.1": "Cache"}) + c.assert_listed_require({"protobuf/1.1": "Cache"}, build=True) From 8ea6dead89aa996ba971dc54a9fad48514e5d255 Mon Sep 17 00:00:00 2001 From: memsharded Date: Tue, 18 Apr 2023 00:40:40 +0200 Subject: [PATCH 2/4] propose track-host too --- conans/client/graph/graph_builder.py | 5 ++++ conans/model/requires.py | 12 +++++---- .../build_requires/build_requires_test.py | 25 +++++++++++++++++++ 3 files changed, 37 insertions(+), 5 deletions(-) diff --git a/conans/client/graph/graph_builder.py b/conans/client/graph/graph_builder.py index 5da864e5bb7..1df9aa01c33 100644 --- a/conans/client/graph/graph_builder.py +++ b/conans/client/graph/graph_builder.py @@ -232,6 +232,11 @@ def _resolved_system_tool(node, require, profile_build, profile_host, resolve_pr return d, ConanFile(str(d)), RECIPE_SYSTEM_TOOL, None def _create_new_node(self, node, require, graph, profile_host, profile_build, graph_lock): + if require.track_host: + req = Requirement(require.ref, headers=True, libs=True, visible=True) + transitive = node.transitive_deps.get(req) + require.ref.version = transitive.require.ref.version + if graph_lock is not None: # Here is when the ranges and revisions are resolved graph_lock.resolve_locked(node, require, self._resolve_prereleases) diff --git a/conans/model/requires.py b/conans/model/requires.py index 2f72c19417c..7b05f4a026a 100644 --- a/conans/model/requires.py +++ b/conans/model/requires.py @@ -11,7 +11,7 @@ class Requirement: """ def __init__(self, ref, *, headers=None, libs=None, build=False, run=None, visible=None, transitive_headers=None, transitive_libs=None, test=None, package_id_mode=None, - force=None, override=None, direct=None, options=None): + force=None, override=None, direct=None, options=None, track_host=None): # * prevents the usage of more positional parameters, always ref + **kwargs # By default this is a generic library requirement self.ref = ref @@ -29,6 +29,7 @@ def __init__(self, ref, *, headers=None, libs=None, build=False, run=None, visib self._direct = direct self.options = options self.overriden_ref = None # to store if the requirement has been overriden (store old ref) + self.track_host = track_host @property def skip(self): @@ -399,10 +400,10 @@ def __init__(self, requires): self._requires = requires def __call__(self, ref, package_id_mode=None, visible=False, run=True, options=None, - override=None): + override=None, track_host=None): # TODO: Check which arguments could be user-defined self._requires.tool_require(ref, package_id_mode=package_id_mode, visible=visible, run=run, - options=options, override=override) + options=options, override=override, track_host=track_host) class TestRequirements: @@ -534,7 +535,7 @@ def test_require(self, ref, run=None, options=None): self._requires[req] = req def tool_require(self, ref, raise_if_duplicated=True, package_id_mode=None, visible=False, - run=True, options=None, override=None): + run=True, options=None, override=None, track_host=None): """ Represent a build tool like "cmake". @@ -548,7 +549,8 @@ def tool_require(self, ref, raise_if_duplicated=True, package_id_mode=None, visi # FIXME: This raise_if_duplicated is ugly, possibly remove ref = RecipeReference.loads(ref) req = Requirement(ref, headers=False, libs=False, build=True, run=run, visible=visible, - package_id_mode=package_id_mode, options=options, override=override) + package_id_mode=package_id_mode, options=options, override=override, + track_host=track_host) if raise_if_duplicated and self._requires.get(req): raise ConanException("Duplicated requirement: {}".format(ref)) self._requires[req] = req diff --git a/conans/test/integration/build_requires/build_requires_test.py b/conans/test/integration/build_requires/build_requires_test.py index dd05e109b3f..556efe53c8a 100644 --- a/conans/test/integration/build_requires/build_requires_test.py +++ b/conans/test/integration/build_requires/build_requires_test.py @@ -773,3 +773,28 @@ def build_requirements(self): c.run("install app") c.assert_listed_require({"protobuf/1.1": "Cache"}) c.assert_listed_require({"protobuf/1.1": "Cache"}, build=True) + + +def test_overriden_host_but_not_build_invisible(): + c = TestClient() + pkg = textwrap.dedent(""" + from conan import ConanFile + class ProtoBuf(ConanFile): + name = "pkg" + version = "0.1" + def requirements(self): + self.requires("protobuf/1.0") + def build_requirements(self): + self.tool_requires("protobuf/1.0", track_host=True) + """) + c.save({"protobuf/conanfile.py": GenConanfile("protobuf"), + "pkg/conanfile.py": pkg, + "app/conanfile.py": GenConanfile().with_requires("pkg/0.1") + .with_requirement("protobuf/1.1", override=True)}) + c.run("create protobuf --version=1.0") + c.run("create protobuf --version=1.1") + c.run("create pkg") + c.run("install app") + print(c.out) + c.assert_listed_require({"protobuf/1.1": "Cache"}) + c.assert_listed_require({"protobuf/1.1": "Cache"}, build=True) From 081452d206408e065dbab98d4318d65190fe7147 Mon Sep 17 00:00:00 2001 From: memsharded Date: Tue, 18 Apr 2023 22:47:40 +0200 Subject: [PATCH 3/4] tests --- conans/client/graph/graph_builder.py | 8 +- conans/model/requires.py | 12 +- .../build_requires/build_requires_test.py | 193 +++++++++++++----- 3 files changed, 156 insertions(+), 57 deletions(-) diff --git a/conans/client/graph/graph_builder.py b/conans/client/graph/graph_builder.py index 1df9aa01c33..08b202a6cc7 100644 --- a/conans/client/graph/graph_builder.py +++ b/conans/client/graph/graph_builder.py @@ -232,9 +232,15 @@ def _resolved_system_tool(node, require, profile_build, profile_host, resolve_pr return d, ConanFile(str(d)), RECIPE_SYSTEM_TOOL, None def _create_new_node(self, node, require, graph, profile_host, profile_build, graph_lock): - if require.track_host: + if require.ref.version == "{host_version}": + if not require.build or require.visible: + raise ConanException(f"{node.ref} require '{require.ref}': 'host_version' can only " + "be used for non-visible tool_requires") req = Requirement(require.ref, headers=True, libs=True, visible=True) transitive = node.transitive_deps.get(req) + if transitive is None: + raise ConanException(f"{node.ref} require '{require.ref}': didn't find a matching " + "host dependency") require.ref.version = transitive.require.ref.version if graph_lock is not None: diff --git a/conans/model/requires.py b/conans/model/requires.py index 7b05f4a026a..2f72c19417c 100644 --- a/conans/model/requires.py +++ b/conans/model/requires.py @@ -11,7 +11,7 @@ class Requirement: """ def __init__(self, ref, *, headers=None, libs=None, build=False, run=None, visible=None, transitive_headers=None, transitive_libs=None, test=None, package_id_mode=None, - force=None, override=None, direct=None, options=None, track_host=None): + force=None, override=None, direct=None, options=None): # * prevents the usage of more positional parameters, always ref + **kwargs # By default this is a generic library requirement self.ref = ref @@ -29,7 +29,6 @@ def __init__(self, ref, *, headers=None, libs=None, build=False, run=None, visib self._direct = direct self.options = options self.overriden_ref = None # to store if the requirement has been overriden (store old ref) - self.track_host = track_host @property def skip(self): @@ -400,10 +399,10 @@ def __init__(self, requires): self._requires = requires def __call__(self, ref, package_id_mode=None, visible=False, run=True, options=None, - override=None, track_host=None): + override=None): # TODO: Check which arguments could be user-defined self._requires.tool_require(ref, package_id_mode=package_id_mode, visible=visible, run=run, - options=options, override=override, track_host=track_host) + options=options, override=override) class TestRequirements: @@ -535,7 +534,7 @@ def test_require(self, ref, run=None, options=None): self._requires[req] = req def tool_require(self, ref, raise_if_duplicated=True, package_id_mode=None, visible=False, - run=True, options=None, override=None, track_host=None): + run=True, options=None, override=None): """ Represent a build tool like "cmake". @@ -549,8 +548,7 @@ def tool_require(self, ref, raise_if_duplicated=True, package_id_mode=None, visi # FIXME: This raise_if_duplicated is ugly, possibly remove ref = RecipeReference.loads(ref) req = Requirement(ref, headers=False, libs=False, build=True, run=run, visible=visible, - package_id_mode=package_id_mode, options=options, override=override, - track_host=track_host) + package_id_mode=package_id_mode, options=options, override=override) if raise_if_duplicated and self._requires.get(req): raise ConanException("Duplicated requirement: {}".format(ref)) self._requires[req] = req diff --git a/conans/test/integration/build_requires/build_requires_test.py b/conans/test/integration/build_requires/build_requires_test.py index 556efe53c8a..f3901220223 100644 --- a/conans/test/integration/build_requires/build_requires_test.py +++ b/conans/test/integration/build_requires/build_requires_test.py @@ -1,3 +1,4 @@ +import json import os import platform import textwrap @@ -749,52 +750,146 @@ def requirements(self): assert "dep/1.0" not in c.out -def test_overriden_host_but_not_build(): - c = TestClient() - pkg = textwrap.dedent(""" - from conan import ConanFile - class ProtoBuf(ConanFile): - name = "pkg" - version = "0.1" - def requirements(self): - self.requires("protobuf/1.0") - def build_requirements(self): - self.tool_requires("protobuf/1.0", visible=True) - """) - c.save({"protobuf/conanfile.py": GenConanfile("protobuf"), - "pkg/conanfile.py": pkg, - "app/conanfile.py": GenConanfile().with_requires("pkg/0.1") - .with_requirement("protobuf/1.1", override=True) - .with_build_requirement("protobuf/1.1", - override=True)}) - c.run("create protobuf --version=1.0") - c.run("create protobuf --version=1.1") - c.run("create pkg") - c.run("install app") - c.assert_listed_require({"protobuf/1.1": "Cache"}) - c.assert_listed_require({"protobuf/1.1": "Cache"}, build=True) - - -def test_overriden_host_but_not_build_invisible(): - c = TestClient() - pkg = textwrap.dedent(""" - from conan import ConanFile - class ProtoBuf(ConanFile): - name = "pkg" - version = "0.1" - def requirements(self): - self.requires("protobuf/1.0") - def build_requirements(self): - self.tool_requires("protobuf/1.0", track_host=True) - """) - c.save({"protobuf/conanfile.py": GenConanfile("protobuf"), - "pkg/conanfile.py": pkg, - "app/conanfile.py": GenConanfile().with_requires("pkg/0.1") - .with_requirement("protobuf/1.1", override=True)}) - c.run("create protobuf --version=1.0") - c.run("create protobuf --version=1.1") - c.run("create pkg") - c.run("install app") - print(c.out) - c.assert_listed_require({"protobuf/1.1": "Cache"}) - c.assert_listed_require({"protobuf/1.1": "Cache"}, build=True) +class TestBuildTrackHost: + + def test_overriden_host_but_not_build(self): + """ + Making the ``tool_requires(..., visible=True)`` works, and allows overriding, but + propagates the build-requirement to protobuf/protoc down the graph, and VirtualBuildEnv + will put ``protoc`` from it in the PATH. Not a problem in majority of cases, but not the + cleanest + """ + c = TestClient() + pkg = textwrap.dedent(""" + from conan import ConanFile + class ProtoBuf(ConanFile): + name = "pkg" + version = "0.1" + def requirements(self): + self.requires("protobuf/1.0") + def build_requirements(self): + self.tool_requires("protobuf/1.0", visible=True) + """) + c.save({"protobuf/conanfile.py": GenConanfile("protobuf"), + "pkg/conanfile.py": pkg, + "app/conanfile.py": GenConanfile().with_requires("pkg/0.1") + .with_requirement("protobuf/1.1", override=True) + .with_build_requirement("protobuf/1.1", + override=True)}) + c.run("create protobuf --version=1.0") + c.run("create protobuf --version=1.1") + c.run("create pkg") + c.run("install app") + c.assert_listed_require({"protobuf/1.1": "Cache"}) + c.assert_listed_require({"protobuf/1.1": "Cache"}, build=True) + + def test_overriden_host_version(self): + """ + Make the tool_requires follow the regular require with the expression "{host_version}" + """ + c = TestClient() + pkg = textwrap.dedent(""" + from conan import ConanFile + class ProtoBuf(ConanFile): + name = "pkg" + version = "0.1" + def requirements(self): + self.requires("protobuf/1.0") + def build_requirements(self): + self.tool_requires("protobuf/{host_version}") + """) + c.save({"protobuf/conanfile.py": GenConanfile("protobuf"), + "pkg/conanfile.py": pkg, + "app/conanfile.py": GenConanfile().with_requires("pkg/0.1") + .with_requirement("protobuf/1.1", override=True)}) + c.run("create protobuf --version=1.0") + c.run("create protobuf --version=1.1") + c.run("create pkg") + c.run("install pkg") # make sure it doesn't crash + c.run("install app") + c.assert_listed_require({"protobuf/1.1": "Cache"}) + c.assert_listed_require({"protobuf/1.1": "Cache"}, build=True) + # verify locks work + c.run("lock create app") + lock = json.loads(c.load("app/conan.lock")) + build_requires = lock["build_requires"] + assert len(build_requires) == 1 + assert "protobuf/1.1" in build_requires[0] + # lock can be used + c.run("install app --lockfile=app/conan.lock") + c.assert_listed_require({"protobuf/1.1": "Cache"}, build=True) + + def test_overriden_host_version_version_range(self): + """ + same as above, but using version ranges instead of overrides + """ + c = TestClient() + pkg = textwrap.dedent(""" + from conan import ConanFile + class ProtoBuf(ConanFile): + name = "pkg" + version = "0.1" + def requirements(self): + self.requires("protobuf/[*]") + def build_requirements(self): + self.tool_requires("protobuf/{host_version}") + """) + c.save({"protobuf/conanfile.py": GenConanfile("protobuf"), + "pkg/conanfile.py": pkg, + "app/conanfile.py": GenConanfile().with_requires("pkg/0.1")}) + c.run("create protobuf --version=1.0") + c.run("create pkg") + c.run("install pkg") # make sure it doesn't crash + c.run("install app") + c.assert_listed_require({"protobuf/1.0": "Cache"}) + c.assert_listed_require({"protobuf/1.0": "Cache"}, build=True) + + c.run("create protobuf --version=1.1") + c.run("install pkg") # make sure it doesn't crash + c.run("install app") + c.assert_listed_require({"protobuf/1.1": "Cache"}) + c.assert_listed_require({"protobuf/1.1": "Cache"}, build=True) + # verify locks work + c.run("lock create app") + lock = json.loads(c.load("app/conan.lock")) + build_requires = lock["build_requires"] + assert len(build_requires) == 1 + assert "protobuf/1.1" in build_requires[0] + # lock can be used + c.run("install app --lockfile=app/conan.lock") + c.assert_listed_require({"protobuf/1.1": "Cache"}, build=True) + + def test_track_host_error_nothost(self): + """ + if no host requirement is defined, it will be an error + """ + c = TestClient() + pkg = textwrap.dedent(""" + from conan import ConanFile + class ProtoBuf(ConanFile): + name = "protobuf" + def build_requirements(self): + self.tool_requires("protobuf/{host_version}") + """) + + c.save({"pkg/conanfile.py": pkg}) + c.run("install pkg", assert_error=True) + assert "ERROR: protobuf/None require 'protobuf/{host_version}': " \ + "didn't find a matching host dependency" in c.out + + def test_track_host_errors_trait(self): + """ + It is not possible to make host_version visible too + """ + c = TestClient() + pkg = textwrap.dedent(""" + from conan import ConanFile + class ProtoBuf(ConanFile): + name = "protobuf" + def requirements(self): + self.tool_requires("other/{host_version}", visible=True) + """) + c.save({"pkg/conanfile.py": pkg}) + c.run("install pkg", assert_error=True) + assert "ERROR: protobuf/None require 'other/{host_version}': 'host_version' " \ + "can only be used for non-visible tool_requires" in c.out From 1b50136ebb68e0e35421d55a30ff5475a2a6fdec Mon Sep 17 00:00:00 2001 From: memsharded Date: Wed, 19 Apr 2023 12:54:06 +0200 Subject: [PATCH 4/4] change brackets --- conans/client/graph/graph_builder.py | 2 +- .../build_requires/build_requires_test.py | 14 +++++++------- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/conans/client/graph/graph_builder.py b/conans/client/graph/graph_builder.py index 08b202a6cc7..fab4f0fa112 100644 --- a/conans/client/graph/graph_builder.py +++ b/conans/client/graph/graph_builder.py @@ -232,7 +232,7 @@ def _resolved_system_tool(node, require, profile_build, profile_host, resolve_pr return d, ConanFile(str(d)), RECIPE_SYSTEM_TOOL, None def _create_new_node(self, node, require, graph, profile_host, profile_build, graph_lock): - if require.ref.version == "{host_version}": + if require.ref.version == "": if not require.build or require.visible: raise ConanException(f"{node.ref} require '{require.ref}': 'host_version' can only " "be used for non-visible tool_requires") diff --git a/conans/test/integration/build_requires/build_requires_test.py b/conans/test/integration/build_requires/build_requires_test.py index f3901220223..abcb75f8cad 100644 --- a/conans/test/integration/build_requires/build_requires_test.py +++ b/conans/test/integration/build_requires/build_requires_test.py @@ -785,7 +785,7 @@ def build_requirements(self): def test_overriden_host_version(self): """ - Make the tool_requires follow the regular require with the expression "{host_version}" + Make the tool_requires follow the regular require with the expression "" """ c = TestClient() pkg = textwrap.dedent(""" @@ -796,7 +796,7 @@ class ProtoBuf(ConanFile): def requirements(self): self.requires("protobuf/1.0") def build_requirements(self): - self.tool_requires("protobuf/{host_version}") + self.tool_requires("protobuf/") """) c.save({"protobuf/conanfile.py": GenConanfile("protobuf"), "pkg/conanfile.py": pkg, @@ -832,7 +832,7 @@ class ProtoBuf(ConanFile): def requirements(self): self.requires("protobuf/[*]") def build_requirements(self): - self.tool_requires("protobuf/{host_version}") + self.tool_requires("protobuf/") """) c.save({"protobuf/conanfile.py": GenConanfile("protobuf"), "pkg/conanfile.py": pkg, @@ -869,12 +869,12 @@ def test_track_host_error_nothost(self): class ProtoBuf(ConanFile): name = "protobuf" def build_requirements(self): - self.tool_requires("protobuf/{host_version}") + self.tool_requires("protobuf/") """) c.save({"pkg/conanfile.py": pkg}) c.run("install pkg", assert_error=True) - assert "ERROR: protobuf/None require 'protobuf/{host_version}': " \ + assert "ERROR: protobuf/None require 'protobuf/': " \ "didn't find a matching host dependency" in c.out def test_track_host_errors_trait(self): @@ -887,9 +887,9 @@ def test_track_host_errors_trait(self): class ProtoBuf(ConanFile): name = "protobuf" def requirements(self): - self.tool_requires("other/{host_version}", visible=True) + self.tool_requires("other/", visible=True) """) c.save({"pkg/conanfile.py": pkg}) c.run("install pkg", assert_error=True) - assert "ERROR: protobuf/None require 'other/{host_version}': 'host_version' " \ + assert "ERROR: protobuf/None require 'other/': 'host_version' " \ "can only be used for non-visible tool_requires" in c.out