Reviewer needed - fix for IcedTea bug#637

[Mark Wielaard]

Hi Andrew,

On Tue, 2011-03-08 at 20:58 +0000, Dr Andrew John Hughes wrote:
> Given this new target kills off jtreg processes, I don't see how you'd
> run it to get a current summary.  Neither would such a target be
> a dependency of jtregcheck as it's never fulfilled.
> 
> What you're thinking of is an independent target where the whole thing is
> just:
> 
> jtregcheck-summary:
> 	for i in hotspot langtools jdk; do \
> 	  if [ -e test/check-$$i.log ] ; then
> 	    echo "--------------- jtreg console summary for $$i ---------------"; \
> 	    egrep -v '^(Passed:|Directory|Re[a-z]+\ written\ to)' test/check-$$i.log; \
> 	  fi
> 	done | tee test/jtreg-summary.log
> 
> and which nothing depends on.  While the tests are running, make jtregcheck-summary
> would be called independently and print the current status.  That's a nice
> idea, but not what this patch is doing.

No, it has indeed two functions. It does the summary and finishes any
stray processes from the jtregcheck target(s). I guess you didn't like
my suggestion for the name, and Pavel's orginal name jtregcheck-finish
was the better one. I don't think the functionality should be split. But
they can be if you want.

> The current jtregcheck target runs after all required checks have been
> run, prints a summary and kills off any processes floating around.

That is what jtregcheck-summary does to. It is the last dependent target
that gets run after all requested tests have been run. It is just that
the target has been split to have the last step independently.

> This change breaks this ordering for no apparent gain.  We now have
> a pointless jtregcheck with no body and a jtregcheck-summary which
> can run at any time and start killing processes.

Maybe I am missing how you think the ordering is broken. make check (or
make jtregcheck) still does the same thing. It does require make check
-k now though (since the other targets can fail). Is that what you are
objecting to?

Cheers,

Mark