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

[John Coomes]

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).

>                                                   ... 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