Skip to content

Qt6 build system prerequisites#4052

Merged
uklotzde merged 3 commits intomixxxdj:mainfrom
Be-ing:qt6_prereqs
Sep 16, 2021
Merged

Qt6 build system prerequisites#4052
uklotzde merged 3 commits intomixxxdj:mainfrom
Be-ing:qt6_prereqs

Conversation

@Be-ing
Copy link
Copy Markdown
Contributor

@Be-ing Be-ing commented Jul 3, 2021

A bit of refactoring before adding an option to build with Qt6 (#4051)

@github-actions github-actions Bot added the build label Jul 3, 2021
@Be-ing Be-ing force-pushed the qt6_prereqs branch 5 times, most recently from 3050953 to 2834755 Compare July 3, 2021 05:36
Comment thread CMakeLists.txt Outdated
@uklotzde
Copy link
Copy Markdown
Contributor

uklotzde commented Jul 4, 2021

Any ideas why the macOS build is failing now?

@Be-ing
Copy link
Copy Markdown
Contributor Author

Be-ing commented Jul 4, 2021

Not really, I still need to work on that

@Holzhaus Holzhaus marked this pull request as draft July 8, 2021 14:53
@Be-ing
Copy link
Copy Markdown
Contributor Author

Be-ing commented Jul 31, 2021

Maybe this should wait until switching to vcpkg for macOS dependencies with Qt statically linked.

@coveralls
Copy link
Copy Markdown

coveralls commented Aug 5, 2021

Pull Request Test Coverage Report for Build 1102806643

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 10 unchanged lines in 2 files lost coverage.
  • Overall coverage decreased (-0.01%) to 26.012%

Files with Coverage Reduction New Missed Lines %
src/engine/enginevumeter.cpp 1 90.24%
src/engine/cachingreader/cachingreader.cpp 9 71.38%
Totals Coverage Status
Change from base Build 1101888073: -0.01%
Covered Lines: 20006
Relevant Lines: 76910

💛 - Coveralls

@Be-ing Be-ing force-pushed the qt6_prereqs branch 2 times, most recently from 7e85af5 to 7dcbf12 Compare August 5, 2021 20:17
Comment thread CMakeLists.txt
That was needed because the old buildserver scripts installed Qt
to a separate prefix from the rest of Mixxx's dependencies. vcpkg
does not do that.
@Be-ing Be-ing marked this pull request as ready for review September 14, 2021 21:25
@Be-ing
Copy link
Copy Markdown
Contributor Author

Be-ing commented Sep 14, 2021

This is no longer failing on copying the Qt plugins into the macOS package now that we are linking Qt statically on macOS with vcpkg. Ready for review.

Comment thread CMakeLists.txt Outdated
install_qt5_plugin(Qt5::QGifPlugin BUNDLE_LIBS "${MIXXX_INSTALL_PREFIX}")
install_qt5_plugin(Qt5::QICOPlugin BUNDLE_LIBS "${MIXXX_INSTALL_PREFIX}")
install_qt5_plugin(Qt5::QSQLiteDriverPlugin BUNDLE_LIBS "${MIXXX_INSTALL_PREFIX}")
install_qt5_plugin(QCocoaIntegrationPlugin BUNDLE_LIBS "${MIXXX_INSTALL_PREFIX}")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this macro be renamed to install_qt_plugin without any version number?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we just remove it now that we're not using it?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The rest of the build system is written with the assumption that a dynamic build should continue to work.
In this sense I would keep it, even though, we can't verify it and it will suffer rotting.
But it will definitely be helpfull in case we want to switch to a dynamic build later.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be better to remove it. Yes, we need to support dynamic linking, but we don't need to do so for macOS.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@uklotzde what do you think about removing this no longer used code?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok. If it is not used then we should probably remove it. Less and simpler is better.

This is not needed since switching to vcpkg and linking Qt
statically.
Copy link
Copy Markdown
Contributor

@uklotzde uklotzde left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, thank you! LGTM

@uklotzde uklotzde merged commit 9fc8b6b into mixxxdj:main Sep 16, 2021
@Be-ing Be-ing deleted the qt6_prereqs branch September 16, 2021 12:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants