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.

Sign up for GitHub

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

Jump to bottom

[Qt] Update to 6.8.0 #41534

Merged
merged 36 commits into from
Nov 22, 2024
Merged

[Qt] Update to 6.8.0 #41534

merged 36 commits into from
Nov 22, 2024

Conversation

Neumann-A
Copy link
Contributor

@Neumann-A Neumann-A commented Oct 12, 2024 •
edited
Loading

  • qtmultimedia:x64-linux fails due to gstreamer pc file containing a non-existant path.
  • qtaplicationmanager:x64-linux: fails due to a missing systemd header? (Solved by not building the watchdog)
  • qtaplicationmanager:arm64-windows: fails due to empty dirs
  • Downstream errors in x64-windows-static-* needs to be investigated (due to open62541)
  • open62541 needs a new release.
  • tinyorm needs a newer release

@xavier2k6 xavier2k6 mentioned this pull request Oct 12, 2024
Closed
@WangWeiLin-MV WangWeiLin-MV added the category:port-update The issue is with a library, which is requesting update new revision label Oct 14, 2024
@BillyONeal
Copy link
Member

/azp run

@BillyONeal
Copy link
Member

Rerunning due to network mistake.

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@xavier2k6
Copy link
Contributor

Closes #41441.

@Neumann-A Neumann-A marked this pull request as ready for review November 3, 2024 17:30
@nickdademo
Copy link
Contributor

Is this on-hold for the planned 6.8.1 release on Nov 21?

@Neumann-A
Copy link
Contributor Author

Is this on-hold for the planned 6.8.1 release on Nov 21?

No just waiting for review.

@nlogozzo
Copy link
Contributor

Is this on-hold for the planned 6.8.1 release on Nov 21?

No just waiting for review.

@BillyONeal

@dg0yt
Copy link
Contributor

dg0yt commented Nov 12, 2024

Actually @WangWeiLin-MV.

@freedbrt
Copy link

Failing on "./vcpkg install qt" on MacOS

vcpkg % ./vcpkg install qt
Computing installation plan...
qtmultimedia[gstreamer] is only supported on 'linux', which does not match arm64-osx. This usually means that there are known build failures, or runtime problems, when building other platforms. To ignore this and attempt to build qtmultimedia anyway, rerun vcpkg with `--allow-unsupported`.

Also after build everything, it is impossible to add qmake to Qt Creator. It says - "Qt version is not properly installed, please run make install"

WangWeiLin-MV
WangWeiLin-MV reviewed Nov 19, 2024
View reviewed changes
Copy link
Contributor

@WangWeiLin-MV WangWeiLin-MV left a comment

Choose a reason for hiding this comment

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

