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

Recipe/20067 armadillo fix cmake targets #20068

Conversation

samuel-emrys
Copy link
Contributor

@samuel-emrys samuel-emrys commented Sep 21, 2023

Specify library name and version: armadillo/12.6.4

CMake has a published FindArmadillo.cmake module that defines the filename to be Armadillo and the target to be Armadillo::Armadillo. This is in contrast to the default recipe armadillo::armadillo currently used.

This should be updated to reflect the upstream convention. This recipe builds on top of #19907 so that it builds in conan 2. #19907 should be merged prior to this because of that.

This also sets the following custom cmake variables to align the usage of this package with the upstream cmake module:

  • ARMADILLO_FOUND - set to true if the library is found
  • ARMADILLO_INCLUDE_DIRS - list of required include directories
  • ARMADILLO_LIBRARIES - list of libraries to be linked
  • ARMADILLO_VERSION_MAJOR - major version number
  • ARMADILLO_VERSION_MINOR - minor version number
  • ARMADILLO_VERSION_PATCH - patch version number
  • ARMADILLO_VERSION_STRING - version number as a string (ex: "1.0.4")
  • ARMADILLO_VERSION_NAME - name of the version

Closes #20067


@github-actions
Copy link
Contributor

🤖 Beep Boop! This pull request is making changes to 'recipes/armadillo//'.

👋 @samuel-emrys you might be interested. 😉

@github-actions
Copy link
Contributor

🤖 Beep Boop! This pull request is making changes to 'recipes/cmake//'.

👋 @Croydon you might be interested. 😉

@github-actions
Copy link
Contributor

🤖 Beep Boop! This pull request is making changes to 'recipes/elfio//'.

👋 @Hopobcn you might be interested. 😉

@samuel-emrys
Copy link
Contributor Author

This obviously brings in too many commits. I'll fix these once the other armadillo PRs are merged.

@github-actions
Copy link
Contributor

🤖 Beep Boop! This pull request is making changes to 'recipes/libalsa//'.

👋 @jwillikers you might be interested. 😉

@github-actions
Copy link
Contributor

🤖 Beep Boop! This pull request is making changes to 'recipes/libarchive//'.

👋 @jwillikers you might be interested. 😉

@github-actions
Copy link
Contributor

🤖 Beep Boop! This pull request is making changes to 'recipes/libcurl//'.

👋 @Hopobcn you might be interested. 😉

@github-actions
Copy link
Contributor

🤖 Beep Boop! This pull request is making changes to 'recipes/libdwarf//'.

👋 @Hopobcn you might be interested. 😉

@github-actions
Copy link
Contributor

🤖 Beep Boop! This pull request is making changes to 'recipes/libpng//'.

👋 @Hopobcn you might be interested. 😉

@github-actions
Copy link
Contributor

🤖 Beep Boop! This pull request is making changes to 'recipes/libunwind//'.

👋 @Hopobcn you might be interested. 😉

@github-actions
Copy link
Contributor

🤖 Beep Boop! This pull request is making changes to 'recipes/libxml2//'.

👋 @Hopobcn you might be interested. 😉

@github-actions
Copy link
Contributor

🤖 Beep Boop! This pull request is making changes to 'recipes/openssl//'.

👋 @Hopobcn @Croydon you might be interested. 😉

@github-actions
Copy link
Contributor

🤖 Beep Boop! This pull request is making changes to 'recipes/protobuf//'.

👋 @Hopobcn you might be interested. 😉

@SpaceIm
Copy link
Contributor

SpaceIm commented Sep 21, 2023

CMake has a published FindArmadillo.cmake module that defines the filename to be Armadillo and the target to be Armadillo::Armadillo. This is in contrast to the default recipe armadillo::armadillo currently used.

That's not true for CMake target. See #20067 (comment)

These target names are defined by the public CMake module published by
Kitware at https://cmake.org/cmake/help/latest/module/FindArmadillo.html
@conan-center-bot

This comment has been minimized.

Add the following legacy CMake variables defined by the upstream FindArmadillo.cmake
module published by Kitware -
https://cmake.org/cmake/help/latest/module/FindArmadillo.html:

