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

[Dmitry Chuyko]

Hi Andrew,

On 7/17/19 11:52 AM, Andrew Haley wrote:
> On 7/16/19 8: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.
> 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__

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.

>
> This:
>
> @@ -212,9 +212,11 @@
>           } else if (count == 0) {
>               // break;
>           } else {
> +/*
>               if (buffer[count - 1] == EOF) {
>                   buffer[count - 1] = '\0';
>               }
> +*/
>
> I'm wondering if this bogus code might have been inherited from Windows.
Yes, something not-posix in ...-posix file. Removed in webrev.01.
>
>> 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.
> +        if ("arm".equals(arch))
> +            return "armv7hl";
>
> Is this actually correct? How do you know it's not, say, armv7hf? I would
> have thought there must be some better way to get the RPM arch. Isn't
> running /bin/arch right on your systems?

Using $(arch) is an interesting question. For now implementation decides 
what kind of bundle to create looking at JVM binary arch. This should 
guarantee that the bundle will fit the same arch of host and some 
compatible architectures (app will be be installable and runnable). And 
it also mean current tests may fail if host and VM archs are different 
(because packages won't be found while may be created and may be working).

'arvmv7hl' is a common build environment/target for arm port and looks 
like a compatible option. We may look into VM binary metadata though to 
get exact build arch.

-Dmitry

>