From bc4ed4d79fd92edd825fc846dfa515c7b3912138 Mon Sep 17 00:00:00 2001 From: James Date: Wed, 26 Jul 2023 17:04:14 +0200 Subject: [PATCH] Implement test_package ``--build-test=missing`` for full control of ``test_package`` deps (#14347) * fix test_package --build=missing, now it works * wip * removed unused .all in BuildMode * fix unittest * wip * change order build_mode * wip * fix tests * working * new test * better defaults * minor -h improvements --- conan/api/subapi/graph.py | 8 +++-- conan/cli/args.py | 2 +- conan/cli/commands/create.py | 11 +++++-- conan/cli/commands/test.py | 5 +-- conan/cli/printers/graph.py | 12 +++---- conans/client/graph/graph.py | 1 - conans/client/graph/graph_binaries.py | 21 ++++++++----- conans/client/graph/graph_builder.py | 29 ----------------- conans/client/graph/install_graph.py | 6 ++-- .../test/integration/command/create_test.py | 31 +++++++++++++++++++ .../integration/command/test_package_test.py | 31 +++++++++++++++---- 11 files changed, 97 insertions(+), 60 deletions(-) diff --git a/conan/api/subapi/graph.py b/conan/api/subapi/graph.py index 92794fa2e6b..4b663342c9f 100644 --- a/conan/api/subapi/graph.py +++ b/conan/api/subapi/graph.py @@ -170,7 +170,8 @@ def load_graph(self, root_node, profile_host, profile_build, lockfile=None, remo deps_graph = builder.load_graph(root_node, profile_host, profile_build, lockfile) return deps_graph - def analyze_binaries(self, graph, build_mode=None, remotes=None, update=None, lockfile=None): + def analyze_binaries(self, graph, build_mode=None, remotes=None, update=None, lockfile=None, + build_modes_test=None, tested_graph=None): """ Given a dependency graph, will compute the package_ids of all recipes in the graph, and evaluate if they should be built from sources, downloaded from a remote server, of if the packages are already in the local Conan cache @@ -181,8 +182,11 @@ def analyze_binaries(self, graph, build_mode=None, remotes=None, update=None, lo :param remotes: list of remotes :param update: (False by default), if Conan should look for newer versions or revisions for already existing recipes in the Conan cache + :param build_modes_test: the --build-test argument + :param tested_graph: In case of a "test_package", the graph being tested """ ConanOutput().title("Computing necessary packages") conan_app = ConanApp(self.conan_api.cache_folder) binaries_analyzer = GraphBinariesAnalyzer(conan_app) - binaries_analyzer.evaluate_graph(graph, build_mode, lockfile, remotes, update) + binaries_analyzer.evaluate_graph(graph, build_mode, lockfile, remotes, update, + build_modes_test, tested_graph) diff --git a/conan/cli/args.py b/conan/cli/args.py index 82771e78159..3313d3280af 100644 --- a/conan/cli/args.py +++ b/conan/cli/args.py @@ -7,7 +7,7 @@ --build="*" Force build from source for all packages. --build=never Disallow build for all packages, use binary packages or fail if a binary - package is not found. Cannot be combined with other '--build' options. + package is not found, it cannot be combined with other '--build' options. --build=missing Build packages from source whose binary package is not found. --build=cascade Build packages from source that have at least one dependency being built from source. diff --git a/conan/cli/commands/create.py b/conan/cli/commands/create.py index 3c0aefa66de..c37c235af86 100644 --- a/conan/cli/commands/create.py +++ b/conan/cli/commands/create.py @@ -21,10 +21,14 @@ def create(conan_api, parser, *args): add_lockfile_args(parser) add_common_install_arguments(parser) parser.add_argument("--build-require", action='store_true', default=False, - help='Whether the provided reference is a build-require') + help='Whether the package being created is a build-require (to be used' + 'as tool_requires() by other packages)') parser.add_argument("-tf", "--test-folder", action=OnceArgument, help='Alternative test folder name. By default it is "test_package". ' 'Use "" to skip the test stage') + parser.add_argument("-bt", "--build-test", action="append", + help="Same as '--build' but only for the test_package requires. By default" + " if not specified it will take the '--build' value if specified") args = parser.parse_args(*args) cwd = os.getcwd() @@ -51,6 +55,8 @@ def create(conan_api, parser, *args): args.build_require) print_profiles(profile_host, profile_build) + if args.build is not None and args.build_test is None: + args.build_test = args.build deps_graph = None if not is_python_require: @@ -91,7 +97,8 @@ def create(conan_api, parser, *args): # The test_package do not make the "conan create" command return a different graph or # produce a different lockfile. The result is always the same, irrespective of test_package run_test(conan_api, test_conanfile_path, ref, profile_host, profile_build, remotes, lockfile, - update=False, build_modes=args.build, tested_python_requires=tested_python_requires) + update=False, build_modes=args.build, build_modes_test=args.build_test, + tested_python_requires=tested_python_requires, tested_graph=deps_graph) conan_api.lockfile.save_lockfile(lockfile, args.lockfile_out, cwd) return {"graph": deps_graph, diff --git a/conan/cli/commands/test.py b/conan/cli/commands/test.py index 1f9af9fdd20..d8f2093fb78 100644 --- a/conan/cli/commands/test.py +++ b/conan/cli/commands/test.py @@ -48,7 +48,7 @@ def test(conan_api, parser, *args): def run_test(conan_api, path, ref, profile_host, profile_build, remotes, lockfile, update, - build_modes, tested_python_requires=None): + build_modes, tested_python_requires=None, build_modes_test=None, tested_graph=None): root_node = conan_api.graph.load_root_test_conanfile(path, ref, profile_host, profile_build, remotes=remotes, @@ -68,7 +68,8 @@ def run_test(conan_api, path, ref, profile_host, profile_build, remotes, lockfil deps_graph.report_graph_error() conan_api.graph.analyze_binaries(deps_graph, build_modes, remotes=remotes, update=update, - lockfile=lockfile) + lockfile=lockfile, build_modes_test=build_modes_test, + tested_graph=tested_graph) print_graph_packages(deps_graph) conan_api.install.install_binaries(deps_graph=deps_graph, remotes=remotes) diff --git a/conan/cli/printers/graph.py b/conan/cli/printers/graph.py index 07efd738a69..d5aa8248305 100644 --- a/conan/cli/printers/graph.py +++ b/conan/cli/printers/graph.py @@ -18,16 +18,16 @@ def print_graph_basic(graph): for node in graph.nodes: if hasattr(node.conanfile, "python_requires"): for r in node.conanfile.python_requires._pyrequires.values(): # TODO: improve interface - python_requires[r.ref] = r.recipe, r.remote, False + python_requires[r.ref] = r.recipe, r.remote if node.recipe in (RECIPE_CONSUMER, RECIPE_VIRTUAL): continue if node.context == CONTEXT_BUILD: - build_requires[node.ref] = node.recipe, node.remote, node.test_package + build_requires[node.ref] = node.recipe, node.remote else: if node.test: - test_requires[node.ref] = node.recipe, node.remote, node.test_package + test_requires[node.ref] = node.recipe, node.remote else: - requires[node.ref] = node.recipe, node.remote, node.test_package + requires[node.ref] = node.recipe, node.remote if node.conanfile.deprecated: deprecated[node.ref] = node.conanfile.deprecated @@ -39,11 +39,9 @@ def _format_requires(title, reqs_to_print): if not reqs_to_print: return output.info(title, Color.BRIGHT_YELLOW) - for ref, (recipe, remote, test_package) in sorted(reqs_to_print.items()): + for ref, (recipe, remote) in sorted(reqs_to_print.items()): if remote is not None: recipe = "{} ({})".format(recipe, remote.name) - if test_package: - recipe = f"(tp) {recipe}" output.info(" {} - {}".format(ref.repr_notime(), recipe), Color.BRIGHT_CYAN) _format_requires("Requirements", requires) diff --git a/conans/client/graph/graph.py b/conans/client/graph/graph.py index ff2aa3b0c12..91f588712e0 100644 --- a/conans/client/graph/graph.py +++ b/conans/client/graph/graph.py @@ -57,7 +57,6 @@ def __init__(self, ref, conanfile, context, recipe=None, path=None, test=False): self.binary_remote = None self.context = context self.test = test - self.test_package = False # True if it is a test_package only package # real graph model self.transitive_deps = OrderedDict() # of _TransitiveRequirement diff --git a/conans/client/graph/graph_binaries.py b/conans/client/graph/graph_binaries.py index 0fc973ad4c1..3c4588a6ea4 100644 --- a/conans/client/graph/graph_binaries.py +++ b/conans/client/graph/graph_binaries.py @@ -304,15 +304,21 @@ def _evaluate_package_id(self, node): with conanfile_exception_formatter(conanfile, "layout"): conanfile.layout() - def evaluate_graph(self, deps_graph, build_mode, lockfile, remotes, update): + def evaluate_graph(self, deps_graph, build_mode, lockfile, remotes, update, build_mode_test=None, + tested_graph=None): self._selected_remotes = remotes or [] # TODO: A bit dirty interfaz, pass as arg instead self._update = update # TODO: Dirty, fix it - test_package = deps_graph.root.conanfile.tested_reference_str is not None - if test_package: - main_mode = BuildMode(["never"]) - test_mode = BuildMode(build_mode) + + if tested_graph is None: + main_mode = BuildMode(build_mode) + test_mode = None # Should not be used at all + mainprefs = None else: - main_mode = test_mode = BuildMode(build_mode) + main_mode = BuildMode(["never"]) + test_mode = BuildMode(build_mode_test) + mainprefs = [str(n.pref) for n in tested_graph.nodes + if n.recipe not in (RECIPE_CONSUMER, RECIPE_VIRTUAL)] + if main_mode.cascade: ConanOutput().warning("Using build-mode 'cascade' is generally inefficient and it " "shouldn't be used. Use 'package_id' and 'package_id_modes' for" @@ -330,7 +336,8 @@ def evaluate_graph(self, deps_graph, build_mode, lockfile, remotes, update): node.conanfile.layout() else: self._evaluate_package_id(node) - build_mode = test_mode if node.test_package else main_mode + build_mode = main_mode if mainprefs is None or str(node.pref) in mainprefs \ + else test_mode if lockfile: locked_prev = lockfile.resolve_prev(node) if locked_prev: diff --git a/conans/client/graph/graph_builder.py b/conans/client/graph/graph_builder.py index a0a3a689d46..0f0948d18d0 100644 --- a/conans/client/graph/graph_builder.py +++ b/conans/client/graph/graph_builder.py @@ -55,7 +55,6 @@ def load_graph(self, root_node, profile_host, profile_build, graph_lock=None): for r in reversed(new_node.conanfile.requires.values())) self._remove_overrides(dep_graph) check_graph_provides(dep_graph) - self._compute_test_package_deps(dep_graph) except GraphError as e: dep_graph.error = e dep_graph.resolved_ranges = self._resolver.resolved_ranges @@ -308,31 +307,3 @@ def _remove_overrides(dep_graph): to_remove = [r for r in node.transitive_deps if r.override] for r in to_remove: node.transitive_deps.pop(r) - - @staticmethod - def _compute_test_package_deps(graph): - """ compute and tag the graph nodes that belong exclusively to test_package - dependencies but not the main graph - """ - root_node = graph.root - tested_ref = root_node.conanfile.tested_reference_str - if tested_ref is None: - return - tested_ref = RecipeReference.loads(root_node.conanfile.tested_reference_str) - tested_ref = str(tested_ref) - # We classify direct dependencies in the "tested" main ones and the "test_package" specific - direct_nodes = [n.node for n in root_node.transitive_deps.values() if n.require.direct] - main_nodes = [n for n in direct_nodes if tested_ref == str(n.ref)] - test_package_nodes = [n for n in direct_nodes if tested_ref != str(n.ref)] - - # Accumulate the transitive dependencies of the 2 subgraphs ("main", and "test_package") - main_graph_nodes = set(main_nodes) - for n in main_nodes: - main_graph_nodes.update(t.node for t in n.transitive_deps.values()) - test_graph_nodes = set(test_package_nodes) - for n in test_package_nodes: - test_graph_nodes.update(t.node for t in n.transitive_deps.values()) - # Some dependencies in "test_package" might be "main" graph too, "main" prevails - test_package_only = test_graph_nodes.difference(main_graph_nodes) - for t in test_package_only: - t.test_package = True diff --git a/conans/client/graph/install_graph.py b/conans/client/graph/install_graph.py index 9fa5fef074a..f9e962e7613 100644 --- a/conans/client/graph/install_graph.py +++ b/conans/client/graph/install_graph.py @@ -301,9 +301,9 @@ def _raise_missing(self, missing): conanfile.output.warning(msg) missing_pkgs = "', '".join(list(sorted([str(pref.ref) for pref in missing_prefs]))) if self._is_test_package: - build_msg = "'conan test' tested packages must exist, and '--build' argument " \ - "is used only for the 'test_package' dependencies, not for the tested " \ - "dependencies" + build_msg = "This is a **test_package** missing binary. You can use --build (for " \ + "all dependencies) or --build-test (exclusive for 'test_package' " \ + "dependencies) to define what can be built from sources" else: if len(missing_prefs) >= 5: build_str = "--build=missing" diff --git a/conans/test/integration/command/create_test.py b/conans/test/integration/command/create_test.py index e82558b8dca..af1c470765e 100644 --- a/conans/test/integration/command/create_test.py +++ b/conans/test/integration/command/create_test.py @@ -739,3 +739,34 @@ def test_name_never(): c.save({"conanfile.py": GenConanfile("never", "0.1")}) c.run("create .") assert "never/0.1: Created package" in c.out + + +def test_create_both_host_build_require(): + c = TestClient() + c.save({"conanfile.py": GenConanfile("protobuf", "0.1").with_settings("build_type"), + "test_package/conanfile.py": GenConanfile().with_build_requires("protobuf/0.1") + .with_test("pass")}) + c.run("create . -s:b build_type=Release -s:h build_type=Debug", assert_error=True) + print(c.out) + # The main "host" Debug binary will be correctly build + c.assert_listed_binary({"protobuf/0.1": ("9e186f6d94c008b544af1569d1a6368d8339efc5", "Build")}) + # But test_package will fail because of the missing "tool_require" in Release + c.assert_listed_binary({"protobuf/0.1": ("efa83b160a55b033c4ea706ddb980cd708e3ba1b", "Missing")}, + build=True, test_package=True) + + c.run("remove * -c") # make sure that previous binary is removed + c.run("create . -s:b build_type=Release -s:h build_type=Debug --build-test=missing") + c.assert_listed_binary({"protobuf/0.1": ("9e186f6d94c008b544af1569d1a6368d8339efc5", "Build")}) + # it used to fail, now it works and builds the test_package "tools_requires" in Release + c.assert_listed_binary({"protobuf/0.1": ("9e186f6d94c008b544af1569d1a6368d8339efc5", "Cache")}, + test_package=True) + c.assert_listed_binary({"protobuf/0.1": ("efa83b160a55b033c4ea706ddb980cd708e3ba1b", "Build")}, + build=True, test_package=True) + + # we can be more explicit about the current package only with "missing:protobuf/*" + c.run("remove * -c") # make sure that previous binary is removed + c.run("create . -s:b build_type=Release -s:h build_type=Debug --build-test=missing:protobuf/*") + c.assert_listed_binary({"protobuf/0.1": ("9e186f6d94c008b544af1569d1a6368d8339efc5", "Build")}) + # it used to fail, now it works and builds the test_package "tools_requires" in Release + c.assert_listed_binary({"protobuf/0.1": ("efa83b160a55b033c4ea706ddb980cd708e3ba1b", "Build")}, + build=True, test_package=True) diff --git a/conans/test/integration/command/test_package_test.py b/conans/test/integration/command/test_package_test.py index 48a03bcfa5f..98e6915bf7c 100644 --- a/conans/test/integration/command/test_package_test.py +++ b/conans/test/integration/command/test_package_test.py @@ -132,11 +132,13 @@ def test_build_all(self): c.run("export tool") c.run("export dep") c.run("create pkg --build=*") + c.assert_listed_binary({"dep/0.1": ("da39a3ee5e6b4b0d3255bfef95601890afd80709", "Build"), "pkg/0.1": ("59205ba5b14b8f4ebc216a6c51a89553021e82c1", "Build")}) - c.assert_listed_require({"tool/0.1": "(tp) Cache"}, build=True, test_package=True) + c.assert_listed_require({"tool/0.1": "Cache"}, build=True, test_package=True) c.assert_listed_binary({"tool/0.1": ("da39a3ee5e6b4b0d3255bfef95601890afd80709", "Build")}, build=True, test_package=True) + # Note we do NOT rebuild the already built binaries c.assert_listed_binary({"dep/0.1": ("da39a3ee5e6b4b0d3255bfef95601890afd80709", "Cache"), "pkg/0.1": ("59205ba5b14b8f4ebc216a6c51a89553021e82c1", "Cache")}, test_package=True) @@ -153,13 +155,31 @@ def test_build_missing(self): c.run("create pkg --build=missing") c.assert_listed_binary({"dep/0.1": ("da39a3ee5e6b4b0d3255bfef95601890afd80709", "Cache"), "pkg/0.1": ("59205ba5b14b8f4ebc216a6c51a89553021e82c1", "Build")}) - c.assert_listed_require({"tool/0.1": "(tp) Cache"}, build=True, test_package=True) + c.assert_listed_require({"tool/0.1": "Cache"}, build=True, test_package=True) c.assert_listed_binary({"tool/0.1": ("da39a3ee5e6b4b0d3255bfef95601890afd80709", "Build")}, build=True, test_package=True) c.assert_listed_binary({"dep/0.1": ("da39a3ee5e6b4b0d3255bfef95601890afd80709", "Cache"), "pkg/0.1": ("59205ba5b14b8f4ebc216a6c51a89553021e82c1", "Cache")}, test_package=True) + def test_build_test_package_dep(self): + c = TestClient() + c.save({"dep/conanfile.py": GenConanfile("dep", "0.1"), + "pkg/conanfile.py": GenConanfile("pkg", "0.1"), + "pkg/test_package/conanfile.py": GenConanfile().with_requires("dep/0.1") + .with_test("pass")}) + c.run("export dep") + c.run('create pkg --build=missing --build-test=""', assert_error=True) + c.assert_listed_binary({"pkg/0.1": ("da39a3ee5e6b4b0d3255bfef95601890afd80709", "Build")}) + c.assert_listed_binary({"dep/0.1": ("da39a3ee5e6b4b0d3255bfef95601890afd80709", "Missing"), + "pkg/0.1": ("da39a3ee5e6b4b0d3255bfef95601890afd80709", "Cache")}, + test_package=True) + c.run("create pkg --build-test=missing") + c.assert_listed_binary({"pkg/0.1": ("da39a3ee5e6b4b0d3255bfef95601890afd80709", "Build")}) + c.assert_listed_binary({"dep/0.1": ("da39a3ee5e6b4b0d3255bfef95601890afd80709", "Build"), + "pkg/0.1": ("da39a3ee5e6b4b0d3255bfef95601890afd80709", "Cache")}, + test_package=True) + class ConanTestTest(unittest.TestCase): @@ -398,7 +418,6 @@ def test_package_missing_binary_msg(): c.run("export .") c.run("test test_package dep/0.1", assert_error=True) assert "ERROR: Missing binary: dep/0.1" in c.out - assert "'conan test' tested packages must exist" in c.out - c.run("test test_package dep/0.1 --build=dep/0.1", assert_error=True) - assert "ERROR: Missing binary: dep/0.1" in c.out - assert "'conan test' tested packages must exist" in c.out + assert "This is a **test_package** missing binary." in c.out + c.run("test test_package dep/0.1 --build=dep/0.1") + c.assert_listed_binary({"dep/0.1": (NO_SETTINGS_PACKAGE_ID, "Build")})