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

[bug] FMT missing builddirs for the component #11870

Closed
Richard-W opened this issue Aug 10, 2022 · 11 comments · Fixed by #11883 or conan-io/conan-center-index#12411
Closed

[bug] FMT missing builddirs for the component #11870

Richard-W opened this issue Aug 10, 2022 · 11 comments · Fixed by #11883 or conan-io/conan-center-index#12411
Assignees
Milestone

Comments

@Richard-W
Copy link

Environment Details

  • os: Ubuntu 22.04
  • gcc version: 11.2
  • conan version: 1.51.1
  • python version: 3.10.4

Steps to reproduce

In an empty directory create a conanfile.txt with the following content:

[requires]
fmt/8.1.1

[generators]
cmake_paths

Issue conan install . and inspect conan_paths.cmake:

set(CONAN_FMT_ROOT "<path_to_conan_cache>/fmt/8.1.1/_/_/package/3bb43d390932310d9ef00e9986e41f02509d79e8")
set(CMAKE_MODULE_PATH  ${CMAKE_MODULE_PATH} ${CMAKE_CURRENT_LIST_DIR})
set(CMAKE_PREFIX_PATH  ${CMAKE_PREFIX_PATH} ${CMAKE_CURRENT_LIST_DIR})

Expectation: CMAKE_MODULE_PATH and CMAKE_PREFIX_PATH contain the value of CONAN_FMT_ROOT
Actual: Neither variable contains the value of CONAN_FMT_ROOT.

Possible cause

The cmake_paths generator generates the CMAKE_* variables by concatenating the build_paths arrays of the dependencies. The default value for this path (which would be the correct value here) is overriden in _call_package_info in conans/client/installer.py if hasattr(conanfile, "layout") is true.

The correct value is set in conan/client/installer.py:659:

conanfile.cpp_info = CppInfo(conanfile.name, package_folder)

But it is later overridden in conan/client/installer.py:679:

if hasattr(conanfile, "layout"):
    # Old cpp info without defaults (the defaults are in the new one)
    conanfile.cpp_info = CppInfo(conanfile.name, package_folder,
                                 default_values=CppInfoDefaultValues())
@lasote
Copy link
Contributor

lasote commented Aug 11, 2022

I've been able to reproduce and debug, I'll provide a fix for the next release.

@lasote lasote added bug and removed bug labels Aug 11, 2022
@lasote
Copy link
Contributor

lasote commented Aug 11, 2022

I realized that is not a bug in Conan, this cames from the recipe of fmt, the default cpp_info for the components in Conan only include by default the following values:

includedirs = ["include"]
libdirs = ["lib"]
bindirs = ["bin"]

So, In case the recipe has some build script, it has to declare the builddirs in the component accordingly:

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

I'm transferring the issue to the conan-center-index repository.

Thanks

@lasote lasote transferred this issue from conan-io/conan Aug 11, 2022
@lasote lasote changed the title [bug] Using cmake_layout breaks cmake_paths generator [bug] FMT missing builddirs for the component Aug 11, 2022
@SpaceIm
Copy link
Contributor

SpaceIm commented Aug 12, 2022

@lasote, I don't understand.
fmt doesn't contain build scripts. It's a conan bug of layout/cmake_paths interaction, not fmt recipe bug. Do you expect all "conan v2" recipes with components to manually set self.components[foo].builddirs = [""] in all components to satisfy the legacy cmake_paths generator?

@lasote
Copy link
Contributor

lasote commented Aug 12, 2022

@SpaceIm, not at all, maybe Richard-W has more information about his usage of fmt and why he needs the CMAKE_MODULE_PATH and CMAKE_PREFIX_PATH containing the CONAN_FMT_ROOT. I assumed there was some scripts there, I didn't check, sorry.

@SpaceIm
Copy link
Contributor

SpaceIm commented Aug 12, 2022

According to https://docs.conan.io/en/latest/reference/generators/cmake_paths.html, it looks like a bug. In the file generated by cmake_paths, CMAKE_MODULE_PATH & CMAKE_PREFIX_PATH should contain root package folders of dependencies.

@Richard-W
Copy link
Author

Richard-W commented Aug 12, 2022

@lasote Context is a project that relies on CMAKE_MODULE_PATH CMAKE_PREFIX_PATH to find its dependencies. This cannot be easily changed since the project also needs to build in somewhat exotic environments where conan is unavailable. Using cmake_paths is a good way to create conan packages of that project without having to touch its build system.

@SpaceIm
Copy link
Contributor

SpaceIm commented Aug 12, 2022

@lasote Context is a project that relies on CMAKE_MODULE_PATH to find its dependencies.

CMAKE_PREFIX_PATH I guess. root package folder of fmt in CMAKE_MODULE_PATH is useless since there is no cmake module file. CMAKE_MODULE_PATH is not used at all in commands like find_file, find_program, find_library etc.

@lasote
Copy link
Contributor

lasote commented Aug 16, 2022

Transfering back to conan.
I have to discuss with the team what to do with this issue. The documentation at here was wrong, cmake_paths never used the root of the packages but the builddirs, and it was the root folder the default for builddirs until, when layout was declared, we moved in this direction.

So now I have to discuss if we should "fix" the cmake_paths generator (not maintained anymore) by adding the root folder always. Tagging it as look_into.

@lasote lasote transferred this issue from conan-io/conan-center-index Aug 16, 2022
@lasote lasote added this to the 1.52 milestone Aug 16, 2022
@Richard-W
Copy link
Author

IMHO the documentation isn't wrong. What was documented aligned with our expectations and observations even if the implementation had different (but then compatible) semantics. The change in behavior actually breaks reasonable expectations that recipe authors may have after reading the docs.

As a side-note: There is no clear indication in the docs that the cmake_paths generator is not maintained anymore. After reading the note on https://docs.conan.io/en/latest/integrations/build_system/cmake.html I expected a deprecation in the future but only after conan.tools.cmake became stable.

@lasote
Copy link
Contributor

lasote commented Aug 16, 2022

IMHO the documentation isn't wrong. What was documented aligned with our expectations and observations even if the implementation had different (but then compatible) semantics. The change in behavior actually breaks reasonable expectations that recipe authors may have after reading the docs.

That is True but the code of the cmake_paths never managed the root folder of the packages. That is a collateral effect of having a builddir declared in the dependencies pointing to ..

As a side-note: There is no clear indication in the docs that the cmake_paths generator is not maintained anymore. After reading the note on https://docs.conan.io/en/latest/integrations/build_system/cmake.html I expected a deprecation in the future but only after conan.tools.cmake became stable.

That is also True, but FYI we stopped long ago to develop Conan 1.X since we are working with 2.X beta.

Anyway I've talked with the team and we will "fix" the cmake_paths generator to add the root folders of the dependencies always because we understand this is breaking just because the dependencies are introducing the layout() method.

@lasote
Copy link
Contributor

lasote commented Aug 16, 2022

This has been fixed at #11883 and will be released at 1.51.3

@lasote lasote closed this as completed Aug 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment