Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[question] Hot to propagate tool_requires dependency to consumer packages. #15143

Open
1 task done
lmglmg opened this issue Nov 20, 2023 · 13 comments
Open
1 task done
Assignees

Comments

@lmglmg
Copy link

lmglmg commented Nov 20, 2023

What is your question?

I have a forked gtest conan package which uses a custom tool when defining ctest targets ( add_test will have this tool as a parameter ). This custom tool is built using conan. It has a package_type = 'application' set.

Using this tool with self.tool_requires('the_tool/[~1]@mycompany/main') works well and fine when building gtest itself, but the issue is that I also need to specify in the consumers of my forked gtest package this build dependency.

If I do not specify this tool_requires dependency, the binary cannot be found because it will not be located in the PATH.

How can I tell to my consumers that they should use the build_requires for this package also. This is an implementation detail in a way, and setting this tool_requires to all consumers is cumbersome.

I am also building for emscripten platform, where this tool is not build as an emscripten application, but must be available to ctest as an host application for the device on which the test is ran.

Is there an easy way to transitively propagate the tool_requires dependency?

Have you read the CONTRIBUTING guide?

  • I've read the CONTRIBUTING guide
@memsharded
Copy link
Member

Hi @lmglmg

Thanks for your questions.

tool_requires, by its nature, they are not transitive. They are private to the package that requires it. Having one package built with cmake doesn't mean that every package that uses it should use that cmake (even that version), one package building with meson or with autotools doesn't mean that every other package that uses it should be using that tool as well. Same with test_requires, they are not transitive, you can have one package using gtest for testing, and another package using catch2, that is perfectly valid. Propagating these requirements down the graph would be a disaster.

What is possible is to inject tool_requires from the profiles. You can define a [tool_requires] section in your profile, and those will apply to all packages in the graph (or only some if you specify some pattern). Please have a look: https://docs.conan.io/2/reference/config_files/profiles.html

@DoDoENT
Copy link
Contributor

DoDoENT commented Nov 20, 2023

@memsharded , I understand that tool_requires is not meant to be transitive.

However, if someone writes self.requires( 'pkg/version', build=True, run=True, visible=True, headers=False, libs=False ), I'd expect that whoever uses this package also gets transitive dependency to pkg/version, albeit in build context, which is what @lmglmg is trying to achieve. Unfortunately, this does not seem to work.

I also had a situation where I packaged a script into a conan package, and this script depends on a tool, and whoever uses the script also needs to use the tool, but in the build context. I ended up making all my dependencies define both tool and script in their requirements, but this is very ugly and not scalable to more than a handful of packages.

@memsharded
Copy link
Member

There are some ways to indicate that a tool_requires need something more, but not for building itself, using regular requires.

Example:

$ conan install --tool-requires=meson/[*] -g VirtualBuildEnv
$ meson --version
'meson' is not recognized as an internal or external command,
operable program or batch file.

$ ninja --version
'ninja' is not recognized as an internal or external command,
operable program or batch file.

$ conanbuild.bat
$ meson --version
Meson works correctly only with python 3.7+.
You have python 3.6.8 (tags/v3.6.8:3c6b436a57, Dec 24 2018, 00:16:47) [MSC v.1916 64 bit (AMD64)].
Please update your environment

$ninja --version
1.11.1

That is, the meson recipe is able to pull the ninja dependency for all consumers, automatically and transparently.
This is achieved with the meson recipe doing just:

    def requirements(self):
        ...
            self.requires("ninja/1.11.1")

The key is that when you are already in the "build" context, you don't need to tool_require something. If you intended the requirement to be propagated down to the user, then it is a regular requires, not a tool_requires with visible=True, it is a different concept.

@memsharded
Copy link
Member

This test also proves that a tool_require can be propagated if visible=True to the consumers, please have a look and let me know (I can submit a PR with the test, it would be worth to include it in the test suite, there are similar ones, I think they still cover the cases, but not as explicit as this one):

class TestToolRequiresTransitivity:
    def test_basic(self):
        c = TestClient()
        tool = textwrap.dedent(r"""
            import os
            from conan import ConanFile
            from conan.tools.files import chdir, save

            class Tool(ConanFile):
                name = "tool"
                version = "0.1"
                def package(self):
                    with chdir(self, self.package_folder):
                        echo = f"@echo off\necho MY-TOOL! {self.name}/{self.version}!!"
                        save(self, "bin/mytool.bat", echo)
                        save(self, "bin/mytool.sh", echo)
                        os.chmod("bin/mytool.sh", 0o777)
            """)
        pkg = textwrap.dedent("""
            from conan import ConanFile
            class Pkg(ConanFile):
                name = "pkg"
                version = "0.1"
                def requirements(self):
                    self.requires("tool/0.1", build=True, visible=True, headers=False, libs=False,
                                  run=True)
            """)
        app = textwrap.dedent("""
            import platform
            from conan import ConanFile
            class App(ConanFile):
                requires = "pkg/0.1"
                def build(self):
                    cmd = "mytool.bat" if platform.system() == "Windows" else "mytool.sh"
                    self.run(cmd)
            """)
        c.save({"tool/conanfile.py": tool,
                "pkg/conanfile.py": pkg,
                "app/conanfile.py": app})
        c.run("create tool")
        c.run("create pkg")
        c.run("build app")
        print(c.out)
        assert "MY-TOOL! tool/0.1!!" in c.out

