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

[Tim Bell]

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
>>
>