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

[Jiri Vanek]

On 03/12/2018 12:46 PM, Jiri Vanek wrote:
> On 03/02/2018 11:24 PM, Alex Kashchenko wrote:
>> On 03/02/2018 02:27 PM, Jiri Vanek wrote:
>>> 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. 
>>
>> Variables need to be fetched from env during compile time, validated (checking for 
>> missing/empty/invalid), substituted by default values where needed and then passed to rustc. This 
>> way inside hardcoded.rs env!() can be used, as all values are already validated in build.rs.
>>
>>
>>>>> 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!.
>>
>> Validation logic may be as complex as needed (e.g. distinguishing build initiated from autoconf 
>> from the one initiated directly/from IDE etc). Just please make sure that dev builds (direct cargo 
>> invocation) work all the time and don't require to add OS-level env vars on dev machine. Something 
>> like this can be used as a starter:
>>
>> println!("cargo:rustc-env=ITW_JRE={}", option_env!("AC_JRE").unwrap_or("JRE-dev-unspecified"));
> 
> Hello!
> 
> One more attempt to make default build pass, and still protect the launcher from being built without 
> vars set.
> 
> Using the original approach (no build.rs), havinf  option_env! to private fields, and having 
> unwrapping gettets in hardcoded.rs.  With difference that the getter will return default instead of 
> failing.
> 
> 
> In addition I would keep the test, ensuring that the necessary values are not empty nor default.
> By that, the default build you require will be kept, and  my fear that default-vars build will leak 
> removed.
> 
> 

Patch with above approach attached.

J.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: cargoisation_testdrivenFail.patch
Type: text/x-patch
Size: 9311 bytes
Desc: not available
URL: <http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20180316/1b09f962/cargoisation_testdrivenFail.patch>