prusnak
commented at 9:34 am on March 25, 2022:
contributor
build, qt: bump Qt5 version to 5.15.3
Qt 5.15.3 release is a patch release made on the top of Qt 5.15.2. As a patch
release, Qt 5.15.3 does not add any new functionality but provides bug fixes
and other improvements.
MarcoFalke added the label
DrahtBot Guix build requested
on Mar 25, 2022
prusnak
commented at 10:00 am on March 25, 2022:
contributor
Are there explicit benefits for our project?
…
UPDATE: Could dont_use_avx_android_x86_64.patch and fix_limits_header.patch be dropped now?
Right, we can now drop the following (upstreamed) patches:
dont_use_avx_android_x86_64.patch
fix_limits_header.patch
fix_bigsur_style.patch
I force pushed c959d296c879e9f628504cb070c26bcbbbf12926 which drops these patches.
I went through the release notes and there are macOS rendering fixes included in the release. Some of them were backported in fix_bigsur_style.patch but maybe there are more …
in
depends/packages/qt.mk:21
in
c959d296c8outdated
I went through the release notes and there are macOS rendering fixes included in the release. Some of them were backported in fix_bigsur_style.patch but maybe there are more …
I’ve verified that all changes in our fix_bigsur_style.patch are backported into 5.15.3:
hebasto
commented at 1:19 pm on March 25, 2022:
member
Some notes about depends/patches/qt/fix_limits_header.patch:
Apparently, patching of qtbase/src/tools/moc/generator.cpp was required only for Qt 5.12.11. It even is not required on the current master branch for Qt 5.15.2. Therefore, it seems safe to drop this diff from the patch, although the line https://github.com/qt/qtbase/blob/v5.15.3-lts-lgpl/src/tools/moc/generator.cpp#L1621 still exists:
I’d like to discuss whether we should backport the https://github.com/qt/qtbase/commit/813a928c7c3cf98670b6043149880ed5c955efb9 commit entirely, not just a diff for src/corelib/text/qbytearraymatcher.h. At least, should we add a diff for src/plugins/platforms/xcb/qxcbwindow.cpp? OTOH, I have not observed any build failures with the current implementation.
hebasto
commented at 1:31 pm on March 25, 2022:
member
DrahtBot
commented at 10:06 pm on March 25, 2022:
contributor
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
Conflicts
Reviewers, this pull request conflicts with the following ones:
#24586 (doc: add more info to dependencies.md by prusnak)
#22708 (build, qt: Add Wayland support for Linux builds with depends by hebasto)
#21778 (build: LLVM 14 & LLD based macOS toolchain by fanquake)
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.
promag
commented at 11:16 am on March 26, 2022:
member
Concept ACK.
hebasto
commented at 11:42 am on March 26, 2022:
member
Was thinking again about Qt security patches, and I believe it would be better to split the relevant discussion into separated issues / PRs.
Therefore, only (1) and (2) are remaining from my #24668 (comment).
prusnak
commented at 11:50 am on March 26, 2022:
contributor
Was thinking again about Qt security patches, and I believe it would be better to split the relevant discussion into separated issues / PRs.
8d83d31 - applied CVE fixes mentioned earlier in the thread
~Note: We might want to backport 8d83d31 into 23.x branch. Right now, it applies cleanly with no conflicts.~ edit: not needed, see below #24668 (comment)
DrahtBot
commented at 2:39 pm on March 27, 2022:
contributor
DrahtBot removed the label
DrahtBot Guix build requested
on Mar 27, 2022
DrahtBot added the label
Build system
on Mar 27, 2022
DrahtBot added the label
Needs rebase
on Mar 29, 2022
prusnak force-pushed
on Mar 29, 2022
prusnak
commented at 9:29 pm on March 29, 2022:
contributor
Rebased
DrahtBot removed the label
Needs rebase
on Mar 29, 2022
fanquake
commented at 8:28 am on March 30, 2022:
member
Updating to 5.15.3 and dropping some patches is fine. Not a priority unless there are some important fixes that we aren’t already patching.
It’s not clear to me why we need these QProcess CVE patches, or why they would need to be backported:
CVE-2022-25255 | https://codereview.qt-project.org/c/qt/qtbase/+/393113
We have a single usage of QProcess in our codebase (macOS only). Looking at the abandoned patch for the 5.15 branch, from what I can tell, this only affects the case where you pass QProcess a binary blah, and it needs to lookup a full path. Given that we pass /usr/bin/open, we shouldn’t fall into either if (!program.contains(QLatin1Char('/'))) { block, which is where the patching is happens, so it’s not clear how we are affected by this CVE.
prusnak
commented at 8:53 am on March 30, 2022:
contributor
It’s not clear to me why we need these … CVE patches
My rationale was that it’s better to have these known Qt vulnerabilities patched defensively now in case we start using the affected functionality some time in the future and no-one will realise that the otherwise perfectly fine change will enable these vulnerabilities.
If that sounds over the top, I am fine with dropping 18588c1 from the PR.
And of course it’s nice to know that older releases are not affected and 18588c1 does not need to be backported.
DrahtBot added the label
Needs rebase
on Apr 1, 2022
build, qt: bump Qt5 version to 5.15.3
Qt 5.15.3 release is a patch release made on the top of Qt 5.15.2. As a patch
release, Qt 5.15.3 does not add any new functionality but provides bug fixes
and other improvements.
https://code.qt.io/cgit/qt/qtreleasenotes.git/about/qt/5.15.3/release-note.md
* dropped patches:
- patches/qt/dont_use_avx_android_x86_64.patch
- patches/qt/fix_bigsur_style.patch
* adjusted patches:
- patches/qt/fix_android_jni_static.patch
- patches/qt/fix_limits_header.patch
- patches/qt/use_android_ndk23.patch
Co-authored-by: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com>
ef20add4c9
build, qt: drop fix_no_printer.patch
The removed patch is not required since switching Qt version from
5.12.11 to 5.15.2.
826cbc470f
build, qt: use one patch per line in depends/packages/qt.mk7f6042849c
prusnak force-pushed
on Apr 1, 2022
prusnak
commented at 3:03 pm on April 1, 2022:
contributor
Rebased
DrahtBot removed the label
Needs rebase
on Apr 1, 2022
fanquake
commented at 2:45 pm on April 2, 2022:
member
hebasto
commented at 4:56 pm on April 2, 2022:
member
If that sounds over the top, I am fine with dropping 18588c1 from the PR.
I think that’s what we can do for now.
Agree. Another concerns about the CVE patches are that it is not clear (at least for me) how they are maintained. I cannot see them in the Qt repository, there are no discussion in https://bugreports.qt.io etc.
Btw, I’m working on removing Qprocess from our codebase.
nit: In 7f6042849c976ef1c7ac90c6aa8982ee1492a4bf“build, qt: use one patch per line in depends/packages/qt.mk” suggesting to place patches, and correspondent patch calls, in lexicographical order to be consistent with same ordering of files. (although, not sure if the latter won’t break something).
prusnak force-pushed
on Apr 2, 2022
hebasto
commented at 5:06 pm on April 2, 2022:
member
This is a metadata mirror of the GitHub repository
bitcoin/bitcoin.
This site is not affiliated with GitHub.
Content is generated from a GitHub metadata backup.
generated: 2024-11-21 12:12 UTC
This site is hosted by @0xB10C More mirrored repositories can be found on mirror.b10c.me