@lmglmg
Copy link
Author

lmglmg commented Nov 21, 2023

This only works if I do not use a CMake generator.

If I use a CMake generator ( because I need to add a CMake test which uses the tool ) CMake configure step fails.

In generators/GTest-release-armv8-data.cmake the following line is added

list(APPEND gtest_FIND_DEPENDENCY_NAME my_tool)

However, no additional CMake files were generated and the configure step fails with the following error:

CMake Error at /Applications/CMake.app/Contents/share/cmake-3.27/Modules/CMakeFindDependencyMacro.cmake:76 (find_package):
  Could not find a package configuration file provided by "my_tool" with
  any of the following names:

    my_toolConfig.cmake
    my_tool-config.cmake

  Add the installation prefix of "my_tool" to CMAKE_PREFIX_PATH or set
  "my_tool_DIR" to a directory containing one of the above files.  If
  "my_tool" provides a separate development package or SDK, be sure it has
  been installed.
Call Stack (most recent call first):
  build/macos-armv8-apple-clang-15.0.0/generators/GTestConfig.cmake:24 (find_dependency)
  CMakeLists.txt:6 (find_package)

I'll try to make a minimal example with the CMake generator which demonstrates this issue.

@memsharded
Copy link
Member

Just in case it helps, CMakeDeps needs to explicitly require config.cmake files to be generated for the build context, it doesn't generate those files by default, please have a look at: https://docs.conan.io/2/reference/tools/cmake/cmakedeps.html#build-context-activated

@lmglmg
Copy link
Author

lmglmg commented Nov 21, 2023

I tried calling cmake.build_context_activated = ["my_tool"] and the my_tool-config.cmake is generated in the package itself (pkg in the example above), but this does not solve the issue for the consumer of the package (or the test package for that matter).

@memsharded
Copy link
Member

You need to define cmake.build_context_activated = ["my_tool"] in the downstream consumer, not in the package, that only applies to the recipe that uses it.

@DoDoENT
Copy link
Contributor

DoDoENT commented Nov 21, 2023

But doesn't that mean that this is needed in every downstream consumer? This appears to be tedious, especially if the tool is actually used internally by the script provided by pkg. Why should every single downstream consumer that uses pkg be aware of the fact that pkg internally uses my_tool? If it has to be, then it's better to specify tool_requires('my_tool/<version>') in every downstream package, but that's the exact thing that we want to avoid - updating dozens of downstream packages just because pkg introduced a new dependency which should be completely transparent for them.

@memsharded
Copy link
Member

But doesn't that mean that this is needed in every downstream consumer? This appears to be tedious, especially if the tool is actually used internally by the script provided by pkg. Why should every single downstream consumer that uses pkg be aware of the fact that pkg internally uses my_tool?

It shouldn't. And there are ways to do similar things as the example above proves. What doesn't work is trying to use that with a build system (CMake) find_package() to locate that tool, because that doesn't run in the scope of the tool-requires, but that runs in the consumer context, and as such would need information in the consumer context, not in the tool-require dependency.

It is possible that I am missing something, the best would be to materialize the use case in a test like the one I provided above in TestToolRequiresTransitivity, as you can see it is pretty self explanatory and self contained, it should be relatively doable to do something in the same line that illustrate the use case. If you could please share things in that form, that would really help understanding the things much faster.

However, it might be possible that we are trying to push tool_requires beyond their intention and design. tool_requires were never designed to propagate to consumers, and they were never designed to be able to work as "accumulators" of other tools to propagate them to consumers with 1 single tool_requires that will bring all the others automatically. While we have seen that it is possible, as we saw in the meson->ninja case, still the intended use case is to have one tool_require for each tool that you need, explicitly, at least as the standard way to use tool_requires and the meson->ninja thing is mostly something anecdotical from ConanCenter recipes.

@DoDoENT
Copy link
Contributor

DoDoENT commented Nov 23, 2023

@memsharded , I've modified your test so that it demonstrates the problem:

import textwrap
import platform
import pytest

from conans.test.utils.tools import TestClient


