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

rectanglebinpack: add rectanglebinpack/cci.20210901 recipe #7112

Conversation

vbieleny
Copy link
Contributor

@vbieleny vbieleny commented Aug 30, 2021

Specify library name and version: rectanglebinpack/cci.20210901

We use this library as a dependency, so we would like to provide this recipe so anyone can use it.


  • I've read the guidelines for contributing.
  • I've followed the PEP8 style guides for Python code in the recipes.
  • I've used the latest Conan client version.
  • I've tried at least one configuration locally with the
    conan-center hook activated.

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

Comment on lines +73 to +74
self.cpp_info.names["cmake_find_package"] = "RectangleBinPack"
self.cpp_info.names["cmake_find_package_multi"] = "RectangleBinPack"
Copy link
Contributor

Choose a reason for hiding this comment

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

targets are not exported

Copy link
Contributor

Choose a reason for hiding this comment

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

Blah #7153 they match upstream


class TestPackageConan(ConanFile):
settings = "os", "compiler", "build_type", "arch"
generators = "cmake", "cmake_find_package"
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
generators = "cmake", "cmake_find_package"
generators = "cmake"

not used

cmake.build()

def test(self):
if not tools.cross_building(self.settings):
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if not tools.cross_building(self.settings):
if not tools.cross_building(self):

self.copy("*.a", dst="lib", keep_path=False)

def package_info(self):
self.cpp_info.libs = tools.collect_libs(self)
Copy link
Contributor

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.libs = tools.collect_libs(self)
self.cpp_info.libs = ["RectangleBinPack"]

Comment on lines 7 to 10
if(NOT "${CMAKE_CXX_STANDARD}")
set(CMAKE_CXX_STANDARD 11)
endif()
set(CMAKE_CXX_STANDARD_REQUIRED ON)
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe move this in a patch and submit it upstream?

It would prefer target_compile_features(RectangleBinPack PUBLIC cxx_std_11)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will submit this to upstream, but since the upstream cmake file has two targets (RectangleBinPack and MaxRectsBinPackTest) which need C++11 support, isn't it better to just define set(CMAKE_CXX_STANDARD 11) instead of defining target_compile_features for both targets?

Copy link
Contributor

@SpaceIm SpaceIm Aug 30, 2021

Choose a reason for hiding this comment

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

No, CMAKE_CXX_STANDARD should never be used in libraries. Nowadays target_compile_features should be preferred in libraries, it allows to ask for a minimum C++ standard (while CMAKE_CXX_STANDARD asks for the upper bound).
Then final project can decide to enforce a specific C++ standard through CMAKE_CXX_STANDARD in all its dependencies.

It also plays better with compiler.cppstd in conan profiles.

It's worth noting that even with if(NOT "${CMAKE_CXX_STANDARD}"), this block is NOT equivalent to target_compile_features(target PUBLIC cxx_std_11):

if(NOT "${CMAKE_CXX_STANDARD}")
    set(CMAKE_CXX_STANDARD 11)
endif()

Indeed, if CMAKE_CXX_STANDARD has not been explicitly set downstream, target_compile_features(target PUBLIC cxx_std_11) may use any standard greater or equal than 11 (I guess CMake prefers default C++ standard of the compiler if possible).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

All right, I'll use target_compile_features, thanks for the explanation!

@vbieleny vbieleny changed the title rectanglebinpack: add rectanglebinpack/cci.20210829 recipe rectanglebinpack: add rectanglebinpack/cci.20210901 recipe Sep 3, 2021
@conan-center-bot
Copy link
Collaborator

All green in build 5 (3e73a4eafc52c5c0d1d89c8c75159e2a8a22756e):

  • rectanglebinpack/cci.20210901@:
    All packages built successfully! (All logs)

@conan-center-bot conan-center-bot merged commit 37f5130 into conan-io:master Sep 7, 2021
@vbieleny vbieleny deleted the package/rectanglebinpack/cci.20210829 branch September 7, 2021 09:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants