build: Remove unnecessary executables from gitian release #7776

pull laanwj wants to merge 1 commits into bitcoin:master from laanwj:2016_03_gitian_release_cleanup changing 4 files +15 −12
  1. laanwj commented at 12:56 pm on March 31, 2016: member

    This removes the following executables from the binary gitian release:

    • test_bitcoin-qt[.exe]
    • bench_bitcoin[.exe] @jonasschnelli and me discussed this on IRC a few days ago - unlike the normal bitcoin_tests which is useful to see if it is safe to run bitcoin on a certain OS/environment combination, there is no good reason to include these. Better to leave them out to reduce the download size.

    Sizes from the 0.12 release:

    02.4M bitcoin-0.12.0/bin/bench_bitcoin.exe
1 22M bitcoin-0.12.0/bin/test_bitcoin-qt.exe

    This adds an option --disable-gui-tests to the build system.

  2. laanwj added the label Build system on Mar 31, 2016
  3. laanwj added the label Needs backport on Mar 31, 2016
  4. MarcoFalke commented at 1:03 pm on March 31, 2016: member
    utACK 8e25246
  5. btcdrak commented at 1:10 pm on March 31, 2016: contributor
    utACK 8e25246
  6. jonasschnelli commented at 1:32 pm on March 31, 2016: contributor
    utACK 8e25246
  7. cdecker commented at 2:27 pm on March 31, 2016: contributor
    utACK 8e25246
  8. theuni commented at 5:05 pm on March 31, 2016: member
    No objections to the changes here, but I think we also want https://github.com/theuni/bitcoin/commit/72946e173b27e8664e2c822729763c38831931e7 ? That prevents them from being installed by ‘make install’ under any config.
  10. laanwj commented at 12:20 pm on April 1, 2016: member

    No objections to the changes here, but I think we also want theuni@72946e1 ?

    I think it’s a bit inconsistent - what if people want them installed? In any case I think it’s a separate change with a separate goal to this, probably should do it as a separate PR.

  11. laanwj commented at 6:39 am on April 2, 2016: member

    I think it’s a bit inconsistent - what if people want them installed?

    A consistent policy there would be “do not install the tests at all”. But then we’d have to take special care to put test_bitcoin into the distribution :( I mean it’s harder to make a case for the build system (which is also used by other distributions which may have other choices) that that one is special, than on the gitian descriptor level. Let’s leave it for another PR anyhow.

    Testing this in gitian now.

  12. theuni commented at 7:08 am on April 2, 2016: member
    Yep, i suppose I agree. ut ACK.
  13. laanwj commented at 8:50 am on April 2, 2016: member
    Linux works UHM I mean doesn’t work, builds but the stupid files are still there :horse: [snip] May have been using the wrong descriptors, retrying…
  14. laanwj commented at 9:50 am on April 2, 2016: member

    It’s not properly disabling test_bitcoin-qt. And this is probably the reason:

    0$ git grep BUILD_TEST_QT
1configure.ac:    BUILD_TEST_QT="test"
2configure.ac:AC_SUBST(BUILD_TEST_QT)

    E.g. we don’t actually use BUILD_TEST_QT anywhere.

  15. build: Remove unnecessary executables from gitian release 
    This removes the following executables from the binary gitian release:

- test_bitcoin-qt[.exe]
- bench_bitcoin[.exe]

@jonasschnelli and me discussed this on IRC a few days ago - unlike the
normal `bitcoin_tests` which is useful to see if it is safe to run
bitcoin on a certain OS/environment combination, there is no good reason
to include these. Better to leave them out to reduce the download
size.

Sizes from the 0.12 release:
```
2.4M bitcoin-0.12.0/bin/bench_bitcoin.exe
 22M bitcoin-0.12.0/bin/test_bitcoin-qt.exe
```
    f063863d1f
  16. laanwj force-pushed on Apr 3, 2016
  17. laanwj commented at 1:15 pm on April 3, 2016: member

    @theuni Pushed a new version which does work. Can you take a look? I removed a few AC_SUBST( which weren’t used at all. But used these variables, which are set at the place where the log message is printed,

    0  BUILD_TEST_QT=""
1  ...
2  AC_MSG_CHECKING([whether to build test_bitcoin-qt])
3  if test x$use_gui_tests$bitcoin_enable_qt_test = xyesyes; then
4    AC_MSG_RESULT([yes])
5    BUILD_TEST_QT="yes"
6  else

    and then use them at the end:

    0AM_CONDITIONAL([ENABLE_TESTS],[test x$BUILD_TEST = xyes])
1AM_CONDITIONAL([ENABLE_QT_TESTS],[test x$BUILD_TEST_QT = xyes])

    I think this is clearer - no need to repeat the logic. But maybe I’m doing something completely against autoconf gospels I don’t know….

  18. theuni commented at 6:08 pm on April 4, 2016: member

    @laanwj Looks ok to me, and works as described in a quick test.

    The one case that’s unclear is “–disable-tests –enable-gui-tests”. As-is it built test_bitcoin-qt but not bitcoin_test. That’s fine by me (I don’t really care either way which one overrides), but it might help to make the help string more clear that the gui tests don’t imply non-gui tests.

  19. laanwj commented at 10:38 am on April 5, 2016: member

    The one case that’s unclear is “–disable-tests –enable-gui-tests”. As-is it built test_bitcoin-qt but not bitcoin_test. That’s fine by me

    That was the intended behavior, yes. This allows the maximum flexibility: if you want the GUI tests but not normal tests, or normal tests but not GUI tests, both is possible. Could try to improve the help string.

    Edit: On second thought

    do not compile GUI tests

    I’m not sure how I would go around making this clearer.

  20. laanwj merged this on Apr 5, 2016
  21. laanwj closed this on Apr 5, 2016
  22. laanwj referenced this in commit 3cc0fb3a23 on Apr 5, 2016
  23. laanwj referenced this in commit a784675a32 on Apr 5, 2016
  24. zander referenced this in commit 163d4bd2f2 on Apr 22, 2016
  25. laanwj referenced this in commit 7bbb81842f on Jun 8, 2016
  26. laanwj referenced this in commit 74c1347482 on Jun 9, 2016
  27. schinzelh referenced this in commit bccc3b9053 on Jun 20, 2016
  28. laanwj removed the label Needs backport on Sep 26, 2016
  29. laanwj added this to the milestone 0.12.2 on Sep 26, 2016
  30. laanwj added the label Needs backport on Sep 26, 2016
  31. zkbot referenced this in commit 4ee9d712b5 on Oct 17, 2016
  32. EvgenijM86 referenced this in commit 0c5360bfb0 on Apr 25, 2017
  33. fanquake removed the label Needs backport on Mar 7, 2018
  34. MarcoFalke locked this on Sep 8, 2021


laanwj MarcoFalke btcdrak jonasschnelli cdecker theuni fanquake

Labels
Build system

Milestone
0.12.branch

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: 2024-11-18 03:12 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me