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

in Defs.gmk and then:

-include $(CUSTOM_MAKE_DIR)/<path-to-file.gmk>

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.

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.

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

Reply via email to