-
Notifications
You must be signed in to change notification settings - Fork 6
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
vcpkg for macOs #18
vcpkg for macOs #18
Conversation
if(${VCPKG_OSX_DEPLOYMENT_TARGET} GREATER "10.14") # Max Version supported by QT | ||
message(STATUS "Qt 5.12.4 only support OSX_DEPLOYMENT_TARGET up to 10.14") | ||
set(VCPKG_OSX_DEPLOYMENT_TARGET "10.14") | ||
if(${VCPKG_OSX_DEPLOYMENT_TARGET} GREATER "10.15") # Max Version supported by QT. This version is defined in mkspecs/common/macx.conf as QT_MAC_SDK_VERSION_MAX |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this change needed if we use 10.12 as the deployment target?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is bugfix from upstream.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm unclear where it comes from. Is it backported from a later Qt version upstream?
Somehow uploading the logs of the Windows build broke:
|
|
why is this is still 2.3 ? |
I guess there are non, because building is skipped due to caching. |
The vcpkg archive does not include cmake nor ccache, so I am still using those binaries from the old build environment. |
Thank you for rebasing and cleaning up the commit messages. It looks like the new custom triplet is not working unfortunately. |
There is no need to take care of these in this PR, but I would like to ship ccache and cmake with our macOS dependencies so macOS developers only need to download one archive besides the Mixxx source code. I do not know if upstream would accept packages for those because vcpkg is focused on libraries, but it is worth asking. |
.github/workflows/build.yml
Outdated
@@ -62,7 +62,7 @@ jobs: | |||
wavpack | |||
${{ matrix.vcpkg_packages_extras }} | |||
VCPKG_DEFAULT_TRIPLET: ${{ matrix.vcpkg_triplet }} | |||
VCPKG_OVERLAY_TRIPLETS: overlay/triplets | |||
#VCPKG_OVERLAY_TRIPLETS: overlay/triplets does not work |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You could open an issue upstream to request this feature.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is make in your path? |
OK, It is building again. Merge? |
I would like to get this working locally or in a PR before merging. There may be changes needed in this branch to make it work still. |
Yes of cause there are probably remaining issues. Wen we have the environment produced here on our download server, I can also continue to work on it fixing the mixxx repro or this one. I think this was the way we have developed the Windows version as well. |
If you make a branch on the mixxxdj/vcpkg repo, GitHub Actions will upload the build to downloads.mixxx.org which can be used for testing a PR in the mixxxdj/mixxx repo. |
Why do you think we need a branch? We have the same with less hassle when we just merge. |
We can rebase a branch as needed until we merge it. |
Please explain the benefit? |
a cleaner git history There is no rush to merge until we know it works IMO. Do you have permission to push a new branch to this repo? |
I don't have interest to do more work for a clean history. This has gone already through a clean up which took me some What is your current build output? Does installing make fix the issue? |
Using an relative path allows to use the original overlays/triplets folder. |
I think it's okay to merge as-is. This PR builds fine and there is no regression risk because we didn't use vcpkg for macOS until now. If further changes are necessary to make macOS build with vcpkg, the git history may as well reflect that. |
4be5121
to
8eb5abb
Compare
OK, I consider this as ready to merge now. It was tested together with Mixxx here: |
Has it been tested by a macOS user? |
My PR to upstream the fdk-aac fork without HE-AAC was merged. I will take care of removing it from our overlay after merging this. |
Changing the default config in this overlay is required, because other required ports like harfbuz depend on the default configuration of this library
This allows using the port with imported targes form the mixxx module folder Fixes: microsoft#18442
Looks good to me as soon as we get confirmation from a macOS user that it is working. |
We have the confirmation that it works in terms of using the new libraries. I think we can merge and fix the found issues in the mixxx repro, linking to this new environment. |
Alright, then I'll merge this now so that multiple people can contribute fixes if necessary instead of putting all the weight on @daschuer's shoulders. It won't make a difference for users/developers until we change the buildenv in the mixxx repo anyway. |
This builds an environment for MacOS
Traget specific overlays have been moved to overlays/x64-osc and overlays/x64-win
The qt version of MacOs is 5.12.4, Windows is on 5.15.2