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

build: make building protobuf optional in depends #16871

Merged
merged 2 commits into from Sep 16, 2019

Conversation

fanquake
Copy link
Member

As mentioned by dongcarl in #15584 (comment), make building protobuf optional in depends. With this change it will only be built if you pass PROTOBUF=1.

@fanquake fanquake added this to the 0.19.0 milestone Sep 14, 2019
@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:

  • #16367 (Multiprocess build support 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.

@fanquake fanquake force-pushed the depends_no_protobuf_by_default branch from 4eb0502 to 8600e92 Compare September 14, 2019 04:19
Those that want to build it can now pass PROTOBUF=1.
@fanquake fanquake force-pushed the depends_no_protobuf_by_default branch from 8600e92 to 107e030 Compare September 14, 2019 04:59
@laanwj
Copy link
Member

laanwj commented Sep 14, 2019

code review ACK 107e030

@DrahtBot
Copy link
Contributor

Gitian builds for commit 4bfef0d (master):

Gitian builds for commit 86e97fc (master and this pull):

@Sjors
Copy link
Member

Sjors commented Sep 16, 2019

tACK 107e030 on macOS 10.14. When I build depends with PROTOBUF=1 then ./configure has bip70 enabled.

laanwj added a commit that referenced this pull request Sep 16, 2019
107e030 build: make protobuf optional in depends (fanquake)
ff6122f doc: clarify protobuf build requirements (fanquake)

Pull request description:

  As mentioned by dongcarl in #15584 (comment), make building `protobuf` optional in depends. With this change it will only be built if you pass `PROTOBUF=1`.

ACKs for top commit:
  laanwj:
    code review ACK 107e030
  Sjors:
    tACK 107e030 on macOS 10.14. When I build depends with `PROTOBUF=1` then `./configure` has `bip70` enabled.

Tree-SHA512: 49bc247a6879aaf55b943a3d0b930544ddef1e69a481955a8bebe0b02c9ad0fe168b93025f34168334cef34bb567478eb98eacab62ba909f2f64fb21119c71b8
@laanwj laanwj merged commit 107e030 into bitcoin:master Sep 16, 2019
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Sep 16, 2019
107e030 build: make protobuf optional in depends (fanquake)
ff6122f doc: clarify protobuf build requirements (fanquake)

Pull request description:

  As mentioned by dongcarl in bitcoin#15584 (comment), make building `protobuf` optional in depends. With this change it will only be built if you pass `PROTOBUF=1`.

ACKs for top commit:
  laanwj:
    code review ACK 107e030
  Sjors:
    tACK 107e030 on macOS 10.14. When I build depends with `PROTOBUF=1` then `./configure` has `bip70` enabled.

Tree-SHA512: 49bc247a6879aaf55b943a3d0b930544ddef1e69a481955a8bebe0b02c9ad0fe168b93025f34168334cef34bb567478eb98eacab62ba909f2f64fb21119c71b8
@fanquake fanquake deleted the depends_no_protobuf_by_default branch October 12, 2019 15:44
deadalnix pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this pull request Apr 2, 2020
Summary:
Backport of core [[bitcoin/bitcoin#16871 | PR16871]].

The logic is reversed wrt core since we enable bip70 by default.

Depends on D5637.

Test Plan:
  make build-osx NO_PROTOBUF=1
Check that protobuf is not built.
  ../configure --prefix=$PWD/../depends/x86_64-apple-darwin16
Check in the summary that bip70 is disabled:
  with bip70  = no

Reviewers: #bitcoin_abc, deadalnix

Reviewed By: #bitcoin_abc, deadalnix

Differential Revision: https://reviews.bitcoinabc.org/D5640
ftrader pushed a commit to bitcoin-cash-node/bitcoin-cash-node that referenced this pull request Aug 17, 2020
Summary:
Backport of core [[bitcoin/bitcoin#16871 | PR16871]].

The logic is reversed wrt core since we enable bip70 by default.

Depends on D5637.

Test Plan:
  make build-osx NO_PROTOBUF=1
Check that protobuf is not built.
  ../configure --prefix=$PWD/../depends/x86_64-apple-darwin16
Check in the summary that bip70 is disabled:
  with bip70  = no

Reviewers: #bitcoin_abc, deadalnix

Reviewed By: #bitcoin_abc, deadalnix

Differential Revision: https://reviews.bitcoinabc.org/D5640
Munkybooty pushed a commit to Munkybooty/dash that referenced this pull request Dec 7, 2021
107e030 build: make protobuf optional in depends (fanquake)
ff6122f doc: clarify protobuf build requirements (fanquake)

Pull request description:

  As mentioned by dongcarl in bitcoin#15584 (comment), make building `protobuf` optional in depends. With this change it will only be built if you pass `PROTOBUF=1`.

ACKs for top commit:
  laanwj:
    code review ACK 107e030
  Sjors:
    tACK 107e030 on macOS 10.14. When I build depends with `PROTOBUF=1` then `./configure` has `bip70` enabled.

Tree-SHA512: 49bc247a6879aaf55b943a3d0b930544ddef1e69a481955a8bebe0b02c9ad0fe168b93025f34168334cef34bb567478eb98eacab62ba909f2f64fb21119c71b8
Munkybooty pushed a commit to Munkybooty/dash that referenced this pull request Dec 15, 2021
107e030 build: make protobuf optional in depends (fanquake)
ff6122f doc: clarify protobuf build requirements (fanquake)

Pull request description:

  As mentioned by dongcarl in bitcoin#15584 (comment), make building `protobuf` optional in depends. With this change it will only be built if you pass `PROTOBUF=1`.

ACKs for top commit:
  laanwj:
    code review ACK 107e030
  Sjors:
    tACK 107e030 on macOS 10.14. When I build depends with `PROTOBUF=1` then `./configure` has `bip70` enabled.

Tree-SHA512: 49bc247a6879aaf55b943a3d0b930544ddef1e69a481955a8bebe0b02c9ad0fe168b93025f34168334cef34bb567478eb98eacab62ba909f2f64fb21119c71b8
Munkybooty pushed a commit to Munkybooty/dash that referenced this pull request Dec 15, 2021
107e030 build: make protobuf optional in depends (fanquake)
ff6122f doc: clarify protobuf build requirements (fanquake)

Pull request description:

  As mentioned by dongcarl in bitcoin#15584 (comment), make building `protobuf` optional in depends. With this change it will only be built if you pass `PROTOBUF=1`.

ACKs for top commit:
  laanwj:
    code review ACK 107e030
  Sjors:
    tACK 107e030 on macOS 10.14. When I build depends with `PROTOBUF=1` then `./configure` has `bip70` enabled.

Tree-SHA512: 49bc247a6879aaf55b943a3d0b930544ddef1e69a481955a8bebe0b02c9ad0fe168b93025f34168334cef34bb567478eb98eacab62ba909f2f64fb21119c71b8
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Dec 16, 2021
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.

None yet

5 participants