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

[Jiri Vanek]

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