[vcpkg-cmake][libpng] Add "OSX_SPLIT_BUILD" feature #22898

omartijn · 2022-02-02T16:41:36Z

Describe the pull request

What does your PR fix?

It enabled universal binaries for cmake-based ports that somehow don't support it (e.g. because they use NASM). One such example is libpng, which now works with this PR

Which triplets are supported/not supported? Have you updated the CI baseline?

All. It also works for libpng when you set VCPKG_OSX_ARCHITECTURES to both arm64 and x86_64

Does your PR follow the maintainer guide?

Yes

If you have added/updated a port: Have you run ./vcpkg x-add-version --all and committed the result?

Yes

github-actions

You have modified or added at least one vcpkg.json where a "license" field is missing.
If you feel able to do so, please consider adding a "license" field to the following files:

ports/libpng/vcpkg.json
ports/vcpkg-cmake/vcpkg.json

Valid values for the license field are listed at https://spdx.org/licenses/

omartijn · 2022-02-03T08:08:50Z

@JackBoosY It seems the pipeline failed x86_windows, but not with any error I can understand. Looks like the "Test Modified Ports and Prepare Test Logs" failed, but the message simply states:

Evaluating: SucceededNode()
Result: False

Is this something I can fix on my end or is this some other issue? Should we just restart the pipeline?

github-actions

You have modified or added at least one vcpkg.json where a "license" field is missing.
If you feel able to do so, please consider adding a "license" field to the following files:

ports/libpng/vcpkg.json

Valid values for the license field are listed at https://spdx.org/licenses/

omartijn · 2022-02-04T14:55:56Z

@JackBoosY: Can you shed a light on this build failure: https://dev.azure.com/vcpkg/public/_build/results?buildId=66849&view=logs&j=878666d5-db33-5b27-9e7d-b0c7ee352005&t=d914c609-a9aa-5880-221f-cbd46db9069e

It says it's failed, but I don't understand why.

JackBoosY · 2022-02-05T08:15:55Z

That's because you also need to update the related docs.
Please download this file and apply it, then commit.

omartijn · 2022-02-05T09:45:24Z

Thanks, @JackBoosY, I would have never guessed that from the output!

JackBoosY · 2022-02-07T02:12:10Z

cc @strega-nil-ms for review this PR.

strega-nil-ms

This is really cool! I'll bring it up to the team 😄

One thing that I'd like to see here is, instead of passing OSX_SPLIT_BUILD to both vcpkg_cmake_configure and vcpkg_cmake_install, just setting a Z_OSX_SPLIT_BUILD cache variable (just like how we do generators).

It also seems like this could be a more general solution; I think it could very easily be adapted for autotools and meson builds, at least, with a helper z_vcpkg_merge_osx_architectures.

I'm sorry it took so long for me to see this PR; it completely slipped under the radar.

docs/maintainers/ports/vcpkg-cmake/vcpkg_cmake_install.md

ports/vcpkg-cmake/vcpkg_cmake_build.cmake

JackBoosY · 2022-04-12T01:23:26Z

Can you please resolve the file conflicts?

Thanks.

omartijn · 2022-04-12T07:24:23Z

Can you please resolve the file conflicts?

Thanks.

Certainly. I thought I had, but I think I must've rebased on an older version of the master branch. Should be good to go now.

JackBoosY · 2022-06-29T02:55:57Z

@ras0219-msft Please review the vcpkg-cmake part.

JackBoosY · 2022-07-08T08:54:01Z

Can you please merge to master first?

omartijn · 2022-07-15T08:14:29Z

Can you please merge to master first?

I guess you mean "merge master in first"? Just as an aside, merging to master would be when we finally get this feature in the master branch.

Anyway, during merging I noticed that master is broken. Somebody has been playing around renaming variables (which happens surprisingly often it seems), parallel_args seems to be renamed to parallel_param, as well as some other variables ending in _args now ending in _param, but this has not been consistently changed. So now there are variables being set and not used.

JackBoosY · 2022-07-18T08:07:32Z

When building mesa:x64-windows:

