RFR: JDK-8075054 Mixed case Windows path break native dependency checks

[Erik Joelsson]

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
>