Request for Review: Conditionalize source so that functionality can be easily included or excluded

[David Holmes]

Hi Joe,

On 18/09/2012 10:30 AM, Joe Provino wrote:
> Sorry, I forgot to provide a link to the new webrev:
>
> http://cr.openjdk.java.net/~jprovino/7189254/webrev.04

The webrev is missing excludeSrc.gmk and the new minimal1.make 
files.(perhaps they were not "hg add"ed?)

I applied your patch to a fresh checkout of hotspot-rt that was situated 
in a jdk8 forest. Initially I hadn't noticed that minimal1.make was 
missing which was a good in a way because it highlighted a logic error 
in make/Makefile in generic_buildminimal1:

+         @if [ -r $(GAMMADIR)/make/$(OSNAME)/makefiles/minimal1.make ] 
; then \
+           $(CD) $(OUTPUTDIR); \
+           $(MAKE) -f $(ABS_OS_MAKEFILE) $(MAKE_ARGS) $(VM_TARGET) ; \
+         else \
+           $(ECHO) "No $(VM_TARGET) for $(OSNAME) 
ARCH_DATA_MODEL=$(ARCH_DATA_MODEL)" ; \
+         fi

If minimal1.make is missing this needs to be a fatal error, because the 
build will fail later anyway because you have added minimal1 related 
targets to the EXPORT_LIST.

FYI I was unable to complete a full build due to build machine issues 
unrelated to this, but I could see minimal get built ok and gamma ran.

>> David, I made all of the changes you wanted (listed in the link below)
>> except for the rule to make optimizedminimal1. optimizedminimal1 would
>> be the same as productminimal1.

Thanks. They all seem to be addressed. One minor modification. In 
make/defs.make can you move:

  102 ifeq ($(USER),)
  103   USER=$(USERNAME)
  104 endif

back up under

   32 # Directory paths and user name
   33 # Unless GAMMADIR is set on the command line, search upward from
   34 # the current directory for a parent directory containing 
"src/share/vm".
   35 # If that fails, look for $GAMMADIR in the environment.
   36 # When the tree of subdirs is built, this setting is stored in 
each flags.make.
   37 GAMMADIR := $(shell until ([ -d dev ]&&echo 
$${GAMMADIR:-/GAMMADIR/}) || ([ -d src/share/vm ]&&pwd); do cd ..; done)
   38 HS_SRC_DIR=$(GAMMADIR)/src
   39 HS_MAKE_DIR=$(GAMMADIR)/make
   40 HS_BUILD_DIR=$(GAMMADIR)/build

so that the comment at #32 remains accurate. No need to regenerate 
webrev just for that.

Looking at the source changes there is an inconsistency with CDS 
handling (as per my direct email re the JPRT crash). arguments.cpp is 
guarding the CDS code using !INCLUDE_SERVICES not !INCLUDE_CDS:

2427 #elif !INCLUDE_SERVICES
2428       vm_exit_during_initialization(
2429           "Dumping a shared archive is not supported int this VM.", 
NULL);
2430 #else

and

3171 #if !INCLUDE_SERVICES
3172   no_shared_spaces();
3173 #endif // INCLUDE_SERVICES

Also note there is a minor type on #2429: int -> in

Thanks,
David


>>
>> joe
>>
>> http://mail.openjdk.java.net/pipermail/hotspot-dev/2012-August/006406.html
>>
>