[710/1008] "C:/Program Files/Microsoft Visual Studio/2022/Enterprise/VC/Tools/MSVC/14.32.31326/bin/Hostx64/x64/cl.exe" "-Isrc\mesa\libmesa.a.p" "-Isrc\mesa" "-I..\src\esa-22.0.2-5fd67a6f31.clean\src\mesa" "-Iinclude" "-I..\src\esa-22.0.2-5fd67a6f31.clean\include" "-Isrc" "-I..\src\esa-22.0.2-5fd67a6f31.clean\src" "-Isrc\mapi" "-I..\src\esa-22.0.2-5fd67a6f31.clean\src\mapi" "-I..\src\esa-22.0.2-5fd67a6f31.clean\src\gallium\include" "-Isrc\gallium\auxiliary" "-I..\src\esa-22.0.2-5fd67a6f31.clean\src\gallium\auxiliary" "-Isrc\mesa\main" "-I..\src\esa-22.0.2-5fd67a6f31.clean\src\mesa\main" "-Isrc\compiler\nir" "-I..\src\esa-22.0.2-5fd67a6f31.clean\src\compiler\nir" "-Isrc\util" "-I..\src\esa-22.0.2-5fd67a6f31.clean\src\util" "-Isrc\mesa\program" "-Isrc\mapi\glapi\gen" "-Isrc\compiler" "-ID:/installed/x64-windows/debug/../include" "-ID:/installed/x64-windows/include" "-DNDEBUG" "/MDd" "/nologo" "/showIncludes" "/utf-8" "/W2" "/std:c11" "/Od" "/Zi" "-D__STDC_CONSTANT_MACROS" "-D__STDC_FORMAT_MACROS" "-D__STDC_LIMIT_MACROS" "-DPACKAGE_VERSION=\"22.0.2\"" "-DPACKAGE_BUGREPORT=\"https://gitlab.freedesktop.org/mesa/mesa/-/issues\"" "-DHAVE_WINDOWS_PLATFORM" "-DUSE_ELF_TLS" "-DUSE_TLS_BEHIND_FUNCTIONS" "-DENABLE_ST_OMX_BELLAGIO=0" "-DENABLE_ST_OMX_TIZONIA=0" "-DEGL_NO_X11" "-D_WIN32_WINNT=0x0A00" "-DWINVER=0x0A00" "-DPIPE_SUBSYSTEM_WINDOWS_USER" "-D_USE_MATH_DEFINES" "-DVC_EXTRALEAN" "-D_CRT_SECURE_NO_WARNINGS" "-D_CRT_SECURE_NO_DEPRECATE" "-D_SCL_SECURE_NO_WARNINGS" "-D_SCL_SECURE_NO_DEPRECATE" "-D_ALLOW_KEYWORD_MACROS" "-D_HAS_EXCEPTIONS=0" "-DNOMINMAX" "-DMISSING_64BIT_ATOMICS" "-DHAS_SCHED_H" "-DHAVE_DLFCN_H" "-DHAVE_STRTOF" "-DHAVE_QSORT_S" "-DHAVE_DIRENT_D_TYPE" "-DHAVE_ZLIB" "-DHAVE_ZSTD" "-DHAVE_COMPRESSION" "-DLLVM_AVAILABLE" "-DMESA_LLVM_VERSION_STRING=\"14.0.4\"" "-DLLVM_IS_SHARED=0" "-DDRAW_LLVM_AVAILABLE" "-DMESA_EXECMEM" "-DVK_USE_PLATFORM_WIN32_KHR" "/wd4018" "/wd4056" "/wd4244" "/wd4267" "/wd4305" "/wd4351" "/wd4756" "/wd4800" "/wd4996" "/wd4291" "/wd4146" "/wd4200" "/wd4624" "/wd4309" "/wd4838" "/wd5105" "/we4020" "/we4024" "/Zc:__cplusplus" "-nologo" "-DWIN32" "-D_WINDOWS" "-W3" "-utf-8" "-MP" "-D_CRT_DECLARE_NONSTDC_NAMES" "-D_DEBUG" "-MDd" "-Z7" "-Ob0" "-Od" "-RTC1" "-D_GDI32_" "/Fdsrc\mesa\libmesa.a.p\meson-generated_.._program_lex.yy.c.pdb" /Fosrc/mesa/libmesa.a.p/meson-generated_.._program_lex.yy.c.obj "/c" src/mesa/program/lex.yy.c
FAILED: src/mesa/libmesa.a.p/meson-generated_.._program_lex.yy.c.obj 
"C:/Program Files/Microsoft Visual Studio/2022/Enterprise/VC/Tools/MSVC/14.32.31326/bin/Hostx64/x64/cl.exe" "-Isrc\mesa\libmesa.a.p" "-Isrc\mesa" "-I..\src\esa-22.0.2-5fd67a6f31.clean\src\mesa" "-Iinclude" "-I..\src\esa-22.0.2-5fd67a6f31.clean\include" "-Isrc" "-I..\src\esa-22.0.2-5fd67a6f31.clean\src" "-Isrc\mapi" "-I..\src\esa-22.0.2-5fd67a6f31.clean\src\mapi" "-I..\src\esa-22.0.2-5fd67a6f31.clean\src\gallium\include" "-Isrc\gallium\auxiliary" "-I..\src\esa-22.0.2-5fd67a6f31.clean\src\gallium\auxiliary" "-Isrc\mesa\main" "-I..\src\esa-22.0.2-5fd67a6f31.clean\src\mesa\main" "-Isrc\compiler\nir" "-I..\src\esa-22.0.2-5fd67a6f31.clean\src\compiler\nir" "-Isrc\util" "-I..\src\esa-22.0.2-5fd67a6f31.clean\src\util" "-Isrc\mesa\program" "-Isrc\mapi\glapi\gen" "-Isrc\compiler" "-ID:/installed/x64-windows/debug/../include" "-ID:/installed/x64-windows/include" "-DNDEBUG" "/MDd" "/nologo" "/showIncludes" "/utf-8" "/W2" "/std:c11" "/Od" "/Zi" "-D__STDC_CONSTANT_MACROS" "-D__STDC_FORMAT_MACROS" "-D__STDC_LIMIT_MACROS" "-DPACKAGE_VERSION=\"22.0.2\"" "-DPACKAGE_BUGREPORT=\"https://gitlab.freedesktop.org/mesa/mesa/-/issues\"" "-DHAVE_WINDOWS_PLATFORM" "-DUSE_ELF_TLS" "-DUSE_TLS_BEHIND_FUNCTIONS" "-DENABLE_ST_OMX_BELLAGIO=0" "-DENABLE_ST_OMX_TIZONIA=0" "-DEGL_NO_X11" "-D_WIN32_WINNT=0x0A00" "-DWINVER=0x0A00" "-DPIPE_SUBSYSTEM_WINDOWS_USER" "-D_USE_MATH_DEFINES" "-DVC_EXTRALEAN" "-D_CRT_SECURE_NO_WARNINGS" "-D_CRT_SECURE_NO_DEPRECATE" "-D_SCL_SECURE_NO_WARNINGS" "-D_SCL_SECURE_NO_DEPRECATE" "-D_ALLOW_KEYWORD_MACROS" "-D_HAS_EXCEPTIONS=0" "-DNOMINMAX" "-DMISSING_64BIT_ATOMICS" "-DHAS_SCHED_H" "-DHAVE_DLFCN_H" "-DHAVE_STRTOF" "-DHAVE_QSORT_S" "-DHAVE_DIRENT_D_TYPE" "-DHAVE_ZLIB" "-DHAVE_ZSTD" "-DHAVE_COMPRESSION" "-DLLVM_AVAILABLE" "-DMESA_LLVM_VERSION_STRING=\"14.0.4\"" "-DLLVM_IS_SHARED=0" "-DDRAW_LLVM_AVAILABLE" "-DMESA_EXECMEM" "-DVK_USE_PLATFORM_WIN32_KHR" "/wd4018" "/wd4056" "/wd4244" "/wd4267" "/wd4305" "/wd4351" "/wd4756" "/wd4800" "/wd4996" "/wd4291" "/wd4146" "/wd4200" "/wd4624" "/wd4309" "/wd4838" "/wd5105" "/we4020" "/we4024" "/Zc:__cplusplus" "-nologo" "-DWIN32" "-D_WINDOWS" "-W3" "-utf-8" "-MP" "-D_CRT_DECLARE_NONSTDC_NAMES" "-D_DEBUG" "-MDd" "-Z7" "-Ob0" "-Od" "-RTC1" "-D_GDI32_" "/Fdsrc\mesa\libmesa.a.p\meson-generated_.._program_lex.yy.c.pdb" /Fosrc/mesa/libmesa.a.p/meson-generated_.._program_lex.yy.c.obj "/c" src/mesa/program/lex.yy.c
cl : Command line warning D9025 : overriding '/W2' with '/W3'
cl : Command line warning D9025 : overriding '/Zi' with '/Z7'
src/compiler/glsl/glsl_lexer.cpp(2): warning C4117: macro name '__STDC_VERSION__' is reserved, '#define' ignored
../src/esa-22.0.2-5fd67a6f31.clean/src/compiler/glsl/glsl_lexer.ll(27): fatal error C1083: Cannot open include file: 'ast.h': No such file or directory

