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.

Sign up for GitHub

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

Add Corrade 2020.06 #2109

Closed
wants to merge 12 commits into from
Closed

Add Corrade 2020.06 #2109

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
629576c
Add Corrade 2020.06
helmesjo Jul 2, 2020
c8b558c
Don't build deprecated by default.
helmesjo Aug 12, 2020
66b2f03
Correctly set cmake defines used by corrade (previous way use invalid…
helmesjo Aug 12, 2020
4425a5a
Include corrade cmake scripts (that in turn read configuration.h to s…
helmesjo Aug 12, 2020
4499dc4
Setup PATH to correctly handle rpaths (dynamic linking) on windows, l…
helmesjo Aug 12, 2020
65b4d86
Changed CMake version to be minimum v3.1
helmesjo Aug 12, 2020
eea7115
Don't manually handle/setup rpaths.
helmesjo Aug 13, 2020
02b9386
Remove printing of CMake defines.
helmesjo Aug 13, 2020
a6a95a8
Don't manually handle conversion of bool from python -> cmake
helmesjo Aug 18, 2020
90b456b
Merge branch 'master' into corrade_2020.06
danimtb Jan 28, 2021
603d46b
Merge branch 'master' of github.com:conan-io/conan-center-index into …
danimtb Jan 29, 2021
764671c
remove duplicated entries
danimtb Jan 29, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion recipes/corrade/all/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
cmake_minimum_required(VERSION 2.8.12)
cmake_minimum_required(VERSION 3.1)
project(cmake_wrapper)

if(EXISTS "${CMAKE_BINARY_DIR}/conanbuildinfo.cmake")
Expand Down
52 changes: 29 additions & 23 deletions recipes/corrade/all/conanfile.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
from conans.errors import ConanInvalidConfiguration
import os


class CorradeConan(ConanFile):
name = "corrade"
description = "Corrade is a multiplatform utility library written in C++11/C++14."
Expand All @@ -25,16 +24,18 @@ class CorradeConan(ConanFile):
"with_pluginmanager": [True, False],
"with_testsuite": [True, False],
"with_utility": [True, False],
"with_rc": [True, False],
}
default_options = {
"shared": False,
"fPIC": True,
"build_deprecated": True,
"build_deprecated": False,
uilianries marked this conversation as resolved.
Show resolved Hide resolved
"with_interconnect": True,
"with_main": True,
"with_pluginmanager": True,
"with_testsuite": True,
"with_utility": True,
"with_rc": True,
}

_source_subfolder = "source_subfolder"
Expand All @@ -58,24 +59,26 @@ def source(self):
def _configure_cmake(self):
if not self._cmake:
self._cmake = CMake(self)
self._cmake.definitions["BUILD_STATIC"] = not self.options.shared
self._cmake.definitions["BUILD_DEPRECARED"] = self.options["build_deprecated"]
self._cmake.definitions["WITH_INTERCONNECT"] = self.options["with_interconnect"]
self._cmake.definitions["WITH_MAIN"] = self.options["with_main"]
self._cmake.definitions["WITH_PLUGINMANAGER"] = self.options["with_pluginmanager"]
self._cmake.definitions["WITH_TESTSUITE"] = self.options["with_testsuite"]
self._cmake.definitions["WITH_UTILITY"] = self.options["with_utility"]
self._cmake.definitions["WITH_RC"] = "ON"
def add_cmake_option(option, value):
var_name = "{}".format(option).upper()
var_value = bool(value)
self._cmake.definitions[var_name] = var_value

for option, value in self.options.items():
add_cmake_option(option, value)

add_cmake_option("BUILD_STATIC", not self.options.shared)
add_cmake_option("BUILD_STATIC_PIC", not self.options.shared and self.options.get_safe("fPIC") == True)
Comment on lines +70 to +71
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
add_cmake_option("BUILD_STATIC", not self.options.shared)
add_cmake_option("BUILD_STATIC_PIC", not self.options.shared and self.options.get_safe("fPIC") == True)
self._cmake.definitions["BUILD_STATIC"] = not self.options.shared
self._cmake.definitions["BUILD_STATIC_PIC"] not self.options.shared and self.options.get_safe("fPIC", False)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not reuse the add_cmake_option(...) method?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

keep it simple and readable like other recipes. you don't need an extra method for a simple logic.


if self.settings.compiler == "Visual Studio":
self._cmake.definitions["MSVC2015_COMPATIBILITY"] = "ON" if self.settings.compiler.version == "14" else "OFF"
self._cmake.definitions["MSVC2017_COMPATIBILITY"] = "ON" if self.settings.compiler.version == "15" else "OFF"
self._cmake.definitions["MSVC2019_COMPATIBILITY"] = "ON" if self.settings.compiler.version == "16" else "OFF"
Comment on lines +74 to +76
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
self._cmake.definitions["MSVC2015_COMPATIBILITY"] = "ON" if self.settings.compiler.version == "14" else "OFF"
self._cmake.definitions["MSVC2017_COMPATIBILITY"] = "ON" if self.settings.compiler.version == "15" else "OFF"
self._cmake.definitions["MSVC2019_COMPATIBILITY"] = "ON" if self.settings.compiler.version == "16" else "OFF"
self._cmake.definitions["MSVC2015_COMPATIBILITY"] = self.settings.compiler.version
self._cmake.definitions["MSVC2017_COMPATIBILITY"] = self.settings.compiler.version
self._cmake.definitions["MSVC2019_COMPATIBILITY"] = self.settings.compiler.version == "16"

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happened to == "14" and == "15"?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also I could've reused add_cmake_option(...) here as well.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

again, don't need an extra method for a very simple logic.


# Corrade uses suffix on the resulting "lib"-folder when running cmake.install()
# Set it explicitly to empty, else Corrade might set it implicitly (eg. to "64")
self._cmake.definitions["LIB_SUFFIX"] = ""

if self.settings.compiler == "Visual Studio":
self._cmake.definitions["CORRADE_MSVC2015_COMPATIBILITY"] = "ON" if self.settings.compiler.version == "14" else "OFF"
self._cmake.definitions["CORRADE_MSVC2017_COMPATIBILITY"] = "ON" if self.settings.compiler.version == "15" else "OFF"
self._cmake.definitions["CORRADE_MSVC2019_COMPATIBILITY"] = "ON" if self.settings.compiler.version == "16" else "OFF"

self._cmake.configure(build_folder=self._build_subfolder)

return self._cmake
Expand All @@ -90,8 +93,11 @@ def package(self):
cmake.install()

share_cmake = os.path.join(self.package_folder, "share", "cmake", "Corrade")
self.copy("UseCorrade.cmake", src=share_cmake, dst=os.path.join(self.package_folder, "lib", "cmake", "Corrade"))
self.copy("CMakeLists.txt", src=share_cmake, dst=os.path.join(self.package_folder, "lib", "cmake", "Corrade"))
self.copy("CorradeConfig.cmake", src=share_cmake, dst=os.path.join(self.package_folder, "lib", "cmake", "Corrade"))
self.copy("CorradeLibSuffix.cmake", src=share_cmake, dst=os.path.join(self.package_folder, "lib", "cmake", "Corrade"))
self.copy("FindCorrade.cmake", src=share_cmake, dst=os.path.join(self.package_folder, "lib", "cmake", "Corrade"))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What specific macro do you need from Corrade? Conan CmakeFindPackage generator should be enough. If there is no specific macro there, please remove FindCorrade/CorradeConfig

Copy link
Author

@helmesjo helmesjo Aug 13, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All the different options passed to CMake when building corrade (that are relevant for consumers) gets written to a configure.h which is packaged along all other includes. This file is then parsed by FindCorrade.cmake. The alternative is to extract the necessary pieces and add to a CMake script that's part of the recipe (if I'm correct it's only the parsing of configure.h that's relevant for consumers).

