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

metal-cpp: Add header only library metal-cpp from Apple #23086

Merged
merged 16 commits into from
Jul 10, 2024

Conversation

irieger
Copy link
Contributor

@irieger irieger commented Mar 13, 2024

Specify library name and version: metal-cpp/x

Add metal-cpp header only library provided by Apple under the Apache 2.0 license: https://developer.apple.com/metal/cpp/

This library wraps some libraries provided by Apple as Objective-C interfaces into C++ interfaces. Headers only provide the wrapping and it requires one file doing the implementation (as described on the linked project page).

Important point: No clear versioning, packages only reference the version of the macOS/iOS SDKs they wrap into C++. Also the library requires relatively modern macOS/iOS versions to be compiled.


metal-cpp is a library to use Metal (Apple GPU programing language) on Apple systems.
This is header only and doesn't link a framework, as not all requirements are equal.
@conan-center-bot

This comment has been minimized.

@irieger
Copy link
Contributor Author

irieger commented Mar 13, 2024

Pinging #17848 here

I use metal-cpp myself and currently copy the headers into my project which I'd like to prevent, which is why I want to have it as a library myself.

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@irieger
Copy link
Contributor Author

irieger commented Mar 27, 2024

Some open questions to discuss:

  1. Version naming: Current suggestion uses the version number of the target macOS version of the headers as the version number. Would it be better to use cci.xyz or something else?
  2. Version checking and settings are a bit unclear to me here. "Building" the header only library obviously just requires unpacking the ZIP and packing it into a conan package. Applying it however depends mainly on the target SDK, so version "13" builds on everything targeting macOS 13 or newer, while version "14.2" requires a config with "os.version=14.2". How to best reflect this?

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@jcar87
Copy link
Contributor

jcar87 commented Jul 9, 2024

Hi @irieger - thank you so much for this PR and for investigating and troubleshooting the SDK version compatibility.

I have pushed a couple refactorings in this PR - while your PR is correct, I think requiring the users to have os.version or os.sdk_version defined may be a tad too far, as doing so would cause all package IDs to be different due to the new settings, and it may be incompatible with what they had before.

As a fallback, I'm proposing to ask xcrun what the version of the SDK is - it is the SDK headers after all that define the compatible versions of macOS (and others). This should work in most cases when the other two are not defined, and should return the baseline SDK version (which is what ultimately will determine whether the required link libraries are available). One is to assume that if xcrun returns this SDK version, this is what will be used de-facto and that the target OS version will typically be just this (or lower), and if the user wants a target version - they would use os.version anyway.

Additionally - added checks for tvOS as well, since it falls under the "iOS" umbrella (if I'm not mistaken, iOS in this context is both iOS and ipadOS, and tvOS is versioned alongside those two). I've moved the dict of versions to Conandata.yml - as that seems to be a better place for "version-dependent" information.

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@irieger
Copy link
Contributor Author

irieger commented Jul 10, 2024

Hey @jcar87 thank you for the proposed changes. I think they make sense.

I just fixed one version requirement and did a small cleanup but from my point of view we could move forward.

Build all 4 versions locally with success.

@conan-center-bot
Copy link
Collaborator

Conan v1 pipeline ✔️

All green in build 20 (b3f415f59a94488389268e533ed0ad3488e3f7a8):

  • metal-cpp/13.3:
    All packages built successfully! (All logs)

  • metal-cpp/14.2:
    All packages built successfully! (All logs)

  • metal-cpp/13:
    All packages built successfully! (All logs)

  • metal-cpp/14:
    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 19 (b3f415f59a94488389268e533ed0ad3488e3f7a8):

  • metal-cpp/14.2:
    All packages built successfully! (All logs)

  • metal-cpp/13.3:
    All packages built successfully! (All logs)

  • metal-cpp/14:
    All packages built successfully! (All logs)

  • metal-cpp/13:
    All packages built successfully! (All logs)

@jcar87
Copy link
Contributor

jcar87 commented Jul 10, 2024

Hey @jcar87 thank you for the proposed changes. I think they make sense.

I just fixed one version requirement and did a small cleanup but from my point of view we could move forward.

Build all 4 versions locally with success.

Thanks so much!

@conan-center-bot conan-center-bot merged commit efd720c into conan-io:master Jul 10, 2024
24 checks passed
@irieger
Copy link
Contributor Author

irieger commented Jul 10, 2024

Nice. Thank you very much for the review and the general work. Working on my side projects is so much more fun now since I switched everything to conan.

@irieger irieger deleted the metalcpp/add-header-only branch July 10, 2024 21:06
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.

5 participants