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

[John Coomes]

David Holmes (David.Holmes at oracle.com) wrote:
> 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.

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

-John