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

[Jiri Vanek]

On 03/02/2018 03:20 PM, Jiri Vanek wrote:
> On 03/02/2018 12:10 PM, Jiri Vanek wrote:
>> 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.
> 
> 
> If nothing else, I think that this validation have to go to separate patch. And probably big one.
> Attached is patch with build.rs removed.

returned also test.
>>
>>
>>>
>>> 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: cargoized5.patch
Type: text/x-patch
Size: 7786 bytes
Desc: not available
URL: <http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20180302/ecc6619e/cargoized5.patch>