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

David Holmes David.Holmes at oracle.com
Mon May 2 23:24:34 UTC 2011


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



More information about the build-dev mailing list