Skip to content

Conversation

fanquake
Copy link
Member

This is an alternative to #19751 that fixes the build without requiring
splitting out libpng. This patch can be dropped once we are building qt
5.12.0 or later.

One of the concerns posted in #19751 was:

the one bundled with Qt is at best a slower "bare minimum".

However for our usage, I don't think the performance of libpng is a concern. If it is, I'd like to see some numbers/justification as to why we should be worried about it.

This patch should be enough to unblock PowerPC binaries (combined with other changes) for 0.21.0, and for 0.22.0, when we switch to qt 5.15.x in depends, we should be able to drop it.

Related upstream issue: https://bugreports.qt.io/browse/QTBUG-66388.

This is an alternative to bitcoin#19751 that fixes the build without requiring
splitting out libpng. This patch can be dropped once we are building qt
5.12.0 or later.
@laanwj
Copy link
Member

laanwj commented Sep 15, 2020

ACK f07fb5a
Given that this solves the underlying problem I prefer this to #19751, in the meantime until it's fixed upstream, so that we can have PowerPC builds for 0.21 with minimal change to the build.

Copy link
Member

@hebasto hebasto left a comment

Choose a reason for hiding this comment

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

@DrahtBot
Copy link
Contributor

Guix builds

File commit 62e3eb9
(master)
commit ab1e6a0
(master and this pull)
*-aarch64-linux-gnu-debug.tar.gz b8b48f5bb0bffe8c... 47c974fb9db879c4...
*-aarch64-linux-gnu.tar.gz c79d3fac7ab8e4f7... 9a3b02f00332776e...
*-arm-linux-gnueabihf-debug.tar.gz 3cc302892ee5f5e3... 265887dc1699748d...
*-arm-linux-gnueabihf.tar.gz 72947a73cad667c4... a82e683de7414916...
*-riscv64-linux-gnu-debug.tar.gz 725c5393abc85a70... 251b3c9d8dd1a022...
*-riscv64-linux-gnu.tar.gz 9e37fecf626fd945... 9ddb1880b0181736...
*-win-unsigned.tar.gz d1272e48baac0e83... 58a62aabb54c68aa...
*-win64-debug.zip 981d4ea93187a2af... 1329c0cd82e37eae...
*-win64-setup-unsigned.exe 1abeb4bcb649cea1... 0199a03f2fdc91a6...
*-win64.zip 83f0f7220202a32b... b5ddadc89e0ea818...
*-x86_64-linux-gnu-debug.tar.gz ab68763b90fb5692... 5fba233283e0913a...
*-x86_64-linux-gnu.tar.gz 0f705864412f280f... c2730218b3d6a826...
*.tar.gz 36c9eb16e5611b4a... 120d0afcb84d790b...
guix_build.log 7a20e928ad9e247f... bd7d3868a7f92aa6...
guix_build.log.diff 3f364c2affe48cdf...

@DrahtBot
Copy link
Contributor

Gitian builds

File commit be3af4f
(master)
commit d3c4766
(master and this pull)
*-osx-unsigned.dmg 704c36f4ca9a4091... b585de337f34967d...
*-osx64.tar.gz 10347bc435144ea1... e914670d4a8b169e...
*-win64-debug.zip 735eec7b6ed71052... d1a384bbd3603a33...
*-win64-setup-unsigned.exe b96aa53196a1779c... de9716257ce2be21...
*-win64.zip c424da189c9d569b... dfe3ab8e0ed13510...
*.tar.gz 7ab768625c41ffab... e6776295d514d6cc...
bitcoin-core-osx-0.21-res.yml 0537c1e5ddfffd45... 4dd87ef6f3883cd6...
bitcoin-core-win-0.21-res.yml 29a2a585d04d069f... f5a801beca4bfd47...
osx-build.log 780dfcd971c2f84c... eafc1fe089ef97a3...
win-build.log 9cc185c27bd8871a... 4e13cdfeb04ebd79...
bitcoin-core-linux-0.21-res.yml 5b3744e6e90a42b8...
bitcoin-core-osx-0.21-res.yml.diff 3dfed4acc402b858...
bitcoin-core-win-0.21-res.yml.diff a9e0cf0a3fb1c3cf...
*-aarch64-linux-gnu-debug.tar.gz 91d173949bebd9aa...
*-aarch64-linux-gnu.tar.gz eb5eff3933ceb407...
*-arm-linux-gnueabihf-debug.tar.gz a5cbfb04c6c880f0...
*-arm-linux-gnueabihf.tar.gz db1f07b6b96dcb9e...
*-riscv64-linux-gnu-debug.tar.gz 8ab06280f4057a91...
*-riscv64-linux-gnu.tar.gz e7edb81331dd11b9...
*-x86_64-linux-gnu-debug.tar.gz 5d1c73345c2ac3f6...
*-x86_64-linux-gnu.tar.gz 470713a950def7e7...
linux-build.log e501237b194ebedc...
osx-build.log.diff a3aadb6631dadcdd...
win-build.log.diff 1af46d8334afc502...