class TestTransitiveRequiresInBuildContext:

    @pytest.mark.skipif(platform.system() == 'Windows', reason="Needs bash support")
    def test_basic(self):
        c = TestClient()
        tool = textwrap.dedent(r"""
            import os
            from conan import ConanFile
            from conan.tools.files import chdir, save

            class Tool(ConanFile):
                name = "tool"
                version = "0.1"
                def package(self):
                    with chdir(self, self.package_folder):
                        echo = f"#!/bin/bash\n@echo off\necho MY-TOOL! {self.name}/{self.version}!!"
                        save(self, "bin/mytool", echo)
                        os.chmod("bin/mytool", 0o777)
        """)

        pkg = textwrap.dedent("""
            import textwrap

            from conan import ConanFile
            from conan.tools.files import chdir, save

            class Pkg(ConanFile):
                name = "pkg"
                version = "0.1"
                def requirements(self):
                    self.requires("tool/0.1", build=True, visible=True, headers=False, libs=False,
                                  run=True)

                def package(self):
                    with chdir(self, self.package_folder):
                        cmake_script = textwrap.dedent('''
                            include_guard( GLOBAL )

                            find_program( mytool_var "mytool" )
                            if ( NOT mytool_var )
                                message( FATAL_ERROR "Cannot find mytool" )
                            else()
                                execute_process( COMMAND ${mytool_var} )
                            endif()
                        ''')

                        save(self, "cmake_helper.cmake", cmake_script)

                def package_id(self):
                    self.info.clear()

                def package_info(self):
                    self.cpp_info.libdirs = []

                    self.cpp_info.set_property('cmake_build_modules', [
                        'cmake_helper.cmake',
                    ])
        """)

        app_cmakelists = textwrap.dedent("""
            cmake_minimum_required( VERSION 3.25 )
            project( testApp )
            find_package( pkg REQUIRED )
        """)

        app = textwrap.dedent("""
            import textwrap

            from conan import ConanFile
            from conan.tools.cmake import cmake_layout, CMake, CMakeDeps, CMakeToolchain

            class Pkg(ConanFile):
                requires = 'pkg/0.1'
                settings = 'os', 'arch', 'build_type', 'compiler'

                def layout(self):
                    cmake_layout(self)

                def generate(self):
                    tc = CMakeToolchain(self)
                    tc.generate()

                    deps = CMakeDeps(self)
                    deps.generate()

                def build(self):
                    cmake = CMake(self)
                    cmake.configure()
                    cmake.build()
        """)

        c.save({
            "tool/conanfile.py": tool,
            "pkg/conanfile.py": pkg,
            "app/CMakeLists.txt": app_cmakelists,
            "app/conanfile.py": app,
        })

        c.run("create tool")
        c.run("create pkg")
        c.run("build app")
        print(c.out)
        assert "MY-TOOL! tool/0.1!!" in c.out

As you can see, nobody never uses find_package( tool ), yet the error happens.

@memsharded
Copy link
Member

This happens because the script is reused from a requires, not from a tool_requires.
Adapting the test to this will pass:

    def test_basic(self):
        c = TestClient()
        tool = textwrap.dedent(r"""
            import os
            from conan import ConanFile
            from conan.tools.files import chdir, save

            class Tool(ConanFile):
                name = "tool"
                version = "0.1"
                def package(self):
                    with chdir(self, self.package_folder):
                        echo = f"@echo off\necho MY-TOOL! {self.name}/{self.version}!!"
                        save(self, "bin/mytool.bat", echo)
                        save(self, "bin/mytool.sh", echo)
                        os.chmod("bin/mytool.sh", 0o777)
        """)

        pkg = textwrap.dedent("""
            import textwrap

            from conan import ConanFile
            from conan.tools.files import chdir, save

            class Pkg(ConanFile):
                name = "pkg"
                version = "0.1"
                def requirements(self):
                    self.requires("tool/0.1", build=True, visible=True, headers=False, libs=False,
                                  run=True)

                def package(self):
                    with chdir(self, self.package_folder):
                        cmake_script = textwrap.dedent('''
                            include_guard( GLOBAL )

                            message(STATUS "MYCMAKE SCRIPT!!!!")
                            find_program( mytool_var "mytool.bat" )
                            if ( NOT mytool_var )
                                message( FATAL_ERROR "Cannot find mytool" )
                            else()
                                execute_process( COMMAND ${mytool_var} )
                            endif()
                        ''')

                        save(self, "cmake_helper.cmake", cmake_script)

                def package_id(self):
                    self.info.clear()

                def package_info(self):
                    self.cpp_info.libdirs = []

                    self.cpp_info.set_property('cmake_build_modules', [
                        'cmake_helper.cmake',
                    ])
        """)

        app_cmakelists = textwrap.dedent("""
            cmake_minimum_required( VERSION 3.25 )
            project( testApp )
            find_package( pkg REQUIRED )
        """)

        app = textwrap.dedent("""
            import textwrap

            from conan import ConanFile
            from conan.tools.cmake import cmake_layout, CMake, CMakeDeps, CMakeToolchain

            class Pkg(ConanFile):
                tool_requires = 'pkg/0.1'
                settings = 'os', 'arch', 'build_type', 'compiler'

                def layout(self):
                    cmake_layout(self)

                def generate(self):
                    tc = CMakeToolchain(self)
                    tc.generate()

                    deps = CMakeDeps(self)
                    deps.build_context_activated=["pkg"]
                    deps.build_context_build_modules=["pkg"]
                    deps.generate()

                def build(self):
                    cmake = CMake(self)
                    cmake.configure()
                    cmake.build()
        """)

        c.save({
            "tool/conanfile.py": tool,
            "pkg/conanfile.py": pkg,
            "app/CMakeLists.txt": app_cmakelists,
            "app/conanfile.py": app,
        })

        c.run("create tool")
        c.run("create pkg")
        c.run("build app")
        print(c.out)
        assert "MY-TOOL! tool/0.1!!" in c.out

