conan-io / conan

Conan - The open-source C and C++ package manager
https://conan.io
MIT License
8.14k stars 970 forks source link

[feature] Better error message when profile defines a tool_require loop by mistake #16714

Open AbrilRBS opened 1 month ago

AbrilRBS commented 1 month ago

What is your suggestion?

We've seen some issues lately with users finding graph loops because of the usage of the [tool_requires] section in the build profile, affecting the same libraries they are trying to automatically include

ie

[tool_requires]
*: foo/1.0

In a build profile would generate a loop, with the solution being to, in this case, not include such library in the include pattern

[tool_requires]
!foo/*: foo/1.0

We need a better error message for these cases, as the users are most of the time not aware where the issue is coming from (we might also need to add a note in the documentation about this too)

Something maybe like this?

diff --git a/conan/api/subapi/profiles.py b/conan/api/subapi/profiles.py
index 62d50fd65..036b2b9c0 100644
--- a/conan/api/subapi/profiles.py
+++ b/conan/api/subapi/profiles.py
@@ -64,6 +64,17 @@ class ProfilesAPI:
         profile_build = self._get_profile(build_profiles, args.settings_build, args.options_build,
                                           args.conf_build, cwd, cache_settings,
                                           profile_plugin, global_conf)
+
+        for pattern, refs in profile_build.tool_requires.items():
+            for ref in refs:
+                if ref.matches(pattern, False):
+                    raise ConanException(f"Tool requirement '{ref}' will create a loop "
+                                         f"declared in the build profile [tool_requires] section "
+                                         f"as it matches its own pattern '{pattern}', "
+                                         f"which will create a loop.\n"
+                                         f"To avoid this, make sure that the pattern does not include '{ref}' "
+                                         f"(!{ref}: {ref} would for example apply to all packages, BUT '{ref}' itself)")
+
         profile_host = self._get_profile(host_profiles, args.settings_host, args.options_host, args.conf_host,
                                          cwd, cache_settings, profile_plugin, global_conf)
         return profile_host, profile_build

Have you read the CONTRIBUTING guide?

memsharded commented 1 month ago

I think there might be some bootstraping situations where there might be a tool-require to a previous version. If the error is being correctly raised as a "loop in the graph", maybe we want to do the extra checks and error messages once the loop is detected and not in the build-profile? I am trying to avoid possible false positives and missing true positives (like if the loop happens because more than 1 tool-require).

memsharded commented 2 weeks ago

I am not sure about how to reproduce this, following test passes:

def test_loop_build_profile():
    c = TestClient(light=True)
    c.save({"fmt/conanfile.py": GenConanfile("fmt", "1.0"),
            "tool/conanfile.py": GenConanfile("tool", "1.0"),
            "profile": "[settings]\nos=Linux\n[tool_requires]\n*:fmt/1.0"})
    c.run("export fmt")
    c.run("export tool")
    c.run("install --requires=fmt/1.0 -pr:b=profile -b=missing")
    print(c.out)

Conan 2 already contains some code to prevent depending on itself:

        for pattern, tool_requires in profile.tool_requires.items():
            if ref_matches(ref, pattern, is_consumer=conanfile._conan_is_consumer):
                for tool_require in tool_requires:  # Do the override
                    if str(tool_require) == str(ref):  # FIXME: Ugly str comparison
                        continue  # avoid self-loop of build-requires in build context