RFR: 8213736: Build fails with LOG=debug on F28 after JDK-8210958

[Severin Gehwolf]

Hi Erik,

Thanks for the review!

On Wed, 2018-11-14 at 09:21 -0800, Erik Joelsson wrote:
> Hello,
> 
> On 2018-11-14 03:27, Severin Gehwolf wrote:
> > Hi,
> > 
> > Thanks for Erik Joelsson for contributing this temporary?! fix for a
> > build issue with LOG=debug.
> > 
> > The actual issue is that on some systems with make 4.x when building
> > with LOG=debug MAKE_TEST_TARGETS receives debug output from the
> > shell'ed make invocation. This in turn changes ALL_NAMED_TESTS which is
> > iterated over in Main.gmk so as to produce test-<name> targets. This
> > fails for some of the supposed test names of: "gmake[1]:", "Leaving",
> > "directory" or "'<top-dir-path>'". All of them are bogus.
> > 
> > In other words, MAKE_TEST_TARGETS gets value:
> > 
> > make-base java-compilation copy-files idea compile-commands gmake[1]: Leaving directory '<top-dir-path>'
> > 
> > instead of:
> > 
> > make-base java-compilation copy-files idea compile-commands
> > 
> > This only happens the second time TestMake.gmk's print-targets is
> > executed. Why is it executed (at least) twice? The chain is something
> > like this:
> > 
> > Init.gmk => InitSupport.gmk (include) => Main.gmk (create-main-targets-include)
> >      => FindTests.gmk (include) => TestMake.gmk (print-targets)
> >      => Modules.gmk (include) => ??? => FindTests.gmk (include) => TestMake.gmk (print-targets)
> 
> What happens here is that Modules.gmk does a -include on a generated 
> file (module-deps.gmk), that has a rule for generating it in the same 
> makefile. Make will check if this file is up to date, if not it will get 
> generated and then make will restart the current top level makefile from 
> the top. It seems like make will continue to evaluate the rest of the 
> current makefile before running the rule however, which is why it 
> continues down in FindTests.gmk one time, then runs the rule for 
> module-deps.gmk, then restarts Main.gmk. It's only on the second go 
> through Main.gmk that the $(shell $(MAKE) -f TestMake.gmk ...) 
> invocation fails. This is the part that really baffles me.

Ugh. Very subtle.

> > As to where Modules.gmk depends on FindTests.gmk (or includes it) is a
> > mystery to me.
> > 
> > The proposed fix is to add --no-print-directory flag to the main make
> > invocation (Main.gmk).
> 
> In my testing it doesn't matter at all what arguments we give make in 
> the $(shell $(MAKE) -f TestMake.gmk ...) call. What matters is the 
> makeflags active in the parent process, which is why adding 
> --no-print-directory in Init.gmk helps. Normally, when running with 
> LOG=info/warn, we have -s in all make calls, which also implies 
> --no-print-directory. That is why this only happens for LOG=debug/trace.
> 
> I think this fix is good enough for now, but perhaps we should consider 
> always running with --no-print-directory even in debug/trace? The 
> directory we run in isn't really relevant considering how we have 
> organized our makefiles. That info is basically assuming a normal 
> recursive makefile structure with a makefile in each src dir. We 
> basically run everything from the top level make dir.
> 
> We could also reconsider the communications model of the targets in 
> FindTests.gmk and avoid the $(shell $(MAKE)) call.
> 
> Regarding the patch, I think we need to add a comment explaining it. 
> Something like:
> 
> # The --no-print-directory is needed to make the call from FindTest.gmk 
> to Test.gmk work
> # with LOG=debug/trace.

Done: http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8213736/webrev.02/

Thanks,
Severin

> /Erik
> 
> > Bug: https://bugs.openjdk.java.net/browse/JDK-8213736
> > webrev: http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8213736/webrev.01/
> > 
> > This allows me to build with LOG=debug again.
> > 
> > Thoughts?
> > 
> > Thanks,
> > Severin
> >