From 5a4c038affbc8933536c6272421caf821e12ae57 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Abril=20Rinc=C3=B3n=20Blanco?= Date: Wed, 3 Jul 2024 12:10:13 +0200 Subject: [PATCH 1/4] Quote build_args in conan graph build-order --- conans/client/graph/install_graph.py | 3 +- .../command_v2/test_info_build_order.py | 44 ++++++++++++++----- 2 files changed, 34 insertions(+), 13 deletions(-) diff --git a/conans/client/graph/install_graph.py b/conans/client/graph/install_graph.py index 65befe30de1..d262ad15a74 100644 --- a/conans/client/graph/install_graph.py +++ b/conans/client/graph/install_graph.py @@ -1,5 +1,6 @@ import json import os +import shlex import textwrap from conan.api.output import ConanOutput @@ -275,7 +276,7 @@ def _build_args(self): cmd = f"--requires={self.ref}" if self.context == "host" else f"--tool-requires={self.ref}" cmd += f" --build={self.ref}" if self.options: - cmd += " " + " ".join(f"-o {o}" for o in self.options) + cmd += " " + " ".join(f"-o {shlex.quote(o)}" for o in self.options) if self.overrides: cmd += f' --lockfile-overrides="{self.overrides}"' return cmd diff --git a/test/integration/command_v2/test_info_build_order.py b/test/integration/command_v2/test_info_build_order.py index cf95afb5429..ecb58796d36 100644 --- a/test/integration/command_v2/test_info_build_order.py +++ b/test/integration/command_v2/test_info_build_order.py @@ -9,7 +9,7 @@ def test_info_build_order(): - c = TestClient() + c = TestClient(light=True) c.save({"dep/conanfile.py": GenConanfile(), "pkg/conanfile.py": GenConanfile().with_requires("dep/0.1"), "consumer/conanfile.txt": "[requires]\npkg/0.1"}) @@ -75,7 +75,7 @@ def test_info_build_order(): def test_info_build_order_configuration(): - c = TestClient() + c = TestClient(light=True) c.save({"dep/conanfile.py": GenConanfile(), "pkg/conanfile.py": GenConanfile().with_requires("dep/0.1"), "consumer/conanfile.txt": "[requires]\npkg/0.1"}) @@ -129,7 +129,7 @@ def test_info_build_order_configuration(): def test_info_build_order_configuration_text_formatter(): - c = TestClient() + c = TestClient(light=True) c.save({"dep/conanfile.py": GenConanfile(), "pkg/conanfile.py": GenConanfile().with_requires("dep/0.1"), "consumer/conanfile.txt": "[requires]\npkg/0.1"}) @@ -144,7 +144,7 @@ def test_info_build_order_configuration_text_formatter(): def test_info_build_order_build_require(): - c = TestClient() + c = TestClient(light=True) c.save({"dep/conanfile.py": GenConanfile(), "pkg/conanfile.py": GenConanfile().with_tool_requires("dep/0.1"), "consumer/conanfile.txt": "[requires]\npkg/0.1"}) @@ -199,7 +199,7 @@ def test_info_build_order_build_require(): def test_info_build_order_options(): - c = TestClient() + c = TestClient(light=True) # The normal default_options do NOT propagate to build_requires, it is necessary to use # self.requires(..., options=xxx) c.save({"tool/conanfile.py": GenConanfile().with_option("myopt", [1, 2, 3]), @@ -253,7 +253,7 @@ def test_info_build_order_options(): def test_info_build_order_merge_multi_product(): - c = TestClient() + c = TestClient(light=True) c.save({"dep/conanfile.py": GenConanfile(), "pkg/conanfile.py": GenConanfile().with_requires("dep/0.1"), "consumer1/conanfile.txt": "[requires]\npkg/0.1", @@ -334,7 +334,7 @@ def test_info_build_order_merge_multi_product(): def test_info_build_order_merge_multi_product_configurations(): - c = TestClient() + c = TestClient(light=True) c.save({"dep/conanfile.py": GenConanfile(), "pkg/conanfile.py": GenConanfile().with_requires("dep/0.1"), "consumer1/conanfile.txt": "[requires]\npkg/0.1", @@ -517,7 +517,7 @@ def test_info_build_order_lockfile_location(): """ the lockfile should be in the caller cwd https://github.com/conan-io/conan/issues/13850 """ - c = TestClient() + c = TestClient(light=True) c.save({"dep/conanfile.py": GenConanfile("dep", "0.1"), "pkg/conanfile.py": GenConanfile("pkg", "0.1").with_requires("dep/0.1")}) c.run("create dep") @@ -529,7 +529,7 @@ def test_info_build_order_lockfile_location(): def test_info_build_order_broken_recipe(): # https://github.com/conan-io/conan/issues/14104 - c = TestClient() + c = TestClient(light=True) dep = textwrap.dedent(""" from conan import ConanFile from conan.tools.files import replace_in_file @@ -549,7 +549,7 @@ def export(self): class TestBuildOrderReduce: @pytest.mark.parametrize("order", ["recipe", "configuration"]) def test_build_order_reduce(self, order): - c = TestClient() + c = TestClient(light=True) c.save({"liba/conanfile.py": GenConanfile("liba", "0.1"), "libb/conanfile.py": GenConanfile("libb", "0.1").with_requires("liba/0.1"), "libc/conanfile.py": GenConanfile("libc", "0.1").with_requires("libb/0.1"), @@ -625,7 +625,7 @@ def test_build_order_merge_reduce(self, order): assert level1[1]["depends"] == [liba2] def test_error_reduced(self): - c = TestClient() + c = TestClient(light=True) c.save({"conanfile.py": GenConanfile("liba", "0.1")}) c.run("graph build-order . --format=json", redirect_stdout="bo1.json") c.run("graph build-order . --order-by=recipe --reduce --format=json", @@ -637,7 +637,7 @@ def test_error_reduced(self): assert "ERROR: Reduced build-order file cannot be merged: bo2.json" def test_error_different_orders(self): - c = TestClient() + c = TestClient(light=True) c.save({"conanfile.py": GenConanfile("liba", "0.1")}) c.run("graph build-order . --format=json", redirect_stdout="bo1.json") c.run("graph build-order . --order-by=recipe --format=json", redirect_stdout="bo2.json") @@ -652,3 +652,23 @@ def test_error_different_orders(self): # different order c.run(f"graph build-order-merge --file=bo3.json --file=bo2.json", assert_error=True) assert "ERROR: Cannot merge build-orders of configuration!=recipe" in c.out + + +def test_build_order_space_in_options(): + tc = TestClient(light=True) + tc.save({"dep/conanfile.py": GenConanfile("dep", "1.0") + .with_option("flags", ["ANY", None]) + .with_option("extras", ["ANY", None]), + "conanfile.txt": textwrap.dedent(""" + [requires] + dep/1.0 + + [options] + dep/*:flags=define=FOO define=BAR define=BAZ + dep/*:extras="cxx=yes gnuext=no" + """)}) + + tc.run("create dep") + tc.run("graph build-order . --order-by=configuration --build=dep/1.0 -f=json", redirect_stdout="order.json") + order = json.loads(tc.load("order.json")) + assert order["order"][0][0]["build_args"] == """--requires=dep/1.0 --build=dep/1.0 -o 'dep/*:extras="cxx=yes gnuext=no"' -o 'dep/*:flags=define=FOO define=BAR define=BAZ'""" From e5300de00c7bef36b7327a26bde1ae9bf95a025e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Abril=20Rinc=C3=B3n=20Blanco?= Date: Wed, 3 Jul 2024 13:59:45 +0200 Subject: [PATCH 2/4] Another approach? --- conans/client/graph/install_graph.py | 2 +- conans/model/options.py | 8 ++++++-- test/integration/command_v2/test_info_build_order.py | 6 ++++-- 3 files changed, 11 insertions(+), 5 deletions(-) diff --git a/conans/client/graph/install_graph.py b/conans/client/graph/install_graph.py index d262ad15a74..9acce95d924 100644 --- a/conans/client/graph/install_graph.py +++ b/conans/client/graph/install_graph.py @@ -276,7 +276,7 @@ def _build_args(self): cmd = f"--requires={self.ref}" if self.context == "host" else f"--tool-requires={self.ref}" cmd += f" --build={self.ref}" if self.options: - cmd += " " + " ".join(f"-o {shlex.quote(o)}" for o in self.options) + cmd += " " + " ".join(f"-o {o}" for o in self.options) if self.overrides: cmd += f' --lockfile-overrides="{self.overrides}"' return cmd diff --git a/conans/model/options.py b/conans/model/options.py index cbf6eaf83da..c18070bbe13 100644 --- a/conans/model/options.py +++ b/conans/model/options.py @@ -29,10 +29,14 @@ def dumps(self, scope=None): if self._value is None: return None important = "!" if self.important else "" + value = self._value + if " " in value: + value = value.replace('"', r'\"') + value = f'"{value}"' if scope: - return "%s:%s%s=%s" % (scope, self._name, important, self._value) + return "%s:%s%s=%s" % (scope, self._name, important, value) else: - return "%s%s=%s" % (self._name, important, self._value) + return "%s%s=%s" % (self._name, important, value) def copy_conaninfo_option(self): # To generate a copy without validation, for package_id info.options value diff --git a/test/integration/command_v2/test_info_build_order.py b/test/integration/command_v2/test_info_build_order.py index ecb58796d36..a65d5a3e658 100644 --- a/test/integration/command_v2/test_info_build_order.py +++ b/test/integration/command_v2/test_info_build_order.py @@ -665,10 +665,12 @@ def test_build_order_space_in_options(): [options] dep/*:flags=define=FOO define=BAR define=BAZ - dep/*:extras="cxx=yes gnuext=no" + dep/*:extras=cxx="yes" gnuext="no" """)}) tc.run("create dep") tc.run("graph build-order . --order-by=configuration --build=dep/1.0 -f=json", redirect_stdout="order.json") order = json.loads(tc.load("order.json")) - assert order["order"][0][0]["build_args"] == """--requires=dep/1.0 --build=dep/1.0 -o 'dep/*:extras="cxx=yes gnuext=no"' -o 'dep/*:flags=define=FOO define=BAR define=BAZ'""" + assert order["order"][0][0]["build_args"] == ''''--requires=dep/1.0 --build=dep/1.0 -o dep/*:extras="cxx=\\"yes\\" gnuext=\\"no\\"" -o dep/*:flags="define=FOO define=BAR define=BAZ"''' + +# '--requires=dep/1.0 --build=dep/1.0 -o dep/*:extras="\\"cxx=yes gnuext=no\\"" -o dep/*:flags="define=FOO define=BAR define=BAZ"' From 426f32b496081d049cbf858f76d202b2b74965b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Abril=20Rinc=C3=B3n=20Blanco?= Date: Tue, 30 Jul 2024 19:19:59 +0200 Subject: [PATCH 3/4] Quote using shlex, remove old approach --- conans/client/graph/install_graph.py | 4 ++-- conans/model/options.py | 8 ++------ test/integration/command_v2/test_info_build_order.py | 6 ++---- 3 files changed, 6 insertions(+), 12 deletions(-) diff --git a/conans/client/graph/install_graph.py b/conans/client/graph/install_graph.py index 9acce95d924..6891391cea7 100644 --- a/conans/client/graph/install_graph.py +++ b/conans/client/graph/install_graph.py @@ -70,7 +70,7 @@ def _build_args(self): cmd = f"--requires={self.ref}" if self.context == "host" else f"--tool-requires={self.ref}" cmd += f" --build={self.ref}" if self.options: - cmd += " " + " ".join(f"-o {o}" for o in self.options) + cmd += " " + " ".join(f"-o={shlex.quote(o)}" for o in self.options) if self.overrides: cmd += f' --lockfile-overrides="{self.overrides}"' return cmd @@ -276,7 +276,7 @@ def _build_args(self): cmd = f"--requires={self.ref}" if self.context == "host" else f"--tool-requires={self.ref}" cmd += f" --build={self.ref}" if self.options: - cmd += " " + " ".join(f"-o {o}" for o in self.options) + cmd += " " + " ".join(f"-o={shlex.quote(o)}" for o in self.options) if self.overrides: cmd += f' --lockfile-overrides="{self.overrides}"' return cmd diff --git a/conans/model/options.py b/conans/model/options.py index c18070bbe13..cbf6eaf83da 100644 --- a/conans/model/options.py +++ b/conans/model/options.py @@ -29,14 +29,10 @@ def dumps(self, scope=None): if self._value is None: return None important = "!" if self.important else "" - value = self._value - if " " in value: - value = value.replace('"', r'\"') - value = f'"{value}"' if scope: - return "%s:%s%s=%s" % (scope, self._name, important, value) + return "%s:%s%s=%s" % (scope, self._name, important, self._value) else: - return "%s%s=%s" % (self._name, important, value) + return "%s%s=%s" % (self._name, important, self._value) def copy_conaninfo_option(self): # To generate a copy without validation, for package_id info.options value diff --git a/test/integration/command_v2/test_info_build_order.py b/test/integration/command_v2/test_info_build_order.py index a65d5a3e658..70e81bd9f16 100644 --- a/test/integration/command_v2/test_info_build_order.py +++ b/test/integration/command_v2/test_info_build_order.py @@ -665,12 +665,10 @@ def test_build_order_space_in_options(): [options] dep/*:flags=define=FOO define=BAR define=BAZ - dep/*:extras=cxx="yes" gnuext="no" + dep/*:extras=cxx="yes" gnuext='no' """)}) tc.run("create dep") tc.run("graph build-order . --order-by=configuration --build=dep/1.0 -f=json", redirect_stdout="order.json") order = json.loads(tc.load("order.json")) - assert order["order"][0][0]["build_args"] == ''''--requires=dep/1.0 --build=dep/1.0 -o dep/*:extras="cxx=\\"yes\\" gnuext=\\"no\\"" -o dep/*:flags="define=FOO define=BAR define=BAZ"''' - -# '--requires=dep/1.0 --build=dep/1.0 -o dep/*:extras="\\"cxx=yes gnuext=no\\"" -o dep/*:flags="define=FOO define=BAR define=BAZ"' + assert order["order"][0][0]["build_args"] == """--requires=dep/1.0 --build=dep/1.0 -o='dep/*:extras=cxx="yes" gnuext='"'"'no'"'"'' -o='dep/*:flags=define=FOO define=BAR define=BAZ'""" From 15debfddd81570338725b511f175e90a3708e460 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Abril=20Rinc=C3=B3n=20Blanco?= Date: Tue, 30 Jul 2024 19:25:56 +0200 Subject: [PATCH 4/4] Remove light=True --- .../command_v2/test_info_build_order.py | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/test/integration/command_v2/test_info_build_order.py b/test/integration/command_v2/test_info_build_order.py index 70e81bd9f16..0fbe367f47b 100644 --- a/test/integration/command_v2/test_info_build_order.py +++ b/test/integration/command_v2/test_info_build_order.py @@ -9,7 +9,7 @@ def test_info_build_order(): - c = TestClient(light=True) + c = TestClient() c.save({"dep/conanfile.py": GenConanfile(), "pkg/conanfile.py": GenConanfile().with_requires("dep/0.1"), "consumer/conanfile.txt": "[requires]\npkg/0.1"}) @@ -75,7 +75,7 @@ def test_info_build_order(): def test_info_build_order_configuration(): - c = TestClient(light=True) + c = TestClient() c.save({"dep/conanfile.py": GenConanfile(), "pkg/conanfile.py": GenConanfile().with_requires("dep/0.1"), "consumer/conanfile.txt": "[requires]\npkg/0.1"}) @@ -129,7 +129,7 @@ def test_info_build_order_configuration(): def test_info_build_order_configuration_text_formatter(): - c = TestClient(light=True) + c = TestClient() c.save({"dep/conanfile.py": GenConanfile(), "pkg/conanfile.py": GenConanfile().with_requires("dep/0.1"), "consumer/conanfile.txt": "[requires]\npkg/0.1"}) @@ -144,7 +144,7 @@ def test_info_build_order_configuration_text_formatter(): def test_info_build_order_build_require(): - c = TestClient(light=True) + c = TestClient() c.save({"dep/conanfile.py": GenConanfile(), "pkg/conanfile.py": GenConanfile().with_tool_requires("dep/0.1"), "consumer/conanfile.txt": "[requires]\npkg/0.1"}) @@ -199,7 +199,7 @@ def test_info_build_order_build_require(): def test_info_build_order_options(): - c = TestClient(light=True) + c = TestClient() # The normal default_options do NOT propagate to build_requires, it is necessary to use # self.requires(..., options=xxx) c.save({"tool/conanfile.py": GenConanfile().with_option("myopt", [1, 2, 3]), @@ -253,7 +253,7 @@ def test_info_build_order_options(): def test_info_build_order_merge_multi_product(): - c = TestClient(light=True) + c = TestClient() c.save({"dep/conanfile.py": GenConanfile(), "pkg/conanfile.py": GenConanfile().with_requires("dep/0.1"), "consumer1/conanfile.txt": "[requires]\npkg/0.1", @@ -334,7 +334,7 @@ def test_info_build_order_merge_multi_product(): def test_info_build_order_merge_multi_product_configurations(): - c = TestClient(light=True) + c = TestClient() c.save({"dep/conanfile.py": GenConanfile(), "pkg/conanfile.py": GenConanfile().with_requires("dep/0.1"), "consumer1/conanfile.txt": "[requires]\npkg/0.1", @@ -517,7 +517,7 @@ def test_info_build_order_lockfile_location(): """ the lockfile should be in the caller cwd https://github.com/conan-io/conan/issues/13850 """ - c = TestClient(light=True) + c = TestClient() c.save({"dep/conanfile.py": GenConanfile("dep", "0.1"), "pkg/conanfile.py": GenConanfile("pkg", "0.1").with_requires("dep/0.1")}) c.run("create dep") @@ -529,7 +529,7 @@ def test_info_build_order_lockfile_location(): def test_info_build_order_broken_recipe(): # https://github.com/conan-io/conan/issues/14104 - c = TestClient(light=True) + c = TestClient() dep = textwrap.dedent(""" from conan import ConanFile from conan.tools.files import replace_in_file @@ -549,7 +549,7 @@ def export(self): class TestBuildOrderReduce: @pytest.mark.parametrize("order", ["recipe", "configuration"]) def test_build_order_reduce(self, order): - c = TestClient(light=True) + c = TestClient() c.save({"liba/conanfile.py": GenConanfile("liba", "0.1"), "libb/conanfile.py": GenConanfile("libb", "0.1").with_requires("liba/0.1"), "libc/conanfile.py": GenConanfile("libc", "0.1").with_requires("libb/0.1"), @@ -625,7 +625,7 @@ def test_build_order_merge_reduce(self, order): assert level1[1]["depends"] == [liba2] def test_error_reduced(self): - c = TestClient(light=True) + c = TestClient() c.save({"conanfile.py": GenConanfile("liba", "0.1")}) c.run("graph build-order . --format=json", redirect_stdout="bo1.json") c.run("graph build-order . --order-by=recipe --reduce --format=json", @@ -637,7 +637,7 @@ def test_error_reduced(self): assert "ERROR: Reduced build-order file cannot be merged: bo2.json" def test_error_different_orders(self): - c = TestClient(light=True) + c = TestClient() c.save({"conanfile.py": GenConanfile("liba", "0.1")}) c.run("graph build-order . --format=json", redirect_stdout="bo1.json") c.run("graph build-order . --order-by=recipe --format=json", redirect_stdout="bo2.json")