<html>
<head>
<meta content="text/html; charset=utf-8" http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
Hi Max<br>
<br>
Thank you for your update and its associated effort.<br>
<br>
I suggest that at least a comment should be added about why and how
non-existing files can be added and the test still serves it's
purpose. In fact I was quite a bit surprised when I found out that
JarUtils.createJar adds the file name as contents if the file cannot
be found. Ideally, we would also add some note about why this was
relevant, about the test not compiling on certain oses with jtreg
together with the acute but it doesn't apply any longer now.
Altogether the test has become even simpler now.<br>
<br>
One more small thing that I just noticed now is that I would prefer
ManifestFileName not to start with a capital letter like the other
nearby variables. I thought testName was too ambiguous a name and
changed it to testClassName. I also reviewed and slightly changed a
few comments again. Of course it will never become perfect but now
it should do. See attached patch.<br>
<br>
The contributed by is fine. I'd be glad to share credits with you
and please accept more flattering for your collaboration.<br>
<br>
Regards,<br>
Philipp<br>
<br>
<br>
<br>
<div class="moz-cite-prefix">On 27.09.2017 03:20, Weijun Wang wrote:<br>
</div>
<blockquote
cite="mid:45B1F083-CDB3-4545-B217-24AF40231625@oracle.com"
type="cite">
<pre wrap="">Hi Philipp
The problem is that when launching by jtreg javac has difficulties writing the class file with the é char into the file system on several OSes.
I've updated the test a little. Now they are not written to files. Fortunately JarUtils can add non-existing entries. The test now passes on all our testing platforms.
<a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~weijun/6695402/webrev.01">http://cr.openjdk.java.net/~weijun/6695402/webrev.01</a>
If you are OK with the new version I'll push them.
Thanks
Max
</pre>
<blockquote type="cite">
<pre wrap="">On Sep 26, 2017, at 10:54 PM, Weijun Wang <a class="moz-txt-link-rfc2396E" href="mailto:weijun.wang@oracle.com"><weijun.wang@oracle.com></a> wrote:
It might be a jtreg issue, but I'll have to get it resolved before pushing your changeset.
--Max
</pre>
<blockquote type="cite">
<pre wrap="">On Sep 26, 2017, at 7:30 PM, Weijun Wang <a class="moz-txt-link-rfc2396E" href="mailto:weijun.wang@oracle.com"><weijun.wang@oracle.com></a> wrote:
Oops, the new test fails on Linux and Solaris.
/scratch/test/jdk/sun/security/tools/jarsigner/LineBrokenMultiByteCharacter.java:54: error: error while writing A1234567890B1234567890C123456789D1?xyz: bad filename RelativeFile[LineBrokenMultiByteCharacter$A1234567890B1234567890C123456789D1?xyz.class]
static class A1234567890B1234567890C123456789D1\u00E9xyz { }
^
1 error
I'll ask the compiler team.
--Max
</pre>
<blockquote type="cite">
<pre wrap="">On Sep 26, 2017, at 3:51 PM, Weijun Wang <a class="moz-txt-link-rfc2396E" href="mailto:weijun.wang@oracle.com"><weijun.wang@oracle.com></a> wrote:
</pre>
<blockquote type="cite">
<pre wrap="">On Sep 26, 2017, at 1:37 PM, Philipp Kunz <a class="moz-txt-link-rfc2396E" href="mailto:philipp.kunz@paratix.ch"><philipp.kunz@paratix.ch></a> wrote:
Hi Max
This time I got it with readAllBytes. Thank you for the hint.
Apparently, UTF characters are allowed in source code, particularly in identifiers here, which also has caused the bug. Even if only for sending patches around I changed it and was surprised to see escaping working not only in strings but also in identifiers.
</pre>
</blockquote>
<pre wrap="">
See <a class="moz-txt-link-freetext" href="https://docs.oracle.com/javase/specs/jls/se8/html/jls-3.html#jls-3.2">https://docs.oracle.com/javase/specs/jls/se8/html/jls-3.html#jls-3.2</a>
I've submitted your change to our testing server. Once it's OK, I'll push the changeset.
I assume "Contributed-by: Philipp Kunz <a class="moz-txt-link-rfc2396E" href="mailto:philipp.kunz@paratix.ch"><philipp.kunz@paratix.ch></a>" is good.
BTW, there are several TAB chars and trailing spaces in your patch. I've removed them.
Thanks for your contribution.
--Max
</pre>
<blockquote type="cite">
<pre wrap="">
When I had another look at the test I came to the conclusion that it does not need what has been named refClassFileName before. The purpose of the test is only to check a signature of a class with a two byte character in its name and not at the same time to verify that if that test failed it is specifically because of the name. If it fails there is a problem no matter why. In the beginning it was handy to see the difference but I don't think it should be kept and maintained so I removed it. For the update signature case a second file to sign is still required though.
I considered multi-byte a one word before but now I also prefer it with a capital b. Anyway, this name might not be the best choice and I changed it to LineBrokenMultiByteCharacter.
See attached patch.
Regards,
Philipp
</pre>
</blockquote>
<pre wrap="">
</pre>
</blockquote>
<pre wrap="">
</pre>
</blockquote>
<pre wrap="">
</pre>
</blockquote>
<pre wrap="">
</pre>
</blockquote>
<br>
<div class="moz-signature">-- <br>
<meta http-equiv="content-type" content="text/html; charset=utf-8">
<title></title>
<br>
<br>
Gruss Philipp<br>
<br>
<br>
<br>
<hr size="2" width="100%"><br>
<img shrinktofit="true"
src="cid:part1.BDBA5550.DA0B3875@paratix.ch" align="bottom"
border="0"><br>
<br>
Paratix GmbH<br>
St Peterhofstatt 11<br>
8001 Zürich<br>
<br>
+41 (0)76 397 79 35<br>
<a href="mailto:philipp.kunz@paratix.ch">philipp.kunz@paratix.ch</a>
</div>
</body>
</html>