Or alternatively mimic it by directly in package_info() setup all these different preprocessor directives. Though this is even more error prone than above, IMO.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't find a real evidence of their usage as cmake definitions. For instance, BUILD_STATIC will be "renamed" to CORRADE_BUILD_STATIC, and only the unit tests are using it. The defines in configure.h are okay, they will be processed by the consumer. Do you see a real usage for them?

https://github.com/mosra/corrade/search?q=CORRADE_UTILITY_USE_ANSI_COLORS
https://github.com/mosra/corrade/search?q=CORRADE_BUILD_STATIC

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There were some comments here regarding some macros defined (that one is in UseCorrade.cmake though). But yeah, FindCorrade.cmake should not be included, but instead just the bare minimal required by consumers.

self.copy("UseCorrade.cmake", src=share_cmake, dst=os.path.join(self.package_folder, "lib", "cmake", "Corrade"))
tools.rmdir(os.path.join(self.package_folder, "share"))

def _sort_libs(self, correct_order, libs, lib_suffix="", reverse_result=False):
Expand All @@ -113,9 +119,13 @@ def package_info(self):
self.cpp_info.names["cmake_find_package"] = "Corrade"
self.cpp_info.names["cmake_find_package_multi"] = "Corrade"

self.cpp_info.builddirs.append(os.path.join("lib", "cmake"))
self.cpp_info.build_modules.append(os.path.join("lib", "cmake", "Corrade", "UseCorrade.cmake"))
self.cpp_info.includedirs.append("include")
self.cpp_info.builddirs.append(os.path.join("lib", "cmake", "Corrade"))
self.cpp_info.build_modules.append(os.path.join("lib", "cmake", "Corrade", "FindCorrade.cmake"))
self.cpp_info.build_modules.append(os.path.join("lib", "cmake", "Corrade", "CorradeLibSuffix.cmake"))
self.cpp_info.build_modules.append(os.path.join("lib", "cmake", "Corrade", "CMakeLists.txt"))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
self.cpp_info.build_modules.append(os.path.join("lib", "cmake", "Corrade", "CMakeLists.txt"))

CMakeLists.txt can be removed, it only aggregate other cmake files, Conan will pass them as build modules

self.cpp_info.build_modules.append(os.path.join("lib", "cmake", "Corrade", "CorradeConfig.cmake"))
self.cpp_info.build_modules.append(os.path.join("lib", "cmake", "Corrade", "UseCorrade.cmake"))

# See dependency order here: https://doc.magnum.graphics/magnum/custom-buildsystems.html
allLibs = [
Expand All @@ -136,8 +146,4 @@ def package_info(self):
if self.settings.os == "Linux":
self.cpp_info.system_libs = ["m", "dl"]

self.cpp_info.builddirs = [os.path.join(self.package_folder, "lib", "cmake", "Corrade")]

bindir = os.path.join(self.package_folder, "bin")
self.output.info("Appending PATH environment variable: {}".format(bindir))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Keep the output info.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So which is it: Keep it, or remove it..?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

keep.

self.env_info.PATH.append(bindir)
self.env_info.PATH.append(os.path.join(self.package_folder, "bin"))