search

conan-io / conan

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

[bug] Conan returns incorrect path for Apple Framework when multiple components have the same framework name #10140

Open T1T4N opened 2 years ago

T1T4N commented 2 years ago

Environment Details (include every applicable attribute)

Description

Since CMake does not yet natively support XCFrameworks I'm looking for alternative solutions to integrating Apple's new format into our existing codebase. After some research, I determined that Conan Components together with the cmake_find_package generator are a perfect fit for the task at hand. I will not go in detail what XCFrameworks are, but for anyone interested, the following blog post has a lot of useful info: PSPDFKit - Supporting XCFrameworks. An XCFramework is basically a folder (+ some metadata) containing different architecture builds (slices) of a framework. It has the following structure:

ExampleFramework.xcframework
├── Info.plist
├── ios-arm64
│   └── ExampleFramework.framework
├── ios-x86_64-simulator
│   └── ExampleFramework.framework
├── tvos-arm64
│   └── ExampleFramework.framework
├── tvos-arm64_x86_64-simulator
│   └── ExampleFramework.framework
├── watchos-arm64_32_armv7k
│   └── ExampleFramework.framework
└── watchos-arm64_x86_64-simulator
    └── ExampleFramework.framework

The conanfile.py for Example Framework looks like this (simplified):

def build(self):
    # Build all slices
    for platform in self._supported_platforms:
        archive_cmd = f"xcrun xcodebuild archive -quiet -project ExampleProject.xcodeproj -scheme 'ExampleFramework' -configuration {self.settings.build_type} -archivePath '{platform}.xcarchive'"
        self.run(archive_cmd)

    # Combine all slices
    create_cmd = f"xcrun xcodebuild -create-xcframework <args> -output '{self.name}.xcframework'"
    self.run(create_cmd)

# "Component" package
def package_info(self):
    self.cpp_info.name = self.name #"ExampleFramework"

    for platform in self._supported_platforms:
        arch_dir = self._arch_for_platform[platform]
        framework_path = os.path.join(self.package_folder, f"{self.name}.xcframework", arch_dir)

        self.cpp_info.components[platform].names["cmake_find_package"] = platform
        self.cpp_info.components[platform].frameworkdirs = [framework_path]
        self.cpp_info.components[platform].frameworks = [self.name]

For each supported architecture of the XCFramework, a component is defined with the name of the platform, e.g: iphoneos, iphonesimulator, macosx (names used by Xcode). NOTE: The framework name specified in self.cpp_info.components[platform].frameworks is identical for every component (self.name) and this can't be trivially renamed (or symlinked) to e.g. ExampleFramework_macosx.framework.

The test_package is a bare minimum CMake project integrating this, with the following CMakeLists.txt:

cmake_minimum_required(VERSION 3.1)
project(PackageTest)

include(CMakePrintHelpers)
set(CMAKE_C_FLAGS "-fmodules")
set(CMAKE_VERBOSE_MAKEFILE ON)

# cmake_find_package generator required
find_package(ExampleFramework REQUIRED COMPONENTS macosx)

add_executable(example example.m)
target_link_libraries(example ExampleFramework::macosx)

Contrary to expectations, running conan create yields the following link error:

/Applications/CMake-3.21.2.app/Contents/bin/cmake -E cmake_link_script CMakeFiles/example.dir/link.txt --verbose=1
/Applications/Xcode-13.1.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/cc -fmodules -O3 -DNDEBUG -arch x86_64 -isysroot /Applications/Xcode-13.1.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX12.0.sdk -mmacosx-version-min=10.14 -Wl,-search_paths_first -Wl,-headerpad_max_install_names CMakeFiles/example.dir/example.m.o -o example -F/Users/user/.conan/data/ExampleFramework/0.1.0/user/testing/package/<package-id>/ExampleFramework.xcframework/ios-arm64  -Wl,-rpath,/Users/user/.conan/data/ExampleFramework/0.1.0/user/testing/package/<package-id>/ExampleFramework.xcframework/ios-arm64 -framework ExampleFramework
ld: warning: ignoring file /Users/user/.conan/data/ExampleFramework/0.1.0/user/testing/package/<package-id>/ExampleFramework.xcframework/ios-arm64/ExampleFramework.framework/ExampleFramework, building for macOS-x86_64 but attempting to link with file built for iOS-arm64
Undefined symbols for architecture x86_64:
  "_OBJC_CLASS_$_TestClass", referenced from:
      objc-class-ref in example.m.o
ld: symbol(s) not found for architecture x86_64

It is very interesting to note that although we specified the macosx component, what FindExampleFramework.cmake configured was actually the path leading to the iphoneos framework in the subfolder ios-arm64.

After a bit of debugging, by inspecting the variables defined by the custom FindExampleFramework.cmake, we observe the following:

and from this, the target property is also incorrect: ❌ ExampleFramework::macosx.INTERFACE_LINK_LIBRARIES = "/Users/user/.conan/data/ExampleFramework/0.1.0/user/testing/package/<package-id>/ExampleFramework.xcframework/ios-arm64/ExampleFramework.framework;$<$<STREQUAL:$<TARGET_PROPERTY:TYPE>,SHARED_LIBRARY>:>;$<$<STREQUAL:$<TARGET_PROPERTY:TYPE>,MODULE_LIBRARY>:>;$<$<STREQUAL:$<TARGET_PROPERTY:TYPE>,EXECUTABLE>:>"

Solution

