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
