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

[David Holmes]

John Coomes said the following on 05/05/11 04:36:
> David Holmes (David.Holmes at oracle.com) wrote:
>> John Coomes said the following on 05/04/11 02:37:
>>> David Holmes (David.Holmes at oracle.com) wrote:
>>>> 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.
>>> FWIW, I prefer the change you've made, but don't feel that strongly
>>> about it.
>>>
>>>> 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).
>>> IMHO, better if an OPENJDK build doesn't use alt-src, at least by
>>> default.  And I suspect you can override HS_ALT_SRC_REL from the gmake
>>> command line, even when OPENJDK==true (haven't tried it, though).
>> No. Unless you use -e a variable's value from the environment will be 
>> overridden by an explicit assignment in the Makefile.  ...
> 
> True about the environment, but I meant this:
> 
> 	gmake product OPENJDK=true HS_ALT_SRC_REL=my_impl
> 
> which will override the assignment within the make file
> (http://www.gnu.org/s/hello/manual/make/Overriding.html#Overriding).

That didn't work for me either. Something went very awry when I was 
working on this bug as things that should have worked simply did not and 
I have no idea why.

David

>>                                                   ... Which means that 
>> the better fix here is:
>>
>> + 36 ifndef HS_ALT_SRC_REL
>>    37   ifneq ($(OPENJDK),true)
>>    38     # This needs to be changed to a more generic location, but we 
>> keep it as this
>>    39     # for now for compatibility
>>    40
>>    41     HS_ALT_SRC_REL=src/closed
>>    42   else
>>    43     HS_ALT_SRC_REL=$(HS_COMMON_SRC_REL)
>>    44   endif
>> + 45 endif
> 
> This version treats HS_ALT_SRC_REL differently from other vars, in
> that a value from the environment overrides the assignment in the
> makefile, even without the -e option to gmake.
> 
> -John