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

[Joe Provino]

On 9/18/2012 2:44 AM, David Holmes wrote:
> 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.

minimal1.make shouldn't be missing.  Do I need the @if or can I just get 
rid of it?

joe

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