[vulkan-validationlayers] Fix discovery of libVkLayer_khronos_validation #19606

samuel-emrys · 2023-09-05T12:55:32Z

Please read #19605 for a full description of the bug this PR solves

Add libdirs to the runtime lib discovery path to enable libVlLayer_khronos_validation.{so,dll,dylib} to be discovered appropriately at runtime. This package exports both a static library and a shared library, of which, only the static library should be linked against. The shared library is loaded dynamically by vulkan-loader, and still needs to be discoverable. Because this is not a shared library, these paths are not exported automatically and therefore are required to be explicitly set.

I felt that option 1 was the most desirable solution here because:

It doesn't introduce the unwanted semantics that setting package_type="shared-library" would have, and potential complications when people actually want to use this only as the static library that it is. Particularly as this relates to the difference in package_id implications
It doesn't impose requirements on the user to depend on this library differently by manually specifying run=True in the requires() call
It doesn't require patching of artifacts

Additional discussion available in https://cpplang.slack.com/archives/C41CWV9HA/p1693906666122339

Closes #19605

I've read the contributing guidelines.
I've used a recent Conan client version close to the currently deployed.
I've tried at least one configuration locally with the conan-center hook activated.

* Add libdirs to the runtime lib discovery path to enable libVlLayer_khronos_validation.{so,dll,dylib} to be discovered appropriately at runtime. This package exports both a static library and a shared library, of which, only the static library should be linked against. The shared library is loaded dynamically by vulkan-loader, and still needs to be discoverable. Because this is not a shared library, these paths are not exported automatically and therefore are required to be explicitly set. Closes conan-io#19605

samuel-emrys · 2023-09-05T12:56:01Z

// cc @ericLemanissier @SpaceIm

github-actions · 2023-09-05T21:18:41Z

Hooks produced the following warnings for commit 9353b84

vulkan-validationlayers/1.3.239.0@#27caa1728c64d78e642a3a14eb92453c

post_package(): WARN: [APPLE RELOCATABLE SHARED LIBS (KB-H077)] install_name dir of these shared libs is not @rpath: libVkLayer_khronos_validation.dylib

vulkan-validationlayers/1.3.236.0@#1e3692580ab87d25eb3fb49835d65cca

post_package(): WARN: [APPLE RELOCATABLE SHARED LIBS (KB-H077)] install_name dir of these shared libs is not @rpath: libVkLayer_khronos_validation.dylib

recipes/vulkan-validationlayers/all/conanfile.py

Co-authored-by: SpaceIm <30052553+SpaceIm@users.noreply.github.com>

github-actions · 2023-09-08T19:30:14Z

Hooks produced the following warnings for commit 0091dbe

vulkan-validationlayers/1.3.239.0@#3262f83f79381c7f3b170f6afe604069

post_package(): WARN: [APPLE RELOCATABLE SHARED LIBS (KB-H077)] install_name dir of these shared libs is not @rpath: libVkLayer_khronos_validation.dylib

vulkan-validationlayers/1.3.236.0@#32fc5b2d87b522dad4e5db2c250399f9

post_package(): WARN: [APPLE RELOCATABLE SHARED LIBS (KB-H077)] install_name dir of these shared libs is not @rpath: libVkLayer_khronos_validation.dylib

samuel-emrys · 2023-12-04T23:41:26Z

@RubenRBS how's this one going? Last update you provided you were going to check with the team that this was the best way forward

AbrilRBS

Thanks a lot for your patience. After looking into it with the team, we've seen that while Conan lacks some built-in support for cases like these (See conan-io/conan#15216 for tracking), this PR is correct :)

valgur · 2023-12-05T09:59:27Z

