build: use a static .tiff for macOS .dmg rather than generating

fanquake commented at 2:05 pm on December 30, 2021: member

For demonstration, after [discussion in #23778](/bitcoin-bitcoin/23778/#issuecomment-1003005503), and the question as to why we can’t just have a background.tiff that we copy into the macOS DMG, and do away with the somewhat convoluted image generation steps.

From my understanding, the only reason we have this image generation as part of our build system is so that forks of Core can adapt the imagery for their own branding via PACKAGE_NAME. It don’t think it provides much value to us, and could just have a static .tiff that we copy into the dmg (replacing the .svg that currently lives in macdeploy/).

Doing this would eliminate the following build dependencies:

For native macOS:

sed (usage in Makefile.am)
librsvg (rsvg-convert)
tiffutil

Linux macOS cross-compile:

sed (usage in Makefille.am)
librsvg
tiffcp
convert (imagemagick)
font-tuffy

Guix Build:

0bash-5.1# find guix-build-$(git rev-parse --short=12 HEAD)/output/ -type f -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum
1c98d67796863f4b1bab0ad600d46bd74e744d94072cbd4bc856a6aeaba3bb329  guix-build-e09773d20a92/output/dist-archive/bitcoin-e09773d20a92.tar.gz
23336f90bab312798cb7665e2b4ae24d1a270fb240647d5fed8dbfcd83e3ed37e  guix-build-e09773d20a92/output/x86_64-apple-darwin/SHA256SUMS.part
38fd680c7ee158c64bad212385df7b0b302c6c2143d4e672b4b0eb5da41f9256d  guix-build-e09773d20a92/output/x86_64-apple-darwin/bitcoin-e09773d20a92-osx-unsigned.dmg
434f54177c2f0700e8cfaf5d85d91e404807cd9d411e22006cdff82653e5f4af2  guix-build-e09773d20a92/output/x86_64-apple-darwin/bitcoin-e09773d20a92-osx-unsigned.tar.gz
5da6b8f54ef755d40330c8eac4f5bd0329637e827be9ee61318600d5d0bdcc3dc  guix-build-e09773d20a92/output/x86_64-apple-darwin/bitcoin-e09773d20a92-osx64.tar.gz

dmg

fanquake added the label macOS on Dec 30, 2021

fanquake added the label Build system on Dec 30, 2021

hebasto commented at 2:21 pm on December 30, 2021: member

Concept ACK.

prusnak commented at 2:21 pm on December 30, 2021: contributor

In my take in #23908 I removed the “PACKAGE_NAME” from the SVG completely, since that was the only reason for the whole SVG dance.

Let’s do the same thing in this PR since the package name is already shown in the window title and under the icon (see examples below).

See https://github.com/bitcoin/bitcoin/pull/23908/files#diff-ed877ecc59088c3387feeabf1b8bb37da20b3101ea17c8e5d794a7b84f6ebba7 + we could also remove the <style>...</style> at the beginning which only defines the used font.

It is also worth keeping the background.svg in the Git even if it is not used directly.

Before:

After:

It is still quite obvious that Bitcoin Core is about to be installed even without the branding embedded in the background.

Feel free to take the resulting TIFF from https://github.com/prusnak/bitcoin/blob/827a590896acbfd5ef630aaa913e7ca37d1d5f5c/contrib/macdeploy/background.tiff

hebasto commented at 3:07 pm on December 30, 2021: member

It is also worth keeping the background.svg in the Git even if it is not used directly.

Agree. And document steps required to convert svg to tiff, that anyone could prove the correctness of the latter.

prusnak commented at 3:14 pm on December 30, 2021: contributor

that anyone could prove the correctness of the latter.

Here is the script I used to generate the TIFF from above:

0<background.svg rsvg-convert -f png -d 36 -p 36 -o background.png
1<background.svg rsvg-convert -f png -d 72 -p 72 -o background@2x.png
2optipng -o 7 background*.png  # optional
3tiffutil -cathidpicheck background.png background@2x.png -out background.tiff

fanquake marked this as ready for review on Dec 31, 2021

fanquake force-pushed on Dec 31, 2021

DrahtBot commented at 6:34 am on December 31, 2021: member

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#23778 (release: Guix 1.4.0 & GCC 10.3 by fanquake)
#21851 (build: support cross-compiling for arm64-apple-darwin 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.

prusnak approved

prusnak commented at 9:31 am on December 31, 2021: contributor

utACK

hebasto approved

hebasto commented at 4:00 pm on December 31, 2021: member

ACK 835c308c6a5b76f07d4e197ef01a4a16edc5e723 (see #23909 (comment)), compiled on Linux Mint 20.2 (x86_64) and on macOS Big Sur 11.6.2 (20G314, Intel).

Guix build:

0$ find guix-build-$(git rev-parse --short=12 HEAD)/output/ -type f -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum
1f98bd97ae7335ef3fd9fc1fd4f91359d99490cb630930248b844ea933453803c  guix-build-835c308c6a5b/output/dist-archive/bitcoin-835c308c6a5b.tar.gz
20e6e70da98ef7837f611dadbd08ab21b2c054f2442a19d1de30ad0835e5c393c  guix-build-835c308c6a5b/output/x86_64-apple-darwin/SHA256SUMS.part
38d36535b0c14144d860246271ccf6457dc7f5323d63a3393c2ac44d707c16add  guix-build-835c308c6a5b/output/x86_64-apple-darwin/bitcoin-835c308c6a5b-osx-unsigned.dmg
4abbc4f6567758dd8ccf2f83da64e386ad392bd7d5c9b61453cde3bde1085d8c0  guix-build-835c308c6a5b/output/x86_64-apple-darwin/bitcoin-835c308c6a5b-osx-unsigned.tar.gz
516e01a91d184455bee35f23d0a3b177328135beb9d5853669f64ac3f24e80f0c  guix-build-835c308c6a5b/output/x86_64-apple-darwin/bitcoin-835c308c6a5b-osx64.tar.gz

Installing bitcoin-835c308c6a5b-osx-unsigned.dmg on macOS Big Sur 11.6.2 (20G314, Intel): Screenshot from 2021-12-31 17-15-36

Zero-1729 approved

Zero-1729 commented at 4:46 pm on December 31, 2021: contributor

tACK 835c308

Compiled and tested on macOS v12.1 (21C52, M1)

MarcoFalke commented at 5:40 pm on December 31, 2021: member

Is there a reason to add a 30KB file to the repo? If yes, it would be good to explain why and how you generated it.

Locally I am getting 9KB (zip) or 22KB (lzw)

Commands:

0tiffcp -c lzw:2 /tmp/pngs/background{,2x}.tiff /tmp/pngs/background.lzw2.tiff
1tiffcp -c zip:p9 /tmp/pngs/background{,2x}.tiff /tmp/pngs/background.zip3p9.tiff

Concept ACK 835c308c6a5b76f07d4e197ef01a4a16edc5e723 🦌

Signature:

 0-----BEGIN PGP SIGNED MESSAGE-----
 1Hash: SHA512
 2
 3Concept ACK 835c308c6a5b76f07d4e197ef01a4a16edc5e723  🦌
 4-----BEGIN PGP SIGNATURE-----
 5
 6iQGzBAEBCgAdFiEE+rVPoUahrI9sLGYTzit1aX5ppUgFAlwqrYAACgkQzit1aX5p
 7pUhnbgv/Zbdg7Xzc8xVd7rZlgYaNx6YfMw1OVAhOr8ybZ1WSrN8vuXmgf+IhtI28
 8x/LosKocFciBnpfz5M8U94FMSL0ck2fOSUDdOMEctop5qmSkc2GVFzkvRwkKhMZq
 9d7fBFmYBcBL5G42liFxZBjIM1N/0ywZzEA5fHpvelCAMCm2uGAxigpy9zXlIaeQi
10DnHth+dfM7lsqRIQIcQGFVIbkzocVU6bpobgHKI7EHjtM9c4qY+QRdrtzNNxABwd
11ROyBFWTQInSTLRGWRYXxiXfbDL4tRP49ciu1VK8CzyWNKWvhVF6fa8J72ZDRT7pM
12XKRq+o27HGAmrxM1NKW6d2JQDWj6KClVdV8gJJG50fr/5P+xTtqFgCTOap2GHMi/
13Hlh9em0MAs6M5/X/MHLotny9lB9Savz40KuZZC7G2G7Isq+mVjDeg2yYAC4TFGdi
14H0CNkpUdXyPacIFbMsq6d2OaWDv0ZvU/GiVsroXJEQEP2p0WQlKJYszgaAheSvhF
15rQFMLbix
16=vah4
17-----END PGP SIGNATURE-----

fanquake force-pushed on Jan 1, 2022

fanquake commented at 9:28 am on January 1, 2022: member

it would be good to explain why and how you generated it.

It came from this branch: https://github.com/prusnak/bitcoin/blob/827a590896acbfd5ef630aaa913e7ca37d1d5f5c/contrib/macdeploy/background.tiff, so I’d assume these were the instructions. I’ve regenerated it now, and it’s about half the size (18KB).

I’ve also removed the .svg. It doesn’t need to remain in the tree, and if anyone wants it they can retrieve it from git (along with the conversion instructions in the makefile).

Added screenshot to PR description, and added @prusnak as co-author given he also opened a PR.

hebasto commented at 5:49 am on January 2, 2022: member

It seems no background in Guix’s bitcoin-*-osx-unsigned.dmg.

fanquake force-pushed on Jan 2, 2022

fanquake commented at 6:38 am on January 2, 2022: member

It seems no background in Guix’s bitcoin-*-osx-unsigned.dmg.

Should be fixed now.

hebasto approved

hebasto commented at 7:15 am on January 2, 2022: member

Approach ACK 692b38d2c47e83f384713961803b3df431eb32ee

Guix build:

0$ find guix-build-$(git rev-parse --short=12 HEAD)/output/ -type f -print0 | env LC_ALL=C sort -z | xargs -r0 ls -l
1-rw-r--r-- 1 hebasto hebasto 10760618 Jan  2 09:03 guix-build-692b38d2c47e/output/dist-archive/bitcoin-692b38d2c47e.tar.gz
2-rw-r--r-- 1 hebasto hebasto 35000686 Jan  2 09:10 guix-build-692b38d2c47e/output/x86_64-apple-darwin/bitcoin-692b38d2c47e-osx64.tar.gz
3-rw-r--r-- 1 hebasto hebasto 16053436 Jan  2 09:09 guix-build-692b38d2c47e/output/x86_64-apple-darwin/bitcoin-692b38d2c47e-osx-unsigned.dmg
4-rw-r--r-- 1 hebasto hebasto 16396639 Jan  2 09:09 guix-build-692b38d2c47e/output/x86_64-apple-darwin/bitcoin-692b38d2c47e-osx-unsigned.tar.gz
5-rw-r--r-- 1 hebasto hebasto      478 Jan  2 09:10 guix-build-692b38d2c47e/output/x86_64-apple-darwin/SHA256SUMS.part

Running bitcoin-692b38d2c47e-osx-unsigned.dmg:

Screenshot from 2022-01-02 09-12-44

hebasto commented at 7:20 am on January 2, 2022: member

CI failure seems related to building out of source tree.

hebasto commented at 7:30 am on January 2, 2022: member

Maybe

0--- a/configure.ac
1+++ b/configure.ac
2@@ -1861,6 +1861,7 @@ AC_CONFIG_LINKS([contrib/devtools/symbol-check.py:contrib/devtools/symbol-check.
3 AC_CONFIG_LINKS([contrib/devtools/test-security-check.py:contrib/devtools/test-security-check.py])
4 AC_CONFIG_LINKS([contrib/devtools/test-symbol-check.py:contrib/devtools/test-symbol-check.py])
5 AC_CONFIG_LINKS([contrib/filter-lcov.py:contrib/filter-lcov.py])
6+AC_CONFIG_LINKS([contrib/macdeploy/background.tiff:contrib/macdeploy/background.tiff])
7 AC_CONFIG_LINKS([test/functional/test_runner.py:test/functional/test_runner.py])
8 AC_CONFIG_LINKS([test/fuzz/test_runner.py:test/fuzz/test_runner.py])
9 AC_CONFIG_LINKS([test/util/test_runner.py:test/util/test_runner.py])

?

(did not test though)

build: use a static .tiff for macOS .dmg over generating

Co-authored-by: Pavol Rusnak <pavol@rusnak.io>

e09773d20a

fanquake force-pushed on Jan 2, 2022

luke-jr commented at 7:59 am on January 2, 2022: member

Concept NACK. There’s no benefit to this; it only serves to sabotage derivative projects.

As mentioned in #23778, there’s a straightforward solution to avoid the Rust dependency by simply not using the Rust-based librsvg…

hebasto commented at 8:06 am on January 2, 2022: member

@luke-jr

it only serves to sabotage derivative projects.

This statement is not true. See OP:

Doing this would eliminate the following build dependencies…

luke-jr commented at 8:11 am on January 2, 2022: member

Not really. You still need those to actually build it. Now you’re just injecting a pre-compiled TIFF into Guix, essentially reverting it back to where things were with gitian (except now the trusted “compiler” isn’t necessarily even reproducible).

And these aren’t even deps for an actual end-user build, only for producing the release artefacts.

jnewbery commented at 8:28 am on January 2, 2022: member

Concept ACK

hebasto approved

hebasto commented at 8:41 am on January 2, 2022: member

ACK e09773d20a9230ba7aa2cbb7e87fdc5187ddfec6

hebasto commented at 8:48 am on January 2, 2022: member

Reviewers could be interested in historical context: #7192.

prusnak commented at 10:23 am on January 2, 2022: contributor

There’s no benefit to this; it only serves to sabotage derivative projects.

This PR makes it actually easier for derivative projects because they do not need to fiddle with the installer background anymore.

luke-jr commented at 3:12 pm on January 2, 2022: member

This PR makes it actually easier for derivative projects because they do not need to fiddle with the installer background anymore.

They didn’t have to before…

fanquake commented at 1:22 am on January 3, 2022: member

Concept NACK. There’s no benefit to this;

I wouldn’t have opened a PR if there was no benefit to doing this. As others have mentioned, and as I laid out in the PR description, the benefit is a reduction in the number of dependencies you need to build Bitcoin Core for macOS. In general, if it’s possible to simplify a build process with no, or near zero downside (which I certainly think is the case here), then I think it’s something we should be doing.

it only serves to sabotage derivative projects.

Not even sure how to respond to this, but I really think you need to get some perspective. What’s changing here is outlined in the image below. Remembering that this is a view that’s only shown during the install procedure, for a single host, and likely, at most, for the few seconds it takes for a user to drag the app icon over to the folder. Note that the project name is still shown in two other places, so it’s not like this is removing the only branding / naming that was present in the DMG. How exactly is anything being sabotaged by this few second view only containing 2 instances of the project name, rather than 3?

both

Not really. You still need those to actually build it.

No, you don’t. Once the .tiff exists in .git, no one has to build it ever again, and thus doesn’t need to have those tools present in their build envinronment. That’s basically the point of this PR.

Now you’re just injecting a pre-compiled TIFF into Guix, essentially reverting it back to where things were with gitian

Comparing the copying of a .tiff into our Guix environment, as the equivalent of reverting back to gitian is absurd.

luke-jr commented at 2:52 am on January 3, 2022: member

the benefit is a reduction in the number of dependencies you need to build Bitcoin Core for macOS

You don’t need the tiff to build Core (only to package it, which is a special case where deps matter quite a bit less).

How exactly is anything being sabotaged by this few second view only containing 2 instances of the project name, rather than 3?

The description in the PR said the image was added unmodified. Having seen the actual tiff, I see the change, and agree it is less problematic than I originally understood it to be. (However, I still think this isn’t a good approach)

Comparing the copying of a .tiff into our Guix environment, as the equivalent of reverting back to gitian is absurd.

It’s like adding a compiled bitcoind binary to git and saying that means we don’t need GCC/LLVM anymore.

fanquake commented at 3:15 am on January 3, 2022: member

(only to package it, which is a special case where deps matter quite a bit less).

I agree it’s a special case, but not in the way you’re seemingly dismissing it. The makeup of our Guix build (release packaging) environment is our most important build environment, thus a very special case. I completely disagree with the notion that “deps matter quite a bit less” there, in fact, I think the opposite is true. In that environment, every additional dependency (and the dependencies of those dependencies) increases the scope for potential compromise, and/or tool/supply chain related attacks and are just more things we have to potentially maintain / vet / install. So I think any change that reduces the build requirements for that environment, is a good one. In this case, we can drop imagemagick, libtiff, librsvg and font-tuffy when performing the macOS cross-compile.

The description in the PR said the image was added unmodified.

Apologies, I’ll update the PR description to reflect the current change. However, there were at least 4 screenshots, showing the new DMG view, posted in this thread before you made your comments.

MarcoFalke added the label DrahtBot Guix build requested on Jan 3, 2022

jarolrod approved

jarolrod commented at 1:42 am on January 5, 2022: member

ACK e09773d20a9230ba7aa2cbb7e87fdc5187ddfec6

Clear benefit in this change.

Guix hashes:

0find guix-build-$(git rev-parse --short=12 HEAD)/output/ -type f -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum
1c98d67796863f4b1bab0ad600d46bd74e744d94072cbd4bc856a6aeaba3bb329  guix-build-e09773d20a92/output/dist-archive/bitcoin-e09773d20a92.tar.gz
23336f90bab312798cb7665e2b4ae24d1a270fb240647d5fed8dbfcd83e3ed37e  guix-build-e09773d20a92/output/x86_64-apple-darwin/SHA256SUMS.part
38fd680c7ee158c64bad212385df7b0b302c6c2143d4e672b4b0eb5da41f9256d  guix-build-e09773d20a92/output/x86_64-apple-darwin/bitcoin-e09773d20a92-osx-unsigned.dmg
434f54177c2f0700e8cfaf5d85d91e404807cd9d411e22006cdff82653e5f4af2  guix-build-e09773d20a92/output/x86_64-apple-darwin/bitcoin-e09773d20a92-osx-unsigned.tar.gz
5da6b8f54ef755d40330c8eac4f5bd0329637e827be9ee61318600d5d0bdcc3dc  guix-build-e09773d20a92/output/x86_64-apple-darwin/bitcoin-e09773d20a92-osx64.tar.gz