Is this related to your changes?

omartijn · 2022-07-18T08:18:36Z

Is this related to your changes?

I'd be very surprised if it were, given that the changes are only enabled on macos. It's possible I made a mistake during one of the merges. I'll check this out later today.

JackBoosY · 2022-07-22T06:56:29Z

@omartijn Any progress?

omartijn · 2022-07-22T08:38:30Z

I've tried building the package on the master branch, and that also failed in the mesa:x64-windows package. So it seems the package is simply broken.

JackBoosY · 2022-07-29T08:47:34Z

Can you please resolve the file conflicts so I can take a look?

Some ports (e.g. libpng) don't support building for multiple architectures at the same time. For these ports, we can now use the OSX_SPLIT_BUILD option, which will perform multiple builds (one for each requested architecture) and then merge them into a universal binary when the build is complete

JackBoosY · 2022-08-02T06:05:01Z

ports/vcpkg-cmake/vcpkg_cmake_configure.cmake

+                        "${CMAKE_COMMAND}" "${arg_SOURCE_PATH}"
+                        -D CMAKE_OSX_ARCHITECTURES=${OSX_ARCHITECTURE}
+                        ${arg_OPTIONS}
+                        ${arg_OPTIONS_RELEASE}
+                        -G "${generator}"
+                        "-DCMAKE_BUILD_TYPE=Release"
+                        "-DCMAKE_INSTALL_PREFIX=${OSX_SPLIT_BUILD_INSTALL_DIR_RELEASE}"


