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

added recipe for extra-cmake-modules #3211

Merged

Conversation

boussaffawalid
Copy link
Contributor

@boussaffawalid boussaffawalid commented Oct 15, 2020

Specify library name and version: lib/1.0

  • 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.

@CLAassistant
Copy link

CLAassistant commented Oct 15, 2020

CLA assistant check
All committers have signed the CLA.

@conan-center-bot
Copy link
Collaborator

Sorry, the build is only launched for Early Access Program users. You can request access writing in this issue.

@conan-center-bot
Copy link
Collaborator

Sorry, the build is only launched for Early Access Program users. You can request access writing in this issue.

@conan-center-bot
Copy link
Collaborator

Sorry, the build is only launched for Early Access Program users. You can request access writing in this issue.

1 similar comment
@conan-center-bot
Copy link
Collaborator

Sorry, the build is only launched for Early Access Program users. You can request access writing in this issue.

recipes/extra-cmake-modules/all/conanfile.py Outdated Show resolved Hide resolved
recipes/extra-cmake-modules/all/conanfile.py Outdated Show resolved Hide resolved
recipes/extra-cmake-modules/all/test_package/conanfile.py Outdated Show resolved Hide resolved
recipes/extra-cmake-modules/all/test_package/conanfile.py Outdated Show resolved Hide resolved
recipes/extra-cmake-modules/all/conanfile.py Outdated Show resolved Hide resolved
from conans import ConanFile, CMake, tools

class ExtracmakemodulesConan(ConanFile):
name = "extra-cmake-modules"
Copy link
Contributor

Choose a reason for hiding this comment

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

Should the name be namespaced?
e.g. kde-extra-cmake-modules

extra-cmake-modules sounds generic.

This is just a question, let's hear the opinion of the others.

Copy link
Member

@uilianries uilianries Oct 19, 2020

Choose a reason for hiding this comment

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

extra-cmake-modules sounds generic.

Indeed, I didn't know about this project, but when searching, it's well referenced to KDE, both Debian packages and Qt points to KDE page. Also, Conda offers extra-cmake-modules by the same name. So I think extra-cmake-modules is okay, as there is no other popular project using the same name or similar.

Copy link
Contributor

Choose a reason for hiding this comment

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

KDE does prefix almost everything with a k. What a shame that they haven't done here the same and leave us with this unfortunate choice 😄

The package got into many Linux distros with the unchanged name. I guess we can leave it at that

https://pkgs.org/download/extra-cmake-modules

recipes/extra-cmake-modules/all/conanfile.py Outdated Show resolved Hide resolved
@conan-center-bot
Copy link
Collaborator

Sorry, the build is only launched for Early Access Program users. You can request access writing in this issue.

This was referenced Oct 19, 2020
recipes/extra-cmake-modules/all/conanfile.py Outdated Show resolved Hide resolved
recipes/extra-cmake-modules/all/conanfile.py Outdated Show resolved Hide resolved
@conan-center-bot
Copy link
Collaborator

Sorry, the build is only launched for Early Access Program users. You can request access writing in this issue.

1 similar comment
@conan-center-bot
Copy link
Collaborator

Sorry, the build is only launched for Early Access Program users. You can request access writing in this issue.

@conan-center-bot
Copy link
Collaborator

An unexpected error happened and has been reported. Help is on its way! 🏇

@conan-center-bot
Copy link
Collaborator

