On May 13, 2011, at 1:06 AM, David Holmes wrote: > Kelly, > > I have trouble following the details of this change. > > Here: > > + # Relative path from an output directory to the image directory > + REL_JDK_IMAGE_DIR = > ../$(OUTPUTDIR_BASENAME-$(DEBUG_NAME))/$(JDK_IMAGE_DIRNAME) >
In the above, if DEBUG_NAME is undefined, the relative path will be to the normal product build. $(OUTPUTDIR_BASENAME-) will evaluate to $(ORIG_OUTPUTDIR_BASENAME) > there's no indication that REL_JDK_IMAGE_DIR pertains to a debug build, but > that is what it refers to. DEBUG should appear in the variable name else it > seems odd to make changes like: > > ALT_OUTPUTDIR=$(ABS_OUTPUTDIR)/../$(PLATFORM)-$(ARCH)-$(DEBUG_NAME) > > (which is obviously a DEBUG path) becomes: > > ALT_OUTPUTDIR=$(ABS_OUTPUTDIR)/$(REL_JDK_OUTPUTDIR) > > (which is not obviously a debug path). This stuff is messy due to the use of $(MAKE) ALT_OUTPUTDIR= It's not obvious when something is a debug path name and not. I tried to only use DEBUG or FASTDEBUG in the name if I knew for sure it would be one. > > I'd want to test this change on a number of our builds before passing further > judgement. I think it is something that may have to wait given where we are > with Java 7. Well, it either works or it doesn't. The biggest impact is to those that set ALT_OUTPUTDIR. Getting this fixed removes a patch from the IcedTea's patch list. -kto > > David > > > > Kelly O'Hair said the following on 05/13/11 06:39: >> Need reviewers. (Omair, you will want to verify this works for IcedTea). >> Some background: this changeset: >> http://hg.openjdk.java.net/jdk7/jdk7/rev/47f6b7db1882 >> Created some issues for people setting ALT_OUTPUTDIR to a vanilla path like >> /tmp/foobar. >> The expectation was that a debug build would show up in /tmp/foobar-debug, >> but it was showing >> up in /tmp/OS-ARCH-debug. >> The original changeset was mostly dealing with a Windows issue where you >> cannot just append >> characters to an existing path and expect that path to be valid, so a >> technique of doing a /../ was used. >> This fix tries to make it a bit more obvious what is going on, although I >> have to admit it's a confusing >> situation regardless. >> 7043700: Regression for IcedTea builds >> http://cr.openjdk.java.net/~ohair/openjdk7/jdk7-build-outdebug-7043700/webrev/ >> -kto
