Merge bitcoin/bitcoin#24279: build: Make $(package)_*_env available to all $(package)_*_cmds

affbf58a1e52a8e60c830be6a9e0347e0ff4c28e build: Move environment variables into `$(package)_config_env` (Hennadii Stepanov)
d44fcd3c976572883bbf7f386bc88e2610dc1a58 build: Make $(package)_*_env available to all $(package)_*_cmds (Hennadii Stepanov)

Pull request description:

  On master (1e7564eca8a688f39c75540877ec3bdfdde766b1) 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](https://github.com/bitcoin/bitcoin/issues/22719) in the depends build system has already become a problem at least two times in the past (https://github.com/bitcoin/bitcoin/pull/16883#issuecomment-683817472 and https://github.com/bitcoin/bitcoin/pull/24134). Each time the problem was solved with other means.

  The initial [solution](https://github.com/bitcoin/bitcoin/pull/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](https://github.com/bitcoin/bitcoin/pull/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:1e7564eca8/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 affbf58a1e52a8e60c830be6a9e0347e0ff4c28e

Tree-SHA512: 0ce2cf82870a7774bdf1592fac50857126ae47da902e349f1092d50109223be9d6a8efd5e92ec08c2ca775b17516482aabaf232378950ade36484a883acc177b
This commit is contained in:
fanquake 2022-12-08 16:41:31 +00:00 committed by pasta
parent 3261092f85
commit 66a3981a7a
No known key found for this signature in database
GPG Key ID: E2F3D7916E722D38
2 changed files with 8 additions and 9 deletions

View File

@ -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 ?=
@ -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)
@ -221,17 +222,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)

View File

@ -34,6 +34,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
@ -273,9 +274,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