Need reviewers: 7043700: Regression for IcedTea builds (e.g. ALT_OUTPUTDIR)

[Kelly O'Hair]

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