Magnus:
Looks good to me as well.
Tim
On 03/13/15 05:56, Erik Joelsson wrote:
Looks good!
/Erik
On 2015-03-13 13:37, Magnus Ihse Bursie wrote:
Bug description:
If the top-level directory of the forest contains mixed case elements
in the path, then the script which checks for native C/C++ headers
dependencies doesn't work properly.
This happens because Visual Studio compiler sometimes (?) produce
lower-case only paths when invoked with the -showIncludes option used
in the dependency tracking, and the script doesn't match these paths
with the mixed case paths produced from the top-level directory.
For example, if the top-level directory is /cygdrive/c/Vadim/jdk9,
then the cl.exe invocation cmdline looks like this:
/cygdrive/c/Vadim/jdk9/.../fixpath.exe -c ...cl ... -c -showIncludes
... /cygdrive/c/Vadim/jdk9/...foo.cpp | tee ... foo.d.raw
The .d.raw file contains these lines (note the lowercase path):
Note: including file:
c:\vadim\jdk9-cpu\jdk\src\java.desktop\share\native\foo.h
My proposed fix:
diff --git a/make/common/NativeCompilation.gmk
b/make/common/NativeCompilation.gmk
--- a/make/common/NativeCompilation.gmk
+++ b/make/common/NativeCompilation.gmk
@@ -60,7 +60,7 @@
-e 's|Note: including file: *||' \
-e 's|\\|/|g' \
-e 's|^\([a-zA-Z]\):|$(UNIX_PATH_PREFIX)/\1|g' \
- -e '/$(subst /,\/,$(TOPDIR))/!d' \
+ -e '\|$(TOPDIR)|I !d' \
-e 's|$$$$| \\|g' \
#
@@ -153,7 +153,7 @@
exit `cat $$($1_$2_DEP).exitvalue`
$(RM) $$($1_$2_DEP).exitvalue
($(ECHO) $$@: \\ \
- && $(SED) $(WINDOWS_SHOWINCLUDE_SED_PATTERN)
$$($1_$2_DEP).raw) > $$($1_$2_DEP)
+ && $(SED) $(WINDOWS_SHOWINCLUDE_SED_PATTERN)
$$($1_$2_DEP).raw) | $(SORT) -u > $$($1_$2_DEP)
endif
# Create a dependency target file from the dependency file.
# Solution suggested by
http://make.mad-scientist.net/papers/advanced-auto-dependency-generation/
Some comments might be warranted.
I learned a bit about sed expressions when fixing this. :) For those
who didn't know, sed expressions consists of two parts, an "address"
and a "command". A typical "s/foo/bar/" is just the command "s" with
some arguments. The "s" command allows the separator to be changed
from / to anything else, eg "s!foo!bar!", and without an address, it
works on all lines.
The problematic line is instead using the "d" (delete), or more
precisely, the "!d" (not delete) command. The expression in front is
the "address", which in this case is a regexp. So lines matching the
regexp would not be deleted (but all others will). Since TOPDIR
contains slashes, the old expression used make $(subst) to espace
these slashes, so they wouldn't be interpreted as ending the address
regexp. A better solution is to use a different character, but for
the address, this require the regexp to be preceeded by a backslash.
Hence, e.g. "\|foo| d" would delete all lines matching foo.
After the expression is what solves this bug. It is an uppercase I,
which is a GNU sed extension to make the regexp case insensitive.
While it is available in GNU sed only, this is not a problem since
it's only being used on Windows, where only GNU sed is supported.
The second changed line is strictly not needed to fix the bug, but
removes the prolific duplication of header files that the .d files
had on Windows before, drastically reducing them in size.
Bug: https://bugs.openjdk.java.net/browse/JDK-8075054
/Magnus