@DrahtBot
Copy link
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:

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.

@DrahtBot
Copy link
Contributor

Gitian builds

File commit 38fd1bd
(master)
commit 4a6f18a
(master and this pull)
*-aarch64-linux-gnu-debug.tar.gz 688ec58564d3ac7c... 1e82ceb3f05710f9...
*-aarch64-linux-gnu.tar.gz 204e92550eff5846... 3781ca8db6d7254f...
*-arm-linux-gnueabihf-debug.tar.gz 93ffd2f93c319e24... 74e0d96d7fd5a0a5...
*-arm-linux-gnueabihf.tar.gz ff0b0cd6ee565db1... 2723f6285002acd0...
*-osx-unsigned.dmg 1066f9e17cb9ce94... 358ace0c14084b9c...
*-osx64.tar.gz 119394084c33e66a... 6364eb45c2ea2408...
*-riscv64-linux-gnu-debug.tar.gz ea79d00064315c13... c035c31179feb351...
*-riscv64-linux-gnu.tar.gz c16fb78920cb0a32... 1b552b32ef3d1280...
*-win64-debug.zip c4421e8c69a53e92... b1e4310749fe05a0...
*-win64-setup-unsigned.exe 7ae2a2d34266d148... 2bba13e1de43ee43...
*-win64.zip d7529f52b3c1b4e1... 23c18c795cdbc11b...
*-x86_64-linux-gnu-debug.tar.gz df73f0011f35afed... 1c95a4489a85ac84...
*-x86_64-linux-gnu.tar.gz 8c32cc396fb2d95d... 40f8bc83c212c6ca...
*.tar.gz 872eb855ca8cf4f6... eae84401de8e3ab4...
bitcoin-core-linux-0.21-res.yml f36c5d93ff84f3e0... 7a5c13acbbded5ae...
bitcoin-core-osx-0.21-res.yml 33666789ee1b395d... 296930ab54a0a6b0...
bitcoin-core-win-0.21-res.yml 326cfa268f817c2d... c22dfc7446070209...
linux-build.log 303e2ab80599f2c0... 78ee72525ebe8fa5...
osx-build.log d3a2739fb322e488... 41387a0418eacd1a...
win-build.log 82bd0fdb349b2a52... 932de4739b90943d...
bitcoin-core-linux-0.21-res.yml.diff 86c217b021ffbef3...
bitcoin-core-osx-0.21-res.yml.diff f44e7672d01e9ef5...
bitcoin-core-win-0.21-res.yml.diff 506a52227e2d0fe7...
linux-build.log.diff 5d214021e08dd5e6...
osx-build.log.diff 949bc0518cf27813...
win-build.log.diff aa83fb3faff1508a...

@theuni
Copy link
Member

theuni commented Sep 22, 2020

I was fine with the approach from #19751 of moving libpng out, but it's tough to argue against a straightforward one-line patch from upstream.

ACK f07fb5a.

@maflcko maflcko merged commit c7eb85d into bitcoin:master Sep 22, 2020
@fanquake fanquake deleted the powerpc_libpng_qt branch September 22, 2020 23:53
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Sep 23, 2020
f07fb5a build: patch qt libpng to fix powerpc build (fanquake)

Pull request description:

  This is an alternative to bitcoin#19751 that fixes the build without requiring
  splitting out libpng. This patch can be dropped once we are building qt
  5.12.0 or later.

  One of the [concerns posted in bitcoin#19751](bitcoin#19751 (comment)) was:

  > the one bundled with Qt is at best a slower "bare minimum".

  However for our usage, I don't think the performance of libpng is a concern. If it is, I'd like to see some numbers/justification as to why we should be worried about it.

  This patch should be enough to unblock PowerPC binaries (combined with other changes) for 0.21.0, and for 0.22.0, when we switch to qt 5.15.x in depends, we should be able to drop it.

  Related upstream issue: https://bugreports.qt.io/browse/QTBUG-66388.

ACKs for top commit:
  laanwj:
    ACK f07fb5a
  theuni:
    ACK f07fb5a.
  hebasto:
    ACK f07fb5a, the patch is the same as https://codereview.qt-project.org/gitweb?p=qt/qtbase.git;a=blobdiff;f=src/3rdparty/libpng/libpng.pro;h=a2f56669b47e09629b7a88a0964091404d6a9b06;hp=577b61d833f7842f3f0d6b1c94a3b3920006695c;hb=dc2aead842f4cdf74f9259d3606c53c8bdae2c6b;hpb=ceeecbae510af6e2d1ebbf865761e4761d404033

Tree-SHA512: 865b843f5049eca80215774274fb7ae0dacccc3dd7f4a2eec93a73057115dcea85e715f919f96441424f9dd902dd97f0a238d96d4074babcee66b30577104009
kwvg added a commit to kwvg/dash that referenced this pull request Nov 24, 2021
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Feb 15, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants