set opencl pkg_config name #8623

dvirtz · 2022-01-04T11:16:04Z

Specify library name and version: opencl-headers

the name of the pc file installed by ocl-icd system package is OpenCL.pc

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.

SpaceIm

update required_conan_version = ">=1.43.0" please

SpaceIm · 2022-01-04T11:21:43Z

recipes/opencl-headers/all/conanfile.py

        self.cpp_info.names["cmake_find_package"] = "OpenCL"
        self.cpp_info.names["cmake_find_package_multi"] = "OpenCL"
+        self.cpp_info.names["pkg_config"] = "OpenCL"
+        self.cpp_info.set_property("cmake_target_name", "OpenCL")


Suggested change

self.cpp_info.set_property("cmake_target_name", "OpenCL")

self.cpp_info.set_property("cmake_target_name", "OpenCL::Headers")

SpaceIm · 2022-01-04T11:23:21Z

recipes/opencl-headers/all/conanfile.py

        self.cpp_info.names["cmake_find_package"] = "OpenCL"
        self.cpp_info.names["cmake_find_package_multi"] = "OpenCL"
+        self.cpp_info.names["pkg_config"] = "OpenCL"


Suggested change

self.cpp_info.names["pkg_config"] = "OpenCL"

self.cpp_info.set_property("pkg_config_name", "OpenCL")

recipes/opencl-headers/all/conanfile.py

SpaceIm · 2022-01-04T13:05:03Z

recipes/opencl-headers/all/conanfile.py

        self.cpp_info.components["_opencl-headers"].names["cmake_find_package"] = "Headers"
        self.cpp_info.components["_opencl-headers"].names["cmake_find_package_multi"] = "Headers"
+        self.cpp_info.components["_opencl-headers"].set_property("cmake_target_name", "OpenCL::Headers")
+        self.cpp_info.components["_opencl-headers"].set_property("pkg_config_name", "OpenCL")
+


Suggested change

You know you can accept suggestions through github? It saves time sometimes.

yes, but I prefer to test that locally before committing

SpaceIm · 2022-01-04T13:06:53Z

recipes/opencl-headers/all/conanfile.py

        self.cpp_info.names["cmake_find_package"] = "OpenCL"
        self.cpp_info.names["cmake_find_package_multi"] = "OpenCL"
+        self.cpp_info.set_property("cmake_target_name", "OpenCL::OpenCL")


Suggested change

self.cpp_info.set_property("cmake_target_name", "OpenCL::OpenCL")

self.cpp_info.set_property("cmake_target_name", "OpenCL::Headers")