I prefer to use dbg_command and rel_command here, can you please make some changes for this?

JackBoosY · 2022-08-02T06:05:53Z

ports/vcpkg-cmake/vcpkg_cmake_configure.cmake

+                        "-DCMAKE_BUILD_TYPE=Release"
+                        "-DCMAKE_INSTALL_PREFIX=${OSX_SPLIT_BUILD_INSTALL_DIR_RELEASE}"
+                    WORKING_DIRECTORY "${build_dir_release}-${OSX_ARCHITECTURE}"
+                    LOGNAME "${arg_LOGFILE_BASE}-rel-${OSX_ARCHITECTURE}"


We may also need to declare SAVE_LOG_FILES here.

JackBoosY · 2022-08-02T06:06:21Z

ports/vcpkg-cmake/vcpkg_cmake_configure.cmake

+                vcpkg_execute_required_process(
+                    COMMAND
+                        "${CMAKE_COMMAND}" "${arg_SOURCE_PATH}"
+                        -D CMAKE_OSX_ARCHITECTURES=${OSX_ARCHITECTURE}
+                        ${arg_OPTIONS}
+                        ${arg_OPTIONS_DEBUG}
+                        -G "${generator}"
+                        "-DCMAKE_BUILD_TYPE=Debug"
+                        "-DCMAKE_INSTALL_PREFIX=${OSX_SPLIT_BUILD_INSTALL_DIR_DEBUG}"
+                    WORKING_DIRECTORY "${build_dir_debug}-${OSX_ARCHITECTURE}"
+                    LOGNAME "${arg_LOGFILE_BASE}-dbg-${OSX_ARCHITECTURE}"
+                )


