-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Use 2.4 vcpkg branch for MacOS and Windows #4225
Conversation
Copied from Be-ing/portaudio
Thank you very much for working on this. I will review it soon. |
Pull Request Test Coverage Report for Build 1159589324
💛 - Coveralls |
7c86ae3
to
fccfc4e
Compare
libdjinterop is in vcpkg now. We should add it to the build environments. But that is no regression from the old environment so we don't need to fix it in this PR. |
There are a lot of these linker warnings from Qt. Are they a problem? |
Regarding the compiler warnings I give up. We have tests, that this build is working and the warnings are IMHO not critical. Maybe be get a responds from my question issued here: microsoft/vcpkg#19699 |
I think we can leave the link warnings as a to-do card on the 2.4 project board. I want to test building this locally on macOS before merging though. |
I made a PR for your branch: daschuer#68 |
I have not reproduced this link error locally. |
This link error did not occur on my PR for your branch. |
It should occur, because NSAppearanceNameDarkAqua is only available from macOS 10.14 but we wan't make Mixxx run on macOS 10.12. |
I think all that is left is to update the vcpkg ZIP archive to the build from merging mixxxdj/vcpkg#24 then we can merge this. |
I don't think so. CMake uses the MACOSX_DEPLOYMENT_TARGET environment variable to set the CMAKE_OSX_DEPLOYMENT_TARGET CMake variable. So I don't think there is any functional diffrence whether one or the other is used. |
Hmm, you are right. Somehow CMAKE_OSX_DEPLOYMENT_TARGET is not getting set to 10.12. If you look at the end of the Configure step in the most recent build where CMake prints all the CMake cache variables you can see For the build that failed it was |
Here is another PR to get CMake to actually set the deployment target to 10.12: daschuer#69 |
Before, GitHub Actions was caching both the ZIP and its extracted contents which took about 2GB.
It seems that CMake sets this to a default value, so checking if it is already set does not work.
CMake: always set CMAKE_OSX_DEPLOYMENT_TARGET=10.12
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.
Let's merge this now and add libdjinterop to the vcpkg archive when my PR is merged upstream. I don't think that will take too long so I don't think we need to bother adding it to the overlay for this temporary issue.
Do you know why we have no linker issues anymore. That is suspicious. I have stated a new vcpkg build without caching in the 2.3.macos branch. |
I do not know why the link errors occurred on that one build. But they have not happened since and I have not seen them in any of my local builds. So I am not too concerned. |
I did a local build with the environment variable
|
By the way, the old Macbook Air I was using before is no longer useful for building Mixxx because its version of clang is too old to support some of the C++17 features we use. :( I tried updating it to macOS 10.13.6 which supports a newer version of XCode which I think should support C++17. It said it was updating the OS but it never actually does. :/ Thanks for making a functioning computer into trash, Apple. But now my roommate has an iMac which is quite fast (faster than GitHub Actions' macOS runners). |
I can confirm that this is still building with a re-build vcpkg. It was original failing because of QT's 10.14 calls when using the 10.13 SDK Now we are using the 11.1 SDK on a macOS 10.15 runner. Interestingly the vcpkg is built with the "native' 10.15 SDK. I have no clue where this difference happens. But anyway, our goal a Mixxx binary running on a macOS 10.12. @Be-ing: can you confirm this with your abandoned Macbook Air? Please also explain how the calls to more recent calls are handled? Does Mixxx than require to update some libraries to their 11.1 version? |
After reading this message, QT checks it with
This has failed originally, because the QT library was build statically with 10.15 where this function exists, while Mixxx uses only 10.13 (sdk download) without this function. Since we link the function statically, it should run also on an older macOS, we have "only" the issue that this is not supported by QT Which already warns using 10.15 but we finally use 11.1
Is this and issue or not? We need to test. |
I tried to confirm this but unfortunately that Macbook Air is now stuck in a boot loop attempting to update macOS :( |
I don't think so. I have not noticed any issues with the current state of this branch nor has anyone else reported issues. We may be able to use So I think we should stop messing with it and merge this branch as it is now that we have a working setup. |
Anything left to do? Merge? |
Yes, go ahead. |
Thank you very much for your work on this tedious task. This puts us on a much more maintainable setup. It is great that we can now take care of both macOS and Windows with one tool. This is already useful for SoundTouch and libdjinterop. |
This PR does the switch and makes it work for macOS and windows.