On Nov 12, 2010, at 3:14 PM, Mike Duigou wrote:
On Nov 6 2010, at 07:07 , David Holmes wrote:
6998016: Incorrect ifdef nesting in sane-gcc-compiler rule
http://cr.openjdk.java.net/~mduigou/6998016.0/webrev/
In Defs-linux.gmk I must be missing something:
ifneq "$(origin ALT_GCC29_COMPILER_PATH)" "undefined"
and
ifdef ALT_GCC29_COMPILER_PATH
seem completely equivalent. The latter is simpler of course.
That's the conclusion I came to as well after scratching my head for
a couple minutes to figure out why $(origin ) was used in this
specific case. The change is to minimize future head scratching.
The ifneq "$(origin ALT_GCC29_COMPILER_PATH)" "undefined"
means that the variable was defined to something, potentially the
empty value.
The ifdef ALT_GCC29_COMPILER_PATH
means that the variable is defined to a non-empty value.
At least that's what I understand it to mean.
I have no objection to the change, I vaguely recall adding the use of
this $(origin) and it had
something to do with allowing an empty value in this variable.
This and the ALT_COMPILER_PATH were variables that were potentially
empty, or a path ending in /
so that you would use $(ALT_COMPILER_PATH)gcc and if it was empty,
gcc would come from PATH.
Personally, these things are a pain, hotspot makefiles ignores it, and
on Windows you are forced to have
the compiler in PATH anyway.
-kto
In Sanity.gmk ... when I reported this problem it was stated that
the correct fix was to swap the order of these two lines:
1486 ifeq ($(PLATFORM), solaris)
1487 ifndef OPENJDK
Not saying what you have is wrong, just different to what was
stated previously.
That organization does make more sense.
Also note the comment at 1483 is out of date as it only refers to
Solaris.
I've now corrected the comment to make it clear that these sanity
tests are about the OJI plugin.
Updated webrev at http://cr.openjdk.java.net/~mduigou/6998016.1/
webrev/
Mike
