RFR (S): JDK-8222778: Packaging Tool (JEP 343) on Linux/AArch64

[Alexander Matveev]

Looks good.

On 7/24/2019 7:30 AM, Dmitry Chuyko wrote:
> Webrev without chdir() and write() changes: 
> http://cr.openjdk.java.net/~dchuyko/8222778/webrev.03/
>
> When it is applied together with webrev.01 [1] from JDK-8228402, 
> sandbox is compiled and passes same jpackage tests on x86, aarch64 and 
> other platforms.
>
> -Dmitry
>
> [1] http://cr.openjdk.java.net/~herrick/8228402/
>
> On 7/19/19 11:31 AM, Andrew Haley wrote:
>> On 7/19/19 12:36 AM, Dmitry Chuyko wrote:
>>> On 7/18/19 11:57 AM, Andrew Haley wrote:
>>>> Hi,
>>>>
>>>> On 7/17/19 10:19 PM, Dmitry Chuyko wrote:
>>>>> On 7/17/19 11:52 AM, Andrew Haley wrote:
>>>>>> This is weird:
>>>>>>
>>>>>> --- 
>>>>>> old/src/jdk.jpackage/linux/native/libapplauncher/LinuxPlatform.cpp 
>>>>>> 2019-07-16 22:11:30.200258978 +0300
>>>>>> +++ 
>>>>>> new/src/jdk.jpackage/linux/native/libapplauncher/LinuxPlatform.cpp 
>>>>>> 2019-07-16 22:11:29.867374851 +0300
>>>>>> @@ -127,7 +127,7 @@
>>>>>>     }
>>>>>>
>>>>>>     void LinuxPlatform::SetCurrentDirectory(TString Value) {
>>>>>> -    chdir(PlatformString(Value).toPlatformString());
>>>>>> +    int ignored = chdir(PlatformString(Value).toPlatformString());
>>>>>>     }
>>>>>>
>>>>>> Firstly it does not fix the problem: you've gone from an unused 
>>>>>> value to an
>>>>>> unused variable. Secondly, we ignore the return value of printf 
>>>>>> all the time;
>>>>>> what's different about this one?
>>>>> chdir() is __wur, which is 
>>>>> /usr/include/aarch64-linux-gnu/sys/cdefs.h:#
>>>>> define __wur __attribute_warn_unused_result__
>>>> OK, got it.
>>>>
>>>>> In some places this warning is disabled (CoreLibraries.gmk,
>>>>> Awt2dLibraries.gmk).
>>>>>
>>>>> It would be more correct to handle the result here as chdir might not
>>>>> succeeded in fact.
>>>> Very much so. We shouldn't try to "shut up the compiler" in this 
>>>> way.Andrew Haley <aph at redhat.com>
>>> I created a separate bug about chdir() and write() usages:
>>> https://bugs.openjdk.java.net/browse/JDK-8228402
>>>
>>> Does it look good to silence warnings for platforms support changes?
>> I think we should fix the bugs, not silence the warnings.
>>
>>> One
>>> more option could be to exclude related changes from current review.
>> OK.
>>