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