David Holmes (david.hol...@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.hol...@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
