build: Define .INTERMEDIATE target once only #20684
Conversation
|
Concept ACK, not willing to upgrade to review ACK |
| SUBDIRS += doc/man | ||
| endif | ||
| .PHONY: deploy FORCE | ||
| .INTERMEDIATE: $(OSX_TEMP_ISO) $(COVERAGE_INFO) |
There was a problem hiding this comment.
Is it safe/correct to have OSX_TEMP_ISO here even though it's not a thing for non-OSX? It is unconditionally assigned a value on line 38.
There was a problem hiding this comment.
Is it safe/correct to have OSX_TEMP_ISO here even though it's not a thing for non-OSX?
"OSX_TEMP_ISO ... [i]s not a thing for non-OSX" seems like a bit vague statement for the next reasons:
- Being a global variable, the
OSX_TEMP_ISOis defined regardless of the place of its evaluation, i.e., on all platforms:
$ make print-OSX_TEMP_ISO
OSX_TEMP_ISO = Bitcoin-Core.temp.iso
-
On macOS, when
BUILD_DARWINis set totrue, the resultedMakefilehas no theBitcoin-Core.temp.isotarget. So one of prerequisites of the.INTERMEDIATEtarget has no recipe of its own.makeis aware of such possible conditions (e.g., https://www.gnu.org/software/make/manual/html_node/Last-Resort.html) -
Being a special target, the
.INTERMEDIATEjust explicitly marks its each prerequisite file as intermediate, regardless if the file is mentioned explicitly in some other way
There was a problem hiding this comment.
Yes I was a bit unclear there, sorry.
I wondered if it might have been better to define OSX_TEMP_ISO as empty on non-DARWIN, instead of assign it a value, so that it's not included in .INTERMEDIATES.
But it is good to know that adding it to .INTERMEDIATES cannot actually trigger the build rule. I guess that addresses my concern.
Guix builds
|
jonatack
left a comment
There was a problem hiding this comment.
Thanks for working on this, I've been seeing this warning since 3-4 days. This patch appears to fix it for me on Debian.
|
Tested and very light review ACK 5e0dedb IDK if there is a better solution but I've been recompiling several times a day with this patch while testing pulls and it resolves the warning while working fine for me. |
5e0dedb build: Define .INTERMEDIATE target once only (Hennadii Stepanov) Pull request description: A new warning was introduced in bitcoin@22437fc (bitcoin#20470): ``` $ ./autogen.sh ... Makefile.am:335: warning: .INTERMEDIATE was already defined in condition !BUILD_DARWIN, which is included in condition TRUE ... Makefile.am:139: ... '.INTERMEDIATE' previously defined here ... ``` Fixed in this PR. ACKs for top commit: jonatack: Tested and very light review ACK 5e0dedb Tree-SHA512: ecf8de79ba394c36ee84e0b8d3ba78587e0f856259e9731e6bbb38d0baebfd083eb44d7ef6a386dd9e4508dd64fec1c2b9a007e175fbd4d986e845b1c300a649
5e0dedb build: Define .INTERMEDIATE target once only (Hennadii Stepanov) Pull request description: A new warning was introduced in bitcoin@22437fc (bitcoin#20470): ``` $ ./autogen.sh ... Makefile.am:335: warning: .INTERMEDIATE was already defined in condition !BUILD_DARWIN, which is included in condition TRUE ... Makefile.am:139: ... '.INTERMEDIATE' previously defined here ... ``` Fixed in this PR. ACKs for top commit: jonatack: Tested and very light review ACK 5e0dedb Tree-SHA512: ecf8de79ba394c36ee84e0b8d3ba78587e0f856259e9731e6bbb38d0baebfd083eb44d7ef6a386dd9e4508dd64fec1c2b9a007e175fbd4d986e845b1c300a649
5e0dedb build: Define .INTERMEDIATE target once only (Hennadii Stepanov) Pull request description: A new warning was introduced in bitcoin@22437fc (bitcoin#20470): ``` $ ./autogen.sh ... Makefile.am:335: warning: .INTERMEDIATE was already defined in condition !BUILD_DARWIN, which is included in condition TRUE ... Makefile.am:139: ... '.INTERMEDIATE' previously defined here ... ``` Fixed in this PR. ACKs for top commit: jonatack: Tested and very light review ACK 5e0dedb Tree-SHA512: ecf8de79ba394c36ee84e0b8d3ba78587e0f856259e9731e6bbb38d0baebfd083eb44d7ef6a386dd9e4508dd64fec1c2b9a007e175fbd4d986e845b1c300a649
A new warning was introduced in 22437fc (#20470):
Fixed in this PR.