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