Same as above.

omartijn · 2022-08-09T11:31:08Z

I've taken a look into using rel_command and dbg_command, and I cannot get it to work the way I expect. I have tried removing the CMAKE_INSTALL_PREFIX from both commands and then passing those as extra flags to vcpkg_execute_required_process (because those are different for regular vs split build)

Somehow, the option is then not correctly processed. I have spent about as much time as I am willing on this, but the constant code churn is taking too much of my time. So I'll leave this PR for anyone who's willing to maintain it.

JackBoosY · 2022-08-19T07:49:06Z

Convert this PR to draft since there is no progress for 10 days.

JackBoosY · 2022-09-16T08:24:41Z

Closing this PR since it seems that no progress is being made. Please ping me to reopen if work is still being done.

github-actions bot approved these changes Feb 2, 2022

View reviewed changes

JackBoosY self-assigned this Feb 3, 2022

JackBoosY added category:port-bug The issue is with a library, which is something the port should already support category:tool-update The issue is with build tool or build script, which requires update or should be executed correctly labels Feb 3, 2022

omartijn force-pushed the osx-split-build branch from 0f1ceb6 to 294ed25 Compare February 3, 2022 14:21

github-actions bot approved these changes Feb 3, 2022

View reviewed changes

omartijn force-pushed the osx-split-build branch from 294ed25 to 5a7ff94 Compare February 3, 2022 14:27

github-actions bot approved these changes Feb 3, 2022

View reviewed changes

BillyONeal changed the title ~~Osx split build~~ [vcpkg-cmake][libpng] Add "OSX_SPLIT_BUILD" feature Feb 4, 2022

JackBoosY added the requires:author-response label Feb 5, 2022

omartijn force-pushed the osx-split-build branch from 5a7ff94 to f6734a6 Compare February 5, 2022 09:45

github-actions bot approved these changes Feb 5, 2022

View reviewed changes

JackBoosY removed the requires:author-response label Feb 6, 2022

JackBoosY requested a review from strega-nil-ms February 7, 2022 02:12

JackBoosY added the requires:vcpkg-team-review This PR or issue requires someone on the vcpkg team to take a further look. label Mar 18, 2022

strega-nil-ms suggested changes Apr 8, 2022

View reviewed changes

docs/maintainers/ports/vcpkg-cmake/vcpkg_cmake_install.md Outdated Show resolved Hide resolved

ports/vcpkg-cmake/vcpkg_cmake_build.cmake Outdated Show resolved Hide resolved

JackBoosY added the requires:author-response label Apr 11, 2022

omartijn force-pushed the osx-split-build branch from f6734a6 to f62fb20 Compare April 11, 2022 12:06

github-actions bot approved these changes Apr 11, 2022

View reviewed changes

omartijn force-pushed the osx-split-build branch from f62fb20 to a76fe5f Compare April 11, 2022 12:10

github-actions bot approved these changes Apr 11, 2022

View reviewed changes

omartijn force-pushed the osx-split-build branch from a76fe5f to 932c97d Compare April 12, 2022 07:23

github-actions bot approved these changes Apr 12, 2022

View reviewed changes

JackBoosY added info:reviewed Pull Request changes follow basic guidelines and removed requires:vcpkg-team-review This PR or issue requires someone on the vcpkg team to take a further look. labels Jun 29, 2022

dan-shaw added requires:vcpkg-team-review This PR or issue requires someone on the vcpkg team to take a further look. and removed info:reviewed Pull Request changes follow basic guidelines labels Jun 30, 2022

omartijn dismissed stale reviews from JackBoosY and github-actions[bot] via 4e5de4f July 15, 2022 09:21

omartijn force-pushed the osx-split-build branch from b958923 to 4e5de4f Compare July 15, 2022 09:21

github-actions bot previously approved these changes Jul 15, 2022

