Skip to content

Commit 3eaf7be

Browse files
committed
Merge bitcoin/bitcoin#24279: build: Make $(package)_*_env available to all $(package)_*_cmds
affbf58 build: Move environment variables into `$(package)_config_env` (Hennadii Stepanov) d44fcd3 build: Make $(package)_*_env available to all $(package)_*_cmds (Hennadii Stepanov) Pull request description: On master (1e7564e) the depends build system, which is based on pure GNU Make, works, but it lacks robustness, and in some corner cases it fails. For example, see bitcoin/bitcoin#22552. Another [bug](bitcoin/bitcoin#22719) in the depends build system has already become a problem at least two times in the past (bitcoin/bitcoin#16883 (comment) and bitcoin/bitcoin#24134). Each time the problem was solved with other means. The initial [solution](bitcoin/bitcoin#19882) had some discussion. Also it was discussed on the IRC meeting in #bitcoin-core-builds channel. This PR, actually, is a resurrection of it, as the bug silently struck pretty [recently](bitcoin/bitcoin#24134). The bug is well described in bitcoin/bitcoin#22719. Here is another, a bit simpler description, which requires only basic shell (bash, dash etc) experience. After creating targets by this code:https://github.com/bitcoin/bitcoin/blob/1e7564eca8a688f39c75540877ec3bdfdde766b1/depends/funcs.mk#L280 a "draft" line of recipe like `$($(1)_config_env) $(call $(1)_config_cmds, $(1))` becomes a shell command sequence `VAR1=foo VAR2=bar command1 && command2` which is supposed to be executed in a [new sub-shell](https://www.gnu.org/software/make/manual/html_node/Execution.html#Execution). Please note that `VAR1=foo VAR2=bar` part is visible for the first `command1` only (for details see shell docs). Example: ```sh $ VAR1="foo" VAR2="bar" echo "begin" && printenv VAR1 && printenv VAR2 && echo "end" begin $ echo $? 1 ``` Using the `export` command is a trivial solution: ```sh $ export VAR1="foo" VAR2="bar"; echo "begin" && printenv VAR1 && printenv VAR2 && echo "end" begin foo bar end $ echo $? 0 ``` As a [new sub-shell](https://www.gnu.org/software/make/manual/html_node/Execution.html#Execution) is invoked for each line of the recipe, there are no side effects of using `export`. It means this solution should not be considered invasive. Fixes bitcoin/bitcoin#22719. --- Also this PR removes no longer needed crutch from `qt.mk`. ACKs for top commit: fanquake: ACK affbf58 Tree-SHA512: 0ce2cf82870a7774bdf1592fac50857126ae47da902e349f1092d50109223be9d6a8efd5e92ec08c2ca775b17516482aabaf232378950ade36484a883acc177b
2 parents 5126e62 + affbf58 commit 3eaf7be

File tree

2 files changed

+8
-9
lines changed

2 files changed

+8
-9
lines changed

depends/funcs.mk

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -87,9 +87,9 @@ $(1)_download_path_fixed=$(subst :,\:,$$($(1)_download_path))
8787
$(1)_fetch_cmds ?= $(call fetch_file,$(1),$(subst \:,:,$$($(1)_download_path_fixed)),$$($(1)_download_file),$($(1)_file_name),$($(1)_sha256_hash))
8888
$(1)_extract_cmds ?= mkdir -p $$($(1)_extract_dir) && echo "$$($(1)_sha256_hash) $$($(1)_source)" > $$($(1)_extract_dir)/.$$($(1)_file_name).hash && $(build_SHA256SUM) -c $$($(1)_extract_dir)/.$$($(1)_file_name).hash && $(build_TAR) --no-same-owner --strip-components=1 -xf $$($(1)_source)
8989
$(1)_preprocess_cmds ?= true
90-
$(1)_build_cmds ?=
91-
$(1)_config_cmds ?=
92-
$(1)_stage_cmds ?=
90+
$(1)_build_cmds ?= true
91+
$(1)_config_cmds ?= true
92+
$(1)_stage_cmds ?= true
9393
$(1)_set_vars ?=
9494

9595

@@ -137,6 +137,7 @@ $(1)_config_env+=$($(1)_config_env_$(host_arch)_$(host_os)) $($(1)_config_env_$(
137137

138138
$(1)_config_env+=PKG_CONFIG_LIBDIR=$($($(1)_type)_prefix)/lib/pkgconfig
139139
$(1)_config_env+=PKG_CONFIG_PATH=$($($(1)_type)_prefix)/share/pkgconfig
140+
$(1)_config_env+=PKG_CONFIG_SYSROOT_DIR=/
140141
$(1)_config_env+=CMAKE_MODULE_PATH=$($($(1)_type)_prefix)/lib/cmake
141142
$(1)_config_env+=PATH=$(build_prefix)/bin:$(PATH)
142143
$(1)_build_env+=PATH=$(build_prefix)/bin:$(PATH)
@@ -214,17 +215,17 @@ $($(1)_configured): | $($(1)_dependencies) $($(1)_preprocessed)
214215
echo Configuring $(1)...
215216
rm -rf $(host_prefix); mkdir -p $(host_prefix)/lib; cd $(host_prefix); $(foreach package,$($(1)_all_dependencies), $(build_TAR) --no-same-owner -xf $($(package)_cached); )
216217
mkdir -p $$(@D)
217-
+{ cd $$(@D); $($(1)_config_env) $($(1)_config_cmds); } $$($(1)_logging)
218+
+{ cd $$(@D); export $($(1)_config_env); $($(1)_config_cmds); } $$($(1)_logging)
218219
touch $$@
219220
$($(1)_built): | $($(1)_configured)
220221
echo Building $(1)...
221222
mkdir -p $$(@D)
222-
+{ cd $$(@D); $($(1)_build_env) $($(1)_build_cmds); } $$($(1)_logging)
223+
+{ cd $$(@D); export $($(1)_build_env); $($(1)_build_cmds); } $$($(1)_logging)
223224
touch $$@
224225
$($(1)_staged): | $($(1)_built)
225226
echo Staging $(1)...
226227
mkdir -p $($(1)_staging_dir)/$(host_prefix)
227-
+{ cd $($(1)_build_dir); $($(1)_stage_env) $($(1)_stage_cmds); } $$($(1)_logging)
228+
+{ cd $($(1)_build_dir); export $($(1)_stage_env); $($(1)_stage_cmds); } $$($(1)_logging)
228229
rm -rf $($(1)_extract_dir)
229230
touch $$@
230231
$($(1)_postprocessed): | $($(1)_staged)

depends/packages/qt.mk

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ $(package)_extra_sources = $($(package)_qttranslations_file_name)
3333
$(package)_extra_sources += $($(package)_qttools_file_name)
3434

3535
define $(package)_set_vars
36+
$(package)_config_env = QT_MAC_SDK_NO_VERSION_CHECK=1
3637
$(package)_config_opts_release = -release
3738
$(package)_config_opts_release += -silent
3839
$(package)_config_opts_debug = -debug
@@ -261,9 +262,6 @@ define $(package)_preprocess_cmds
261262
endef
262263

263264
define $(package)_config_cmds
264-
export PKG_CONFIG_SYSROOT_DIR=/ && \
265-
export PKG_CONFIG_LIBDIR=$(host_prefix)/lib/pkgconfig && \
266-
export QT_MAC_SDK_NO_VERSION_CHECK=1 && \
267265
cd qtbase && \
268266
./configure -top-level $($(package)_config_opts)
269267
endef

0 commit comments

Comments
 (0)