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
