Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adjust configure so that only bip70 is disabled when protobuf is missing instead of the GUI #14564

Merged
merged 1 commit into from Dec 6, 2018

Conversation

Projects
None yet
6 participants
@jameshilliard
Copy link
Contributor

commented Oct 25, 2018

This change ensures that the GUI is still built even if protobuf is missing unless --enable-bip70 is passed to configure. If protobuf is present bip70 support will be compiled in unless --disable-bip70 is passed.

@Sjors

This comment has been minimized.

Copy link
Member

commented Oct 25, 2018

tACK 3852aff on macOS 10.14

With protobuf installed:
with protobuf

Without protobuf installed:
without

Without protobuf installed, with --enable-bip70:
schermafbeelding 2018-10-25 om 11 53 26

(when checking out this branch for the first time, you need a make (dist?)clean in order to see the bip70 configure option)

@jameshilliard

This comment has been minimized.

Copy link
Contributor Author

commented Oct 25, 2018

I think ./autogen.sh will also get you the configure option.

@fanquake

This comment has been minimized.

Copy link
Member

commented Oct 26, 2018

testing 3852aff:

with protobuf and ./configure:

with gui / qt = yes
  with bip70  = yes
  with qr     = yes
with zmq      = yes

with protobuf and ./configure --disable-bip70:

with gui / qt = yes
  with bip70  = no
  with qr     = yes
with zmq      = yes

without protobuf and ./configure:

with gui / qt = yes
  with bip70  = no
  with qr     = yes
with zmq      = yes

without protobuf and ./configure --enable-bip70:

checking whether to build test_bitcoin-qt... yes
checking whether to build BIP70 support... configure: error: protobuf missing
@DrahtBot

This comment has been minimized.

Copy link
Contributor

commented Oct 27, 2018

Gitian builds for commit f4e4ea1 (master):

Gitian builds for commit 5d78c857f3117cacaea4275b474c164e3e88bbd2 (master and this pull):

@MarcoFalke

This comment has been minimized.

Copy link
Member

commented Oct 27, 2018

Gitian fails with

  CXXLD    test/test_bitcoin.exe
  GEN      qt/forms/ui_addressbookpage.h
  GEN      qt/forms/ui_askpassphrasedialog.h
  GEN      qt/forms/ui_coincontroldialog.h
  GEN      qt/forms/ui_editaddressdialog.h
  GEN      qt/forms/ui_helpmessagedialog.h
  GEN      qt/forms/ui_intro.h
  GEN      qt/forms/ui_modaloverlay.h
  GEN      qt/forms/ui_openuridialog.h
  GEN      qt/forms/ui_optionsdialog.h
  GEN      qt/forms/ui_overviewpage.h
  GEN      qt/forms/ui_receivecoinsdialog.h
  GEN      qt/forms/ui_receiverequestdialog.h
  GEN      qt/forms/ui_debugwindow.h
  GEN      qt/forms/ui_sendcoinsdialog.h
  GEN      qt/forms/ui_sendcoinsentry.h
  GEN      qt/forms/ui_signverifymessagedialog.h
  GEN      qt/forms/ui_transactiondescdialog.h
  GEN      qt/paymentrequest.pb.h
/bin/bash: --cpp_out=qt: command not found
Makefile:10790: recipe for target 'qt/paymentrequest.pb.h' failed
make[2]: *** [qt/paymentrequest.pb.h] Error 127
make[2]: Leaving directory '/home/ubuntu/build/bitcoin/distsrc-i686-w64-mingw32/src'
Makefile:10248: recipe for target 'all-recursive' failed
make[1]: *** [all-recursive] Error 1
make[1]: Leaving directory '/home/ubuntu/build/bitcoin/distsrc-i686-w64-mingw32/src'
Makefile:774: recipe for target 'all-recursive' failed
make: *** [all-recursive] Error 1

@MarcoFalke MarcoFalke closed this Oct 27, 2018

@MarcoFalke MarcoFalke reopened this Oct 27, 2018

@MarcoFalke

This comment has been minimized.

Copy link
Member

commented Oct 27, 2018

Travis failure:

In file included from ./qt/paymentserver.h:40:0,
                 from qt/bitcoin.cpp:25:
./qt/paymentrequestplus.h:10:10: fatal error: qt/paymentrequest.pb.h: No such file or directory
 #include <qt/paymentrequest.pb.h>
          ^~~~~~~~~~~~~~~~~~~~~~~~
compilation terminated.
Makefile:8804: recipe for target 'qt/qt_bitcoin_qt-bitcoin.o' failed
make[2]: *** [qt/qt_bitcoin_qt-bitcoin.o] Error 1
make[2]: Leaving directory '/home/travis/build/bitcoin/bitcoin/build/bitcoin-i686-w64-mingw32/src'
Makefile:10248: recipe for target 'all-recursive' failed
make[1]: Leaving directory '/home/travis/build/bitcoin/bitcoin/build/bitcoin-i686-w64-mingw32/src'
make[1]: *** [all-recursive] Error 1
make: *** [all-recursive] Error 1
Makefile:774: recipe for target 'all-recursive' failed
@jameshilliard

This comment has been minimized.

Copy link
Contributor Author

commented Oct 27, 2018

@ken2812221 does this seem to be a similar issue to #14568?

@DrahtBot

This comment has been minimized.

Copy link
Contributor

commented Nov 1, 2018

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #10102 ([experimental] Multiprocess bitcoin by ryanofsky)

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.

@jameshilliard jameshilliard force-pushed the jameshilliard:bip70-disable-check branch Nov 10, 2018

@jameshilliard jameshilliard force-pushed the jameshilliard:bip70-disable-check branch to 58c5cc9 Nov 10, 2018

@jameshilliard

This comment has been minimized.

Copy link
Contributor Author

commented Nov 10, 2018

This should be fixed now, can we trigger a new gitian test build?

@DrahtBot

This comment has been minimized.

Copy link
Contributor

commented Nov 12, 2018

Gitian builds for commit edc7152 (master):

Gitian builds for commit 8ba6b08d0066e507298f5f1f2261d2f7ffd0e588 (master and this pull):

@gmaxwell

This comment has been minimized.

Copy link
Member

commented Nov 21, 2018

utACK

@MarcoFalke MarcoFalke merged commit 58c5cc9 into bitcoin:master Dec 6, 2018

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

MarcoFalke added a commit that referenced this pull request Dec 6, 2018

Merge #14564: Adjust configure so that only bip70 is disabled when pr…
…otobuf is missing instead of the GUI

58c5cc9 Adjust configure so that only bip70 is disabled when protobuf is missing instead of the GUI (James Hilliard)

Pull request description:

  This change ensures that the GUI is still built even if protobuf is missing unless --enable-bip70 is passed to configure. If protobuf is present bip70 support will be compiled in unless --disable-bip70 is passed.

Tree-SHA512: 432d2fbefec5436503d8aa8994e4efaf760d88bfd5249af031b502b356852e8fd56362f86420f9ffe78498649079d0f1b68c327960b215d83c275800626ad275

luke-jr added a commit to bitcoinknots/bitcoin that referenced this pull request Dec 24, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.