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?
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.
> 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?
--
Andrew Haley (he/him)
Java Platform Lead Engineer
Red Hat UK Ltd. <https://www.redhat.com>
https://keybase.io/andrewhaley
EAC8 43EB D3EF DB98 CC77 2FAD A5CD 6035 332F A671
