Skip to content
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

deps.qt: Remove Qt5 and update Qt6 build scripts #188

Merged
merged 5 commits into from
Jun 15, 2023

Conversation

PatTheMav
Copy link
Member

Description

Removes Qt5 builds and simplifies Qt6 build matrix to only provide the following builds:

  • macOS Universal (Release + dSYMs)
  • Windows x64 (RelWithDebInfo + PDBs)

Motivation and Context

Improve CI efficiency and simplify build matrix for OBS Studio builds.

How Has This Been Tested?

Built Qt6 on Windows 11 for x64 and arm64.

Types of changes

  • Code cleanup (non-breaking change which makes code smaller or more readable)

Checklist:

  • My code has been run through clang-format.
  • I have read the contributing document.
  • My code is not on the master branch.
  • The code has been tested.
  • All commit messages are properly formatted and commits squashed where appropriate.
  • I have included updates to all appropriate documentation.

@PatTheMav PatTheMav force-pushed the qt-cleanup branch 5 times, most recently from 96c640b to 3895870 Compare May 31, 2023 23:41
@PatTheMav PatTheMav force-pushed the qt-cleanup branch 2 times, most recently from 45373a6 to 5d635dd Compare June 1, 2023 01:39
@derrod
Copy link
Member

derrod commented Jun 1, 2023

With cmake 3.0, will we still need 32 bit deps for the remaining 32 bit components (game capture, vcam, etc.) or could they be stripped down further?

@PatTheMav
Copy link
Member Author

With cmake 3.0, will we still need 32 bit deps for the remaining 32 bit components (game capture, vcam, etc.) or could they be stripped down further?

That's implemented in #174.

@PatTheMav PatTheMav marked this pull request as ready for review June 1, 2023 13:09
@PatTheMav PatTheMav force-pushed the qt-cleanup branch 19 times, most recently from 65e8d69 to 6ac25b3 Compare June 3, 2023 23:43
Copy link
Member

@RytoEX RytoEX left a comment

Choose a reason for hiding this comment

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

Nit.

deps.qt/qt6.ps1 Outdated Show resolved Hide resolved
@PatTheMav PatTheMav force-pushed the qt-cleanup branch 9 times, most recently from 3342c5d to 1280333 Compare June 13, 2023 15:34
Copy link
Member

@RytoEX RytoEX left a comment

Choose a reason for hiding this comment

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

CI: Remove Qt5 and not longer needed architectures from Qt builds

not -> no

Some questions, some nits.

Local compile tests worked.

.github/actions/package-windows-qt/action.yml Outdated Show resolved Hide resolved
.github/actions/package-windows-qt/action.yml Outdated Show resolved Hide resolved
.github/actions/package-windows-qt/action.yml Outdated Show resolved Hide resolved
.github/actions/package-windows-qt/action.yml Outdated Show resolved Hide resolved
@PatTheMav PatTheMav force-pushed the qt-cleanup branch 2 times, most recently from b0cf71a to 93e5672 Compare June 13, 2023 23:43
@PatTheMav
Copy link
Member Author

Down to nits. Will test locally shortly.

I noticed that there is scheduled.yaml, but the other files are all .yml. GitHub seems to go with .yml. Is there a particular reason for using .yaml there?

IIRC the standard suggests using yaml by default and I think that's what Sublime Text uses by default. I intend to move all files to yaml at some point anyway, but this just fell through the cracks.

@PatTheMav PatTheMav force-pushed the qt-cleanup branch 3 times, most recently from 76ccbc2 to 5e029c1 Compare June 14, 2023 13:43
@PatTheMav PatTheMav force-pushed the qt-cleanup branch 2 times, most recently from 8578106 to af3b971 Compare June 15, 2023 13:30
Copy link
Member

@RytoEX RytoEX left a comment

Choose a reason for hiding this comment

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

Some of the commits are bit heavier or more entangled with other changes than I prefer (sometimes with cosmetic changes), but it's probably more work than it's worth to split this up further or reorganize the commits. Looks good to me otherwise, and local testing showed no further issues when building with these artifacts.

@PatTheMav PatTheMav changed the title deps.qt: Remove Qt5 and simplify Qt6 build matrix deps.qt: Remove Qt5 and update Qt6 build scripts Jun 15, 2023
@RytoEX RytoEX merged commit cc19125 into obsproject:master Jun 15, 2023
@PatTheMav PatTheMav deleted the qt-cleanup branch June 15, 2023 22:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants