[vcpkg osx] Fix building for non-x86_64 archs on macOS (OSX) and fixes escapes for some params#12657
[vcpkg osx] Fix building for non-x86_64 archs on macOS (OSX) and fixes escapes for some params#12657Deadpikle wants to merge 7 commits intomicrosoft:masterfrom
Conversation
Use VCPKG_TARGET_ARCHITECTURE for CMAKE_OSX_ARCHITECTURES as well This fixes osx triplets that want to build for an arch other than x86_64
| @@ -0,0 +1,5 @@ | |||
| set(VCPKG_TARGET_ARCHITECTURE x86_64 arm64) | |||
There was a problem hiding this comment.
I think this should be:
| set(VCPKG_TARGET_ARCHITECTURE x86_64 arm64) | |
| set(VCPKG_TARGET_ARCHITECTURE x64 arm64) |
| endif() | ||
|
|
||
| # make sure to escape multiple architectures in VCPKG_TARGET_ARCHITECTURE | ||
| string(REPLACE ";" "\\;" _VCPKG_TARGET_ARCHITECTURE "${VCPKG_TARGET_ARCHITECTURE}") |
There was a problem hiding this comment.
And then here:
| string(REPLACE ";" "\\;" _VCPKG_TARGET_ARCHITECTURE "${VCPKG_TARGET_ARCHITECTURE}") | |
| string(REPLACE ";" "\\;" _VCPKG_TARGET_ARCHITECTURE "${VCPKG_TARGET_ARCHITECTURE}") | |
| string(REPLACE "x64" "x86_64" _VCPKG_TARGET_ARCHITECTURE "${_VCPKG_TARGET_ARCHITECTURE}") |
thus, we actually write the vcpkg names for these targets, as opposed to the cmake names. I'm not sure if this is the right way, but it makes sense to me... cc @ras0219-msft ?
There was a problem hiding this comment.
The reason I think this is that in x86-windows, we set VCPKG_TARGET_ARCHITECTURE to x86, not Win32.
There was a problem hiding this comment.
Also, this should break x64-osx, so these suggestions will fix it; otherwise, we'll have to change x64-osx.
| "-DVCPKG_CRT_LINKAGE=${VCPKG_CRT_LINKAGE}" | ||
| "-DVCPKG_LINKER_FLAGS=${VCPKG_LINKER_FLAGS}" | ||
| "-DVCPKG_TARGET_ARCHITECTURE=${VCPKG_TARGET_ARCHITECTURE}" | ||
| "-DVCPKG_TARGET_ARCHITECTURE=${_VCPKG_TARGET_ARCHITECTURE}" |
There was a problem hiding this comment.
Should this be passed only if !TARGET_IS_OSX?
If you changed VCPKG_TARGET_ARCHITECTURE to use x86_64, prettify failed with the error .
|
After reviewing this code and examining the various uses and cases for For example, how do we handle the many existing libraries that check the target architecture to add target-specific code (such as assembly files)? This is an issue in CMake as much as it is in other buildsystems. I don't think it is realistic to assert that every buildsystem and every library will be patchable to support lipo'd binaries -- and even a single link in the chain breaks the entire system. There are other obvious issues, such as where do you stuff flags specifically for one or the other target? Maybe CMake has generator expressions, but again, what about autoconf/make/bazel/meson/etc? Until we have a credible solution for these issues, I don't see how vcpkg can properly support universal binaries. I suspect a solution will at a minimum require vcpkg to invoke separate underlying builds for each architecture (similar to how we have separate release and debug builds), and then owning the Therefore, because any short-term solution is going to be a hack, I think that the design here should adjust away from how to properly support it (since I do not think we have answers to all the above questions at this time) and instead focus on what exactly @Deadpikle is looking to achieve in their specific situation. Because we already support tl;dr:
[1] |
|
Thanks for the feedback on the PR and for taking a deeper look. I think it's safe to say that the PR could probably be closed/shelved and then the following action items taken:
This would keep things fairly separated for now and avoid getting into the deeper issues you've noted. Note: I can do without items 2 and 3 for my use case -- I can manually set up my dependency building script to |
|
The main concern for me is that it's really important that this gets done before the full ARM macOS devices are released. We need to make sure that we get some engineering time on this. |
So, focusing on that specifically, how do we get that fixed? Does it just need to have I would really like to have the list fixes; those are just goodness. I think it would be ideal to open a new PR with just those changes (list fixes + whatever is needed to fix |
I believe that's what was needed, yes. I will take a look at this again on Monday and verify that unless something else comes up and will submit new PR(s) as needed to keep things clean. |
If you changed VCPKG_TARGET_ARCHITECTURE to use x86_64, the prettify command failed to parse things properly. This was found during the changes made for microsoft#12657 and was completed by @strega-nil (thanks!).
e.g., if you set multiple architectures for VCPKG_OSX_ARCHITECTURES, the arguments were not escaped properly. This PR fixes that. The code from this PR was pulled from microsoft#12657 and was done entirely by @strega-nil.
OK, yes, all that was needed to fix the
With those PRs, I should be able to do everything I want to do with my own triplets or whatnot. I see that there's some discussion above, so I'm hesitant to close this PR directly until that discussion is done. Really appreciate all who have been involved thus far. :) |
If you changed VCPKG_TARGET_ARCHITECTURE to use x86_64, the prettify command failed to parse things properly. This was found during the changes made for #12657 and was completed by @strega-nil (thanks!).
If you changed VCPKG_TARGET_ARCHITECTURE to use x86_64, the prettify command failed to parse things properly. This was found during the changes made for microsoft#12657 and was completed by @strega-nil (thanks!).
|
Closing since we have other PRs merged that do the same thing; thanks so much @Deadpikle for your contributions! |
I discovered upon some tests with Xcode 12 that the
arm64-osxtriplet was not correct and didn't actually build forarm64based on the output oflipo -archs mylib.a. After a long conversation in Discord, we discovered a few issues and made a few fixes:-DCMAKE_OSX_ARCHITECTURESshould be set/used in order to properly tell cmake which architectures to build for. Based on the conversation in Discord, it was decided to setCMAKE_OSX_ARCHITECTURESbased on the value ofVCPKG_TARGET_ARCHITECTUREif the target was iOS or macOS.VCPKG_TARGET_IS_IOSdefinition was added.VCPKG_TARGET_ARCHITECTUREforarm64withx86_64, a new community triplet was added.In theory, now you can run commands like
./vcpkg install libzip[core] --triplet x64_arm64-osxand get a build that, with alipo -archs libzip.acheck, will contain bothx86_64andarm64code.CI:
No CI changes have been made.
Special thanks
Many thanks to @strega-nil and @ras0219-msft for their large amount of help in figuring things out in Discord. Thank you!