Request for review: 7036525 Disable alternative source mechanism for OPENJDK builds

[David Holmes]

Fixed cc to hotspot-dev

-------- Original Message --------
Subject: Re: Request for review: 7036525 Disable alternative source 
mechanism	for OPENJDK builds
Date: Tue, 03 May 2011 09:24:34 +1000
From: David Holmes <David.Holmes at oracle.com>
Organization: Oracle Corporation
To: Keith McGuigan <keith.mcguigan at oracle.com>
CC: John Coomes <John.Coomes at oracle.com>,        build-dev 
<build-dev at openjdk.java.net>,        hotspot-dev at openjdk.dev.java.net
References: <4DB90020.7090708 at oracle.com> 
<19898.63905.375477.693659 at oracle.com> 
<33B3C808-9DA2-4776-8E05-5B96FE7359AD at oracle.com>

Hi Keith,

Keith McGuigan said the following on 05/03/11 02:13:
> On Apr 29, 2011, at 1:47 PM, John Coomes wrote:
> 
>> David Holmes (David.Holmes at oracle.com) wrote:
>>> http://cr.openjdk.java.net/~dholmes/7036525/webrev/
>>>
>>> Simple but crude. If OPENJDK is defined then the Hotspot "alternative
>>> source" mechanism is effectively disabled by checking for a non-existent
>>> path. This allows people using the alt-src mechanism to select which
>>> type of build they want in a way that is consistent with how builds of
>>> OPENJDK are done in the rest of the JDK.
>>>
>>> Tested by checking the "errorReporter.cpp" location in builds
>>> with/without OPENJDK set, and with/without src/closed present.
>>>
>>> This will be pushed into hotspot-rt/hotspot for hs21-b11
>>
>> Hi David,
>>
>>  38 ifneq ($(OPENJDK),true)
>>  39   HS_ALT_SRC_REL=src/closed
>>  40 else
>>  41   HS_ALT_SRC=NO_SUCH_PATH
>>  42 endif
>>  43 HS_COMMON_SRC=$(GAMMADIR)/$(HS_COMMON_SRC_REL)
>>  44 HS_ALT_SRC=$(GAMMADIR)/$(HS_ALT_SRC_REL)
>>
>> The 'if' block sets HS_ALT_SRC_*REL*, but the else block sets
>> HS_ALT_SRC (no *REL*), and that is overwritten on line 44.
>>
>> I think it works because after line 44, HS_ALT_SRC == $(GAMMADIR)/,
>> but I doubt that was intended.
>>
>> You could change line 41 to
>>
>>     HS_ALT_SRC_REL=$(HS_COMMON_SRC_REL)
>>
>> Then when OPENJDK=true, HS_ALT_SRC==HS_COMMON_SRC, and you don't have
>> to rely on NO_SUCH_PATH.
> 
> I'd prefer to leave it as NO_SUCH_PATH.  That way tests for the 
> existence of that directory (or files in those directories) will 
> correctly fail and won't mistakenly indicates that alt-srcs exist.  I 
> believe there are a number of tests like that already, but I'll bet it 
> currently works fine (unintentionally) if HS_ALT_SRC == HS_COMMON_SRC.

I've just made the change as John suggested and to be honest I don't
know why I didn't think of that myself. I do see your point though, by
setting it the same the build will always use the ALT_SRC in the OpenJDK
case - but this will be fine because it is the same as COMMON_SRC. This
is only used to generate the Makefiles during the buildtree phase so I
don't think it is really a concern either way.

To be honest I'm doubting the whole rationale for this change as it
means that an OPENJDK build will never use the alt-src mechanism, when
according to the comments alt-src was also intended to be used by others
for introducing alternative code into their builds/distributions. In
those cases you may well want both alt-src and OPENJDK (given that
OPENJDK could be being set at the top-level JDK makefile).

David