Depends: Fix Qt's rcc determinism #13732

pull Fuzzbawls wants to merge 1 commits into bitcoin:master from Fuzzbawls:btc_fix-rcc-determinism changing 5 files +21 −1
  1. Fuzzbawls commented at 2:00 PM on July 21, 2018: contributor

    With the update to Qt 5.9 having been merged, Qt's rcc tool now embeds a file's last modified time in it's output. Since the build system generates temporary files for all locale translations (*.qm files) at build time, the resulting qrc_bitcoin_locale.cpp file was always being generated in a non-deterministic way.

    This is a backport of https://bugreports.qt.io/browse/QTBUG-62511, which is included in Qt versions 5.11+, that allows for an environment variable (QT_RCC_SOURCE_DATE_OVERRIDE) to override the behavior described above. This environment variable is in turn set in the gitian descriptors, as that is where determinism is vital for release purposes.

    Prior to this, the qt_libbitcoinqt_a-qrc_bitcoin_locale.o object file (included into libbitcoinqt.a) was returning a different sha256sum for each and every build, regardless of file contents change, thus breaking determinism in the resulting binaries.

    This should fix #13731

  2. fanquake requested review from theuni on Jul 21, 2018
  3. fanquake added the label Build system on Jul 21, 2018
  4. fanquake added this to the milestone 0.17.0 on Jul 21, 2018
  5. ken2812221 commented at 2:05 PM on July 21, 2018: contributor

    I assume that this can be solved by adding rcc into faketime wrapper.

  6. MarcoFalke added the label Needs gitian build on Jul 21, 2018
  7. MarcoFalke commented at 5:41 PM on July 21, 2018: member

    @DrahtBot Have some fun here!

  8. practicalswift commented at 5:50 PM on July 21, 2018: contributor

    Concept ACK. Nice first time-contribution. Thanks!

    Determinism is our friend!

  9. DrahtBot removed the label Needs gitian build on Jul 21, 2018
  10. Fuzzbawls commented at 10:26 PM on July 21, 2018: contributor

    adding rcc to the list of faketime wrapped binaries is problematic in that the rcc binary location is also defined in the share/config.site file that gets used for gitian builds, which does not use the $PATH environment variable, and the build system isn't really set up to allow for separate paths for each individual Qt tool.

    I don't think that would be a viable alternative anyways, as we would then need to ensure that the last modified timestamp of every generated *.qm file is static (faketime again?).

    There is an alternative that is less code than this, but I'm not sure if it would be acceptable either as it could potentially interfere with non-gitian builds:

    --- src/Makefile.qt.include
+++ src/Makefile.qt.include
@@ -430,7 +430,7 @@
 $(QT_QRC_LOCALE_CPP): $(QT_QRC_LOCALE) $(QT_QM)	[@test](/bitcoin-bitcoin/contributor/test/) -f $(RCC)	[@cp](/bitcoin-bitcoin/contributor/cp/) -f $< $(@D)/temp_$(<F)
