RFR: 8301197: Make sure use of printf is correct and actually needed

[Magnus Ihse Bursie]

On Thu, 3 Apr 2025 20:15:00 GMT, Mikael Vidstedt <mikael at openjdk.org> wrote:

>> We have been sloppy in our use of `printf` in make code. Most of the time, we should really use `echo` instead. If we do need to use `printf`, we should never inline make or shell variables into the formatting string, since they may contain `%` which will be interpreted as formatting. Instead, we should always use `%s` and pass the variable as an argument to `printf`.
>> 
>> I've checked the entire code base for usages of $PRINTF, and converted most of them to $ECHO, and made sure the remaining ones are correct. I also discovered some additional ugly stuff in relation to this, which I fixed.
>
> make/Docs.gmk line 267:
> 
>> 265: 	$$(call LogInfo, Creating overview.html for $1)
>> 266: 	$$(call MakeDir, $$(@D))
>> 267: 	$$(PRINTF) "%s" '$$($1_OVERVIEW_TEXT)' > $$@
> 
> For my education, why isn't this just `$$(ECHO) '$$($1_OVERVIEW_TEXT)' > $$@` ?

That would add an extra \n at the end. Maybe it does not matter so much, but I did not want to change anything.

> make/Init.gmk line 163:
> 
>> 161: 	            $(PARALLEL_TARGETS) $(COMPARE_BUILD_MAKE) $(BUILD_LOG_PIPE) || \
>> 162: 	        ( exitcode=$$? && \
>> 163: 	        $(ECHO) "" $(BUILD_LOG_PIPE_SIMPLE) && \
> 
> Just for my understanding, is the empty string needed, or "just" a stylistic choice?

Yeah, it is a stylistic choice to indicate that the line was intentionally left empty.

> make/autoconf/help.m4 line 295:
> 
>> 293:   $ECHO "* HS debug level: $HOTSPOT_DEBUG_LEVEL"
>> 294:   $ECHO "* JVM variants:   $JVM_VARIANTS"
>> 295:   $ECHO "* JVM features:   "
> 
> I believe this will now add a newline where there was none earlier, so the actual features will show up on a new line?

Good catch; I'll fix that.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/24415#discussion_r2027743799
PR Review Comment: https://git.openjdk.org/jdk/pull/24415#discussion_r2027744477
PR Review Comment: https://git.openjdk.org/jdk/pull/24415#discussion_r2027745056