It seems that Conan's cmake_find_package generator cannot deal with different components declaring the same framework name. I have traced the behavior to the macro conan_find_apple_frameworks in conans/client/generators/cmake_find_package_common.py. Specifically, it checks if(CONAN_FRAMEWORK_${_FRAMEWORK}_FOUND) but this global variable doesn't take the different FRAMEWORKS_DIRS given to the macro, which can be a disjoint-sets between different components.

The easiest solution to this is to add an unset(CONAN_FRAMEWORK_${_FRAMEWORK}_FOUND CACHE) immediately before the find_library call. Personally, I think this is acceptable, as the macro throws a FATAL_ERROR if a framework isn't found.

From CMake's find_library documentation:

If the library is found the result is stored in the variable and the search will not be repeated unless the variable is cleared.

I verified that this works as expected by monkey-patching the macro in test_package/conanfile.py:

import textwrap
from conans.client.generators.cmake_find_package_common import CMakeFindPackageCommonMacros
CMakeFindPackageCommonMacros.apple_frameworks_macro = textwrap.dedent("""
    macro(conan_find_apple_frameworks FRAMEWORKS_FOUND FRAMEWORKS FRAMEWORKS_DIRS)
        if(APPLE)
            foreach(_FRAMEWORK ${FRAMEWORKS})
                # BUGFIX: Different components with the same framework name should still attempt to find the library
                unset(CONAN_FRAMEWORK_${_FRAMEWORK}_FOUND CACHE)

                # https://cmake.org/pipermail/cmake-developers/2017-August/030199.html
                find_library(CONAN_FRAMEWORK_${_FRAMEWORK}_FOUND NAME ${_FRAMEWORK} PATHS ${FRAMEWORKS_DIRS} CMAKE_FIND_ROOT_PATH_BOTH)
                if(CONAN_FRAMEWORK_${_FRAMEWORK}_FOUND)
                    list(APPEND ${FRAMEWORKS_FOUND} ${CONAN_FRAMEWORK_${_FRAMEWORK}_FOUND})
                else()
                    message(FATAL_ERROR "Framework library ${_FRAMEWORK} not found in paths: ${FRAMEWORKS_DIRS}")
                endif()
            endforeach()
        endif()
    endmacro()
""")

A more elaborate solution would require defining separate CONAN_FRAMEWORK_ variables for each component, and e.g. passing the component name to this function as a suffix.

In any case, I cannot see a reason why this use-case shouldn't be supported (same framework name in different components) and it would certainly make the lives of all Apple devs using Conan much easier in the future.

I'll be very happy to provide more information and code if necessary, as well as answer any questions that might arise.

memsharded commented 2 years ago

Hi @T1T4N

I don't know if you have checked the new integrations in https://docs.conan.io/en/latest/reference/conanfile/tools/cmake.html. Those are the ones that will be valid in Conan 2.0. Our current efforts are on these. From Conan 1.43, they allow defining the property cpp_info.set_property("cmake_target_name", <global target name>).

Maybe it would be worth trying with the new integrations, as they are still experimental, much easier to be fixed if necessary, and in any case it will be necessary to migrate to them before 2.0 (we are already in 2.0.0-alpha1)?

T1T4N commented 2 years ago

Hello @memsharded, Thanks for sharing the integrations documentation. I can see here that cpp_info.components is mentioned. Does that mean that the experimental Components feature is going to be present in Conan 2.0? Additionally, is there any way to define components info using set_property?

Regarding my detailed bug description above, I wanted to give the full context to prevent unnecessary questions, but if it is too broad, please focus on the Solution section detailing the issue in conan_find_apple_frameworks.

As you mentioned that you've focused all efforts on 2.0, I understand that you might be a bit time-constrained. Would you prefer if I open a PR with the bugfix I detailed above?

franramirez688 commented 1 year ago

Hi @T1T4N

We keep figuring out the issue. What I wonder is if it could solve the problem something like this:

    def package_info(self):
        self.cpp_info.name = self.name  # "ExampleFramework"

        # Getting the platform name depending on the OS (and ARCH perhaps???)
        platform = self._supported_platforms[str(self.settings.os)]
        arch_dir = self._arch_for_platform[platform]
        framework_path = os.path.join(self.package_folder, f"{self.name}.xcframework", arch_dir)

       # Create one unique component
        self.cpp_info.components["ComponentPlatform"].names["cmake_find_package"] = platform
        self.cpp_info.components["ComponentPlatform"].frameworkdirs = [framework_path]
        self.cpp_info.components["ComponentPlatform"].frameworks = [self.name]

I'm saying this because if you have the same Framework name but it depends on the OS/ARCH, why don't you create one component depending directly on your settings? Then, when you consume it in your recipe you'll be sure you're using the correct Framework by pointing out its folder.

T1T4N commented 1 year ago

Hi @franramirez688,

Thanks for replying on the original issue! Your suggestion would theoretically work, as it avoids the issue (not having 2 components with the same framework name).

It's a fact that Apple's design approach to XCFrameworks is completely orthogonal to Conan's design, and your suggestion is a good solution when different platforms are in question (e.g. not mixing macOS + iOS), but from a usability perspective it is not ideal in the (most common) case of iOS + iOS Simulator, as Xcode parses the metadata in the XCFramework for proper linking of the flavor, which can be dynamically changed in the IDE.

The ideal solution is to get conan_find_apple_frameworks fixed as IMO it is a purely logical bug.

My current "temporary solution" to the problem is implementing #12088 directly in our conanfile.py.