Skip to content

Commit

Permalink
Merge bitcoin#24279: build: Make $(package)_*_env available to all …
Browse files Browse the repository at this point in the history 
…`$(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#22552.

  Another [bug](bitcoin#22719) in the depends build system has already become a problem at least two times in the past (bitcoin#16883 (comment) and bitcoin#24134). Each time the problem was solved with other means.

  The initial [solution](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#24134).

  The bug is well described in 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#22719.

  ---

  Also this PR removes no longer needed crutch from `qt.mk`.

ACKs for top commit:
  fanquake:
    ACK affbf58

Tree-SHA512: 0ce2cf82870a7774bdf1592fac50857126ae47da902e349f1092d50109223be9d6a8efd5e92ec08c2ca775b17516482aabaf232378950ade36484a883acc177b
  • Loading branch information
@fanquake
fanquake authored and jagdeep sidhu committed Dec 8, 2022
1 parent fdda07d commit 5dde8cb
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 9 deletions.
13 changes: 7 additions & 6 deletions depends/funcs.mk
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,9 +87,9 @@ $(1)_download_path_fixed=$(subst :,\:,$$($(1)_download_path))
$(1)_fetch_cmds ?= $(call fetch_file,$(1),$(subst \:,:,$$($(1)_download_path_fixed)),$$($(1)_download_file),$($(1)_file_name),$($(1)_sha256_hash))
$(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)
$(1)_preprocess_cmds ?= true
$(1)_build_cmds ?=
$(1)_config_cmds ?=
$(1)_stage_cmds ?=
$(1)_build_cmds ?= true
$(1)_config_cmds ?= true
$(1)_stage_cmds ?= true
$(1)_set_vars ?=


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

$(1)_config_env+=PKG_CONFIG_LIBDIR=$($($(1)_type)_prefix)/lib/pkgconfig
$(1)_config_env+=PKG_CONFIG_PATH=$($($(1)_type)_prefix)/share/pkgconfig
$(1)_config_env+=PKG_CONFIG_SYSROOT_DIR=/
$(1)_config_env+=CMAKE_MODULE_PATH=$($($(1)_type)_prefix)/lib/cmake
$(1)_config_env+=PATH=$(build_prefix)/bin:$(PATH)
$(1)_build_env+=PATH=$(build_prefix)/bin:$(PATH)
Expand Down Expand Up @@ -214,17 +215,17 @@ $($(1)_configured): | $($(1)_dependencies) $($(1)_preprocessed)
echo Configuring $(1)...
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); )
mkdir -p $$(@D)
+{ cd $$(@D); $($(1)_config_env) $($(1)_config_cmds); } $$($(1)_logging)
+{ cd $$(@D); export $($(1)_config_env); $($(1)_config_cmds); } $$($(1)_logging)
touch $$@
$($(1)_built): | $($(1)_configured)
echo Building $(1)...
mkdir -p $$(@D)
+{ cd $$(@D); $($(1)_build_env) $($(1)_build_cmds); } $$($(1)_logging)
+{ cd $$(@D); export $($(1)_build_env); $($(1)_build_cmds); } $$($(1)_logging)
touch $$@
$($(1)_staged): | $($(1)_built)
echo Staging $(1)...
mkdir -p $($(1)_staging_dir)/$(host_prefix)
+{ cd $($(1)_build_dir); $($(1)_stage_env) $($(1)_stage_cmds); } $$($(1)_logging)
+{ cd $($(1)_build_dir); export $($(1)_stage_env); $($(1)_stage_cmds); } $$($(1)_logging)
rm -rf $($(1)_extract_dir)
touch $$@
$($(1)_postprocessed): | $($(1)_staged)
Expand Down
4 changes: 1 addition & 3 deletions depends/packages/qt.mk
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ $(package)_extra_sources = $($(package)_qttranslations_file_name)
$(package)_extra_sources += $($(package)_qttools_file_name)

define $(package)_set_vars
$(package)_config_env = QT_MAC_SDK_NO_VERSION_CHECK=1
$(package)_config_opts_release = -release
$(package)_config_opts_release += -silent
$(package)_config_opts_debug = -debug
Expand Down Expand Up @@ -261,9 +262,6 @@ define $(package)_preprocess_cmds
endef

define $(package)_config_cmds
export PKG_CONFIG_SYSROOT_DIR=/ && \
export PKG_CONFIG_LIBDIR=$(host_prefix)/lib/pkgconfig && \
export QT_MAC_SDK_NO_VERSION_CHECK=1 && \
cd qtbase && \
./configure -top-level $($(package)_config_opts)
endef
Expand Down

0 comments on commit 5dde8cb

Please sign in to comment.