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

Add recipe for libvulkan-loader package #21893

Merged
merged 17 commits into from
Feb 18, 2023
Merged

Conversation

traversaro
Copy link
Contributor

@traversaro traversaro commented Jan 28, 2023

This adds the recipe for the vulkan-loader package.

Repo: https://github.com/KhronosGroup/Vulkan-Loader .
Repology page: https://repology.org/project/vulkan-loader/versions .

The package name starts with lib given the discussion in #19764 (review) .

Checklist

  • Title of this PR is meaningful: e.g. "Adding my_nifty_package", not "updated meta.yaml".
  • License file is packaged (see here for an example).
  • Source is from official source.
  • Package does not vendor other packages. (If a package uses the source of another package, they should be separate packages or the licenses of all packages need to be packaged).
  • If static libraries are linked in, the license of the static library is packaged.
  • Package does not ship static libraries. If static libraries are needed, follow CFEP-18.
  • Build number is 0.
  • A tarball (url) rather than a repo (e.g. git_url) is used in your recipe (see here for more details).
  • GitHub users listed in the maintainer section have posted a comment confirming they are willing to be listed there.
  • When in trouble, please check our knowledge base documentation before pinging a team.

@conda-forge-webservices
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I wanted to let you know that I linted all conda-recipes in your PR (recipes/vulkan-loader) and found some lint.

Here's what I've got...

For recipes/vulkan-loader:

  • Failed to even lint the recipe, probably because of a conda-smithy bug 😢. This likely indicates a problem in your meta.yaml, though. To get a traceback to help figure out what's going on, install conda-smithy and run conda smithy recipe-lint . from the recipe directory.

@conda-forge-webservices
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipes/vulkan-loader) and found it was in an excellent condition.

@traversaro
Copy link
Contributor Author

@conda-forge/help-c-cpp The PR is ready for review.

build:
number: 0
ignore_run_exports_from:
- libxcb
Copy link
Contributor

Choose a reason for hiding this comment

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

why?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

At least the version that is being packaged here was just using the headers of xcb, without linking the library, see https://github.com/KhronosGroup/Vulkan-Loader/blob/sdk-1.3.239.0/CMakeLists.txt#L180 . So keeping the run_exports was creating warnings (or an error? I can't remember) during the build. I noticed that this just changed in the latest master in KhronosGroup/Vulkan-Loader#1136, so probably we will need anyhow to get rid of this in the next release.

summary: Khronos official Vulkan ICD desktop loader for Windows, Linux, and MacOS.

extra:
feedstock-name: vulkan-loader
Copy link
Contributor

Choose a reason for hiding this comment

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

why this distinction?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In the past I packaged several repositories called <something> that just contained a C/C++ library, so I initially created a feedstock called lib<something>-feedstock, and then more packages were added (such as <something>-python, etc etc), and it turned out to be confusing for contributors to understand why the feedstock was called lib<something>-feedstock while the original repo was called <something>, and the feedstock name could not be (easily) changed. So, I started to create all the feedstosk by using the original repo name, a bit like debian source packages are named. Anyhow, I do not feel strongly about this, so I am happy to just name the feedstock as well libvulkan-loader.

Copy link
Contributor

Choose a reason for hiding this comment

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

thank you for explaining.

@hmaarrfk
Copy link
Contributor

So OSX seems to install a few things in ${PREFIX}/loader is that expected?

-- Installing: $PREFIX/loader/vulkan.framework
-- Installing: $PREFIX/loader/vulkan.framework/Resources
-- Installing: $PREFIX/loader/vulkan.framework/Versions
-- Installing: $PREFIX/loader/vulkan.framework/Versions/A
-- Installing: $PREFIX/loader/vulkan.framework/Versions/A/Resources
-- Installing: $PREFIX/loader/vulkan.framework/Versions/A/Resources/Info.plist
-- Installing: $PREFIX/loader/vulkan.framework/Versions/A/vulkan
-- Installing: $PREFIX/loader/vulkan.framework/Versions/Current
-- Installing: $PREFIX/loader/vulkan.framework/vulkan
-- Installing: $PREFIX/lib/libvulkan.1.3.239.dylib
-- Installing: $PREFIX/lib/libvulkan.1.dylib
-- Installing: $PREFIX/lib/libvulkan.dylib

Otherwise, this looks great.

cc @rossant

xref: conda-forge/cdt-builds#56

@traversaro
Copy link
Contributor Author

So OSX seems to install a few things in ${PREFIX}/loader is that expected?

-- Installing: $PREFIX/loader/vulkan.framework
-- Installing: $PREFIX/loader/vulkan.framework/Resources
-- Installing: $PREFIX/loader/vulkan.framework/Versions
-- Installing: $PREFIX/loader/vulkan.framework/Versions/A
-- Installing: $PREFIX/loader/vulkan.framework/Versions/A/Resources
-- Installing: $PREFIX/loader/vulkan.framework/Versions/A/Resources/Info.plist
-- Installing: $PREFIX/loader/vulkan.framework/Versions/A/vulkan
-- Installing: $PREFIX/loader/vulkan.framework/Versions/Current
-- Installing: $PREFIX/loader/vulkan.framework/vulkan
-- Installing: $PREFIX/lib/libvulkan.1.3.239.dylib
-- Installing: $PREFIX/lib/libvulkan.1.dylib
-- Installing: $PREFIX/lib/libvulkan.dylib

Otherwise, this looks great.

cc @rossant

xref: conda-forge/cdt-builds#56

Thanks for noticing this, I guess there should be some option to disable the installation of the library as a macOS framework, I will check this.

@traversaro
Copy link
Contributor Author

traversaro commented Feb 18, 2023

$PREFIX/loader/vulkan.framework

I disable framework installation with a one-line four lines patch.

@hmaarrfk
Copy link
Contributor

Looks good to me. Thanks!

@hmaarrfk hmaarrfk merged commit 966165c into conda-forge:main Feb 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

2 participants