No (same review than #8623 (comment))

can you explain that? from the docs it seems that this recipe generates the target OpenCL::OpenCL for both the cmake_find_package and the cmake_find_package_multi generators.

It's an unwanted side effect of cmake_find_package, the truth is OpenCL-Headers source code which only provides OpenCL::Headers target. set_property() is better in this case, and we should drop those side effects in CMakeDeps.

SpaceIm · 2022-01-04T21:59:11Z

recipes/opencl-headers/all/conanfile.py

        self.cpp_info.components["_opencl-headers"].names["cmake_find_package"] = "Headers"
        self.cpp_info.components["_opencl-headers"].names["cmake_find_package_multi"] = "Headers"
+        self.cpp_info.components["_opencl-headers"].set_property("cmake_target_name", "OpenCL::Headers")
+        self.cpp_info.components["_opencl-headers"].set_property("pkg_config_name", "OpenCLHeaders")


Suggested change

self.cpp_info.components["_opencl-headers"].set_property("pkg_config_name", "OpenCLHeaders")

self.cpp_info.components["_opencl-headers"].set_property("pkg_config_name", "OpenCLHeaders")

No, if "official" pkg-config file is OpenCL.pc, just set pkg_config_name to OpenCL.

but this causes PkgConfigDeps to generate a self including file:

Name: OpenCL Description: Conan package: OpenCL Version: 2021.06.30 Requires: OpenCL

with this change it generates

Name: OpenCL Description: Conan package: OpenCL Version: 2021.06.30 Requires: OpenCLHeaders

and

prefix=/home/conan/.conan/data/opencl-headers/2021.06.30/_/_/package/5ab84d6acfe1f23c4fae0ab88f26e3a396351ac9 libdir1=${prefix}/lib includedir1=${prefix}/include Name: OpenCLHeaders Description: Conan component: OpenCLHeaders Version: 2021.06.30 Libs: -L"${libdir1}" -F Frameworks Cflags: -I"${includedir1}"

I would say it's a bug of PkgConfigDeps, it's still experimental.

what would you expect the generator to do?

Actually i would expect the component pkg-config file to override the global pkg-config file like it happens for pkg-config generator

@SpaceIm, I think that here maybe the best would be not to declare the pkg_config_name property in the component at all, just for the root cpp_info, so Conan will create a new name for the component (OpenCL-_opencl-headers.pc in this case) that is included in the OpenCL.pc that is the one we want.
Doing that names will not collide and also not declaring the property with other invented named we don't promote names that are not official... Also, for Conan 1.45 we have added a warning to try to avoid names overlapping when using PkgConfigDeps. WDYT?

Conan 1.45 will bring the same behavior for PkgConfigDeps. It's already tested here conan-io/conan#10344. The documentation is pending but it's ongoing.
In summary, you can put:

self.cpp_info.components["_opencl-headers"].set_property("pkg_config_name", "OpenCL")

and you're not going to create the self including file anymore.

ghost · 2022-01-31T16:22:30Z

I detected other pull requests that are modifying opencl-headers/all recipe:

opencl-headers: modernize #9259

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

conan-center-bot · 2022-03-11T09:10:33Z

Failure in build 3 (838d75e0de581b19d508fa3d318768cba801ad17):

opencl-headers/2022.01.04@:
All packages built successfully! (All logs)
opencl-headers/2021.04.29@:
All packages built successfully! (All logs)
opencl-headers/2021.06.30@:
All packages built successfully! (All logs)
opencl-headers/2020.06.16@:
All packages built successfully! (All logs)
opencl-headers/2020.12.18@:
An unexpected error happened and has been reported
opencl-headers/2020.03.13@:
All packages built successfully! (All logs)

^{Note: To save resources, CI tries to finish as soon as an error is found. For this reason you might find that not all the references have been launched or not all the configurations for a given reference. Also, take into account that we cannot guarantee the order of execution as it depends on CI workload and workers availability.}

SpaceIm · 2022-03-11T09:35:37Z

this PR can be closed, pkg-config name has been fixed already in #9259

SpaceIm suggested changes Jan 4, 2022

View reviewed changes

This comment has been minimized.

Sign in to view

conan-center-bot added the Unexpected Error label Jan 4, 2022

SpaceIm reviewed Jan 4, 2022

View reviewed changes

This comment has been minimized.

Sign in to view

conan-center-bot removed the Unexpected Error label Jan 4, 2022

SpaceIm previously approved these changes Jan 4, 2022

View reviewed changes

This comment has been minimized.

Sign in to view

prince-chrismc mentioned this pull request Jan 4, 2022

Pull Requests Ready to be Reviewed prince-chrismc/conan-center-index-pending-review#1

Open

dvirtz dismissed SpaceIm’s stale review via dca3351 January 4, 2022 20:20

This comment has been minimized.

Sign in to view

SpaceIm reviewed Jan 4, 2022

View reviewed changes

SpaceIm mentioned this pull request Jan 11, 2022

libsamplerate: fix cmake names for CMakeDeps #8716

Merged

4 tasks

conan-center-bot requested a review from SSE4 January 12, 2022 12:04

czoido mentioned this pull request Jan 14, 2022

[feature] Make the generation order of .pc files deterministic for PkgConfigDeps and pkg_config conan-io/conan#10341

Closed

conan-center-bot requested a review from memsharded January 25, 2022 12:03

This was referenced Jan 31, 2022

[service] Potentially conflicting recipes #3571

Closed

opencl-headers: Add version 2022.01.04 #9201

Merged

ghost mentioned this pull request Feb 4, 2022

opencl-headers: modernize #9259

Merged

4 tasks

conan-center-bot requested a review from jgsogo February 17, 2022 12:01

conan-center-bot requested a review from uilianries March 7, 2022 12:04

This comment has been minimized.

Sign in to view

conan-center-bot added the Failed label Mar 11, 2022

dvirtz added 4 commits March 11, 2022 15:29

set opencl pkg_config name

59d692b

apply review changes

c20419b

apply more suggestions

c661cc7

component file name should be different

838d75e

SSE4 force-pushed the xilinx-runtime branch from 067f57f to 838d75e Compare March 11, 2022 08:31

conan-center-bot added the Unexpected Error label Mar 11, 2022

SSE4 closed this Mar 11, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

set opencl pkg_config name #8623

set opencl pkg_config name #8623

dvirtz commented Jan 4, 2022 •

edited

Loading

SpaceIm left a comment •

edited

Loading

SpaceIm Jan 4, 2022

SpaceIm Jan 4, 2022

This comment has been minimized.

SpaceIm Jan 4, 2022

dvirtz Jan 4, 2022

SpaceIm Jan 4, 2022 •

edited

Loading

dvirtz Jan 4, 2022

SpaceIm Jan 4, 2022 •

edited

Loading

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

SpaceIm Jan 4, 2022 •

edited

Loading

dvirtz Jan 4, 2022

SpaceIm Jan 7, 2022

dvirtz Jan 7, 2022

SpaceIm Jan 11, 2022

czoido Jan 12, 2022

franramirez688 Jan 19, 2022

ghost commented Jan 31, 2022 •

edited by ghost

Loading

This comment has been minimized.

conan-center-bot commented Mar 11, 2022

SpaceIm commented Mar 11, 2022

	self.cpp_info.set_property("cmake_target_name", "OpenCL")
	self.cpp_info.set_property("cmake_target_name", "OpenCL::Headers")

	self.cpp_info.names["pkg_config"] = "OpenCL"
	self.cpp_info.set_property("pkg_config_name", "OpenCL")

	self.cpp_info.components["_opencl-headers"].set_property("pkg_config_name", "OpenCLHeaders")
	self.cpp_info.components["_opencl-headers"].set_property("pkg_config_name", "OpenCLHeaders")

set opencl pkg_config name #8623

set opencl pkg_config name #8623

Conversation

dvirtz commented Jan 4, 2022 • edited Loading

SpaceIm left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment has been minimized.

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SpaceIm Jan 4, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SpaceIm Jan 4, 2022 • edited Loading

Choose a reason for hiding this comment

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

SpaceIm Jan 4, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ghost commented Jan 31, 2022 • edited by ghost Loading

This comment has been minimized.

conan-center-bot commented Mar 11, 2022

SpaceIm commented Mar 11, 2022

dvirtz commented Jan 4, 2022 •

edited

Loading

SpaceIm left a comment •

edited

Loading

SpaceIm Jan 4, 2022 •

edited

Loading

SpaceIm Jan 4, 2022 •

edited

Loading

SpaceIm Jan 4, 2022 •

edited

Loading

ghost commented Jan 31, 2022 •

edited by ghost

Loading