[rfc][icedtea-web]detecting jre in rust lunchers

[Jiri Vanek]

Hello Alex.

I'm terribly sorry for long delay. There were issues I had to prioritize. Please see pm.

The patch is adapted to most of the issues you rebuked.
...
> 
> Test-only utils.rs doesn't belong to src, and when moving it to tests some (most?) other tests can 
> go with it.

Skipped for now. Will be handled once the testbase grow. Oook?
Current tests should be considered as unittests. In same matter, the test_utils are cfg out from 
non-test run.

A
> 
> 
>> Rust clearly saysi unittests to source file, integration tests to test folder. All those are 
>> unittests. So kept as it was.
>>>
...>>
>> To create a scratch dir in build dir seems quite nasty to me. Imho there is no reason to not use 
>> regular tmp...
> 
> Please make the build to not write its files anywhere except its own build directory.
> 
> I currently have 1360 entries in my /tmp dir and don't need more of them. I know that /tmp handling 
> depends on distro, but there is no reason to abuse it.

Done!

> 
> 
...

>> Done. ..jsut with unwraps. How do you recommend to get rid of them? Using expect?  /me hopaing for 
>> better trick:(
> 
> Yes, please use "expect", if there are no better alternatives.

Done!
> 
> 
...
>> Wait - you dont like TestLogger? Then there must be fundametnal misunderstanding somewhere.
>> Thw logger part was done only because of windows. Because of you had concerns about stdout 
>> behaviour and usefulness.
> 
> I think you can just use writeln to a log file.
> 
> 
>> I'm +1  to rework this to simple stdout/err, however I'm afraid some loggingbottleneck will be 
>> ncessary if the rust launchers wants to mimic java-part's logging.
> 
> The whole this point is a matter of taste.


I left it as it is. The *proper* logging will be separate patch.
> 
> 
>>> 7. I suggest changing "struct Property" to a tuple of Strings. Otherwise, if you really want to 
...
>>>
>>
>> Updated patch attached.
> 
> Minor things/typos:
> 
> jvm_from_properties.rs:
> 
> get_itw_global_config_file and get_itw_legacy_global_config_file - literals shouldn't be raw.
> 
> 
Fixed!

Although I'm not sure how "path| is now better then r"path"
> jvm_from_properties_resolver.rs:
> 
> order of methods in TestLogger doesn't match the one in trait declaration.
Ugh....
  log, info, get_registry... Are same.. waht do I miss?

> 
> 
> os_access.rs:
> 
> order of methods in Linux doesn't match the one in trait declaration.
I guess that is the same as one before. This fixing both by moving get_registry to end of Os itself.
> 
> 
> property.rs:
> 
> inside Property::fmt write result is not checked (possibly intentionally omitted?).
Hmm. Yes. Even example in doc.rust-lang do so.  What kind f check do you suggest? What it would be for?
> 
> typo in method name "che[kc]_property"
fixed
> 
> 
> utils.rs:
> 
> typos in method names "debug[g]able_remove_file" and "debug[g]able_remove_dir"

Fixes.
> 
> 
>>
>>     TYVM!
>>
> 
> 


-- 
Jiri Vanek
Senior QE engineer, OpenJDK QE lead, Mgr.
Red Hat Czech
jvanek at redhat.com    M: +420775390109
-------------- next part --------------
A non-text attachment was scrubbed...
Name: cargizedJdkFRecognition3.patch
Type: text/x-patch
Size: 37637 bytes
Desc: not available
URL: <http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20180814/3bb93cbe/cargizedJdkFRecognition3-0001.patch>