Customized make file inclusion (original) (raw)

Kelly O'Hair kelly.ohair at oracle.com
Thu Jan 19 22:05:37 UTC 2012


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 NOSUCHPATH, I would not set it at all, and avoid the $(shell) to see if it exists completely. Just don't set CUSTOMMAKEDIR if it does not exist. Then you don't need the HASCUSTOMMAKE, it can just be ifdef CUSTOMMAKEDIR 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 HASCUSTOMMAKE and the shell usage. Incorporating your suggestion it simplifies to: ifneq ($(OPENJDK),true) CUSTOMMAKEDIR = $(BUILDDIR)/closed endif Or something like: # Custom makefile additions ifneq ($(OPENJDK),true) CUSTOMMAKEDIR = $(BUILDDIR)/closed # This directory should exist if OPENJDK is not "true" ifeq ($(shell if [ ! -d $(CUSTOMMAKEDIR) ]; then echo "missing"; fi),missing) x:=$(error "ERROR: Not OPENJDK, and missing $(CUSTOMMAKEDIR)") 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 $(CUSTOMMAKEDIR)/<path-to-file.gmk> And: # Custom makefile additions ifdef CUSTOMMAKEDIR include $(CUSTOMMAKEDIR)/<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 CUSTOMMAKEDIR 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/gctQfVffuo/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) ! CUSTOMMAKEDIRREL=closed ! else ! CUSTOMMAKEDIRREL=NOSUCHPATH ! endif ! CUSTOMMAKEDIR=$(BUILDDIR)/$(CUSTOMMAKEDIRREL) + # Use this variable to guard inclusion of the custom files + HASCUSTOMMAKE := (shellif[−d(shell if [ -d (shellif[d(CUSTOMMAKEDIR) ]; then echo 1; else echo 0; fi) ... + ifeq ($(HASCUSTOMMAKE),1) + include $(CUSTOMMAKEDIR)/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 custominclude() 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) CUSTOMMAKEDIRREL=closed else CUSTOMMAKEDIRREL=NOSUCHPATH endif CUSTOMMAKEDIR=$(BUILDDIR)/$(CUSTOMMAKEDIRREL) and then -include $(CUSTOMMAKEDIR)/<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



More information about the build-dev mailing list