[rfc][icedtea-web] improved home finding to use variables first

[jiri Vanek]

----- Original Message -----
From: "Alex Kashchenko" <akashche at redhat.com>
To: "Jiri Vanek" <jvanek at redhat.com>
Cc: "IcedTea Distro List" <distro-pkg-dev at openjdk.java.net>
Sent: Friday, December 7, 2018 12:13:20 PM
Subject: Re: [rfc][icedtea-web] improved home finding to use variables first

On 12/07/2018 09:10 AM, Jiri Vanek wrote:
>>>>>> 
>>>>>> [...]
>>>>>> 
>>>>>>
>>>>> I think it is better to avoid using deprecated API in a new code, especially with Rust fast pacing
>>>>> release cycle.
>>>>
>>>> Thsi is very very bad idea... We replaced one waring by six warnings...
>>>
>>> Why not fix the warnings which can be fixed and suppress others?
>> 
>> The fix is the same as for env:home_dir - to use implementation from crates.io
> 
> I am talking about the warnings in a launcher code, currently (without 
> the patch) there are 11 warnings. They are all harmless ("dead_code" 
> type), but they produce a lot of garbage output that makes it easy to 
> miss possibly important new ones. It should be trivial to suppress 
> harmless warnings with #[allow(dead_code)], 
> #[allow(non_camel_case_types)] and similar annotations.

Interesting. That is not what I do see.  [1]

10 |     match env::home_dir() {
   = note: #[warn(deprecated)] on by default
  -> this is discussed right now

warning: unused variable: `jre_dir`
103 |         fn spawn_java_process(&self, jre_dir: &std::path::PathBuf, args: &Vec<String>) -> std::process::Child {
    |                                      ^^^^^^^ help: consider using `_jre_dir` instead
    = note: #[warn(unused_variables)] on by default

Introduced freshly.  jre_dir will be used in future development

warning: unused variable: `args`
103 |         fn spawn_java_process(&self, jre_dir: &std::path::PathBuf, args: &Vec<String>) -> std::process::Child {
    |                                                                    ^^^^ help: consider using `_args` instead

args will be used in future development

warning: constant item is never used: `LAUNCHER_BOOTCLASSPATH`
 --> src/hardcoded_paths.rs:3:1
3 | const LAUNCHER_BOOTCLASSPATH: Option<&'static str> = option_env!("LAUNCHER_BOOTCLASSPATH");
  | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

6 times same: #[warn(dead_code)], leading from constant never used.
All those constants will be used in future development or removed.

So you can not say that there are any real warnings right now. On contrary the new implementation of get_home is bringing many nasty warnings :(


>>>> Udated patch attached... But I heavily vote for initial env::home_dir
>>>
>>> linux_pwd module uses linux-specific calls and is not compilable on windows. Please guard it with
>>> #[cfg(unix)] where appropriate. Even if we use env::home_dir (that I think is a bad idea) in this
>>> patch, we are going to have some platform-specific code anyway that needs to be conditionally excluded.
>> 
>> I'm still not sure how will be handled OsAccess = new Linux() x new Windows()
>> Theoretically this should be the onl palce where any  #[cfg(unix)] shouldbe used.
>> Eh both
>> GetUserProfileDirectoryW [1] or SHGetKnownFolderPath [2].
>>    and
>> getuid(),  getpwuid(uid: uid_t)
>> 
>> Are compilable on both platforms, but failing in runtime if architecture goes wrong.
>
> They are not compilable, even removing usage of "std::os::unix" package 
> (that doesn't exist on windows), build will fail on linking stage. GCC 
> linker (that is used in x86_64-pc-windows-gnu Rust) fails with 
> "undefined reference to `getuid'".

Thanx. noted.

j.

[1]
  Compiling launcher v1.8.0 (file:///home/jvanek/Desktop/icedtea-web/launcher.in.javaws)                                                                                                                                                     
warning: use of deprecated item 'std::env::home_dir': This function's behavior is unexpected and probably not what you want. Consider using the home_dir function from https://crates.io/crates/dirs instead.
  --> src/dirs_paths_helper.rs:10:11
   |
10 |     match env::home_dir() {
   |           ^^^^^^^^^^^^^
   |
   = note: #[warn(deprecated)] on by default

warning: unused variable: `jre_dir`
   --> src/property_from_files_resolver.rs:103:38
    |
103 |         fn spawn_java_process(&self, jre_dir: &std::path::PathBuf, args: &Vec<String>) -> std::process::Child {
    |                                      ^^^^^^^ help: consider using `_jre_dir` instead
    |
    = note: #[warn(unused_variables)] on by default

warning: unused variable: `args`
   --> src/property_from_files_resolver.rs:103:68
    |
103 |         fn spawn_java_process(&self, jre_dir: &std::path::PathBuf, args: &Vec<String>) -> std::process::Child {
    |                                                                    ^^^^ help: consider using `_args` instead

warning: constant item is never used: `LAUNCHER_BOOTCLASSPATH`
 --> src/hardcoded_paths.rs:3:1
  |
3 | const LAUNCHER_BOOTCLASSPATH: Option<&'static str> = option_env!("LAUNCHER_BOOTCLASSPATH");
  | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  |
  = note: #[warn(dead_code)] on by default

warning: constant item is never used: `JAVAWS_SPLASH_LOCATION`
 --> src/hardcoded_paths.rs:4:1
  |
4 | const JAVAWS_SPLASH_LOCATION: Option<&'static str> = option_env!("JAVAWS_SPLASH_LOCATION");
  | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

warning: constant item is never used: `NETX_JAR`
 --> src/hardcoded_paths.rs:9:1
  |
9 | const NETX_JAR: Option<&'static str> = option_env!("NETX_JAR");
  | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

warning: constant item is never used: `PLUGIN_JAR`
  --> src/hardcoded_paths.rs:10:1
   |
10 | const PLUGIN_JAR: Option<&'static str> = option_env!("PLUGIN_JAR");
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

warning: constant item is never used: `JSOBJECT_JAR`
  --> src/hardcoded_paths.rs:11:1
   |
11 | const JSOBJECT_JAR: Option<&'static str> = option_env!("JSOBJECT_JAR");
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

    Finished dev [unoptimized + debuginfo] target(s) in 3.14s
     Running target/debug/deps/launcher-77c219f2162556a5