JDK 14 RFR: 8216354: Syntax error in toolchain_windows.m4

[David Holmes]

Erik, Simon,

Thanks for clarifying things. It would have been somewhat clearer if a 
new bug for the space-in-path problem has been filed rather then 
reopening the existing issue.

Cheers,
David

On 13/09/2019 1:10 am, Erik Joelsson wrote:
> Hello David,
> 
> The initial report was about this construct not working at all, as Bash 
> will not expand wildcards inside of quotes:
> 
> "$with_ucrt_dll_dir/*.dll"
> 
> In 13 I changed that to remove the quotes to make the statement work at 
> all. The drawback then, as Simon pointed out, was that if 
> $with_ucrt_dll_dir contains a space, then it won't work. When declaring 
> JDK-8216354 as already fixed, I did not consider the case with space.
> 
> In my opinion, the proposed construct is the preferred one. The set of 
> dlls are known and do not contain spaces. It's however common practice 
> on Windows to install these dlls in directories that contain spaces. I 
> would support backporting this fix as far back as anyone wants to, 
> though the patch will not apply cleanly since different update lines 
> currently have different variants of the statement.
> 
> Note that internally at Oracle, this code path is not used in any 
> official builds, so we are unaffected by this change, which is also why 
> we have not detected it.
> 
> /Erik
> 
> On 2019-09-11 15:07, David Holmes wrote:
>> Hi Simon, Erik,
>>
>> I'm a bit confused. Initially 8216354 was reported as already being 
>> fixed by 8215445. But the fix proposed here actually reverts a change 
>> from 8215445 which means that far from fixing this issue, 8215445 
>> actually introduced it - correct?
>>
>> But 8215445 was only fixed in 13 - no backports - so it can't be 
>> responsible for any problem in 8u or 11u. So some other bug must have 
>> originally fixed this before 13, but was not itself backported to 8u 
>> or 11u.
>>
>> However the original version of the code was introduced by 8202557 in 
>> JDK 11 and was backported to 8u202/8u211. So the code there should be 
>> okay - no?
>>
>> The original code (still in 8u, I didn't check 11u) was:
>>
>> "$with_ucrt_dll_dir/*.dll"
>>
>> and the current proposed fix is:
>>
>> "$with_ucrt_dll_dir/"*.dll
>>
>> which suggests the new fix is less robust if the dll name were to 
>> contain a space rather than the path (but that's probably not good 
>> practice anyway). Or is there some further subtlety I'm missing here?
>>
>> Thanks,
>> David
>> -----
>>
>>
>> On 12/09/2019 3:55 am, Simon Tooke wrote:
>>> This is a trivial fix that has been sitting around since JDK8.
>>>
>>> At one time, 8216354 was marked "won't fix", but I've seen people
>>> (including myself) run up against this issue enough that I think it
>>> should be addressed.  It's been reopened, and I have a webrev ready that
>>> I've tested in the jdk repo, on Windows.  It fixes the issue as
>>> described in the bug description.  I have tested the patch using a
>>> Cygwin build but not a WSL build.
>>>
>>> This patch alters a change introduced in
>>> https://hg.openjdk.java.net/jdk/jdk/annotate/50677f43ac3d/make/autoconf/toolchain_windows.m4#l746 
>>>
>>> That change introduced an issue that prevented the use of directories
>>> with spaces.
>>>
>>> If this patch is accepted, I intend to backport it to 11u and 8u.
>>>
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8216354
>>>
>>> Webrev: http://cr.openjdk.java.net/~stooke/webrevs/jdk-8216354-jdk14/00/
>>>
>>> Thank you,
>>>
>>> -Simon
>>>
>>>
>>>