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-boun...@openjdk.java.net> On Behalf Of Erik > Joelsson > Sent: Donnerstag, 12. September 2019 17:10 > To: David Holmes <david.hol...@oracle.com>; Simon Tooke > <sto...@redhat.com>; build-dev <build-dev@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 > >> > >> > >>
