add moltenvk/1.1.1 #4634
add moltenvk/1.1.1 #4634
Conversation
|
Some configurations of 'moltenvk/1.1.1' failed in build 1 (
|
I have this header in MacOSX10.15.sdk (xcode 12) |
|
@uilianries @SSE4 @danimtb @jgsogo This version of MoltenVK seems to require Macos >= 10.11, at least at runtime (https://github.com/KhronosGroup/MoltenVK/blob/master/README.md#building-moltenvk). I'll assume that xcode 10 has macos-sdk 10.14 (https://en.wikipedia.org/wiki/Xcode#Xcode_7.0_-_10.x_(since_Free_On-Device_Development)), and since it fails, it's not sufficient for moltenVK 1.1.1 |
|
Some configurations of 'moltenvk/1.1.1' failed in build 2 (
|
|
Well, macos sdk still too low. Now it's some property missing in a Metal object See KhronosGroup/MoltenVK#1187, and specifically this comment KhronosGroup/MoltenVK#1187 (comment). I guess I'll have to introduce more hacks in this recipe (raise in Anyway this version of MoltenVK can't build with current Macos agents in CI. Xcode 11.4 required at least. But the generated runtime can be used with older versions. |
|
Some configurations of 'moltenvk/1.1.1' failed in build 3 (
|
|
Failure in build 4 (
|
On the other hand, MoltenVK can be consumed by older versions of XCode
SpaceIm
force-pushed
the
new/moltenvk/1.1.1
branch
from
c54fd10 to
51370b6
Compare
|
Failure in build 5 (
|
|
MoltenVK 1.0.41 (released in April 2020, Vulkan 1.2.135.0) might be compiled with XCode 11.0 MoltenVK 1.0.40 might be compiled with XCode < 11 |
|
Failure in build 6 (
|
|
Failure in build 7 (
|
|
Expect a different result now with apple-clang 12.0 added to the configurations:: #4636 |
|
Failure in build 8 (
|
|
Failure in build 9 (
|
recipes/moltenvk/all/conanfile.py
Outdated
| def package_id(self): | ||
| del self.info.settings.compiler.version |
There was a problem hiding this comment.
Do not remove the compiler.version. Even if it only works with apple-clang 12, it is needed.
There was a problem hiding this comment.
The problem is that MoltenVK > 1.0.41 must be built with XCode >= 12, but can be consumed with XCode < 12...
Is is ok if I set self.info.settings.compiler.version to 12 in package_id() if self.settings.compiler.version < 12?
There was a problem hiding this comment.
That's the use-case for validate + compatible_packages: we declare that the recipe can only be built with XCode 12, but then in the compatible_packages we list all the versions that can consume it (it will be the same scenario for compiler.cppstd).
It is the idea I tried to ilustrate here (conan-io/examples#73), have a look at the conanfile.py, more or less:
class Recipe(ConanFile):
def configure(self):
if self.settings.os not in ["Macos", "iOS", "tvOS"]:
raise ConanInvalidConfiguration("MoltenVK only supported on MacOS, iOS and tvOS")
def validate(self):
if tools.Version(self.settings.compiler.version) < "12":
raise ConanInvalidConfiguration("MoltenVK {} requires XCode 12 or higher at build time".format(self.version))
def package_id(self):
for xcode in ("9", "10", "11"):
compatible_pkg = self.info.clone()
compatible_pkg.settings.compiler.version = xcode
self.compatible_packages.append(compatible_pkg)The problem with removing compiler.version is that CCI identifies that it is the same package-id for all the macOS builds (9.1, 10, 11, 12), CCI removes duplicates and chooses to build with the oldest of all the compilers that are available. CCI chooses apple-clang 9.1 and then it founds the ConanInvalidConfiguration in the build() method.
There was a problem hiding this comment.
Thanks for the explanation about CI logic.
Regarding package_id(), shouldn't it be something like this?
def package_id(self):
if tools.Version(self.settings.compiler.version) < "12.0":
compatible_pkg = self.info.clone()
compatible_pkg.settings.compiler.version = "12.0"
self.compatible_packages.append(compatible_pkg)So I don't fully understand what happen in validate() (and how it interacts with package_id()). It won't raise if compiler version is < 12 and a binary is available? How? I thought that validate() was called even when we don't build.
There was a problem hiding this comment.
🤕 you are right regarding package_id() implementation.
Conan captures the ConanInvalidConfiguration if it is raised from the validate() method. And will it use in different ways depending on the command. If it conan info, Conan will print that the computed package-ID is invalid instead of raising the error, if it is a conan create/build then it will raise.
Besides the UX, we are preparing for Conan 2.0 where two-profiles would be the default. Still we don't know what is going to happen with the build-requires, if we will expand the full graph always (also the build-requires). If that is the case, when you are installing packages for a host, you really don't care (and probably don't know) the profile used to build them, the recipes in the build context will probably get a default (?) empty (?) profile that will likely raise a ConanInvalidConfiguration.... but we will capture the exception unless you really want to use/build those binaries and Conan needs to retrieve/build the package. This is WIP, probably it will change, but it fits with the better UX experience.
|
All green in build 10 (
|
|
💪 |
There was a problem hiding this comment.
No longer blocking, but still it is better if we move the conditions from build() to validate(). Following this suggestion #4062 I'm modifying the CI to raise if the exception is raised from build() now that we have validate() in place. The change stills need to reviewed and approved.
|
All green in build 11 (
|
|
As you've seen, there is a positive side effect: CCI knows that those configurations are not valid to build (we read the |
|
Nice, thanks. It would be nice if someone could test that we can consume the recipe with apple-clang < 12 in profile , and package built with apple-clang 12 is in cache. |
| @@ -0,0 +1,8 @@ | |||
| sources: | |||
| "1.1.1": | |||
There was a problem hiding this comment.
1.1.2 was released 6 hours ago, should we add this new version?
There was a problem hiding this comment.
It requires vulkan-headers 1.2.170, not yet in CCI. And I prefer to wait sdk tag of this vulkan-headers version. Usually each MoltenVK version refers to a SDK version of Vulkan.
| "1.0.44": "1.2.148.0", | ||
| "1.0.43": "1.2.141.0", | ||
| "1.0.42": "1.2.141.0", | ||
| "1.0.41": "1.2.135.0", | ||
| "1.0.40": "1.2.131.1", | ||
| "1.0.39": "1.1.130.0", | ||
| "1.0.38": "1.1.126.0", |
There was a problem hiding this comment.
Do we need all these versions if we don't add them to our conandata.yml?
There was a problem hiding this comment.
I prefer to keep this for documentation, there are very strict requirements on dependencies versions and I don't want to lose this information.
There was a problem hiding this comment.
Ok, if someone wants an older version, it will be quite easy to add it.
There was a problem hiding this comment.
Actually I've tested all versions from 1.0.39 to 1.1.1, and it works.
|
All green in build 12 (
|
| include(conanbuildinfo.cmake) | ||
| conan_basic_setup(TARGETS) | ||
|
|
||
| if(NOT (CMAKE_SYSTEM_NAME STREQUAL "Darwin" OR |
There was a problem hiding this comment.
Where did you find this cmake file?
There was a problem hiding this comment.
I wrote it from scratch
#4634 (comment)
|
All green in build 14 (
|
Specify library name and version: moltenvk/1.1.1
conan-center hook activated.
closes #4353
I gave up with upstream xcode build files (mainly due to upstream mechanism for dependencies handling) and decided to write a CMakeLists from scratch. Usage of GLOB and GLOB_RECURSE should allow some portability across versions.
If someone is able to provide a recipe relying on upstream XCode build files, with the guarantee to honor conan profile, and not relying on vendored dependencies, feel free to contribute.
I still don't know how to avoid that consumers have vulkan-loader in their dependency graph if they link to MoltenVK (you can have vulkan-loader and MoltenVK in the dependency graph if MoltenVK is used as an ICD).