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