-	$(AM_V_GEN) QT_SELECT=$(QT_SELECT) $(RCC) -name bitcoin_locale $(@D)/temp_$(<F) | \
+	$(AM_V_GEN) QT_SELECT=$(QT_SELECT) $(RCC) --format-version 1 -name bitcoin_locale $(@D)/temp_$(<F) | \
 	  $(SED) -e '/^\*\*.*Created:/d' -e '/^\*\*.*by:/d' > $@	[@rm](/bitcoin-bitcoin/contributor/rm/) $(@D)/temp_$(<F)

    This is a runtime option to rcc that forces the older format version to be used, which specifically does not encode file timestamps into the result. There was some concern about using such an override however in the Qt bug report's discussion thread.

  11. MarcoFalke added the label Needs gitian build on Jul 22, 2018
  12. DrahtBot removed the label Needs gitian build on Jul 22, 2018
  13. MarcoFalke commented at 2:20 AM on July 22, 2018: member

    Indeed, looks like the results are stable now.

  14. DrahtBot commented at 1:17 PM on July 22, 2018: member

    <!--e57a25ab6845829454e8d69fc972939a-->Note to reviewers: This pull request conflicts with the following ones:

    • #13710 ([depends] Add riscv qt depends support for cross compiling bitcoin-qt by ken2812221)
    • #13696 (Add aarch64 qt depends support for cross compiling bitcoin-qt by TheCharlatan)
    • #13665 ([WIP][build] Add risc-v support to gitian by ken2812221)

    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.

  15. laanwj commented at 4:13 PM on July 22, 2018: member

    Concept ACK Thanks a lot for figuring this out.! In the longer run, it would be a good idea to get deterministic-build support pushed upstream (@theuni has some experience doing this for binutils).

  16. Fuzzbawls commented at 11:02 PM on July 22, 2018: contributor

    @laanwj In this particular case, upstream Qt did indeed "fix" the issue, but only for versions 5.11+. Even though the QTBUG was closed and marked invalid, a proposed change addressing the issue was merged. (https://github.com/qt/qtbase/commit/38271e9298dcf48652a6e2e08414a940a97867fa)

    I agree though, it would be great if the patch made it into Qt's 5.9 branch and a 5.9.7 version released so it wouldn't need to be explicitely applied in depends here. Until then, however, I think this is the best that can be done while still keeping Qt 5.9.

  17. in depends/packages/qt.mk:137 in 927bb45455 outdated 
     132 | @@ -133,6 +133,7 @@ define $(package)_preprocess_cmds
 133 |    patch -p1 -i $($(package)_patch_dir)/fix_qt_pkgconfig.patch &&\
 134 |    patch -p1 -i $($(package)_patch_dir)/fix_configure_mac.patch &&\
 135 |    patch -p1 -i $($(package)_patch_dir)/fix_no_printer.patch &&\
 136 | +  patch -p1 -i $($(package)_patch_dir)/fix_rcc_determinism.patch &&\
    theuni commented at 11:43 PM on July 22, 2018:

    We need to export the variable here as well so that non-gitian builds get stable results. See below where QT_RCC_TEST is set to 1.

    Fuzzbawls commented at 5:44 AM on July 23, 2018:

    Noted, will add this soon

    Fuzzbawls commented at 8:13 AM on July 24, 2018:

    swapped out the build_env variable further up in this file and didn't notice any break in gitian results determinism.

  18. in contrib/gitian-descriptors/gitian-linux.yml:46 in 927bb45455 outdated 
      42 | @@ -43,6 +43,7 @@ script: |
  43 |    HOST_LDFLAGS=-static-libstdc++
  44 |  
  45 |    export QT_RCC_TEST=1
  46 | +  export QT_RCC_SOURCE_DATE_OVERRIDE=1
    theuni commented at 11:44 PM on July 22, 2018:

    QT_RCC_TEST was the variable for this in the past. If QT_RCC_SOURCE_DATE_OVERRIDE is the new one, let's just replace QT_RCC_TEST. I assume there's no need for both.

    Same goes for the other descriptors as well.

    Fuzzbawls commented at 8:14 AM on July 24, 2018:

    removed QT_RCC_TEST=1 from gitian descriptors and didn't notice any break in gitian results determinism.

  19. theuni commented at 11:52 PM on July 22, 2018: member

    @laanwj Yes, ideally qt's configure would have an option for disabling timestamps by default, which is quickly becoming the norm for host-side build tools.

  20. Fuzzbawls force-pushed on Jul 24, 2018
  21. Fuzzbawls commented at 8:22 AM on July 24, 2018: contributor

    Rebased and pushed to incorporate feedback from @theuni.

    One thing of note: While gitian builds are now deterministic, non-gitian builds that use --prefix= or CONFIG_SITE= pointing at a depends path are only deterministic if they also set QT_RCC_SOURCE_DATE_OVERRIDE=1 at compile time

    CONFIG_SITE=/home/fuzzbawls/bitcoin/depends/x86_64-unknown-linux/share/config.site ./configure
QT_RCC_SOURCE_DATE_OVERRIDE=1 make

    some further tweaking to depends/config.site.in and/or depends/funcs.mk...or configure.ac may be in order to automatically export that environment variable for non-gitian builds, but that may be broadening the scope just a bit too much here given the upcoming release cycle.

  22. theuni commented at 6:10 PM on July 24, 2018: member

    @Fuzzbawls Sorry to do this to you, but I just had a look at the qt source, and it turns out that RCC_TEST has a totally different function: it serves as the seed for a hash function that ends up dictating the order of file entries. So my request above was completely wrong, we need both :(

    Would you mind putting it back? utACK after that.

  23. Fix Qt's rcc determinism for depends/gitian
    Backport of https://bugreports.qt.io/browse/QTBUG-62511 to resolve
locale determinism during the build process.
    6b5506a286
  24. Fuzzbawls force-pushed on Jul 25, 2018
  25. Fuzzbawls commented at 11:04 AM on July 26, 2018: contributor

    @theuni Thanks...I had a feeling it may still be needed...just hadn't had the time to do extensive build testing. I've reverted to re-add the QT_RCC_TEST=1 env vars where appropriate and so far gitian builds are looking nice and consistent.

    There is still an outstanding issue of non-gitian builds (that use a depends target) producing inconsistent results with RCC...perhaps that can be it's own issue/PR?

  26. jonasschnelli commented at 12:51 PM on July 27, 2018: contributor

    Looks good. Gitian Build: https://bitcoin.jonasschnelli.ch/build/719

  27. MarcoFalke added the label Needs gitian build on Jul 28, 2018
  28. MarcoFalke deleted a comment on Jul 28, 2018
  29. MarcoFalke deleted a comment on Jul 28, 2018
  30. DrahtBot commented at 1:35 AM on July 29, 2018: member

    <!--a722867cd34abeea1fadc8d60700f111-->

    Gitian builds for commit ef4fac0ea5b4891f4529e4b59dfd1f7aeb3009b5 (master):

    Gitian builds for commit c2fbdcd7d926841e34d5d5b24be3da781fbc48fc (master and this pull):

  31. DrahtBot removed the label Needs gitian build on Jul 29, 2018
  32. MarcoFalke added the label Needs gitian build on Jul 29, 2018
  33. MarcoFalke commented at 1:52 AM on July 29, 2018: member

    utACK 6b5506a28632b57c0c75c7cc66c0bc35d419b682

  34. DrahtBot commented at 12:02 PM on July 29, 2018: member

    <!--a722867cd34abeea1fadc8d60700f111-->

    Gitian builds for commit ef4fac0ea5b4891f4529e4b59dfd1f7aeb3009b5 (master):

    Gitian builds for commit c2fbdcd7d926841e34d5d5b24be3da781fbc48fc (master and this pull):

  35. DrahtBot removed the label Needs gitian build on Jul 29, 2018
  36. MarcoFalke merged this on Jul 29, 2018
  37. MarcoFalke closed this on Jul 29, 2018
  38. MarcoFalke referenced this in commit e8ffec69f7 on Jul 29, 2018
  39. Fuzzbawls deleted the branch on Jan 10, 2020
  40. PastaPastaPasta referenced this in commit 746cb90cb7 on Jul 29, 2020
  41. UdjinM6 referenced this in commit bccd313d7f on Dec 17, 2020
  42. fanquake referenced this in commit 8837f1ebde on Jun 3, 2021
  43. gades referenced this in commit 9bf5d61d1f on Jun 24, 2021
  44. CryptoCentric referenced this in commit 6f217b953c on Jul 2, 2021
  45. DrahtBot locked this on Feb 15, 2022
Contributors
Fuzzbawlsken2812221MarcoFalkepracticalswiftDrahtBotlaanwjtheunijonasschnelli
Labels
Build system

Milestone
0.17.0

Linked (view graph)
#13731 Gitian builds no longer deterministic#21654 build, qt: Make Qt rcc output always deterministic
github-metadata-mirror

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: 2026-04-17 06:15 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me