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

[Erik Joelsson]

Looks good!

/Erik

On 2018-11-14 10:38, Severin Gehwolf wrote:
> 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
>>>