build, ci: Add file-based logging for individual packages #19952
Conversation
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
|
Concept ACK |
|
cc @dongcarl |
|
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. |
|
Concept ACK. Does the ci print the log on failure? |
|
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); |
|
Rebased 68459dd -> 4b676b1 (pr19952.02 -> pr19952.03). |
dongcarl
reviewed
Nov 12, 2020
depends/README.md
Outdated
| @@ -121,6 +121,8 @@ build script logic) are searched for among the host system packages using | |||
| <dd>(EXPERTS ONLY) When cross-compiling for macOS, use Clang found in the | |||
| 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> | |||
There was a problem hiding this comment.
Could you add more details? For example, where would we expect these log files to appear?
There was a problem hiding this comment.
If wording in the updated OP is ok, I'll push it into the docs.
|
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
|
Updated 4b676b1 -> 46e2ff7 (pr19952.03 -> pr19952.04):
The example of CI output in case of error during depends build is here. |
|
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 |
|
Updated 46e2ff7 -> bdd1056 (pr19952.04 -> pr19952.05) due to the conflict with #22283. |
|
Rebased bdd1056 -> 0b9357b (pr19952.05 -> pr19952.06) due to the conflict with #24212. |
This was referenced Feb 6, 2022
|
Rebased 0b9357b -> 3c5c583 (pr19952.06 -> pr19952.07) due to the conflict with #23998. |
Guix builds: |
e87f5fc to
eb4b01e
Compare
|
Rebased 3c5c583 -> eb4b01e (pr19952.07 -> pr19952.08) due to the conflict with #24522. |
|
Friendly ping @dongcarl @MarcoFalke |
|
Concept ACK |
This change silences depends build using LOG=1.
|
Rebased eb4b01e -> 86c2889 (pr19952.08 -> pr19952.09) due to the conflict with #24285. |
|
Going to test this. |
|
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
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.
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.