Customized make file inclusion

[Kelly O'Hair]

I think that looks fine.

-kto

On Jan 18, 2012, at 9:29 PM, David Holmes wrote:

> FYI here is proposed webrev:
> 
> http://cr.openjdk.java.net/~dholmes/7130909/webrev/
> 
> CR 7130909 was filed.
> 
> David
> 
> On 19/01/2012 1:30 PM, David Holmes wrote:
>> Hi Kelly,
>> 
>> On 19/01/2012 2:22 AM, Kelly O'Hair wrote:
>>> 
>>> On Jan 17, 2012, at 7:17 PM, David Holmes wrote:
>>> 
>>>> On 18/01/2012 7:07 AM, Kelly O'Hair wrote:
>>>>> Generally, I have no objection to this change.
>>>>> 
>>>>> But instead of setting it to NO_SUCH_PATH, I would not set it at
>>>>> all, and avoid the $(shell) to see if it
>>>>> exists completely. Just don't set CUSTOM_MAKE_DIR if it does not exist.
>>>>> Then you don't need the HAS_CUSTOM_MAKE, it can just be ifdef
>>>>> CUSTOM_MAKE_DIR
>>>> 
>>>> But I/we want the default setting to occur in the build files and not
>>>> require that it be set externally. My final suggestion gets rid of
>>>> the HAS_CUSTOM_MAKE and the shell usage. Incorporating your
>>>> suggestion it simplifies to:
>>>> 
>>>> ifneq ($(OPENJDK),true)
>>>> CUSTOM_MAKE_DIR = $(BUILDDIR)/closed
>>>> endif
>>> 
>>> Or something like:
>>> 
>>> # Custom makefile additions
>>> ifneq ($(OPENJDK),true)
>>> CUSTOM_MAKE_DIR = $(BUILDDIR)/closed
>>> # This directory should exist if OPENJDK is not "true"
>>> ifeq ($(shell if [ ! -d $(CUSTOM_MAKE_DIR) ]; then echo "missing";
>>> fi),missing)
>>> x:=$(error "ERROR: Not OPENJDK, and missing $(CUSTOM_MAKE_DIR)")
>>> endif
>>> endif
>> 
>> Not sure we need to force it to be available. Seems this kind of thing
>> belongs in a sanity test if anywhere, but personally I don't think it
>> has to be enforced.
>> 
>>>> 
>>>> in Defs.gmk and then:
>>>> 
>>>> -include $(CUSTOM_MAKE_DIR)/<path-to-file.gmk>
>>> 
>>> And:
>>> 
>>> # Custom makefile additions
>>> ifdef CUSTOM_MAKE_DIR
>>> include $(CUSTOM_MAKE_DIR)/<path-to-file.gmk>
>>> endif
>> 
>> The -include is accounting not only for the directory not being present
>> but also the actual file. Over time we may add these include hooks to
>> support a number of different use-cases but that doesn't mean that
>> anybody involved in any single use-case needs to provide every single
>> custom file that might exist.
>> 
>>>> 
>>>> as needed. Though I may get an error with CUSTOM_MAKE_DIR being
>>>> undefined in which case I'll need to declare it for OpenJDK builds.
>>> 
>>> Undefined is the same as being defined as empty in terms of expansion
>>> $(var)
>> 
>> Was unsure as I see examples where people ensure a variable is always
>> defined in both an if and else clause - even if one is empty.
>> 
>> Thanks,
>> David
>> -----
>> 
>>>> 
>>>>> Not sure I like the word CUSTOM, but that's just a name debate.
>>>> 
>>>> It customizes the build logic. I preferred it to the "alt" name we
>>>> use for hotspot as here it isn't an alternative but an addition.
>>>> Anyway just a name game.
>>> 
>>> Not a big deal, but kind of liked the piggyback idea. ;^)
>>> 
>>> -kto
>>> 
>>>> 
>>>> Thanks,
>>>> David
>>>> 
>>>>> Maybe ADDON or MODDING or MODS or oo oo PIGGYBACK? ;^)
>>>>> http://3.bp.blogspot.com/-cHeD4qJSB44/TbMtSsAiY4I/AAAAAAAAA1c/gctQfVffu_o/s1600/PiggyBack.jpg
>>>>> 
>>>>> 
>>>>> -kto
>>>>> 
>>>>> On Jan 15, 2012, at 9:01 PM, David Holmes wrote:
>>>>> 
>>>>>> Presently the build system contains some files, and directives
>>>>>> involving those files, that supports the building of the Java SE
>>>>>> Embedded product. What I would like to do is remove direct support
>>>>>> for this and replace it with a more general mechanism that allows
>>>>>> for custom make files to be included from an arbitrary location -
>>>>>> which then allows for the removal of the embedded specific files.
>>>>>> 
>>>>>> Initially I've modeled this on some other conditional mechanisms by
>>>>>> defining the default location for non-OpenJDK builds, checking its
>>>>>> existence and then using that as a guard for the individual files.
>>>>>> 
>>>>>> http://cr.openjdk.java.net/~dholmes/custom-make/webrev/
>>>>>> 
>>>>>> ! ifneq ($(OPENJDK),true)
>>>>>> ! CUSTOM_MAKE_DIR_REL=closed
>>>>>> ! else
>>>>>> ! CUSTOM_MAKE_DIR_REL=NO_SUCH_PATH
>>>>>> ! endif
>>>>>> ! CUSTOM_MAKE_DIR=$(BUILDDIR)/$(CUSTOM_MAKE_DIR_REL)
>>>>>> 
>>>>>> + # Use this variable to guard inclusion of the custom files
>>>>>> + HAS_CUSTOM_MAKE := $(shell if [ -d $(CUSTOM_MAKE_DIR) ]; then
>>>>>> echo 1; else echo 0; fi)
>>>>>> 
>>>>>> ...
>>>>>> 
>>>>>> + ifeq ($(HAS_CUSTOM_MAKE),1)
>>>>>> + include $(CUSTOM_MAKE_DIR)/Defs.gmk
>>>>>> + endif
>>>>>> 
>>>>>> Initially there are only two hooks for these custom files:
>>>>>> 
>>>>>> 1. At the end common/Defs.gmk (as it gets included by all the
>>>>>> Makefiles)
>>>>>> 2. In the top-level JDK Makefile, including a custom Release.gmk
>>>>>> 
>>>>>> Naturally these map to the existing uses of the *-embedded.gmk files.
>>>>>> 
>>>>>> It then occurred to me that if someone wanted to add an additional
>>>>>> hook somewhere else, that it might be best to check for the
>>>>>> existence of the actual file rather than a root directory for all
>>>>>> such files. That led me to consider a function
>>>>>> custom_include(<filename>) to hide the existence logic. But I
>>>>>> wasn't sure if a function could contain an include directive and
>>>>>> upon checking on that I discovered a much simpler approach: the
>>>>>> -include directive. This is like the include directive but does not
>>>>>> trigger an error if the file does not exist. So we would simply
>>>>>> have this in Defs.gmk:
>>>>>> 
>>>>>> ifneq ($(OPENJDK),true)
>>>>>> CUSTOM_MAKE_DIR_REL=closed
>>>>>> else
>>>>>> CUSTOM_MAKE_DIR_REL=NO_SUCH_PATH
>>>>>> endif
>>>>>> CUSTOM_MAKE_DIR=$(BUILDDIR)/$(CUSTOM_MAKE_DIR_REL)
>>>>>> 
>>>>>> and then
>>>>>> 
>>>>>> -include $(CUSTOM_MAKE_DIR)/<path-to-file.gmk>
>>>>>> 
>>>>>> wherever we needed it (again initially only the two locations I
>>>>>> mentioned).
>>>>>> 
>>>>>> I'd like to hear people's opinions on this.
>>>>>> 
>>>>>> Thanks,
>>>>>> David
>>>>> 
>>>