IMO, since the vulkan-validationlayers more-or-less packages only a runtime library (i.e. a plugin), the package_type would be more accurate and useful as application. It sets the self.requires() defaults for the package more appropriately as headers=False, libs=False, run=True (https://docs.conan.io/2.0/reference/conanfile/methods/requirements.html#package-type-trait-inferring).

See also #21495. The static utility library together with its headers has been removed in the latest versions and only the runtime one remains.

samuel-emrys · 2023-12-05T13:53:29Z

IMO, since the vulkan-validationlayers more-or-less packages only a runtime library (i.e. a plugin), the package_type would be more accurate and useful as application. It sets the self.requires() defaults for the package more appropriately as headers=False, libs=False, run=True (https://docs.conan.io/2.0/reference/conanfile/methods/requirements.html#package-type-trait-inferring).\n\nSee also #21495. The static utility library together with its headers has been removed in the latest versions and only the runtime one remains.

A couple of points:

I think that despite the fact that application package type may set the correct traits, it's semantically confusing - this isn't an application, so we shouldn't call it such - my preference would be to improve the model that Conan has for handling plugins rather than using application to avoid confusion. My read is that this is the goal of [question] traits propagaton for static library that has runtime loaded plugins conan#15216.
Current versions of this library require static lib support. Suggest any improvements to handle newer versions are better placed in additional PRs

github-actions · 2023-12-05T19:44:13Z

Hooks produced the following warnings for commit 836f103

vulkan-validationlayers/1.3.239.0@#badb48246ad9137d129e1ca51cd51fba

post_package(): WARN: [APPLE RELOCATABLE SHARED LIBS (KB-H077)] install_name dir of these shared libs is not @rpath: libVkLayer_khronos_validation.dylib

vulkan-validationlayers/1.3.236.0@#57c74f0cd23ae14df0b425d1f0c2db15

post_package(): WARN: [APPLE RELOCATABLE SHARED LIBS (KB-H077)] install_name dir of these shared libs is not @rpath: libVkLayer_khronos_validation.dylib

samuel-emrys · 2023-12-11T20:07:56Z

@RubenRBS @franramirez688 could either of you trigger a build of the necessary spirv-tools binary?

AbrilRBS · 2023-12-11T20:49:30Z

I triggered the build this morning, it's progressing nicely now, I'll relaunch the PR once it's done, thanks a lot for your patience

conan-center-bot · 2023-12-21T11:03:15Z

Conan v1 pipeline ✔️

All green in build 7 (dd1fe23b2072c4ce4ca0bbee2323a5a17ca4bce4):

vulkan-validationlayers/1.3.239.0:
All packages built successfully! (All logs)
vulkan-validationlayers/1.3.236.0:
All packages built successfully! (All logs)
vulkan-validationlayers/1.3.231.1:
All packages built successfully! (All logs)
vulkan-validationlayers/1.3.211.0:
All packages built successfully! (All logs)
vulkan-validationlayers/1.3.216.0:
All packages built successfully! (All logs)
vulkan-validationlayers/1.3.224.1:
All packages built successfully! (All logs)

Conan v2 pipeline ✔️

Note: Conan v2 builds are now mandatory. Please read our discussion about it.

All green in build 6 (dd1fe23b2072c4ce4ca0bbee2323a5a17ca4bce4):

vulkan-validationlayers/1.3.239.0:
All packages built successfully! (All logs)
vulkan-validationlayers/1.3.236.0:
All packages built successfully! (All logs)
vulkan-validationlayers/1.3.231.1:
All packages built successfully! (All logs)
vulkan-validationlayers/1.3.216.0:
All packages built successfully! (All logs)
vulkan-validationlayers/1.3.224.1:
All packages built successfully! (All logs)
vulkan-validationlayers/1.3.211.0:
All packages built successfully! (All logs)

AbrilRBS · 2023-12-21T11:45:45Z

Sorry that it took so long to get this merged after approval, my bad on that one, thanks a lot for your patience :)

…r_khronos_validation * [vulkan-validationlayers] Fix discovery of libVkLayer_khronos_validation * Add libdirs to the runtime lib discovery path to enable libVlLayer_khronos_validation.{so,dll,dylib} to be discovered appropriately at runtime. This package exports both a static library and a shared library, of which, only the static library should be linked against. The shared library is loaded dynamically by vulkan-loader, and still needs to be discoverable. Because this is not a shared library, these paths are not exported automatically and therefore are required to be explicitly set. Closes conan-io#19605 * Remove notification of runtime_lib_discovery_path update Co-authored-by: SpaceIm <30052553+SpaceIm@users.noreply.github.com> --------- Co-authored-by: SpaceIm <30052553+SpaceIm@users.noreply.github.com> Co-authored-by: Rubén Rincón Blanco <rubenrb@jfrog.com>

prince-chrismc mentioned this pull request Sep 5, 2023

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

Open

This comment has been minimized.

Sign in to view

SpaceIm reviewed Sep 6, 2023

View reviewed changes

recipes/vulkan-validationlayers/all/conanfile.py Outdated Show resolved Hide resolved

Remove notification of runtime_lib_discovery_path update

0091dbe

Co-authored-by: SpaceIm <30052553+SpaceIm@users.noreply.github.com>

SpaceIm approved these changes Sep 7, 2023

View reviewed changes

This comment has been minimized.

Sign in to view

SSE4 approved these changes Sep 10, 2023

View reviewed changes

conan-center-bot requested review from jcar87 and franramirez688 September 13, 2023 11:44

jwillikers approved these changes Sep 29, 2023

View reviewed changes

AbrilRBS removed request for jcar87 and franramirez688 October 23, 2023 10:10

AbrilRBS self-assigned this Oct 23, 2023

This was referenced Oct 23, 2023

[service] Potentially conflicting recipes #3571

Closed

vulkan-validationlayers/all: bump deps #20744

Merged

ghost mentioned this pull request Oct 30, 2023

Bump version for vulkan-validationlayers/1.3.243.0 #20836

Open

3 tasks

conan-center-bot requested a review from AbrilRBS November 1, 2023 11:42

StellaSmith approved these changes Nov 1, 2023

View reviewed changes

ghost mentioned this pull request Nov 30, 2023

vulkan-validationlayers: update versions, improve package_type and test_package #21495

Open

franramirez688 approved these changes Dec 5, 2023

View reviewed changes

jcar87 mentioned this pull request Dec 5, 2023

[question] traits propagaton for static library that has runtime loaded plugins conan-io/conan#15216

Open

1 task

AbrilRBS approved these changes Dec 5, 2023

View reviewed changes

This comment has been minimized.

Sign in to view

Merge branch 'master' into recipe/19605-vvl-runtime-discovery

836f103

conan-center-bot added Failed Missing dependencies Build failed due missing dependencies in Conan Center labels Dec 5, 2023

This comment has been minimized.

Sign in to view

conan-center-bot added the Unexpected Error label Dec 8, 2023

This comment has been minimized.

Sign in to view

Merge branch 'master' into recipe/19605-vvl-runtime-discovery

dd1fe23

conan-center-bot removed Unexpected Error Failed Missing dependencies Build failed due missing dependencies in Conan Center labels Dec 21, 2023

conan-center-bot merged commit ee18955 into conan-io:master Dec 21, 2023
43 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[vulkan-validationlayers] Fix discovery of libVkLayer_khronos_validation #19606

[vulkan-validationlayers] Fix discovery of libVkLayer_khronos_validation #19606

samuel-emrys commented Sep 5, 2023 •

edited

Loading

samuel-emrys commented Sep 5, 2023

This comment has been minimized.

github-actions bot commented Sep 5, 2023

This comment has been minimized.

github-actions bot commented Sep 8, 2023

samuel-emrys commented Dec 4, 2023

AbrilRBS left a comment

valgur commented Dec 5, 2023 •

edited

Loading

This comment has been minimized.

samuel-emrys commented Dec 5, 2023

This comment has been minimized.

github-actions bot commented Dec 5, 2023

This comment has been minimized.

samuel-emrys commented Dec 11, 2023

AbrilRBS commented Dec 11, 2023

conan-center-bot commented Dec 21, 2023

AbrilRBS commented Dec 21, 2023

[vulkan-validationlayers] Fix discovery of libVkLayer_khronos_validation #19606

[vulkan-validationlayers] Fix discovery of libVkLayer_khronos_validation #19606

Conversation

samuel-emrys commented Sep 5, 2023 • edited Loading

samuel-emrys commented Sep 5, 2023

This comment has been minimized.

github-actions bot commented Sep 5, 2023

This comment has been minimized.

github-actions bot commented Sep 8, 2023

samuel-emrys commented Dec 4, 2023

AbrilRBS left a comment

Choose a reason for hiding this comment

valgur commented Dec 5, 2023 • edited Loading

This comment has been minimized.

samuel-emrys commented Dec 5, 2023

This comment has been minimized.

github-actions bot commented Dec 5, 2023

This comment has been minimized.

samuel-emrys commented Dec 11, 2023

AbrilRBS commented Dec 11, 2023

conan-center-bot commented Dec 21, 2023

Conan v1 pipeline ✔️

Conan v2 pipeline ✔️

AbrilRBS commented Dec 21, 2023

samuel-emrys commented Sep 5, 2023 •

edited

Loading

valgur commented Dec 5, 2023 •

edited

Loading