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

Keith McGuigan keith.mcguigan at oracle.com
Mon May 2 16:13:52 UTC 2011


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.

--
- Keith



More information about the build-dev mailing list