View reviewed changes

martijn-resolume added 2 commits August 1, 2022 11:58

Update libpng to use split build on osx for universal build

1b6a2db

omartijn dismissed github-actions[bot]’s stale review via 1b6a2db August 1, 2022 10:04

omartijn force-pushed the osx-split-build branch from 4e5de4f to 1b6a2db Compare August 1, 2022 10:04

github-actions bot approved these changes Aug 1, 2022

View reviewed changes

JackBoosY suggested changes Aug 2, 2022

View reviewed changes

JackBoosY added the requires:author-response label Aug 5, 2022

JackBoosY removed the requires:vcpkg-team-review This PR or issue requires someone on the vcpkg team to take a further look. label Aug 12, 2022

JackBoosY marked this pull request as draft August 19, 2022 07:49

JackBoosY removed the requires:author-response label Aug 19, 2022

JackBoosY closed this Sep 16, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[vcpkg-cmake][libpng] Add "OSX_SPLIT_BUILD" feature #22898

[vcpkg-cmake][libpng] Add "OSX_SPLIT_BUILD" feature #22898

omartijn commented Feb 2, 2022

github-actions bot left a comment

omartijn commented Feb 3, 2022

github-actions bot left a comment

omartijn commented Feb 4, 2022

JackBoosY commented Feb 5, 2022

omartijn commented Feb 5, 2022

JackBoosY commented Feb 7, 2022

strega-nil-ms left a comment •

edited

Loading

JackBoosY commented Apr 12, 2022

omartijn commented Apr 12, 2022

JackBoosY commented Jun 29, 2022

JackBoosY commented Jul 8, 2022

omartijn commented Jul 15, 2022

JackBoosY commented Jul 18, 2022 •

edited

Loading

omartijn commented Jul 18, 2022

JackBoosY commented Jul 22, 2022

omartijn commented Jul 22, 2022

JackBoosY commented Jul 29, 2022

JackBoosY Aug 2, 2022

JackBoosY Aug 2, 2022

JackBoosY Aug 2, 2022

omartijn commented Aug 9, 2022

JackBoosY commented Aug 19, 2022

JackBoosY commented Sep 16, 2022

[vcpkg-cmake][libpng] Add "OSX_SPLIT_BUILD" feature #22898

[vcpkg-cmake][libpng] Add "OSX_SPLIT_BUILD" feature #22898

Conversation

omartijn commented Feb 2, 2022

What does your PR fix?

Which triplets are supported/not supported? Have you updated the CI baseline?

Does your PR follow the maintainer guide?

If you have added/updated a port: Have you run ./vcpkg x-add-version --all and committed the result?

github-actions bot left a comment

Choose a reason for hiding this comment

omartijn commented Feb 3, 2022

github-actions bot left a comment

Choose a reason for hiding this comment

omartijn commented Feb 4, 2022

JackBoosY commented Feb 5, 2022

omartijn commented Feb 5, 2022

JackBoosY commented Feb 7, 2022

strega-nil-ms left a comment • edited Loading

Choose a reason for hiding this comment

JackBoosY commented Apr 12, 2022

omartijn commented Apr 12, 2022

JackBoosY commented Jun 29, 2022

JackBoosY commented Jul 8, 2022

omartijn commented Jul 15, 2022

JackBoosY commented Jul 18, 2022 • edited Loading

omartijn commented Jul 18, 2022

JackBoosY commented Jul 22, 2022

omartijn commented Jul 22, 2022

JackBoosY commented Jul 29, 2022

JackBoosY Aug 2, 2022

Choose a reason for hiding this comment

JackBoosY Aug 2, 2022

Choose a reason for hiding this comment

JackBoosY Aug 2, 2022

Choose a reason for hiding this comment

omartijn commented Aug 9, 2022

JackBoosY commented Aug 19, 2022

JackBoosY commented Sep 16, 2022

If you have added/updated a port: Have you run `./vcpkg x-add-version --all` and committed the result?

strega-nil-ms left a comment •

edited

Loading

JackBoosY commented Jul 18, 2022 •

edited

Loading