Otherwise, no, it is not possible to have app-(normal-requires)->pkg_with_script-(tool_require+visible)->exe_tool, and have the information propagated via CMake find_package(). As proved above, it is possible to have it, and can actually be very easily leveraged directly in Conan recipes, for example, calling the transitive "tool" executable works. But the CMake find_package() system is not that hackable and CMakeDeps has already a huge ton of complexity to try to add more complicated scenarios. It is simply too convoluted, as commented above, the tool_requires system is not designed to be propagated downstream, coupling it to the CMake find_xxx which cannot support build/host context, is simply too much.

Note that your defined requires approach, without changing to tool_requires also work if we do not use the find_package(), but plain old include():

pkg = textwrap.dedent("""
            import textwrap

            from conan import ConanFile
            from conan.tools.files import chdir, save

            class Pkg(ConanFile):
                name = "pkg"
                version = "0.1"
                def requirements(self):
                    self.requires("tool/0.1", build=True, visible=True, headers=False, libs=False,
                                  run=True)

                def package(self):
                    with chdir(self, self.package_folder):
                        cmake_script = textwrap.dedent('''
                            include_guard( GLOBAL )
                            message(STATUS "INCLUDED CMAKE SXEIPT!!!!!!")
                            execute_process( COMMAND mytool.bat)
                        ''')

                        save(self, "cmake_helper.cmake", cmake_script)

                def package_id(self):
                    self.info.clear()

                def package_info(self):
                    self.cpp_info.builddirs = ["."]
                    self.cpp_info.libdirs = []
        """)

        app_cmakelists = textwrap.dedent("""
            cmake_minimum_required( VERSION 3.25 )
            project( testApp )
            include("cmake_helper")
        """)

It is the find_package() crossing host->build context that becomes unfeasible in practice.

@DoDoENT
Copy link
Contributor

DoDoENT commented Nov 23, 2023

Hi @memsharded ,

unfortunately, this

                    deps.build_context_activated=["pkg"]
                    deps.build_context_build_modules=["pkg"]

does not work for us because it requires that every single package that uses gtest will need to have this in its generate method. It could be possibly done via python_requires, however it's still ugly, as it requires two different dependencies to a single thing.

As @lmglmg already described, our use case is the following:

  • there is a gtest package that contains both the Google test framework (compiled to the host architecture), and it also contains a cmake build module gtest.cmake that provides helper functions for registering c++ tests into ctest. This script also manages resource deployment, as in the case of c++ tests that need to run on iOS, Android, or Emscripten, the resources for testing purposes need to be bundled into a test somehow.
  • specifically for Emscripten, this gtest.cmake script will configure ctest in a way to launch every single test like this: myserver test-target.html --test-parameters
  • the myserver part here is the custom server, similar to emscripten's emrun, that launches the HTTP server, launches the chrome, and attaches to the chrome debug console to capture console log. The myserver, of course, needs to be used in the build context, which brings us back to the original problem:
  • our gtest needs to have a dependency on myserver in build context only if self.settings.os == 'Emscripten' and this needs to be propagated downstream so that any package that uses gtest as its test_requires or normal requires (i.e. packages that extend the functionality of gtest for certain special use cases) can automatically use myserver in build context

Our current workaround that @lmglmg created is to have a bootstrap part in cmake code that literally invokes execute_process( COMMAND conan install --requires myserver/[~1] --profile <build-profile-inferred-using-different-helper-script> --format json OUTPUT_VARIABLE install_output_var ) and then extract the path of the tool from install_output_var and some JSON postprocessing.

This works, but:

  • it's ugly, a terribly ugly
  • it's essentially a "hidden dependency", not visible in conan.lock and or using "normal" conan methodology
  • it would be much nicer if this dependency could be modeled via conan as I described in my test case above

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants