Need official reviewers: hotspot makefile changes from build-infra

Magnus Ihse Bursie magnus.ihse.bursie at oracle.com
Wed Oct 17 12:42:16 PDT 2012


First, I think I'm the author of most of these changes but perhaps not 
all, I need to check the webrev more carefully to figure that out. :-)

On 2012-10-17 05:32, David Holmes wrote:

> I got a real sense of deja-vu looking at this. I don't understand what 
> happened to the TEST_IN_BUILD changes - somehow half of that work got 
> left behind ??

Yep. By mistake, what was pushed into mainline was the first attempt at 
the test-in-build fix, not the latest one. :-( I've been mailing 
privately to Kelly on the best way to resolve this, but he's been away 
for a while and haven't had time to sort it out. I'm not sure what's 
happening now, if pushing this patch would result in the correct code or 
not. Even though it will need more work, I think it would be a good 
thing to separate that out from this current patch.

>
> As for the $(LOG_INFO) changes, I don't get it. This kind of change 
> almost makes sense:
>
> -      _JUNK_ := $(shell echo >&2 "INFO: ALT_OBJCOPY=$(ALT_OBJCOPY)")
> +      _JUNK_ := $(if $(LOG_INFO),,$(shell echo >&2 "INFO: 
> ALT_OBJCOPY=$(ALT_OBJCOPY)"))
>
> I think the intent is that if $(LOG_INFO) is not empty then we will 
> echo the information. But the way it is written we do the echo if 
> LOG_INFO is empty - which I don't understand. But I don't understand 
> all the changes of the form:
>
> @echo $(LOG_INFO) ...
>
> What is that supposed to do? What value will LOG_INFO have?

I understand your confusion. Here's the trick: If you want to print 
information at the "info" level (that is, you have enabled "info" or any 
more detailed level), then LOG_INFO will be empty. Otherwise, it will 
contain "> /dev/null". That means, if you have a log level less detailed 
than info, all messages at the info level will be hidden. But this value 
can also be used as a check. I don't really like to do it that way, but 
the "JUNK/shell echo >&2" stuff gave me no option. (For me that's really 
weird, because "info" is for me like: "this is non-essential, you can 
ignore it if you want" and writing to stderr is like "this is really 
essential, do not ignore this!", and echoing "info" to stderr is... 
well... messed up.)

>
> That aside in top.make where you add:
>
> +    @echo Making vm...
>
> shouldn't that be
>
> +    @echo $(LOG_INFO) Making vm...

The idea here was that this top-level message should be at a "higher" 
log level, so even if you hide all "info" level messages, you should 
still see that we're about to create the vm.

/Magnus



More information about the build-infra-dev mailing list