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

[Alexander Matveev]

Hi Dmitry,

Updated webrev looks fine.

Thanks,
Alexander

On 7/17/2019 1:44 PM, Dmitry Chuyko wrote:
> Ok, here is the updated webrev: 
> http://cr.openjdk.java.net/~dchuyko/8222778/webrev.01/
>
> -Dmitry
>
> On 7/17/19 11:03 PM, Alexander Matveev wrote:
>> Hi Dmitry,
>>
>> I also do not see point of keeping this block, so lets remove it 
>> instead of keeping it commented.
>>
>> Thanks,
>> Alexander
>>
>> On 7/16/2019 3:50 PM, Dmitry Chuyko wrote:
>>> Alexander, thanks for having a look,
>>>
>>> On 7/17/19 12:30 AM, Alexander Matveev wrote:
>>>> Hi Dmitry,
>>>>
>>>> http://cr.openjdk.java.net/~dchuyko/8222778/webrev.00/src/jdk.jpackage/unix/native/libapplauncher/PosixPlatform.cpp.frames.html 
>>>>
>>>> Why code between lines 215 and 219 was disabled? Not sure what it 
>>>> tries to do, if it tries to guarantee NULL termination we should 
>>>> probably keep it or allocate buffer with extra null or read 
>>>> (sizeof(buffer)-1). I think EOF defined as -1.
>>>
>>> gcc 5.4.0 on Linux reports an error:
>>>
>>> sandbox/src/jdk.jpackage/unix/native/libapplauncher/PosixPlatform.cpp: 
>>> In member function ‘bool PosixProcess::ReadOutput()’:
>>>
>>> sandbox/src/jdk.jpackage/unix/native/libapplauncher/PosixPlatform.cpp:215:35: 
>>> error: comparison is always false due to limited range of data type 
>>> [-Werror=type-limits]
>>>              if (buffer[count - 1] == EOF) {
>>>
>>> Here buffer is char[] and read(int, void *, size_t) is used.
>>>
>>> It won't be right to keep it commented, to me it looks like this 
>>> block can be removed. If not, there should be some other fix like 
>>> adding ifdefs for some platforms or using call different from read().
>>>
>>> -Dmitry
>>>
>>>> Otherwise looks fine.
>>>>
>>>> Thanks,
>>>> Alexander
>>>>
>>>> On 7/16/2019 12:55 PM, Dmitry Chuyko wrote:
>>>>> Hello,
>>>>>
>>>>> Please review a small patch that mostly fixes jpackage test for 
>>>>> Linux aarch64 and also arm,x86,power. It is prepared for 
>>>>> 'JDK-8200758-branch' branch of open 'sandbox' repo.
>>>>>
>>>>> There are few parts:
>>>>>
>>>>> 1. LinuxPlatform.cpp and IniFile.cpp got small fixes for compiler 
>>>>> warnings.
>>>>>
>>>>> 2. LinuxDebBundler.getArch() now maps only x86_64 to amd64, x86 is 
>>>>> still mapped to i386, and other archs map to themselves.
>>>>>
>>>>> 3. In tests, new method getRpmArch() was added to 
>>>>> linux/base/Base.java, it maps JVMs os.arch to default rpmbuild 
>>>>> arch. Multiple tests were modified to use that method instead of 
>>>>> "x86_64" in rpm file name. Some timeouts were increased.
>>>>>
>>>>> bug: https://bugs.openjdk.java.net/browse/JDK-8222778
>>>>>
>>>>> webrev: http://cr.openjdk.java.net/~dchuyko/8222778/webrev.00/
>>>>>
>>>>> testing: test/jdk/tools/jpackage jtreg tests pass on Ubuntu 16.04 
>>>>> with rpmbuild on x86_64, aarch64, arm, x86 and power, except 
>>>>> deb/MaintainerTest (fails everywhere similarly to x86_64 because 
>>>>> of extra "Unknown" name in email).
>>>>>
>>>>> I didin't cover s390 as we in BellSoft currently don't build on 
>>>>> that arch. On typical armv7 hw increased or default timeouts are 
>>>>> still too low, while they are fine for some relatively weak 
>>>>> aarch64 machines. Deb tests run especially slow because of 
>>>>> dpkg-deb itself. I used "force-unsafe-io" option in 
>>>>> /etc/dpkg/dpkg.cfg, it does reduce packaging time but still not 
>>>>> enough to have really fast tests.
>>>>>
>>>>> -Dmitry
>>>>>
>>>>>
>>>>
>>