Skip to content

Commit d819840

Browse files
committed
Merge bitcoin/bitcoin#27041: Build: Improve handling of suppressed logging in Makefiles
1b1ffbd Build: Log when test -f fails in Makefile (TheCharlatan) 541012e Build: Use AM_V_GEN in Makefiles where appropriate (TheCharlatan) Pull request description: This PR triages some behavior around Makefile recipe echoing suppression. When generating new files as part of the Makefile the recipe is sometimes suppressed with $(AM_V_GEN) and sometimes with `@`. We should prefer $(AM_V_GEN), since this also prints the lines in silent mode. This is arguably more in style with the current recipe echoing. Before: `Generated test/data/script_tests.json.h` Now: ` GEN test/data/script_tests.json.h` A side effect of this change is that the recipe for generating build.h is now echoed on each make run. Arguably this makes its generation more transparent. Sometimes the error emitted by `test -f` is currently thrown without any logging. This makes it a bit harder to debug. Instead, print a helpful log message to point the developer in the right direction. Alternatively this could have been implemented by just removing the recipe echo suppression (@), but the subsequent make output became too noisy. ACKs for top commit: fanquake: ACK 1b1ffbd Tree-SHA512: e31869fab25e72802b692ce6735f9561912caea903c1577101b64c9cb115c98de01a59300e8ffe7b05b998345c1b64a79226231d7d1453236ac338c62dc9fbb3
2 parents f7036a4 + 1b1ffbd commit d819840

File tree

3 files changed

+8
-10
lines changed

3 files changed

+8
-10
lines changed

src/Makefile.am

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -357,7 +357,7 @@ BITCOIN_CORE_H = \
357357

358358
obj/build.h: FORCE
359359
@$(MKDIR_P) $(builddir)/obj
360-
@$(top_srcdir)/share/genbuild.sh "$(abs_top_builddir)/src/obj/build.h" \
360+
$(AM_V_GEN) $(top_srcdir)/share/genbuild.sh "$(abs_top_builddir)/src/obj/build.h" \
361361
"$(abs_top_srcdir)"
362362
libbitcoin_util_a-clientversion.$(OBJEXT): obj/build.h
363363

@@ -1049,7 +1049,7 @@ clean-local:
10491049
-rm -rf test/__pycache__
10501050

10511051
.rc.o:
1052-
@test -f $(WINDRES)
1052+
@test -f $(WINDRES) || (echo "windres $(WINDRES) not found, but is required to compile windows resource files"; exit 1)
10531053
## FIXME: How to get the appropriate modulename_CPPFLAGS in here?
10541054
$(AM_V_GEN) $(WINDRES) $(DEFS) $(DEFAULT_INCLUDES) $(INCLUDES) $(CPPFLAGS) -DWINDRES_PREPROC -i $< -o $@
10551055

@@ -1104,12 +1104,11 @@ endif
11041104

11051105
%.raw.h: %.raw
11061106
@$(MKDIR_P) $(@D)
1107-
@{ \
1107+
$(AM_V_GEN) { \
11081108
echo "static unsigned const char $(*F)_raw[] = {" && \
11091109
$(HEXDUMP) -v -e '8/1 "0x%02x, "' -e '"\n"' $< | $(SED) -e 's/0x ,//g' && \
11101110
echo "};"; \
11111111
} > "$@.new" && mv -f "$@.new" "$@"
1112-
@echo "Generated $@"
11131112

11141113
include Makefile.minisketch.include
11151114

src/Makefile.qt.include

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -371,13 +371,13 @@ translate: $(srcdir)/qt/bitcoinstrings.cpp $(QT_FORMS_UI) $(QT_FORMS_UI) $(BITCO
371371
@rm -f $(srcdir)/qt/locale/bitcoin_en.xlf.old
372372

373373
$(QT_QRC_LOCALE_CPP): $(QT_QRC_LOCALE) $(QT_QM)
374-
@test -f $(RCC)
374+
@test -f $(RCC) || (echo "rcc $(RCC) not found, but is required for generating qrc cpp files"; exit 1)
375375
@cp -f $< $(@D)/temp_$(<F)
376376
$(AM_V_GEN) QT_SELECT=$(QT_SELECT) $(RCC) -name bitcoin_locale --format-version 1 $(@D)/temp_$(<F) > $@
377377
@rm $(@D)/temp_$(<F)
378378

379379
$(QT_QRC_CPP): $(QT_QRC) $(QT_FORMS_H) $(QT_RES_FONTS) $(QT_RES_ICONS) $(QT_RES_ANIMATION)
380-
@test -f $(RCC)
380+
@test -f $(RCC) || (echo "rcc $(RCC) not found, but is required for generating qrc cpp files"; exit 1)
381381
$(AM_V_GEN) QT_SELECT=$(QT_SELECT) $(RCC) -name bitcoin --format-version 1 $< > $@
382382

383383
CLEAN_QT = $(nodist_qt_libbitcoinqt_a_SOURCES) $(QT_QM) $(QT_FORMS_H) qt/*.gcda qt/*.gcno qt/temp_bitcoin_locale.qrc
@@ -404,7 +404,7 @@ bitcoin_qt_apk: FORCE
404404
cd qt/android && ./gradlew build
405405

406406
ui_%.h: %.ui
407-
@test -f $(UIC)
407+
@test -f $(UIC) || (echo "uic $(UIC) not found, but is required for generating ui headers"; exit 1)
408408
@$(MKDIR_P) $(@D)
409409
$(AM_V_GEN) QT_SELECT=$(QT_SELECT) $(UIC) -o $@ $< || (echo "Error creating $@"; false)
410410

@@ -415,6 +415,6 @@ moc_%.cpp: %.h
415415
$(AM_V_GEN) QT_SELECT=$(QT_SELECT) $(MOC) $(DEFAULT_INCLUDES) $(QT_INCLUDES_UNSUPPRESSED) $(MOC_DEFS) $< > $@
416416

417417
%.qm: %.ts
418-
@test -f $(LRELEASE)
418+
@test -f $(LRELEASE) || (echo "lrelease $(LRELEASE) not found, but is required for generating translations"; exit 1)
419419
@$(MKDIR_P) $(@D)
420420
$(AM_V_GEN) QT_SELECT=$(QT_SELECT) $(LRELEASE) -silent $< -qm $@

src/Makefile.test.include

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -422,10 +422,9 @@ endif
422422

423423
%.json.h: %.json
424424
@$(MKDIR_P) $(@D)
425-
@{ \
425+
$(AM_V_GEN) { \
426426
echo "namespace json_tests{" && \
427427
echo "static unsigned const char $(*F)[] = {" && \
428428
$(HEXDUMP) -v -e '8/1 "0x%02x, "' -e '"\n"' $< | $(SED) -e 's/0x ,//g' && \
429429
echo "};};"; \
430430
} > "[email protected]" && mv -f "[email protected]" "$@"
431-
@echo "Generated $@"

0 commit comments

Comments
 (0)