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

[David Holmes]

Hi Kelly,

Well as they say "the proof of the pudding is in the eating" and while 
the actual make logic changes seem a little puzzling to me the end 
result is good. The use of <os>-<arch>-fastdebug had caused problems for 
builds that had differentiators beyond OS and ARCH.

So thumbs up from me.

Thanks,
david

Kelly O'Hair said the following on 05/14/11 02:06:
> 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
>