macOS: fix host build, SHM names and createBlob failures; add presubmit job#151
macOS: fix host build, SHM names and createBlob failures; add presubmit job#151jovemexausto wants to merge 4 commits into
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
367ef17 to
273e368
Compare
|
Reopened after signing the CLA. I hope this fix is useful to the project; it was essential for my current research, and I’m happy to help with any follow-up. |
|
The companion patch in libkrun is here. Together they unblock the macOS gfxstream blob-mapping path. |
gurchetansingh
left a comment
gurchetansingh
left a comment
There was a problem hiding this comment.
Can you add which MacOS builds configuration you're using (I assume) to the Gitlab CI/CD via modifying the actions?
cd830b4 to
193aaa4
Compare
|
Hi! I addressed both comments in this PR:
I should be candid here: I initially scoped this PR narrowly to a concrete bug fix, but that turned out to be incomplete because the change really only makes sense with a working macOS build path. I overlooked that at first. The complete macOS stack is already working in my companion branch, with a passing build here. If you'd prefer, I can fold that macOS host-build stack into this PR so the CI job reflects a fully green macOS path. |
193aaa4 to
be7de40
Compare
|
Yeah, it makes sense to add all of the MacOS build support patches, to ensure CI/CD coverage of the use case. Can you add them here? Also, make Thanks! |
|
Thanks for the follow-up, @gurchetansingh! I'll take some time this weekend to go through and address all your points |
macOS has no memfd_create(); fall back to shm_open() for the SHARED_MEMORY share type there. POSIX.1-2017 (System Interfaces, shm_open) requires the name to begin with a '/' for portable behavior, so normalize names that lack the leading slash. Per review, the normalization is applied universally (not just on macOS) so callers never need to care about it, with the POSIX requirement cited inline. Signed-off-by: Marcus Figueiredo <figueiredo@protonmail.com>
Signed-off-by: Marcus Figueiredo <figueiredo@protonmail.com>
The macOS host build has bit-rotted in ways no current CI exercises:
- meson.build: declare the 'objc' and 'objcpp' languages, required to
compile mac_native.m and native_sub_window_cocoa.mm on Darwin; without
them meson cannot even configure ("No host machine compiler for ...").
- host/gles_compat.h: the GLES-compat shim typedef'd EGLNativeWindowType
as `unsigned int`, which truncates pointers on 64-bit platforms (the
real <EGL/eglplatform.h> type is pointer-sized, e.g. `void*` on
Apple). native_sub_window_cocoa.mm returns an NSView* through this
type, so the shim must stay ABI-compatible.
- host/frame_buffer.h, host/native_sub_window.h: the GLES-disabled
branch still included "GlesCompat.h"; the file on disk is
gles_compat.h. No configuration that takes this branch was ever
compiled by CI, so the rename went unnoticed.
- host/iostream, host/snapshot: these static libraries have no sources,
which GNU ar tolerates but macOS (BSD) ar rejects ("no archive
members specified"). Add an intentionally-empty translation unit on
Darwin.
- host/meson.build, host/gl/meson.build: hoist the GL include-path
declarations out of the `use_gles` conditional; the Vulkan server
includes GLES dispatch headers even in Vulkan-only builds, and
include_directories() on paths costs nothing when GLES is disabled.
host/color_buffer.cpp: guard the ColorBufferGl using-declaration and
the vulkanOnly derivation for GLES-disabled builds.
With this, the default host build (meson setup -Ddefault_library=static
-Dgfxstream-build=host) configures and compiles to completion on
macOS/AArch64 with Homebrew meson/ninja/molten-vk/vulkan-loader.
Vulkan-only (-Ddecoders=vulkan) now configures but still has further
compile errors in GLES-entangled sources; that is left for a follow-up.
Signed-off-by: Marcus Figueiredo <figueiredo@protonmail.com>
Per review, the macOS build lives in presubmit.yaml alongside the other platform builds instead of a separate workflow file. The job mirrors the Linux meson job's structure (same step names, checkout pinned to the same SHA) and builds the host renderer with the Vulkan loader + MoltenVK from Homebrew: meson setup -Ddefault_library=static -Dgfxstream-build=host This is the configuration used to run gfxstream as the virtio-gpu backend on Apple Silicon hosts (HVF VMMs such as libkrun), where the Vulkan device is MoltenVK on Metal. It also guards the macOS-only code paths (ObjC sources, BSD ar, the GLES-compat shim) that no existing job compiles. Signed-off-by: Marcus Figueiredo <figueiredo@protonmail.com>
be7de40 to
a6023ab
Compare
Hey @gurchetansingh, thanks for the review! Moved the macOS job into presubmit.yaml next to the other platform builds and dropped the separate workflow file. Heads up though: while wiring that up I found the macOS host build doesn't actually compile on main right now, so the job would've just been red. Added one commit with the minimal fixes: project() was missing objc/objcpp (needed for the .m/.mm files), gles_compat.h typedefs EGLNativeWindowType as unsigned int which truncates pointers on 64-bit (native_sub_window_cocoa.mm returns an NSView* through it), the iostream/snapshot static libs have no sources which BSD ar refuses to archive, and frame_buffer.h still includes "GlesCompat.h" from before the rename to gles_compat.h. Built clean on an M-series mac with the exact commands the job runs (brew meson/ninja/molten-vk/vulkan-loader). Also squashed the earlier shm_open fixup into its parent while rebasing, so the series is clean. Vulkan-only (-Ddecoders=vulkan) configures now but still doesn't fully compile, there's more GLES-entangled stuff in there. I'd rather do that in a follow-up than bloat this PR. |
jovemexausto
changed the title
gurchetansingh
left a comment
gurchetansingh
left a comment
There was a problem hiding this comment.
Vulkan-only (-Ddecoders=vulkan) configures now but still doesn't fully compile, there's more GLES-entangled stuff in there. I'd rather do that in a follow-up than bloat this PR.
Ah, so you're testing with gfxstream GLES? I'm just trying to figure out which configuration you've gotten working.
Note: I don't really mind a MR with 10+ commits if you need it to get it working.
| # SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| project('gfxstream', 'cpp', 'c', | ||
| project('gfxstream', 'cpp', 'c', 'objc', 'objcpp', |
There was a problem hiding this comment.
Causes CI failures on Windows/Linux: you need to add_languages(..) selectively based on the host OS.
2 participants
Summary
shm_open()names to be POSIX-compliant (leading/) on all platforms, and useshm_open()on macOS where there's nomemfd_create().stream_renderer_create_blob()instead of always returning success.project()was missing theobjc/objcpplanguages, so meson can't even configure withmac_native.m/native_sub_window_cocoa.mmin the tree.gles_compat.htypedefsEGLNativeWindowTypeasunsigned int, which truncates pointers on 64-bit.native_sub_window_cocoa.mmreturns anNSView*through it.host/iostreamandhost/snapshotare static libraries with no sources, which BSDarrefuses to archive.frame_buffer.h/native_sub_window.hstill include"GlesCompat.h"from before the rename togles_compat.h.presubmit.yaml(per review) so these paths stay compiled.Why
gfxstream works as the virtio-gpu backend on Apple Silicon hosts (MoltenVK on Metal, HVF VMMs like libkrun), but no CI compiles the macOS paths, so they rot. This gets the default host build green on macOS and keeps it that way.
Validation
meson setup -Ddefault_library=static -Dgfxstream-build=host+meson compile, deps from Homebrew (meson, ninja, molten-vk, vulkan-loader).Note:
-Ddecoders=vulkan(Vulkan-only) now configures but still doesn't fully compile; that's coming in a follow-up PR.