build, ci: Add file-based logging for individual packages#19952
Merged
laanwj merged 2 commits intobitcoin:masterfrom Apr 14, 2022
Merged
build, ci: Add file-based logging for individual packages#19952laanwj merged 2 commits intobitcoin:masterfrom
laanwj merged 2 commits intobitcoin:masterfrom
Conversation
Contributor
|
Concept ACK |
Member
Author
|
cc @dongcarl |
Contributor
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ConflictsReviewers, this pull request conflicts with the following ones:
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. |
Member
|
Concept ACK. Does the ci print the log on failure? |
Member
Author
|
Updated 95b5dfa -> 68459dd (pr19952.01 -> pr19952.02):
An example of failure logging in CI -- https://travis-ci.org/github/hebasto/bitcoin/builds/732364252 with the following patch: --- a/depends/patches/qt/freetype_back_compat.patch
+++ b/depends/patches/qt/freetype_back_compat.patch
@@ -22,7 +22,7 @@ index 3f543755..8ecc1c8c 100644
}
#if defined(FT_FONT_FORMATS_H)
- const char *fmt = FT_Get_Font_Format(face);
-+ const char *fmt = FT_Get_X11_Font_Format(face);
++ const char *fmt = FT_Get_X11_Font_Format(face)
if (fmt && qstrncmp(fmt, "CFF", 4) == 0) {
FT_Bool no_stem_darkening = true;
FT_Error err = FT_Property_Get(qt_getFreetype(), "cff", "no-stem-darkening", &no_stem_darkening); |
Member
Author
|
Rebased 68459dd -> 4b676b1 (pr19952.02 -> pr19952.03). |
dongcarl
reviewed
Nov 12, 2020
depends/README.md
Outdated
| system's <code>$PATH</code> rather than the default prebuilt release of Clang | ||
| from llvm.org. Clang 8 or later is required.</dd> | ||
| <dt>LOG</dt> | ||
| <dd>Use file-based logging for individual packages</dd> |
Contributor
There was a problem hiding this comment.
Could you add more details? For example, where would we expect these log files to appear?
Member
Author
There was a problem hiding this comment.
If wording in the updated OP is ok, I'll push it into the docs.
Member
|
Concept ACK, I think not being verbose unless the level of detail is really needed (say, on a failure) is good for build/test commands. Makes it easier to spot actual problems. |
This was referenced Dec 8, 2020
Member
Author
|
Updated 4b676b1 -> 46e2ff7 (pr19952.03 -> pr19952.04):
The example of CI output in case of error during depends build is here. |
Contributor
|
Hmmm, I've been thinking about this... And wondering if we could use diff --git a/depends/funcs.mk b/depends/funcs.mk
index 3ddeaf0378..2de5f67b5c 100644
--- a/depends/funcs.mk
+++ b/depends/funcs.mk
@@ -80,7 +80,7 @@ $(1)_download_path_fixed=$(subst :,\:,$$($(1)_download_path))
# The default behavior for tar will try to set ownership when running as uid 0 and may not succeed, --no-same-owner disables this behavior
$(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 && tar --no-same-owner --strip-components=1 -xf $$($(1)_source)
-$(1)_preprocess_cmds ?= true
+$(1)_preprocess_cmds ?=
$(1)_build_cmds ?=
$(1)_config_cmds ?=
$(1)_stage_cmds ?=
@@ -177,7 +177,7 @@ endef
define int_add_cmds
ifneq ($(LOG),)
-$(1)_logging = >>$$($(1)_build_log) 2>&1 || { if test -f $$($(1)_build_log); then cat $$($(1)_build_log); fi; exit 1; }
+$(1)_logging = exec >> >(tee -a $$($(1)_build_log)) 2>&1;
endif
$($(1)_fetched):
@@ -196,23 +196,23 @@ $($(1)_preprocessed): | $($(1)_extracted)
$(AT)echo Preprocessing $(1)...
$(AT)mkdir -p $$(@D) $($(1)_patch_dir)
$(AT)$(foreach patch,$($(1)_patches),cd $(PATCHES_PATH)/$(1); cp $(patch) $($(1)_patch_dir) ;)
- $(AT){ cd $$(@D); $(call $(1)_preprocess_cmds, $(1)); } $$($(1)_logging)
+ $(AT)( $$($(1)_logging) cd $$(@D); $(call $(1)_preprocess_cmds, $(1)) )
$(AT)touch $$@
$($(1)_configured): | $($(1)_dependencies) $($(1)_preprocessed)
$(AT)echo Configuring $(1)...
$(AT)rm -rf $(host_prefix); mkdir -p $(host_prefix)/lib; cd $(host_prefix); $(foreach package,$($(1)_all_dependencies), tar --no-same-owner -xf $($(package)_cached); )
$(AT)mkdir -p $$(@D)
- $(AT)+{ cd $$(@D); $($(1)_config_env) $(call $(1)_config_cmds, $(1)); } $$($(1)_logging)
+ $(AT)+( $$($(1)_logging) cd $$(@D); $($(1)_config_env) $(call $(1)_config_cmds, $(1)) )
$(AT)touch $$@
$($(1)_built): | $($(1)_configured)
$(AT)echo Building $(1)...
$(AT)mkdir -p $$(@D)
- $(AT)+{ cd $$(@D); $($(1)_build_env) $(call $(1)_build_cmds, $(1)); } $$($(1)_logging)
+ $(AT)+( $$($(1)_logging) cd $$(@D); $($(1)_build_env) $(call $(1)_build_cmds, $(1)) )
$(AT)touch $$@
$($(1)_staged): | $($(1)_built)
$(AT)echo Staging $(1)...
$(AT)mkdir -p $($(1)_staging_dir)/$(host_prefix)
- $(AT)+{ cd $($(1)_build_dir); $($(1)_stage_env) $(call $(1)_stage_cmds, $(1)); } $$($(1)_logging)
+ $(AT)+( $$($(1)_logging) cd $($(1)_build_dir); $($(1)_stage_env) $(call $(1)_stage_cmds, $(1)) )
$(AT)rm -rf $($(1)_extract_dir)
$(AT)touch $$@
$($(1)_postprocessed): | $($(1)_staged)I supposed POSIX sh doesn't have process substitution... But perhaps for depends we can set |
Member
Author
Member
Author
|
Updated 46e2ff7 -> bdd1056 (pr19952.04 -> pr19952.05) due to the conflict with #22283. |
Member
Author
|
Rebased bdd1056 -> 0b9357b (pr19952.05 -> pr19952.06) due to the conflict with #24212. |
This was referenced Feb 6, 2022
Member
Author
|
Rebased 0b9357b -> 3c5c583 (pr19952.06 -> pr19952.07) due to the conflict with #23998. |
Member
Author
Guix builds: |
hebasto
force-pushed
the
200913-log
branch
2 times, most recently
from
e87f5fc to
eb4b01e
Compare
Member
Author
|
Rebased 3c5c583 -> eb4b01e (pr19952.07 -> pr19952.08) due to the conflict with #24522. |
Member
Author
|
Friendly ping @dongcarl @MarcoFalke |
Member
|
Concept ACK |
This change silences depends build using LOG=1.
Member
Author
|
Rebased eb4b01e -> 86c2889 (pr19952.08 -> pr19952.09) due to the conflict with #24285. |
Member
|
Going to test this. |
Member
|
Tested ACK 86c2889 I know it's not the intended use but I patched this into the guix build, as that's the only depends build i do at the moment: --- a/contrib/guix/libexec/build.sh
+++ b/contrib/guix/libexec/build.sh
@@ -198,7 +198,7 @@ esac
####################
# Build the depends tree, overriding variables that assume multilib gcc
-make -C depends --jobs="$JOBS" HOST="$HOST" \
+make -C depends --jobs="$JOBS" HOST="$HOST" LOG=1 \
${V:+V=1} \
${SOURCES_PATH+SOURCES_PATH="$SOURCES_PATH"} \
${BASE_CACHE+BASE_CACHE="$BASE_CACHE"} \The log was clean. A lot less spammy, this is good!: After the build, the individual log files could be found in the cache: I also corrupted a patch intentionally to see if the logging would still report the error in that case, it does: |
sidhujag
pushed a commit
to syscoin/syscoin
that referenced
this pull request
Apr 14, 2022
… packages 86c2889 ci: Make log verbose in error case only (Hennadii Stepanov) 7f65088 depends: Add file-based logging for individual packages (Hennadii Stepanov) Pull request description: This PR adds file-based logging for individual packages in depends. To use this feature one should provide `LOG=1`. A log file is printed out automatically in case of a build error. After successful build log files are being moved along with package archives: ``` $ make -C depends HOST=x86_64-w64-mingw32 LOG=1 $ find ./depends/built/x86_64-w64-mingw32 -name '*.log' | sort ./depends/built/x86_64-w64-mingw32/bdb/bdb-4.8.30-5100a099801.log ./depends/built/x86_64-w64-mingw32/boost/boost-1_71_0-313f82dc7de.log ./depends/built/x86_64-w64-mingw32/libevent/libevent-2.1.12-stable-3fa27048d5e.log ./depends/built/x86_64-w64-mingw32/libnatpmp/libnatpmp-4536032ae32268a45c073a4d5e91bbab4534773a-9db4850dd32.log ./depends/built/x86_64-w64-mingw32/miniupnpc/miniupnpc-2.2.2-75d9a1807e0.log ./depends/built/x86_64-w64-mingw32/native_b2/native_b2-1_71_0-3bf253c19bf.log ./depends/built/x86_64-w64-mingw32/qrencode/qrencode-3.4.4-dfac87af599.log ./depends/built/x86_64-w64-mingw32/qt/qt-5.15.2-9304e03d3ac.log ./depends/built/x86_64-w64-mingw32/sqlite/sqlite-3320100-455acafa7be.log ./depends/built/x86_64-w64-mingw32/zeromq/zeromq-4.3.1-5ff627ec84a.log ``` An example of CI tasks with package build errors -- https://cirrus-ci.com/task/5275741788045312 Closes bitcoin#16368. ACKs for top commit: laanwj: Tested ACK 86c2889 Tree-SHA512: 497f2146fd2e38c952124aecfd80ebb42be22bbc5dc59521491545f4465fc38f23da7787a0caea5686b7c30aa862f2b0c02092ae3fe863e80a5ddd14b3d324b9
This file contains hidden or 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
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
7 participants
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.
This PR adds file-based logging for individual packages in depends. To use this feature one should provide
LOG=1.A log file is printed out automatically in case of a build error. After successful build log files are being moved along with package archives:
An example of CI tasks with package build errors -- https://cirrus-ci.com/task/5275741788045312
Closes #16368.