All versions/* checked with vcpkg x-add-version

ports/qtapplicationmanager/portfile.cmake
Comment on lines +67 to +80

file(GLOB_RECURSE qttools "${CURRENT_PACKAGES_DIR}/tools/Qt6/bin/*")
if(NOT qttools AND VCPKG_CROSSCOMPILING)
file(REMOVE_RECURSE "${CURRENT_PACKAGES_DIR}/tools/Qt6/bin/")
endif()

if(VCPKG_TARGET_IS_WINDOWS AND VCPKG_CROSSCOMPILING AND VCPKG_TARGET_ARCHITECTURE STREQUAL "arm64")
file(REMOVE_RECURSE
"${CURRENT_PACKAGES_DIR}/bin/"
"${CURRENT_PACKAGES_DIR}/debug/bin/"
"${CURRENT_PACKAGES_DIR}/tools/"
)
endif()

Copy link
Contributor

Choose a reason for hiding this comment

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

Would this result in having to native-compiling for arm64 Windows?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You should rather ask if VCPKG_TARGET_ARCHITECTURE STREQUAL "arm64" can be removed and if the error also exists if you crosscompile from arm64 to x64 but i cannot answer that question since I cannot test it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Why is here check target architecture and delete these files? My local build takes a lot of time so can't test quickly.

Copy link
Contributor

@WangWeiLin-MV WangWeiLin-MV Nov 21, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

The local build of arm64-windows on x64-windows passed after removing this section.

Copy link
Contributor

Choose a reason for hiding this comment

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

The local build of arm64-windows on x64-windows passed after removing this section.

Did you have any failed local post-build checks? Removing these directories was made due to failed Azure pipeline: https://dev.azure.com/vcpkg/public/_build/results?buildId=108756&view=logs&jobId=84da4f4a-968a-5b5c-16e9-f444c845396d&j=84da4f4a-968a-5b5c-16e9-f444c845396d&t=410b6c71-3bf9-5b7a-84d3-17ae677a92aa

P.S. Native arm64-windows support can be improved in subsequent pull request (provided that anyone even cares about it). I think I'm not the only one looking forward to this getting merged for x64-windows, and *-linux.

Copy link
Contributor

Choose a reason for hiding this comment

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

Did you have any failed local post-build checks?

Yes, it passed locally.

Removing these directories was made due to failed Azure pipeline: https://dev.azure.com/vcpkg/public/_build/results?buildId=108756&view=logs&jobId=84da4f4a-968a-5b5c-16e9-f444c845396d&j=84da4f4a-968a-5b5c-16e9-f444c845396d&t=410b6c71-3bf9-5b7a-84d3-17ae677a92aa

Thanks for the information, I will look into this failure later.

ports/qtbase/GLIB2-static.patch Show resolved Hide resolved
ports/qtbase/qmake-arm64.patch Show resolved Hide resolved
ports/qtbase/fix-missing-symbol.patch Show resolved Hide resolved
ports/qtmultimedia/ffmpeg-compile-def.patch
Copy link
Contributor

Choose a reason for hiding this comment

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

Recommended reporting to upstream https://bugreports.qt.io/browse/QTBUG-126817

ports/qtwebengine/blink-include-fixes.patch
Copy link
Contributor

Choose a reason for hiding this comment

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

The source code has been modified, requires upstream confirmation or source link.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is an include (order?) fix and doesn't require upstream confirmation according to the maintainer guidelines.
Don't ask me how upstream even compiled this since without this it runs static_assert assert that the forward declared classes are incomplete.

Copy link
Member

Choose a reason for hiding this comment

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

I'm sorry, I missed this:

This is an include (order?) fix and doesn't require upstream confirmation according to the maintainer guidelines.

That's not correct. https://learn.microsoft.com/vcpkg/contributing/maintainer-guide#notify-upstream-owners-for-upstream-relevant-patches

We will skip this waiting period if we have high confidence that the change is correct. Examples of high confidence patches include, but are not limited to:
[...]
*Adding missing #includes.

We can merge stuff without upstream having accepted that does this, but there is still duty to notify.

WangWeiLin-MV
WangWeiLin-MV approved these changes Nov 22, 2024
View reviewed changes
Copy link
Contributor

@WangWeiLin-MV WangWeiLin-MV left a comment

Choose a reason for hiding this comment

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

The port qt installation tests pass with the following triplets:

  • arm64-windows
  • x64-windows

@WangWeiLin-MV WangWeiLin-MV added the info:reviewed Pull Request changes follow basic guidelines label Nov 22, 2024
@BillyONeal BillyONeal merged commit ed03146 into microsoft:master Nov 22, 2024
16 checks passed
@BillyONeal
Copy link
Member

Thanks for the update!

@Neumann-A Neumann-A deleted the update_qt_6.8.0 branch November 22, 2024 22:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vserdyuk vserdyuk vserdyuk left review comments

@BillyONeal BillyONeal BillyONeal left review comments

@WangWeiLin-MV WangWeiLin-MV WangWeiLin-MV approved these changes

Assignees

@WangWeiLin-MV WangWeiLin-MV

Labels
category:port-update The issue is with a library, which is requesting update new revision info:reviewed Pull Request changes follow basic guidelines
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
9 participants
@Neumann-A @BillyONeal @xavier2k6 @nickdademo @nlogozzo @dg0yt @freedbrt @vserdyuk @WangWeiLin-MV