Some configurations of 'extra-cmake-modules/5.75.0' failed in build 9 (8d7fcc81e11269cb0367708a7fe0a2e67a41c3be):

  • Linux x86_64, Release, gcc 4.9, libstdc++
    • Hooks errors detected:
      • [HOOK - conan-center.py] post_package(): ERROR: [MATCHING CONFIGURATION (KB-H014)] Empty package (https://github.com/conan-io/conan-center-index/blob/master/docs/error_knowledge_base.md#KB-H014)
      • [HOOK - conan-center.py] post_package(): ERROR: [MATCHING CONFIGURATION (KB-H014)] Packaged artifacts does not match the settings used: os=None, compiler=None (https://github.com/conan-io/conan-center-index/blob/master/docs/error_knowledge_base.md#KB-H014)
      • [HOOK - conan-center.py] post_package(): ERROR: [CMAKE-MODULES-CONFIG-FILES (KB-H016)] The conan-center repository doesn't allow the packages to contain CMake find modules or config files. The packages have to be located using generators and the declared cpp_infoinformation (https://github.com/conan-io/conan-center-index/blob/master/docs/error_knowledge_base.md#KB-H016)
      • [HOOK - conan-center.py] post_package(): ERROR: [CMAKE-MODULES-CONFIG-FILES (KB-H016)] Found files: ./res/ECM/cmake/ECMConfig.cmake; ./res/ECM/find-modules/FindCanberra.cmake; ./res/ECM/find-modules/FindEGL.cmake; ./res/ECM/find-modules/FindFontconfig.cmake; ./res/ECM/find-modules/FindGLIB2.cmake; ./res/ECM/find-modules/FindGperf.cmake; ./res/ECM/find-modules/FindIcoTool.cmake; ./res/ECM/find-modules/FindInotify.cmake; ./res/ECM/find-modules/FindKF5.cmake; ./res/ECM/find-modules/FindLibExiv2.cmake; ./res/ECM/find-modules/FindLibGit2.cmake; ./res/ECM/find-modules/FindOpenEXR.cmake; ./res/ECM/find-modules/FindPhoneNumber.cmake; ./res/ECM/find-modules/FindPoppler.cmake; ./res/ECM/find-modules/FindPulseAudio.cmake; ./res/ECM/find-modules/FindPythonModuleGeneration.cmake; ./res/ECM/find-modules/FindQHelpGenerator.cmake; ./res/ECM/find-modules/FindQtWaylandScanner.cmake; ./res/ECM/find-modules/FindReuseTool.cmake; ./res/ECM/find-modules/FindSasl2.cmake; ./res/ECM/find-modules/FindSeccomp.cmake; ./res/ECM/find-modules/FindSharedMimeInfo.cmake; ./res/ECM/find-modules/FindTaglib.cmake; ./res/ECM/find-modules/FindUDev.cmake; ./res/ECM/find-modules/FindWayland.cmake; ./res/ECM/find-modules/FindWaylandProtocols.cmake; ./res/ECM/find-modules/FindWaylandScanner.cmake; ./res/ECM/find-modules/FindX11_XCB.cmake; ./res/ECM/find-modules/FindXCB.cmake (https://github.com/conan-io/conan-center-index/blob/master/docs/error_knowledge_base.md#KB-H016)
    • Log download
    • Log error Download
    • Profile
  • Access to all the logs

@boussaffawalid
Copy link
Contributor Author

  • KB-H014 : I don't understand this error, clarification please!
  • KB-H016: This package provide cmake helper modules so I think this is a false positive error. Any way to disable this check for this recipe ?

@Croydon
Copy link
Contributor

Croydon commented Oct 21, 2020

Reading H016, this makes me wonder if those should actually be packaged... extra-cmake-modules has more scripts and NOT only find scripts, right?

The thing is, Conan is supposed to generate find scripts, so if you have a dependency tree with extra-cmake-modules and lets say OpenEXR, then you might end up having two conflicting FindOpenEXR,cmake scripts. And if you don't let Conan generate find scripts and you actually use those from the package then it raises the question if those are compatible with Conan and would find dependencies provided by Conan

🤔

@madebr
Copy link
Contributor

madebr commented Oct 21, 2020

I think this one and ignition-cmake should remove the FindXXX.cmake scripts for finding dependencies of third-party depenencies.
In ignition-cmake's case, the main installed FinxIgnitionCMake.cmake contains extra code that should be carefully modelled using conan.
The fact that conan is currently unable to generate multipe FindXXX.cmake scripts for one recipe also hampers this process.

@boussaffawalid
Copy link
Contributor Author

ECM does not only contains find package scripts but also other cmake modules.
I can exclude the find scripts from the package but this will result to something different than the actual ECM package.

This means that if somebody have OpenEXR installed at the system level and he relay on ECM to find it, He won't be able to achieve that with the current package.

@joxoby
Copy link
Contributor

joxoby commented Oct 21, 2020

I think this one and ignition-cmake should remove the FindXXX.cmake scripts for finding dependencies of third-party depenencies.

Maybe relevant: gazebosim/gz-cmake#119

@conan-center-bot
Copy link
Collaborator

Some configurations of 'extra-cmake-modules/5.75.0' failed in build 10 (5a5afba4b4c4f42654bd2ce386f05a9ceb58438c):

  • Linux x86_64, Release, gcc 4.9, libstdc++
    • Hooks errors detected:
      • [HOOK - conan-center.py] post_package(): ERROR: [CMAKE-MODULES-CONFIG-FILES (KB-H016)] The conan-center repository doesn't allow the packages to contain CMake find modules or config files. The packages have to be located using generators and the declared cpp_infoinformation (https://github.com/conan-io/conan-center-index/blob/master/docs/error_knowledge_base.md#KB-H016)
      • [HOOK - conan-center.py] post_package(): ERROR: [CMAKE-MODULES-CONFIG-FILES (KB-H016)] Found files: ./res/ECM/cmake/ECMConfig.cmake (https://github.com/conan-io/conan-center-index/blob/master/docs/error_knowledge_base.md#KB-H016)
    • Log download
    • Log error Download
    • Profile
  • Access to all the logs

@boussaffawalid
Copy link
Contributor Author

I've excluded all Find*.cmake from the package.
ECMConfig.cmake cannot be excluded thought as it define some variables that are used by the cmake modules provided in this package, unless there a way to define custom variables in the FindECM.cmake generated by conan.

This package provides cmake helper modules and obviously will only be used by cmake. so it is pointless to define self.cpp_info.names[“generator”] .

@uilianries
Copy link
Member

Can you add to build_modules, to be loaded automatically?

@madebr
Copy link
Contributor

madebr commented Oct 22, 2020

Can you add to build_modules, to be loaded automatically?

I don't know about extra-cmake-cmodules, but it is not always desired to blanket load all cmake modules automatically.

@uilianries
Copy link
Member

Yes, ecm has a lot of modules, and probably you don't it, but for main modules. For example, ECMConfig.

@madebr
Copy link
Contributor

madebr commented Oct 22, 2020

Still it is a common pattern to first do a find_package such that some directory is added to CMAKE_MODULE_PATH, and then include the files you need.

@uilianries
Copy link
Member

My point is about mandatory cmake files, not optional. I can use CMAKE_MODULE_PATH and include what I need, but my question is what is mandatory if I use ECM.

@madebr
Copy link
Contributor

madebr commented Oct 22, 2020

Not a lot, looking at its documentation at https://api.kde.org/ecm/manual/ecm.7.html#id3

@boussaffawalid
Copy link
Contributor Author

Nothing needs to be included to build_modules. ECM handle including the required modules itself.

Here is the code from ECMConfig.cmake

set(ECM_FIND_MODULE_DIR "${PACKAGE_PREFIX_DIR}/res/ECM/find-modules/")
set(ECM_MODULE_DIR "${PACKAGE_PREFIX_DIR}/res/ECM/modules/")
set(ECM_KDE_MODULE_DIR "${PACKAGE_PREFIX_DIR}/res/ECM/kde-modules/")
set(ECM_PREFIX "${PACKAGE_PREFIX_DIR}")
set(ECM_MODULE_PATH "${ECM_MODULE_DIR}" "${ECM_FIND_MODULE_DIR}" "${ECM_KDE_MODULE_DIR}")
set(ECM_GLOBAL_FIND_VERSION "${ECM_FIND_VERSION}")

include("${ECM_MODULE_DIR}/ECMUseFindModules.cmake")

This mean when u run find_package(ECM REQUIRED NO_MODULE) cmake will load needed ECM modules.

@uilianries
Copy link
Member

Great, so we can skip it from hooks.

@uilianries
Copy link
Member

conan-io/hooks#248

@danimtb danimtb requested a review from uilianries October 28, 2020 13:39
@conan-center-bot
Copy link
Collaborator

All green in build 11 (e6bb5d3397bb34d409d2a37c3927aa6fd8a81e0f)! 😊

  • extra-cmake-modules/5.75.0: Generated 1 packages. All logs here

@@ -0,0 +1,5 @@
int success()
Copy link
Contributor

Choose a reason for hiding this comment

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

is this supposed to be merged?

Copy link
Contributor

Choose a reason for hiding this comment

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

yes, there is no C++ interface to test with, dummy example is okay

@conan-center-bot conan-center-bot merged commit 9e38f4d into conan-io:master Dec 15, 2020
cacay added a commit to cacay/conan-center-index that referenced this pull request Dec 15, 2020
* master: (73 commits)
  (conan-io#3878) opengl: support FreeBSD
  (conan-io#3882) [system-cci] Fix hooks
  (conan-io#3881) [szip] Fix hooks
  (conan-io#3880) [tcl] Fix hooks
  (conan-io#3866) xorg: add freebsd packages
  (conan-io#3825) Adds baikal-p7 5.6
  (conan-io#3758) minimal recipe for pciutils/3.7.0
  (conan-io#3211) added recipe for extra-cmake-modules
  (conan-io#3870) tl: fix compiler version check again
  (conan-io#3865) bump eigen/3.3.9
  (conan-io#3864) add functionalplus/0.2.13-p0
  bump fmt/7.1.3 (conan-io#3694)
  (conan-io#3858) fix: zulu-openjdk package info
  (conan-io#3836) gmp: bump + enable building static libraries on MSVC
  (conan-io#3812) Add logr v0.1.0
  (conan-io#3786) add Open62541 recipe v1.0.3 and v1.1.3
  (conan-io#3696) openh264: refactor + add 2.1.1 release
  (conan-io#3607) add botan/2.17.2
  (conan-io#3854) Add PEGTL 3.0.0
  (conan-io#3859) bump spdlog/1.8.2
  ...
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.

9 participants