[rfc][icedtea-web] moving rust launchers to cargo

[Jiri Vanek]

On 03/02/2018 02:07 AM, Alex Kashchenko wrote:
> Hi,
> 
> Reviewing (ignoring autoconf changes):

sad, But sure.
> 
> 1. please rename "hardcoded_paths" - values there are not limited to paths (maybe to just "hardcoded")

done.
> 
> 2. add the following build.rs file to the rust-launcher dir ( see: 
> https://doc.rust-lang.org/cargo/reference/build-scripts.html ) to have better control over 
> extracting values from env variables:
> 
> fn main() {
>      // validation logic here for all vars
>      println!("cargo:rustc-env=JRE={}", "foo1");
>      println!("cargo:rustc-env=JAVA={}", "foo2");
>      ...
> }
> 
> 3. in hardcoded.rs make all variable fetching non-optional (use only env! macro) and fill the 
> default values in build.rs
> 
> 4. drop tests from hardcoded.rs - this logic goes to build.rs


I'm not sure how you expect this to work. The ones with options would be dropped at the end of 
development, or made !env only.  So I would let it be. If you insists, I can convert them imediately 
with soem defaults, but that is not what println!("cargo:rustc-env=JAVA={}", "foo2"); is doing.
It *replace*  the value from env!.

I would ratehr stay with curent state
  - optionals (and promiss that they will disapear ot be converted to env! onl
  - stay with the dummy test to invoke them

build.rs do not sounds to me like used correctly here.


> 
> 5. drop module declaration from hardcoded.rs

done


Tahxn for nits!

J.
> 
> 
> On 02/26/2018 12:53 PM, Jiri Vanek wrote:
>> Environment variables propagation check moved from test-time to compile time.
>>
>> Thanx goes to Alkex!
>>     J.
>> On 02/15/2018 01:51 PM, Jiri Vanek wrote:
>>> As agreed on privte discussion, this pathc is moving the responsibility to build of native 
>>> launchers to cargo (from ourt makefiel and plain rustc).
>>>
>>> 2018-02-15  Jiri Vanek <jvanek at redhat.com>
>>>      * configure.ac: added check for cargo
>>>      * .hgignore: added target and Cargo.lock
>>>      * .Makefile.am: dropped all launcher.in/libhardoced_paths_*.rs targets and rustc targets.
>>>      (launcher.build/$(javaws) launcher.build/$(itweb_settings) launcher.build/$(policyeditor))
>>>      adapted to use cargo. Variables handled in switch. (clean) now cleans all launcher.in*
>>>      * rust-launcher/Cargo.toml: primitive declaration of package
>>>      * rust-launcher/src/hardcoded_paths.rs: copied from rust-launcher/hardoced_paths.rs.in
>>>      adapted to get substitution via cargo rather then by sed.  Added test and wrapping getters
>>>      * rust-launcher/src/main.rs: copied from rust-launcher/launchers.rs. Adapted imports to new 
>>> infrastructure,s till just reprinting hardcoded stuff
>>>
>>> Thanx!
>>>    J.
>>>
>>
>>
> 
> 


-- 
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: cargoized3.patch
Type: text/x-patch
Size: 8148 bytes
Desc: not available
URL: <http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20180302/b740eded/cargoized3.patch>