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

[Langer, Christoph]

Hi,

I've also played with this already and support Simon's patch.

Simon, shall I sponsor it for you?

Best regards
Christoph

> -----Original Message-----
> From: build-dev <build-dev-bounces at openjdk.java.net> On Behalf Of Erik
> Joelsson
> Sent: Donnerstag, 12. September 2019 17:10
> To: David Holmes <david.holmes at oracle.com>; Simon Tooke
> <stooke at redhat.com>; build-dev <build-dev at openjdk.java.net>
> Subject: Re: JDK 14 RFR: 8216354: Syntax error in toolchain_windows.m4
> 
> 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/autocon
> f/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
> >>
> >>
> >>