conan-io / conan

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

[feature] Add pre_generate/post_generate hooks #11094

Closed datalogics-kam closed 2 years ago

datalogics-kam commented 2 years ago

Some background

On macOS, we often have different versions of Xcode in flight, and we want to be able to find the Xcode that corresponds to the compiler.version in the settings. We search for all Xcode apps in /Applications and probe them for the clang version that matches compiler.version, and then set DEVELOPER_DIR to point at that Xcode.

I recently moved this from years-old tooling around Conan to the pre_build and pre_package hooks (and post hooks that revert the change). Works great for conan create, packages built in response to conan install, and for our projects that currently funnel all build work through conan build.

Why a new hook?

I'm working on a particular project, which will be our first production project to use CMakeToolchain, and I'm trying to make the workflow work like it would in Conan 2, especially since we can't use conan build to just build in Conan 2. In a development settings, the user would do conan install, followed by cmake and cmake --build.

To do that, in the generate() method, the conanfile places as much information as possible about compilers, flags, etc, into the conan_toolchain.cmake file, using the excellent support for modifying the blocks. That way, if a user loads the CMake project into an IDE, it just works™.

Along the way, we also want to get DEVELOPER_DIR into conanbuild.sh. It's easy enough to make another environment.

Currently, that generate() method has to duplicate the code that finds the right Xcode. If there were hooks for pre_generate() and post_generate(), the same code could supply DEVELOPER_DIR to generate(), build(), and package().

Thoughts? Are there alternatives I hadn't considered?

memsharded commented 2 years ago

Hi @datalogics-kam

In Conan 2.0 we are implementing in our internal code, and recommending the same pattern: recipes should never guess anything. All configuration should be provided already fully defined. In this case, it could be something like this:

The core principle is to decouple configuration from execution. The configuration for the location of Xcode needs to be done once, for the profile, and not potentially dozens of times, once inside each recipe. Recipes should get the data they need to operate, it is not the purpose of a recipe to guess which "ingredients" it needs to cook itself.

I would need to understand better the bit (and post hooks that revert the change). Which change needs to be reverted? Isn't just a thing of defining the folder location to the build system, or is something changed at the system level for this to work?

datalogics-kam commented 2 years ago

@memsharded Hey, thanks for getting back to me.

recipes should never guess anything. All configuration should be provided already fully defined.

I agree with that, and that's encapsulated in something like compiler.version=13.0.

Lets call it DEVELOPER_DIR.

DEVELOPER_DIR isn't just an arbitrary name; it's an environment variable that tells all the command-line development tools in macOS where to look for their actual implementation. If you set DEVELOPER_DIR, it's a per-process way of doing xcode-select to choose an Xcode installation to use. When the new Conan tools use xcrun to find the SDK or a tool, xcrun uses DEVELOPER_DIR to know where to look.

Similarly, CMake will use DEVELOPER_DIR to find the default tools and the SDK, etc. I think it calls xcrun as well.

You can encode your resolution of the conf for tools.xcode:dev_dir in the profile plugin.

Ok, I found #11077. I'm not entirely sure that helps, because then wouldn't the recipes have to know what to do with that conf variable?

Plus, I'd want to augment, not replace, the plugin that's there, and looking at the code in the PR, if define a plugin, I'd lose the functionality that's built-in.

The core principle is to decouple configuration from execution. The configuration for the location of Xcode needs to be done once, for the profile, and not potentially dozens of times, once inside each recipe.

I agree with not repeating this configuration, which is why I am doing it in something global like a hook.

The way I see it the configuration is the compiler and compiler.version. Finding the right DEVELOPER_DIR is part of the execution. Finding the correct compiler installation is already automated by Conan and CMake for Visual Studio builds (in the sense that I say the generator is "Visual Studio 16 2019" and I have no idea where the compiler files are), and what I want to do is have a better place to automate it for Xcode at our company, to make it easy on users.

The elegance of having a pre_validate() hook is this:

Which change needs to be reverted?

Going by the example in the doc, it's good to restore any environment variables set in a hook, so the post-hook sets DEVELOPER_DIR to its previous state (which could be unset).

datalogics-kam commented 2 years ago

Lacking a hook for validate(), I think for our internal projects, I'm going to use a python_requires to supply functionality that can be called from generate().

Now that I've got a complete new project using conan.tools, I see that what I'm really doing is making up for the fact that there is no way to say "Find the correct IDE installation for the given compiler settings" on macOS, analogous to conan.tools.microsoft.VCVars on Windows.

If there's interest, I could double-check with management and contribute code that would put the correct DEVELOPER_DIR into the buildenv.

memsharded commented 2 years ago

Yes, I think this would be interesting. It would be great to align on the approach, because I still think that this would find a better location in the profile_plugin than in a hook.

So I think it would look something like this in the profile_plugin (pseudo-code):

def profile_plugin(profile):
    settings = profile.settings
    buildenv= profile.buildenv
    compiler = settings.get("compiler")
    if compiler == "apple-clang" and not buildenv.get("DEVELOPER_DIR"):  # If not defined
          developer_dir = "do the magic here and define the dir"
          try:
              buildenv.define("DEVELOPER_DIR") = developer_dir
          except ConanException:
              pass

In this way:

datalogics-kam commented 2 years ago

I like the idea of getting this code somewhere where it shows in the configuration in the log. I found the profile_plugin code on the develop2 branch, so I got a little familiar with it.

VCVars() is finding Visual Studio without having to have anything defined. I mean, Conan doesn't really need vswhere, because a stock installation sets variables like VS2019INSTALLDIR and the bat file is at %VS2019INSTALLDIR%\VC\Auxiliary\Build. In fact, Conan 1.48 is producing files that look like:

> more .\conanvcvars.bat
@echo off
set "VSCMD_START_DIR=%CD%" && call "C:\Program Files (x86)\Microsoft Visual Studio\2019\Professional\VC/Auxiliary/Build/vcvarsall.bat"  amd64

...and I didn't have to set a conf variable.

This code execute just once, for the Conan command not once per-package, which seems better.

I had the code I put in my pre_build() hook use a functools.lru_cache for the searching part, which works great. It searches once, and then just plugs the environment for each requirement it builds. I guess the profile_plugin wouldn't need that...unless somehow more than one profile gets involved (build/host profiles?).

In any event, I got permission to post this code, so here's what the developer_dir hook does: https://gist.github.com/datalogics-kam/4be470e6603ca80903ca4f10d7edcf1b

maikelvdh commented 2 years ago

We would like to second this pre/post hook feature for generate for different use-case, namely we would like to produce a mapping file between the dependencies target name and include folder. This mapping file we could then use in turn with cpp-dependencies to validate if the CMakeLists.txt are correctly specifying all the direct target dependencies.

memsharded commented 2 years ago

Hooks added in https://github.com/conan-io/conan/pull/11593, for Conan 2.0-beta.2