Review took longer than I planned. Your fix looks good.

In addition to conversion to java, fix has good enhancements needed to tests. For instance - changing non-default digest algorithm to SHA-1 from SHA-256 for AlgOptions.sh. Thanks for these changes.

Thanks,
Rajan

On 3/25/19 8:20 PM, Weijun Wang wrote:
Ding-dong.

On Mar 5, 2019, at 8:26 AM, Weijun Wang <weijun.w...@oracle.com> wrote:

Webrev updated at

   https://cr.openjdk.java.net/~weijun/8180573/webrev.01

BTW, last time I mistakenly removed ExportPrivateKeyNoPwd.java which is used by 
ListKeychainStore.sh. It's now back.

Thanks,
Max

On Feb 15, 2019, at 9:31 PM, Weijun Wang <weijun.w...@oracle.com> wrote:

Hi Philipp,

In most cases, it's just about creating a non-empty file; in some case, the content is 
relevant. For the former, I will change it to something like "new byte[10]"; 
for the latter, I'll use getBytes(UTF_8).

Thanks,
Max


On Feb 15, 2019, at 5:34 PM, Philipp Kunz <philipp.k...@paratix.ch> wrote:

Hi Max,

I don't know if it is important enough, certainly not a serious issue.
In your patch, for example in DiffEnd.java and a few other tests, Strings are 
encoded to byte streams with String.getBytes() which uses the default platform 
character set to encode the strings.
Manifests, however, always use UTF-8 as the character set to encode. In my 
opinion, getBytes(java.nio.charset.StandardCharsets.UTF_8) would be appropriate 
to specify the encoding in a platform-independent way.
Now the manifests used in the tests use so few different characters that this 
might not even make a real difference because the first around 100 characters 
or so of most character sets are the same in most encodings.
I suspect that the encoding might also have been platform-dependent in at least 
some of the previous shell tests and therefore this aspect might as well be 
addressed separately and is not strictly necessary to just properly convert 
shell tests to java.

Regards,
Philipp


On Wed, 2019-02-13 at 11:01 +0800, Weijun Wang wrote:
Please review the fix at


https://cr.openjdk.java.net/~weijun/8180573/webrev.00/


Notes:

- Most changes are just .sh -> .java

- StorePasswordsByShell.sh combined into StorePasswords.java

- In most cases, JarUtils is called to create a jar file. Sometimes the jar 
command is called because of delicate differences, for example, jar adds 
directory entries also.

- The i18n tests are completely manual described in i18n.html. Old i18n.java is 
useless, now is also empty.

- Copyright year updated to 2019, @bug unchanged.

Two files not converted yet:

- ./keytool/console.sh

This is a manual test calling old versions of JDK.

- ./keytool/ListKeychainStore.sh

I tried on this one but "security list-keychains -s ..." has no effect on mach5 
machines when calling by ProcessTools. No idea why, I've created a separate bug 
(JDK-8218886) for it.

Thanks,
Max



Reply via email to