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

[Alexander Matveev]

Hi Dmitry,

Looks good.

Thanks,
Alexander

On 7/25/2019 5:38 AM, Dmitry Chuyko wrote:
> I realized we also need better detection of arch for deb control file. 
> It can be done by using 'dpkg --print-architecture' on host. 
> LinuxDebBundler was corrected.
>
> New webrev: http://cr.openjdk.java.net/~dchuyko/8222778/webrev.04/
>
> Patch was re-tested together with webrev.02 from JDK-8228402.
>
> -Dmitry
>
> On 7/25/19 12:56 AM, Alexander Matveev wrote:
>> 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.
>>>>
>>