depends: Only use D_DARWIN_C_SOURCE when building miniupnpc on darwin #12466
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Obvious utACK, sorry for merging before @theuni could get a look at it. |
|
@laanwj No worries! Unfortunately this isn't quite as obvious as it looks. This change would clobber any default cflags. @fanquake Could you give this a try? diff --git a/depends/packages/miniupnpc.mk b/depends/packages/miniupnpc.mk
index 9976db43c2..0a17f07e22 100644
--- a/depends/packages/miniupnpc.mk
+++ b/depends/packages/miniupnpc.mk
@@ -5,11 +5,10 @@ $(package)_file_name=$(package)-$($(package)_version).tar.gz
$(package)_sha256_hash=90dda8c7563ca6cd4a83e23b3c66dbbea89603a1675bfdb852897c2c9cc220b7
define $(package)_set_vars
-$(package)_build_opts=CC="$($(package)_cc)"
+$(package)_build_opts=CC="$($(package)_cc)" AR="$($(package)_ar)"
$(package)_build_opts_darwin=OS=Darwin LIBTOOL="$($(package)_libtool)"
$(package)_build_opts_mingw32=-f Makefile.mingw
-$(package)_build_env+=CFLAGS="$($(package)_cflags) $($(package)_cppflags)" AR="$($(package)_ar)"
-$(package)_build_env+=CFLAGS=-D_DARWIN_C_SOURCE
+$(package)_cppflags_darwin+=-D_DARWIN_C_SOURCE
endef
define $(package)_preprocess_cmds
@@ -19,7 +18,7 @@ define $(package)_preprocess_cmds
endef
define $(package)_build_cmds
- $(MAKE) libminiupnpc.a $($(package)_build_opts)
+ CFLAGS="$($(package)_cflags) $($(package)_cppflags)" $(MAKE) libminiupnpc.a $($(package)_build_opts)
endef
define $(package)_stage_cmds |
|
BTW; isn't this something that should ideally be fixed upstream? |
fanquake
force-pushed
the
fix-miniupnpc-darwin
branch
2 times, most recently
from
535a8ee to
e650112
Compare
|
@theuni using the patch you suggested, -D_DARWIN_C_SOURCE is now appearing twice in the build flags: I've pushed up your changes without the cppflags_darwin addition, we'll see what Travis thinks. |
|
Looks like -D_DARWIN_C_SOURCE is being added without the specific cppflags_darwin +=. |
|
@fanquake good catch! Upstream did add the define, as @laanwj mentioned that they should. Our issue turns out to be that upstream added cross-build detection, which we partially override by defining OS. If we just let it be discovered as intended, all is fine. The problem you're seeing now is that LIBTOOL isn't defined (apple has a libtool utility that can handle ar's features, it's unrelated to gnu libtool). Looks like the fix is as simple as: --- a/depends/packages/miniupnpc.mk
+++ b/depends/packages/miniupnpc.mk
@@ -6,10 +6,9 @@ $(package)_sha256_hash=90dda8c7563ca6cd4a83e23b3c66dbbea89603a1675bfdb852897c2c9
define $(package)_set_vars
$(package)_build_opts=CC="$($(package)_cc)"
-$(package)_build_opts_darwin=OS=Darwin LIBTOOL="$($(package)_libtool)"
+$(package)_build_opts_darwin=LIBTOOL="$($(package)_libtool)"
$(package)_build_opts_mingw32=-f Makefile.mingw
$(package)_build_env+=CFLAGS="$($(package)_cflags) $($(package)_cppflags)" AR="$($(package)_ar)"
-$(package)_build_env+=CFLAGS=-D_DARWIN_C_SOURCE
endef
define $(package)_preprocess_cmds |
fanquake
force-pushed
the
fix-miniupnpc-darwin
branch
from
e650112 to
65754fd
Compare
fanquake
force-pushed
the
fix-miniupnpc-darwin
branch
from
65754fd to
992f568
Compare
laanwj
added a commit
that referenced
this pull request
Mar 5, 2018
…pnpc on darwin 992f568 depends: Only use D_DARWIN_C_SOURCE when building miniupnpc on darwin (fanquake) Pull request description: Only use D_DARWIN_C_SOURCE when building on darwin, so we don't inadvertently introduce issues elsewhere. cc @theuni Tree-SHA512: e49a8456ba2b9925c06e62c73e139152b6d63cc5a4cee66944e41c863ca9103e98ac81a5718eceb3d0885a677fc53ece34062b02c304a05c3280e094965e856a
deadalnix
pushed a commit
to Bitcoin-ABC/bitcoin-abc
that referenced
this pull request
Mar 19, 2020
Summary: ``` miniupnpc changelog: http://miniupnp.free.fr/files/changelog.php?file=miniupnpc-2.0.20180203.tar.gz 2.0.20180203 includes fixes for the recent buffer overflow and segfault issues, see miniupnp/miniupnp#268. expat changelog: https://github.com/libexpat/libexpat/blob/R_2_2_5/expat/Changes 2.2.2 & 2.2.3 included security fixes. Also includes latest config.guess and config.sub. ``` This updates our depends version for expat, miniupnpc, config.guess and config.sub. The ccache version is **NOT** updated (see D5503). It also includes a fix for miniupnpc. Backport of core [[bitcoin/bitcoin#12402 | PR12402]] and [[bitcoin/bitcoin#12466 | PR12466]]. Depends on D5502 and D5503. *Note to reviewers:* the diff ends up fairly large due to the pull of config.guess and config.sub. However this is mostly formatting changes (`${FOO}` vs `$FOO` in particular), and there was no merge conflict during the cherry-pick. Test Plan: Run the Gitian builds twice, ensure the build is still deterministic. Reviewers: #bitcoin_abc, deadalnix Reviewed By: #bitcoin_abc, deadalnix Subscribers: deadalnix Differential Revision: https://reviews.bitcoinabc.org/D5504
ftrader
pushed a commit
to bitcoin-cash-node/bitcoin-cash-node
that referenced
this pull request
May 19, 2020
Summary: ``` miniupnpc changelog: http://miniupnp.free.fr/files/changelog.php?file=miniupnpc-2.0.20180203.tar.gz 2.0.20180203 includes fixes for the recent buffer overflow and segfault issues, see miniupnp/miniupnp#268. expat changelog: https://github.com/libexpat/libexpat/blob/R_2_2_5/expat/Changes 2.2.2 & 2.2.3 included security fixes. Also includes latest config.guess and config.sub. ``` This updates our depends version for expat, miniupnpc, config.guess and config.sub. The ccache version is **NOT** updated (see D5503). It also includes a fix for miniupnpc. Backport of core [[bitcoin/bitcoin#12402 | PR12402]] and [[bitcoin/bitcoin#12466 | PR12466]]. Depends on D5502 and D5503. *Note to reviewers:* the diff ends up fairly large due to the pull of config.guess and config.sub. However this is mostly formatting changes (`${FOO}` vs `$FOO` in particular), and there was no merge conflict during the cherry-pick. Test Plan: Run the Gitian builds twice, ensure the build is still deterministic. Reviewers: #bitcoin_abc, deadalnix Reviewed By: #bitcoin_abc, deadalnix Subscribers: deadalnix Differential Revision: https://reviews.bitcoinabc.org/D5504
PastaPastaPasta
pushed a commit
to PastaPastaPasta/dash
that referenced
this pull request
Jun 9, 2020
…g miniupnpc on darwin 992f568 depends: Only use D_DARWIN_C_SOURCE when building miniupnpc on darwin (fanquake) Pull request description: Only use D_DARWIN_C_SOURCE when building on darwin, so we don't inadvertently introduce issues elsewhere. cc @theuni Tree-SHA512: e49a8456ba2b9925c06e62c73e139152b6d63cc5a4cee66944e41c863ca9103e98ac81a5718eceb3d0885a677fc53ece34062b02c304a05c3280e094965e856a
PastaPastaPasta
pushed a commit
to PastaPastaPasta/dash
that referenced
this pull request
Jun 9, 2020
…g miniupnpc on darwin 992f568 depends: Only use D_DARWIN_C_SOURCE when building miniupnpc on darwin (fanquake) Pull request description: Only use D_DARWIN_C_SOURCE when building on darwin, so we don't inadvertently introduce issues elsewhere. cc @theuni Tree-SHA512: e49a8456ba2b9925c06e62c73e139152b6d63cc5a4cee66944e41c863ca9103e98ac81a5718eceb3d0885a677fc53ece34062b02c304a05c3280e094965e856a
PastaPastaPasta
pushed a commit
to PastaPastaPasta/dash
that referenced
this pull request
Jun 10, 2020
…g miniupnpc on darwin 992f568 depends: Only use D_DARWIN_C_SOURCE when building miniupnpc on darwin (fanquake) Pull request description: Only use D_DARWIN_C_SOURCE when building on darwin, so we don't inadvertently introduce issues elsewhere. cc @theuni Tree-SHA512: e49a8456ba2b9925c06e62c73e139152b6d63cc5a4cee66944e41c863ca9103e98ac81a5718eceb3d0885a677fc53ece34062b02c304a05c3280e094965e856a
PastaPastaPasta
pushed a commit
to PastaPastaPasta/dash
that referenced
this pull request
Jun 10, 2020
…g miniupnpc on darwin 992f568 depends: Only use D_DARWIN_C_SOURCE when building miniupnpc on darwin (fanquake) Pull request description: Only use D_DARWIN_C_SOURCE when building on darwin, so we don't inadvertently introduce issues elsewhere. cc @theuni Tree-SHA512: e49a8456ba2b9925c06e62c73e139152b6d63cc5a4cee66944e41c863ca9103e98ac81a5718eceb3d0885a677fc53ece34062b02c304a05c3280e094965e856a
PastaPastaPasta
pushed a commit
to PastaPastaPasta/dash
that referenced
this pull request
Jun 11, 2020
…g miniupnpc on darwin 992f568 depends: Only use D_DARWIN_C_SOURCE when building miniupnpc on darwin (fanquake) Pull request description: Only use D_DARWIN_C_SOURCE when building on darwin, so we don't inadvertently introduce issues elsewhere. cc @theuni Tree-SHA512: e49a8456ba2b9925c06e62c73e139152b6d63cc5a4cee66944e41c863ca9103e98ac81a5718eceb3d0885a677fc53ece34062b02c304a05c3280e094965e856a
PastaPastaPasta
pushed a commit
to PastaPastaPasta/dash
that referenced
this pull request
Jun 11, 2020
…g miniupnpc on darwin 992f568 depends: Only use D_DARWIN_C_SOURCE when building miniupnpc on darwin (fanquake) Pull request description: Only use D_DARWIN_C_SOURCE when building on darwin, so we don't inadvertently introduce issues elsewhere. cc @theuni Tree-SHA512: e49a8456ba2b9925c06e62c73e139152b6d63cc5a4cee66944e41c863ca9103e98ac81a5718eceb3d0885a677fc53ece34062b02c304a05c3280e094965e856a
PastaPastaPasta
pushed a commit
to PastaPastaPasta/dash
that referenced
this pull request
Jun 12, 2020
…g miniupnpc on darwin 992f568 depends: Only use D_DARWIN_C_SOURCE when building miniupnpc on darwin (fanquake) Pull request description: Only use D_DARWIN_C_SOURCE when building on darwin, so we don't inadvertently introduce issues elsewhere. cc @theuni Tree-SHA512: e49a8456ba2b9925c06e62c73e139152b6d63cc5a4cee66944e41c863ca9103e98ac81a5718eceb3d0885a677fc53ece34062b02c304a05c3280e094965e856a
PastaPastaPasta
pushed a commit
to PastaPastaPasta/dash
that referenced
this pull request
Jun 14, 2020
…g miniupnpc on darwin 992f568 depends: Only use D_DARWIN_C_SOURCE when building miniupnpc on darwin (fanquake) Pull request description: Only use D_DARWIN_C_SOURCE when building on darwin, so we don't inadvertently introduce issues elsewhere. cc @theuni Tree-SHA512: e49a8456ba2b9925c06e62c73e139152b6d63cc5a4cee66944e41c863ca9103e98ac81a5718eceb3d0885a677fc53ece34062b02c304a05c3280e094965e856a
gades
pushed a commit
to cosanta/cosanta-core
that referenced
this pull request
Jun 24, 2021
…g miniupnpc on darwin 992f568 depends: Only use D_DARWIN_C_SOURCE when building miniupnpc on darwin (fanquake) Pull request description: Only use D_DARWIN_C_SOURCE when building on darwin, so we don't inadvertently introduce issues elsewhere. cc @theuni Tree-SHA512: e49a8456ba2b9925c06e62c73e139152b6d63cc5a4cee66944e41c863ca9103e98ac81a5718eceb3d0885a677fc53ece34062b02c304a05c3280e094965e856a
CryptoCentric
pushed a commit
to absolute-community/absolute
that referenced
this pull request
Jul 2, 2021
…g miniupnpc on darwin 992f568 depends: Only use D_DARWIN_C_SOURCE when building miniupnpc on darwin (fanquake) Pull request description: Only use D_DARWIN_C_SOURCE when building on darwin, so we don't inadvertently introduce issues elsewhere. cc @theuni Tree-SHA512: e49a8456ba2b9925c06e62c73e139152b6d63cc5a4cee66944e41c863ca9103e98ac81a5718eceb3d0885a677fc53ece34062b02c304a05c3280e094965e856a
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Only use D_DARWIN_C_SOURCE when building on darwin, so we don't inadvertently introduce issues elsewhere.
cc @theuni