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

[David Holmes]

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