* ARMADILLO_FOUND
* ARMADILLO_INCLUDE_DIRS
* ARMADILLO_LIBRARIES
* ARMADILLO_VERSION_MAJOR
* ARMADILLO_VERSION_MINOR
* ARMADILLO_VERSION_PATCH
* ARMADILLO_VERSION_STRING
* ARMADILLO_VERSION_NAME
@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

Copy link
Contributor

github-actions bot commented Dec 5, 2023

Hooks produced the following warnings for commit f5f1404
armadillo/12.6.4@#99258f7f6c86526c4f323f155fcec8e5
post_package_info(): WARN: [CMAKE FILE NOT IN BUILD FOLDERS (KB-H019)] The *.cmake files have to be placed in a folder declared as `cpp_info.builddirs`. Currently folders declared: {'/home/conan/w/prod-v1/bsr/66846/fcced/.conan/data/armadillo/12.6.4/_/_/package/f83ae54dfb18a82abc3b5aed84601da6156de5ce/'}
post_package_info(): WARN: [CMAKE FILE NOT IN BUILD FOLDERS (KB-H019)] Found files: ./lib/cmake/conan-official-armadillo-variables.cmake
post_package_info(): WARN: [CMAKE FILE NOT IN BUILD FOLDERS (KB-H019)] The *.cmake files have to be placed in a folder declared as `cpp_info.builddirs`. Currently folders declared: {'/home/conan/w/prod-v1/bsr/66846/efffa/.conan/data/armadillo/12.6.4/_/_/package/3064342dc568993daa08048dd305eec75d8b8361/'}
post_package_info(): WARN: [CMAKE FILE NOT IN BUILD FOLDERS (KB-H019)] The *.cmake files have to be placed in a folder declared as `cpp_info.builddirs`. Currently folders declared: {'/home/conan/w/prod-v1/bsr/66846/cecda/.conan/data/armadillo/12.6.4/_/_/package/4b4f748ca307c35af345b319d69de65b064ad1e4/'}
post_package_info(): WARN: [CMAKE FILE NOT IN BUILD FOLDERS (KB-H019)] The *.cmake files have to be placed in a folder declared as `cpp_info.builddirs`. Currently folders declared: {'/home/conan/w/prod-v1/bsr/66846/fcead/.conan/data/armadillo/12.6.4/_/_/package/a8c70666eef94ce7c936220ddb5edbe913c23b14/'}
post_package_info(): WARN: [CMAKE FILE NOT IN BUILD FOLDERS (KB-H019)] The *.cmake files have to be placed in a folder declared as `cpp_info.builddirs`. Currently folders declared: {'/home/conan/w/prod-v1/bsr/66846/bedea/.conan/data/armadillo/12.6.4/_/_/package/9ac1552f96db92901cd34d133e849423a5ad9e62/'}
post_package_info(): WARN: [CMAKE FILE NOT IN BUILD FOLDERS (KB-H019)] The *.cmake files have to be placed in a folder declared as `cpp_info.builddirs`. Currently folders declared: {'/home/conan/w/prod-v1/bsr/66846/ccedb/.conan/data/armadillo/12.6.4/_/_/package/8bf12b820b024ddaf86530f89279383f1bb278f1/'}
post_package_info(): WARN: [CMAKE FILE NOT IN BUILD FOLDERS (KB-H019)] The *.cmake files have to be placed in a folder declared as `cpp_info.builddirs`. Currently folders declared: {'/home/conan/w/prod-v1/bsr/66846/fefae/.conan/data/armadillo/12.6.4/_/_/package/8863722b7758260ec3acc41cce2346f178cec4d7/'}
post_package_info(): WARN: [CMAKE FILE NOT IN BUILD FOLDERS (KB-H019)] The *.cmake files have to be placed in a folder declared as `cpp_info.builddirs`. Currently folders declared: {'/home/conan/w/prod-v1/bsr/66846/efbea/.conan/data/armadillo/12.6.4/_/_/package/739d3b2f43aad0de964b6f41730ec44340e5d365/'}
post_package_info(): WARN: [CMAKE FILE NOT IN BUILD FOLDERS (KB-H019)] The *.cmake files have to be placed in a folder declared as `cpp_info.builddirs`. Currently folders declared: {'/home/conan/w/prod-v1/bsr/66846/dcdcc/.conan/data/armadillo/12.6.4/_/_/package/6ec2b4f85686b1eb44ea591fffff36d314743ac9/'}

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@samuel-emrys
Copy link
Contributor Author

@SpaceIm @RubenRBS a review would be appreciated, noting my comment in #20067 (comment)

@ghost
Copy link

ghost commented Dec 26, 2023

I detected other pull requests that are modifying armadillo/all recipe:

This message is automatically generated by https://github.com/ericLemanissier/conan-center-conflicting-prs so don't hesitate to report issues/improvements there.

Add alias targets for wide compatibility with existing CMake scripts.
The upstream CMake find module doesn't define what the name of the
targets should be, so this provides coverage for the wide usage of
different possible combinations found in the wild.

This is based on inspection of target_link_library patterns for
armadillo usage found on github.
… aliases

* Make armadillo::armadillo the primary CMake target because the
  upstream CMake module doesn't enforce this behaviour, and the policy
  of CCI is to use packagename::packagename when no target is enforced
* Make Armadillo::Armadillo and armadillo CMake target aliases as these
  are common in the wild, and this will facilitate conan package
  compatibility with existing build scripts.
* Remove Armadillo::armadillo and Armadillo as CMake targets as these
  have not been observed in the wild.
@conan-center-bot

This comment has been minimized.

* Remove Armadillo::armadillo and Armadillo as CMake targets as these
  have not been observed in the wild.
@samuel-emrys
Copy link
Contributor Author

I've reverted back to using Armadillo::Armadillo as the primary target, as it seems like this is all conan 1.x can support when the module name is FindArmadillo.cmake. I've retained armadillo::armadillo as an alias in conan 2.x, so the net effect is nil in conan 2.x, but using Armadillo::Armadillo as the cmake target allows us to maintain a consistent interface between 1.x, 2.x and upstream.

@conan-center-bot

This comment has been minimized.

@conan-center-bot conan-center-bot requested a review from AbrilRBS May 7, 2024 11:41
@samuel-emrys samuel-emrys requested a review from valgur May 7, 2024 12:43
@conan-center-bot

This comment has been minimized.

@conan-center-bot
Copy link
Collaborator

Conan v1 pipeline ✔️

All green in build 1 (b57d85b658caf01146936b11780f53172c8c18b3):

  • armadillo/10.7.0:
    All packages built successfully! (All logs)

  • armadillo/10.7.3:
    All packages built successfully! (All logs)

  • armadillo/11.4.3:
    All packages built successfully! (All logs)

  • armadillo/12.6.4:
    All packages built successfully! (All logs)

  • armadillo/12.2.0:
    All packages built successfully! (All logs)

Copy link
Contributor

github-actions bot commented May 7, 2024

Hooks produced the following warnings for commit b57d85b
armadillo/10.7.0@#d4861960525bf94ce0b20f1e37a08ade
post_package(): WARN: [MISSING SYSTEM LIBS (KB-H043)] Library '.\bin\armadillo.dll' links to system library 'shlwapi' but it is not in cpp_info.system_libs.
armadillo/10.7.3@#c71d65416818656632d0926795124556
post_package(): WARN: [MISSING SYSTEM LIBS (KB-H043)] Library '.\bin\armadillo.dll' links to system library 'shlwapi' but it is not in cpp_info.system_libs.
armadillo/11.4.3@#3dbfcf0447d376e86bfba24109633ef7
post_package(): WARN: [MISSING SYSTEM LIBS (KB-H043)] Library '.\bin\armadillo.dll' links to system library 'shlwapi' but it is not in cpp_info.system_libs.

@conan-center-bot conan-center-bot merged commit 4efceac into conan-io:master May 7, 2024
43 checks passed
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.

[package] armadillo/12.2.0 Doesn't use upstream cmake target
6 participants