From 29de20dbc22e0b68698a1b9cb1241ae5861a6b9a Mon Sep 17 00:00:00 2001 From: Alexander Zvegintsev Date: Tue, 4 Mar 2025 20:55:45 +0000 Subject: [PATCH 01/46] 8280991: [XWayland] No displayChanged event after setDisplayMode call Reviewed-by: honkar, prr --- .../classes/sun/awt/X11GraphicsDevice.java | 38 ++++++++++++++- test/jdk/ProblemList.txt | 1 - .../FullscreenWindowProps.java | 6 ++- .../NoResizeEventOnDMChangeTest.java | 46 ++++++++++++++++++- 4 files changed, 87 insertions(+), 4 deletions(-) diff --git a/src/java.desktop/unix/classes/sun/awt/X11GraphicsDevice.java b/src/java.desktop/unix/classes/sun/awt/X11GraphicsDevice.java index b17b4c456447d..39ecb13c6a4d0 100644 --- a/src/java.desktop/unix/classes/sun/awt/X11GraphicsDevice.java +++ b/src/java.desktop/unix/classes/sun/awt/X11GraphicsDevice.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 1997, 2024, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 1997, 2025, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -72,6 +72,15 @@ public final class X11GraphicsDevice extends GraphicsDevice private boolean shutdownHookRegistered; private int scale; + // Wayland clients are by design not allowed to change the resolution in Wayland. + // XRandR in Xwayland is just an emulation, it doesn't actually change the resolution. + // This emulation is per window/x11 client, so different clients can have + // different emulated resolutions at the same time. + // So any request to get the current display mode will always return + // the original screen resolution, even if we are in emulated resolution. + // To handle this situation, we store the last set display mode in this variable. + private volatile DisplayMode xwlCurrentDisplayMode; + public X11GraphicsDevice(int screennum) { this.screen = screennum; this.scale = initScaleFactor(); @@ -117,6 +126,20 @@ public int scaleDown(int x) { private Rectangle getBoundsImpl() { Rectangle rect = pGetBounds(getScreen()); + + if (XToolkit.isOnWayland() && xwlCurrentDisplayMode != null) { + // XRandR resolution change in Xwayland is an emulation, + // and implemented in such a way that multiple display modes + // for a device are only available in a single screen scenario, + // if we have multiple screens they will each have a single display mode + // (no emulated resolution change is available). + // So we don't have to worry about x and y for a screen here. + rect.setSize( + xwlCurrentDisplayMode.getWidth(), + xwlCurrentDisplayMode.getHeight() + ); + } + if (getScaleFactor() != 1) { rect.x = scaleDown(rect.x); rect.y = scaleDown(rect.y); @@ -400,10 +423,19 @@ private DisplayMode getDefaultDisplayMode() { @Override public synchronized DisplayMode getDisplayMode() { if (isFullScreenSupported()) { + if (XToolkit.isOnWayland() && xwlCurrentDisplayMode != null) { + return xwlCurrentDisplayMode; + } + DisplayMode mode = getCurrentDisplayMode(screen); if (mode == null) { mode = getDefaultDisplayMode(); } + + if (XToolkit.isOnWayland()) { + xwlCurrentDisplayMode = mode; + } + return mode; } else { if (origDisplayMode == null) { @@ -474,6 +506,10 @@ public synchronized void setDisplayMode(DisplayMode dm) { dm.getWidth(), dm.getHeight(), dm.getRefreshRate()); + if (XToolkit.isOnWayland()) { + xwlCurrentDisplayMode = dm; + } + // update bounds of the fullscreen window w.setBounds(0, 0, dm.getWidth(), dm.getHeight()); diff --git a/test/jdk/ProblemList.txt b/test/jdk/ProblemList.txt index 374019138406d..5ba6b33433c51 100644 --- a/test/jdk/ProblemList.txt +++ b/test/jdk/ProblemList.txt @@ -499,7 +499,6 @@ java/awt/image/multiresolution/MultiResolutionJOptionPaneIconTest.java 8274106 m # Wayland related -java/awt/FullScreen/FullscreenWindowProps/FullscreenWindowProps.java 8280991 linux-x64 java/awt/FullScreen/SetFullScreenTest.java 8332155 linux-x64 ############################################################################ diff --git a/test/jdk/java/awt/FullScreen/FullscreenWindowProps/FullscreenWindowProps.java b/test/jdk/java/awt/FullScreen/FullscreenWindowProps/FullscreenWindowProps.java index f5f77d47319d7..66f358d64ce41 100644 --- a/test/jdk/java/awt/FullScreen/FullscreenWindowProps/FullscreenWindowProps.java +++ b/test/jdk/java/awt/FullScreen/FullscreenWindowProps/FullscreenWindowProps.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2020, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2020, 2025, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -52,6 +52,10 @@ public void paint(Graphics g) { super.paint(g); g.setColor(Color.GREEN); g.fillRect(0, 0, getWidth(), getHeight()); + g.setColor(Color.RED); + DisplayMode displayMode = + getGraphicsConfiguration().getDevice().getDisplayMode(); + g.drawString(displayMode.toString(), 100, 100); } }; try { diff --git a/test/jdk/java/awt/FullScreen/NoResizeEventOnDMChangeTest/NoResizeEventOnDMChangeTest.java b/test/jdk/java/awt/FullScreen/NoResizeEventOnDMChangeTest/NoResizeEventOnDMChangeTest.java index 455ad2f2b0a3e..c7277e8b96caa 100644 --- a/test/jdk/java/awt/FullScreen/NoResizeEventOnDMChangeTest/NoResizeEventOnDMChangeTest.java +++ b/test/jdk/java/awt/FullScreen/NoResizeEventOnDMChangeTest/NoResizeEventOnDMChangeTest.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2007, 2021, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2007, 2025, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -26,6 +26,8 @@ * @bug 6646411 * @summary Tests that full screen window and its children receive resize event when display mode changes + * @library /test/lib + * @build jdk.test.lib.Platform jtreg.SkippedException * @run main/othervm NoResizeEventOnDMChangeTest * @run main/othervm -Dsun.java2d.d3d=false NoResizeEventOnDMChangeTest */ @@ -44,9 +46,21 @@ import java.awt.event.ComponentEvent; import java.awt.event.WindowAdapter; import java.awt.event.WindowEvent; +import java.io.BufferedReader; +import java.io.IOException; + +import static java.util.concurrent.TimeUnit.SECONDS; +import jdk.test.lib.Platform; +import jtreg.SkippedException; public class NoResizeEventOnDMChangeTest { + public static void main(String[] args) { + if (Platform.isOnWayland() && !isFixDelivered()) { + throw new SkippedException("Test skipped because fix was not" + + "delivered in current GnomeShell version"); + } + final GraphicsDevice gd = GraphicsEnvironment. getLocalGraphicsEnvironment().getDefaultScreenDevice(); @@ -231,4 +245,34 @@ public synchronized int getDmChanges() { return dmChanges; } } + + private static boolean isFixDelivered() { + try { + Process process = + new ProcessBuilder("/usr/bin/gnome-shell", "--version") + .start(); + + try (BufferedReader reader = process.inputReader()) { + if (process.waitFor(2, SECONDS) && process.exitValue() == 0) { + String line = reader.readLine(); + if (line != null) { + System.out.println("Gnome shell version: " + line); + String[] versionComponents = line + .replaceAll("[^\\d.]", "") + .split("\\."); + + if (versionComponents.length >= 1) { + return Integer.parseInt(versionComponents[0]) > 42; + } + } + } + } + } catch (IOException + | InterruptedException + | IllegalThreadStateException + | NumberFormatException ignored) { + } + + return false; + } } From 38b4d46c1ff3701d75ff8347e5edbb01acd9b512 Mon Sep 17 00:00:00 2001 From: Cesar Soares Lucas Date: Tue, 4 Mar 2025 21:44:40 +0000 Subject: [PATCH 02/46] 8351081: Off-by-one error in ShenandoahCardCluster Reviewed-by: wkemper --- src/hotspot/share/gc/shenandoah/shenandoahScanRemembered.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/hotspot/share/gc/shenandoah/shenandoahScanRemembered.hpp b/src/hotspot/share/gc/shenandoah/shenandoahScanRemembered.hpp index f5acb4f10eded..e28f2f030522a 100644 --- a/src/hotspot/share/gc/shenandoah/shenandoahScanRemembered.hpp +++ b/src/hotspot/share/gc/shenandoah/shenandoahScanRemembered.hpp @@ -402,7 +402,7 @@ class ShenandoahCardCluster: public CHeapObj { ShenandoahCardCluster(ShenandoahDirectCardMarkRememberedSet* rs) { _rs = rs; - _object_starts = NEW_C_HEAP_ARRAY(crossing_info, rs->total_cards(), mtGC); + _object_starts = NEW_C_HEAP_ARRAY(crossing_info, rs->total_cards() + 1, mtGC); // the +1 is to account for card table guarding entry for (size_t i = 0; i < rs->total_cards(); i++) { _object_starts[i].short_word = 0; } From 20ea218ce52f79704445acfe2d4a3dc9d04e86d2 Mon Sep 17 00:00:00 2001 From: Dean Long Date: Tue, 4 Mar 2025 23:10:52 +0000 Subject: [PATCH 03/46] 8336042: Caller/callee param size mismatch in deoptimization causes crash Co-authored-by: Richard Reingruber Reviewed-by: pchilanomate, rrich, vlivanov, never --- .../aarch64/abstractInterpreter_aarch64.cpp | 3 +- .../cpu/arm/abstractInterpreter_arm.cpp | 9 ++ .../cpu/ppc/abstractInterpreter_ppc.cpp | 9 ++ .../cpu/riscv/abstractInterpreter_riscv.cpp | 3 +- .../cpu/s390/abstractInterpreter_s390.cpp | 7 ++ .../cpu/x86/abstractInterpreter_x86.cpp | 5 +- src/hotspot/share/interpreter/bytecode.hpp | 2 + .../share/interpreter/bytecode.inline.hpp | 10 ++ src/hotspot/share/runtime/deoptimization.cpp | 21 ++-- src/hotspot/share/runtime/vframeArray.cpp | 6 +- .../jtreg/compiler/jsr292/MHDeoptTest.java | 97 +++++++++++++++++++ 11 files changed, 158 insertions(+), 14 deletions(-) create mode 100644 test/hotspot/jtreg/compiler/jsr292/MHDeoptTest.java diff --git a/src/hotspot/cpu/aarch64/abstractInterpreter_aarch64.cpp b/src/hotspot/cpu/aarch64/abstractInterpreter_aarch64.cpp index a3c729fdd56e7..b543c96f3b81b 100644 --- a/src/hotspot/cpu/aarch64/abstractInterpreter_aarch64.cpp +++ b/src/hotspot/cpu/aarch64/abstractInterpreter_aarch64.cpp @@ -149,7 +149,8 @@ void AbstractInterpreter::layout_activation(Method* method, #ifdef ASSERT if (caller->is_interpreted_frame()) { - assert(locals < caller->fp() + frame::interpreter_frame_initial_sp_offset, "bad placement"); + assert(locals <= caller->interpreter_frame_expression_stack(), "bad placement"); + assert(locals >= interpreter_frame->sender_sp() + max_locals - 1, "bad placement"); } #endif diff --git a/src/hotspot/cpu/arm/abstractInterpreter_arm.cpp b/src/hotspot/cpu/arm/abstractInterpreter_arm.cpp index 075db4736f132..978011491a0a7 100644 --- a/src/hotspot/cpu/arm/abstractInterpreter_arm.cpp +++ b/src/hotspot/cpu/arm/abstractInterpreter_arm.cpp @@ -131,6 +131,15 @@ void AbstractInterpreter::layout_activation(Method* method, intptr_t* locals = interpreter_frame->sender_sp() + max_locals - 1; +#ifdef ASSERT + if (caller->is_interpreted_frame()) { + // Test exact placement on top of caller args + intptr_t* l2 = caller->interpreter_frame_last_sp() + caller_actual_parameters - 1; + assert(l2 <= caller->interpreter_frame_expression_stack(), "bad placement"); + assert(l2 >= locals, "bad placement"); + } +#endif + interpreter_frame->interpreter_frame_set_locals(locals); BasicObjectLock* montop = interpreter_frame->interpreter_frame_monitor_begin(); BasicObjectLock* monbot = montop - moncount; diff --git a/src/hotspot/cpu/ppc/abstractInterpreter_ppc.cpp b/src/hotspot/cpu/ppc/abstractInterpreter_ppc.cpp index cc094ad4f995b..beadce336379e 100644 --- a/src/hotspot/cpu/ppc/abstractInterpreter_ppc.cpp +++ b/src/hotspot/cpu/ppc/abstractInterpreter_ppc.cpp @@ -128,6 +128,15 @@ void AbstractInterpreter::layout_activation(Method* method, caller->interpreter_frame_esp() + caller_actual_parameters : caller->sp() + method->max_locals() - 1 + (frame::java_abi_size / Interpreter::stackElementSize); +#ifdef ASSERT + if (caller->is_interpreted_frame()) { + assert(locals_base <= caller->interpreter_frame_expression_stack(), "bad placement"); + const int caller_abi_bytesize = (is_bottom_frame ? frame::top_ijava_frame_abi_size : frame::parent_ijava_frame_abi_size); + intptr_t* l2 = caller->sp() + method->max_locals() - 1 + (caller_abi_bytesize / Interpreter::stackElementSize); + assert(locals_base >= l2, "bad placement"); + } +#endif + intptr_t* monitor_base = caller->sp() - frame::ijava_state_size / Interpreter::stackElementSize; intptr_t* monitor = monitor_base - (moncount * frame::interpreter_frame_monitor_size()); intptr_t* esp_base = monitor - 1; diff --git a/src/hotspot/cpu/riscv/abstractInterpreter_riscv.cpp b/src/hotspot/cpu/riscv/abstractInterpreter_riscv.cpp index 843a58e28d712..00a6877684af2 100644 --- a/src/hotspot/cpu/riscv/abstractInterpreter_riscv.cpp +++ b/src/hotspot/cpu/riscv/abstractInterpreter_riscv.cpp @@ -141,7 +141,8 @@ void AbstractInterpreter::layout_activation(Method* method, #ifdef ASSERT if (caller->is_interpreted_frame()) { - assert(locals < caller->fp() + frame::interpreter_frame_initial_sp_offset, "bad placement"); + assert(locals <= caller->interpreter_frame_expression_stack(), "bad placement"); + assert(locals >= interpreter_frame->sender_sp() + max_locals - 1, "bad placement"); } #endif diff --git a/src/hotspot/cpu/s390/abstractInterpreter_s390.cpp b/src/hotspot/cpu/s390/abstractInterpreter_s390.cpp index 37aa9d9a0e8a3..e815542a51e43 100644 --- a/src/hotspot/cpu/s390/abstractInterpreter_s390.cpp +++ b/src/hotspot/cpu/s390/abstractInterpreter_s390.cpp @@ -182,6 +182,13 @@ void AbstractInterpreter::layout_activation(Method* method, intptr_t* sender_sp; if (caller->is_interpreted_frame()) { sender_sp = caller->interpreter_frame_top_frame_sp(); +#ifdef ASSERT + assert(locals_base <= caller->interpreter_frame_expression_stack(), "bad placement"); + // Test caller-aligned placement vs callee-aligned + intptr_t* l2 = (caller->sp() + method->max_locals() - 1 + + frame::z_parent_ijava_frame_abi_size / Interpreter::stackElementSize); + assert(locals_base >= l2, "bad placement"); +#endif } else if (caller->is_compiled_frame()) { sender_sp = caller->fp() - caller->cb()->frame_size(); // The bottom frame's sender_sp is its caller's unextended_sp. diff --git a/src/hotspot/cpu/x86/abstractInterpreter_x86.cpp b/src/hotspot/cpu/x86/abstractInterpreter_x86.cpp index 68ac5b6ca9a97..6680b8c4c0313 100644 --- a/src/hotspot/cpu/x86/abstractInterpreter_x86.cpp +++ b/src/hotspot/cpu/x86/abstractInterpreter_x86.cpp @@ -87,7 +87,10 @@ void AbstractInterpreter::layout_activation(Method* method, #ifdef ASSERT if (caller->is_interpreted_frame()) { - assert(locals < caller->fp() + frame::interpreter_frame_initial_sp_offset, "bad placement"); + // Test exact placement on top of caller args + intptr_t* l2 = caller->interpreter_frame_last_sp() + caller_actual_parameters - 1; + assert(l2 <= caller->interpreter_frame_expression_stack(), "bad placement"); + assert(l2 >= locals, "bad placement"); } #endif diff --git a/src/hotspot/share/interpreter/bytecode.hpp b/src/hotspot/share/interpreter/bytecode.hpp index 870fcb7784ec7..de83cfe5d710d 100644 --- a/src/hotspot/share/interpreter/bytecode.hpp +++ b/src/hotspot/share/interpreter/bytecode.hpp @@ -226,6 +226,8 @@ class Bytecode_invoke: public Bytecode_member_ref { bool has_appendix(); + bool has_member_arg() const; + int size_of_parameters() const; private: diff --git a/src/hotspot/share/interpreter/bytecode.inline.hpp b/src/hotspot/share/interpreter/bytecode.inline.hpp index 43e0cf2991d60..139f477d3971f 100644 --- a/src/hotspot/share/interpreter/bytecode.inline.hpp +++ b/src/hotspot/share/interpreter/bytecode.inline.hpp @@ -28,6 +28,7 @@ #include "interpreter/bytecode.hpp" #include "oops/cpCache.inline.hpp" +#include "prims/methodHandles.hpp" inline bool Bytecode_invoke::has_appendix() { if (invoke_code() == Bytecodes::_invokedynamic) { @@ -37,4 +38,13 @@ inline bool Bytecode_invoke::has_appendix() { } } +inline bool Bytecode_invoke::has_member_arg() const { + // NOTE: We could resolve the call and use the resolved adapter method here, but this function + // is used by deoptimization, where resolving could lead to problems, so we avoid that here + // by doing things symbolically. + // + // invokedynamic instructions don't have a class but obviously don't have a MemberName appendix. + return !is_invokedynamic() && MethodHandles::has_member_arg(klass(), name()); +} + #endif // SHARE_INTERPRETER_BYTECODE_INLINE_HPP diff --git a/src/hotspot/share/runtime/deoptimization.cpp b/src/hotspot/share/runtime/deoptimization.cpp index 4a51e2cbd92a2..167b3095fa9f2 100644 --- a/src/hotspot/share/runtime/deoptimization.cpp +++ b/src/hotspot/share/runtime/deoptimization.cpp @@ -36,6 +36,7 @@ #include "gc/shared/collectedHeap.hpp" #include "gc/shared/memAllocator.hpp" #include "interpreter/bytecode.hpp" +#include "interpreter/bytecode.inline.hpp" #include "interpreter/bytecodeStream.hpp" #include "interpreter/interpreter.hpp" #include "interpreter/oopMapCache.hpp" @@ -641,11 +642,12 @@ Deoptimization::UnrollBlock* Deoptimization::fetch_unroll_info_helper(JavaThread bool caller_was_method_handle = false; if (deopt_sender.is_interpreted_frame()) { methodHandle method(current, deopt_sender.interpreter_frame_method()); - Bytecode_invoke cur = Bytecode_invoke_check(method, deopt_sender.interpreter_frame_bci()); - if (cur.is_invokedynamic() || cur.is_invokehandle()) { - // Method handle invokes may involve fairly arbitrary chains of - // calls so it's impossible to know how much actual space the - // caller has for locals. + Bytecode_invoke cur(method, deopt_sender.interpreter_frame_bci()); + if (cur.has_member_arg()) { + // This should cover all real-world cases. One exception is a pathological chain of + // MH.linkToXXX() linker calls, which only trusted code could do anyway. To handle that case, we + // would need to get the size from the resolved method entry. Another exception would + // be an invokedynamic with an adapter that is really a MethodHandle linker. caller_was_method_handle = true; } } @@ -748,9 +750,14 @@ Deoptimization::UnrollBlock* Deoptimization::fetch_unroll_info_helper(JavaThread } #endif + int caller_actual_parameters = -1; // value not used except for interpreted frames, see below + if (deopt_sender.is_interpreted_frame()) { + caller_actual_parameters = callee_parameters + (caller_was_method_handle ? 1 : 0); + } + UnrollBlock* info = new UnrollBlock(array->frame_size() * BytesPerWord, caller_adjustment * BytesPerWord, - caller_was_method_handle ? 0 : callee_parameters, + caller_actual_parameters, number_of_frames, frame_sizes, frame_pcs, @@ -939,7 +946,7 @@ JRT_LEAF(BasicType, Deoptimization::unpack_frames(JavaThread* thread, int exec_m if (Bytecodes::is_invoke(cur_code)) { Bytecode_invoke invoke(mh, iframe->interpreter_frame_bci()); cur_invoke_parameter_size = invoke.size_of_parameters(); - if (i != 0 && !invoke.is_invokedynamic() && MethodHandles::has_member_arg(invoke.klass(), invoke.name())) { + if (i != 0 && invoke.has_member_arg()) { callee_size_of_parameters++; } } diff --git a/src/hotspot/share/runtime/vframeArray.cpp b/src/hotspot/share/runtime/vframeArray.cpp index 1640b08c47911..e17961fc424d3 100644 --- a/src/hotspot/share/runtime/vframeArray.cpp +++ b/src/hotspot/share/runtime/vframeArray.cpp @@ -25,6 +25,7 @@ #include "classfile/vmSymbols.hpp" #include "code/vmreg.inline.hpp" #include "interpreter/bytecode.hpp" +#include "interpreter/bytecode.inline.hpp" #include "interpreter/interpreter.hpp" #include "memory/allocation.inline.hpp" #include "memory/resourceArea.hpp" @@ -610,10 +611,7 @@ void vframeArray::unpack_to_stack(frame &unpack_frame, int exec_mode, int caller methodHandle caller(current, elem->method()); methodHandle callee(current, element(index - 1)->method()); Bytecode_invoke inv(caller, elem->bci()); - // invokedynamic instructions don't have a class but obviously don't have a MemberName appendix. - // NOTE: Use machinery here that avoids resolving of any kind. - const bool has_member_arg = - !inv.is_invokedynamic() && MethodHandles::has_member_arg(inv.klass(), inv.name()); + const bool has_member_arg = inv.has_member_arg(); callee_parameters = callee->size_of_parameters() + (has_member_arg ? 1 : 0); callee_locals = callee->max_locals(); } diff --git a/test/hotspot/jtreg/compiler/jsr292/MHDeoptTest.java b/test/hotspot/jtreg/compiler/jsr292/MHDeoptTest.java new file mode 100644 index 0000000000000..61dd330cfdd70 --- /dev/null +++ b/test/hotspot/jtreg/compiler/jsr292/MHDeoptTest.java @@ -0,0 +1,97 @@ +/* + * Copyright (c) 2025, Oracle and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +package compiler.jsr292; + +import java.lang.invoke.MethodHandle; +import java.lang.invoke.MethodHandles; +import java.lang.invoke.MethodHandles.Lookup; +import java.lang.invoke.MethodType; +import java.lang.reflect.Method; + +/* + * @test + * @bug 8336042 + * @library /test/lib / + * + * @run main/bootclasspath/othervm -Xbatch -XX:-TieredCompilation compiler.jsr292.MHDeoptTest + * + */ +public class MHDeoptTest { + + static int xx = 0; + + public static void main(String[] args) throws Throwable { + MethodHandle mh1 = MethodHandles.lookup().findStatic(MHDeoptTest.class, "body1", MethodType.methodType(int.class)); + MethodHandle mh2 = MethodHandles.lookup().findStatic(MHDeoptTest.class, "body2", MethodType.methodType(int.class)); + MethodHandle[] arr = new MethodHandle[] {mh2, mh1}; + + for (MethodHandle mh : arr) { + for (int i = 1; i < 50_000; i++) { + xx = i; + mainLink(mh); + } + } + + } + + static int mainLink(MethodHandle mh) throws Throwable { + return (int)mh.invokeExact(); + } + + static int cnt = 1000; + + static int body1() { + int limit = 0x7fff; + // uncommon trap + if (xx == limit) { + // OSR + for (int i = 0; i < 50_000; i++) { + } + ++cnt; + ++xx; + } + if (xx == limit + 1) { + return cnt + 1; + } + return cnt; + } + + static int body2() { + int limit = 0x7fff; + int dummy = 0; + // uncommon trap + if (xx == limit) { + // OSR + for (int i = 0; i < 50_000; i++) { + } + ++cnt; + ++xx; + } + if (xx == limit + 1) { + return cnt + 1; + } + return cnt; + } + +} From 62fa33a8704aef9fd08a8221f4fde217ab749dfc Mon Sep 17 00:00:00 2001 From: Jatin Bhateja Date: Wed, 5 Mar 2025 01:34:15 +0000 Subject: [PATCH 04/46] 8351158: Incorrect APX EGPR register save ordering Reviewed-by: kvn, sviswanathan --- src/hotspot/cpu/x86/sharedRuntime_x86_64.cpp | 35 ++++++++++---------- 1 file changed, 17 insertions(+), 18 deletions(-) diff --git a/src/hotspot/cpu/x86/sharedRuntime_x86_64.cpp b/src/hotspot/cpu/x86/sharedRuntime_x86_64.cpp index bbe62db33f00e..d3e7e23678ae7 100644 --- a/src/hotspot/cpu/x86/sharedRuntime_x86_64.cpp +++ b/src/hotspot/cpu/x86/sharedRuntime_x86_64.cpp @@ -101,24 +101,23 @@ class RegisterSaver { ymm_off = xmm_off + (XSAVE_AREA_YMM_BEGIN - XSAVE_AREA_BEGIN)/BytesPerInt, DEF_YMM_OFFS(0), DEF_YMM_OFFS(1), - // 2..15 are implied in range usage - r31_off = xmm_off + (XSAVE_AREA_EGPRS - XSAVE_AREA_BEGIN)/BytesPerInt, - r31H_off, - r30_off, r30H_off, - r29_off, r29H_off, - r28_off, r28H_off, - r27_off, r27H_off, - r26_off, r26H_off, - r25_off, r25H_off, - r24_off, r24H_off, - r23_off, r23H_off, - r22_off, r22H_off, - r21_off, r21H_off, - r20_off, r20H_off, - r19_off, r19H_off, - r18_off, r18H_off, + r16_off = xmm_off + (XSAVE_AREA_EGPRS - XSAVE_AREA_BEGIN)/BytesPerInt, + r16H_off, r17_off, r17H_off, - r16_off, r16H_off, + r18_off, r18H_off, + r19_off, r19H_off, + r20_off, r20H_off, + r21_off, r21H_off, + r22_off, r22H_off, + r23_off, r23H_off, + r24_off, r24H_off, + r25_off, r25H_off, + r26_off, r26H_off, + r27_off, r27H_off, + r28_off, r28H_off, + r29_off, r29H_off, + r30_off, r30H_off, + r31_off, r31H_off, opmask_off = xmm_off + (XSAVE_AREA_OPMASK_BEGIN - XSAVE_AREA_BEGIN)/BytesPerInt, DEF_OPMASK_OFFS(0), DEF_OPMASK_OFFS(1), @@ -265,7 +264,7 @@ OopMap* RegisterSaver::save_live_registers(MacroAssembler* masm, int additional_ if (UseAPX) { int base_addr = XSAVE_AREA_EGPRS; off = 0; - for(int n = 16; n < Register::number_of_registers; n++) { + for (int n = 16; n < Register::number_of_registers; n++) { __ movq(Address(rsp, base_addr+(off++*8)), as_Register(n)); } } From b1a21b563e3ae13fa5c409a4f0c04686c3f5b34a Mon Sep 17 00:00:00 2001 From: Fei Yang Date: Wed, 5 Mar 2025 02:17:22 +0000 Subject: [PATCH 05/46] 8351101: RISC-V: C2: Small improvement to MacroAssembler::revb Reviewed-by: fjiang, mli --- src/hotspot/cpu/riscv/macroAssembler_riscv.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/hotspot/cpu/riscv/macroAssembler_riscv.cpp b/src/hotspot/cpu/riscv/macroAssembler_riscv.cpp index f2bdd20890bdf..d3daef7664d04 100644 --- a/src/hotspot/cpu/riscv/macroAssembler_riscv.cpp +++ b/src/hotspot/cpu/riscv/macroAssembler_riscv.cpp @@ -2892,7 +2892,6 @@ void MacroAssembler::revb(Register Rd, Register Rs, Register tmp1, Register tmp2 slli(tmp1, tmp1, 8); } srli(Rd, Rs, 56); - zext(Rd, Rd, 8); orr(Rd, tmp1, Rd); } From 75f028b46b245bdcbde8391af69020befda66b7d Mon Sep 17 00:00:00 2001 From: Emanuel Peter Date: Wed, 5 Mar 2025 10:01:26 +0000 Subject: [PATCH 06/46] 8348657: compiler/loopopts/superword/TestEquivalentInvariants.java timed out Reviewed-by: thartmann --- .../compiler/loopopts/superword/TestEquivalentInvariants.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/hotspot/jtreg/compiler/loopopts/superword/TestEquivalentInvariants.java b/test/hotspot/jtreg/compiler/loopopts/superword/TestEquivalentInvariants.java index 09b087bee5434..b3032b1fa4e9d 100644 --- a/test/hotspot/jtreg/compiler/loopopts/superword/TestEquivalentInvariants.java +++ b/test/hotspot/jtreg/compiler/loopopts/superword/TestEquivalentInvariants.java @@ -39,7 +39,7 @@ * i.e. where the invariants have the same summands, but in a different order. * @modules java.base/jdk.internal.misc * @library /test/lib / - * @run driver compiler.loopopts.superword.TestEquivalentInvariants + * @run driver/timeout=1200 compiler.loopopts.superword.TestEquivalentInvariants */ public class TestEquivalentInvariants { From de29ef3bf3a029f99f340de9f093cd20544217fd Mon Sep 17 00:00:00 2001 From: Sergey Chernyshev Date: Wed, 5 Mar 2025 10:32:36 +0000 Subject: [PATCH 07/46] 8343191: Cgroup v1 subsystem fails to set subsystem path Co-authored-by: Severin Gehwolf Reviewed-by: sgehwolf, mbaesken --- src/hotspot/os/linux/cgroupUtil_linux.cpp | 30 ++++- .../os/linux/cgroupV1Subsystem_linux.cpp | 77 +++++++++-- .../os/linux/cgroupV2Subsystem_linux.cpp | 6 +- .../cgroupv1/CgroupV1SubsystemController.java | 42 +++--- .../runtime/test_cgroupSubsystem_linux.cpp | 78 ++++++++++- .../docker/TestMemoryWithSubgroups.java | 126 ++++++++++++++++++ .../CgroupV1SubsystemControllerTest.java | 17 ++- .../cgroup/TestCgroupSubsystemFactory.java | 34 ++++- .../TestDockerMemoryMetricsSubgroup.java | 120 +++++++++++++++++ 9 files changed, 488 insertions(+), 42 deletions(-) create mode 100644 test/hotspot/jtreg/containers/docker/TestMemoryWithSubgroups.java create mode 100644 test/jdk/jdk/internal/platform/docker/TestDockerMemoryMetricsSubgroup.java diff --git a/src/hotspot/os/linux/cgroupUtil_linux.cpp b/src/hotspot/os/linux/cgroupUtil_linux.cpp index bc0e018d6be03..b52ef87dcaee2 100644 --- a/src/hotspot/os/linux/cgroupUtil_linux.cpp +++ b/src/hotspot/os/linux/cgroupUtil_linux.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2024, Red Hat, Inc. + * Copyright (c) 2024, 2025, Red Hat, Inc. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -49,12 +49,18 @@ int CgroupUtil::processor_count(CgroupCpuController* cpu_ctrl, int host_cpus) { } void CgroupUtil::adjust_controller(CgroupMemoryController* mem) { + assert(mem->cgroup_path() != nullptr, "invariant"); + if (strstr(mem->cgroup_path(), "../") != nullptr) { + log_warning(os, container)("Cgroup memory controller path at '%s' seems to have moved to '%s', detected limits won't be accurate", + mem->mount_point(), mem->cgroup_path()); + mem->set_subsystem_path("/"); + return; + } if (!mem->needs_hierarchy_adjustment()) { // nothing to do return; } log_trace(os, container)("Adjusting controller path for memory: %s", mem->subsystem_path()); - assert(mem->cgroup_path() != nullptr, "invariant"); char* orig = os::strdup(mem->cgroup_path()); char* cg_path = os::strdup(orig); char* last_slash; @@ -62,7 +68,8 @@ void CgroupUtil::adjust_controller(CgroupMemoryController* mem) { julong phys_mem = os::Linux::physical_memory(); char* limit_cg_path = nullptr; jlong limit = mem->read_memory_limit_in_bytes(phys_mem); - jlong lowest_limit = phys_mem; + jlong lowest_limit = limit < 0 ? phys_mem : limit; + julong orig_limit = ((julong)lowest_limit) != phys_mem ? lowest_limit : phys_mem; while ((last_slash = strrchr(cg_path, '/')) != cg_path) { *last_slash = '\0'; // strip path // update to shortened path and try again @@ -83,7 +90,7 @@ void CgroupUtil::adjust_controller(CgroupMemoryController* mem) { limit_cg_path = os::strdup("/"); } assert(lowest_limit >= 0, "limit must be positive"); - if ((julong)lowest_limit != phys_mem) { + if ((julong)lowest_limit != orig_limit) { // we've found a lower limit anywhere in the hierarchy, // set the path to the limit path assert(limit_cg_path != nullptr, "limit path must be set"); @@ -93,6 +100,7 @@ void CgroupUtil::adjust_controller(CgroupMemoryController* mem) { mem->subsystem_path(), lowest_limit); } else { + log_trace(os, container)("Lowest limit was: " JLONG_FORMAT, lowest_limit); log_trace(os, container)("No lower limit found for memory in hierarchy %s, " "adjusting to original path %s", mem->mount_point(), orig); @@ -104,19 +112,26 @@ void CgroupUtil::adjust_controller(CgroupMemoryController* mem) { } void CgroupUtil::adjust_controller(CgroupCpuController* cpu) { + assert(cpu->cgroup_path() != nullptr, "invariant"); + if (strstr(cpu->cgroup_path(), "../") != nullptr) { + log_warning(os, container)("Cgroup cpu controller path at '%s' seems to have moved to '%s', detected limits won't be accurate", + cpu->mount_point(), cpu->cgroup_path()); + cpu->set_subsystem_path("/"); + return; + } if (!cpu->needs_hierarchy_adjustment()) { // nothing to do return; } log_trace(os, container)("Adjusting controller path for cpu: %s", cpu->subsystem_path()); - assert(cpu->cgroup_path() != nullptr, "invariant"); char* orig = os::strdup(cpu->cgroup_path()); char* cg_path = os::strdup(orig); char* last_slash; assert(cg_path[0] == '/', "cgroup path must start with '/'"); int host_cpus = os::Linux::active_processor_count(); int cpus = CgroupUtil::processor_count(cpu, host_cpus); - int lowest_limit = host_cpus; + int lowest_limit = cpus < host_cpus ? cpus: host_cpus; + int orig_limit = lowest_limit != host_cpus ? lowest_limit : host_cpus; char* limit_cg_path = nullptr; while ((last_slash = strrchr(cg_path, '/')) != cg_path) { *last_slash = '\0'; // strip path @@ -138,7 +153,7 @@ void CgroupUtil::adjust_controller(CgroupCpuController* cpu) { limit_cg_path = os::strdup(cg_path); } assert(lowest_limit >= 0, "limit must be positive"); - if (lowest_limit != host_cpus) { + if (lowest_limit != orig_limit) { // we've found a lower limit anywhere in the hierarchy, // set the path to the limit path assert(limit_cg_path != nullptr, "limit path must be set"); @@ -148,6 +163,7 @@ void CgroupUtil::adjust_controller(CgroupCpuController* cpu) { cpu->subsystem_path(), lowest_limit); } else { + log_trace(os, container)("Lowest limit was: %d", lowest_limit); log_trace(os, container)("No lower limit found for cpu in hierarchy %s, " "adjusting to original path %s", cpu->mount_point(), orig); diff --git a/src/hotspot/os/linux/cgroupV1Subsystem_linux.cpp b/src/hotspot/os/linux/cgroupV1Subsystem_linux.cpp index a6ac2822b251d..8d9c3edb72a24 100644 --- a/src/hotspot/os/linux/cgroupV1Subsystem_linux.cpp +++ b/src/hotspot/os/linux/cgroupV1Subsystem_linux.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2019, 2024, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2019, 2025, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -37,6 +37,47 @@ /* * Set directory to subsystem specific files based * on the contents of the mountinfo and cgroup files. + * + * The method determines whether it runs in + * - host mode + * - container mode + * + * In the host mode, _root is equal to "/" and + * the subsystem path is equal to the _mount_point path + * joined with cgroup_path. + * + * In the container mode, it can be two possibilities: + * - private namespace (cgroupns=private) + * - host namespace (cgroupns=host, default mode in cgroup V1 hosts) + * + * Private namespace is equivalent to the host mode, i.e. + * the subsystem path is set by concatenating + * _mount_point and cgroup_path. + * + * In the host namespace, _root is equal to host's cgroup path + * of the control group to which the containerized process + * belongs to at the moment of creation. The mountinfo and + * cgroup files are mirrored from the host, while the subsystem + * specific files are mapped directly at _mount_point, i.e. + * at /sys/fs/cgroup//, the subsystem path is + * then set equal to _mount_point. + * + * A special case of the subsystem path is when a cgroup path + * includes a subgroup, when a containerized process was associated + * with an existing cgroup, that is different from cgroup + * in which the process has been created. + * Here, the _root is equal to the host's initial cgroup path, + * cgroup_path will be equal to host's new cgroup path. + * As host cgroup hierarchies are not accessible in the container, + * it needs to be determined which part of cgroup path + * is accessible inside container, i.e. mapped under + * /sys/fs/cgroup//. + * In Docker default setup, host's cgroup path can be + * of the form: /docker//, + * from which only is mapped. + * The method trims cgroup path from left, until the subgroup + * component is found. The subsystem path will be set to + * the _mount_point joined with the subgroup path. */ void CgroupV1Controller::set_subsystem_path(const char* cgroup_path) { if (_cgroup_path != nullptr) { @@ -49,28 +90,36 @@ void CgroupV1Controller::set_subsystem_path(const char* cgroup_path) { _cgroup_path = os::strdup(cgroup_path); stringStream ss; if (_root != nullptr && cgroup_path != nullptr) { + ss.print_raw(_mount_point); if (strcmp(_root, "/") == 0) { - ss.print_raw(_mount_point); + // host processes and containers with cgroupns=private if (strcmp(cgroup_path,"/") != 0) { ss.print_raw(cgroup_path); } - _path = os::strdup(ss.base()); } else { - if (strcmp(_root, cgroup_path) == 0) { - ss.print_raw(_mount_point); - _path = os::strdup(ss.base()); - } else { - char *p = strstr((char*)cgroup_path, _root); - if (p != nullptr && p == _root) { - if (strlen(cgroup_path) > strlen(_root)) { - ss.print_raw(_mount_point); - const char* cg_path_sub = cgroup_path + strlen(_root); - ss.print_raw(cg_path_sub); - _path = os::strdup(ss.base()); + // containers with cgroupns=host, default setting is _root==cgroup_path + if (strcmp(_root, cgroup_path) != 0) { + if (*cgroup_path != '\0' && strcmp(cgroup_path, "/") != 0) { + // When moved to a subgroup, between subgroups, the path suffix will change. + const char *suffix = cgroup_path; + while (suffix != nullptr) { + stringStream pp; + pp.print_raw(_mount_point); + pp.print_raw(suffix); + if (os::file_exists(pp.base())) { + ss.print_raw(suffix); + if (suffix != cgroup_path) { + log_trace(os, container)("set_subsystem_path: cgroup v1 path reduced to: %s.", suffix); + } + break; + } + log_trace(os, container)("set_subsystem_path: skipped non-existent directory: %s.", suffix); + suffix = strchr(suffix + 1, '/'); } } } } + _path = os::strdup(ss.base()); } } diff --git a/src/hotspot/os/linux/cgroupV2Subsystem_linux.cpp b/src/hotspot/os/linux/cgroupV2Subsystem_linux.cpp index 62e8cac3a62b2..cbadbb9db02ff 100644 --- a/src/hotspot/os/linux/cgroupV2Subsystem_linux.cpp +++ b/src/hotspot/os/linux/cgroupV2Subsystem_linux.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2020, 2022, Red Hat Inc. + * Copyright (c) 2020, 2025, Red Hat Inc. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -292,6 +292,10 @@ jlong memory_swap_limit_value(CgroupV2Controller* ctrl) { } void CgroupV2Controller::set_subsystem_path(const char* cgroup_path) { + if (_cgroup_path != nullptr) { + os::free(_cgroup_path); + } + _cgroup_path = os::strdup(cgroup_path); if (_path != nullptr) { os::free(_path); } diff --git a/src/java.base/linux/classes/jdk/internal/platform/cgroupv1/CgroupV1SubsystemController.java b/src/java.base/linux/classes/jdk/internal/platform/cgroupv1/CgroupV1SubsystemController.java index 051b4da5f78d8..fd325a8f8b4e9 100644 --- a/src/java.base/linux/classes/jdk/internal/platform/cgroupv1/CgroupV1SubsystemController.java +++ b/src/java.base/linux/classes/jdk/internal/platform/cgroupv1/CgroupV1SubsystemController.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2018, 2022, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2018, 2025, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -25,6 +25,9 @@ package jdk.internal.platform.cgroupv1; +import java.lang.System.Logger.Level; +import java.nio.file.Path; +import java.nio.file.Files; import jdk.internal.platform.CgroupSubsystem; import jdk.internal.platform.CgroupSubsystemController; @@ -44,27 +47,36 @@ public CgroupV1SubsystemController(String root, String mountPoint) { public void setPath(String cgroupPath) { if (root != null && cgroupPath != null) { + String path = mountPoint; if (root.equals("/")) { + // host processes and containers with cgroupns=private if (!cgroupPath.equals("/")) { - path = mountPoint + cgroupPath; + path += cgroupPath; } - else { - path = mountPoint; - } - } - else { - if (root.equals(cgroupPath)) { - path = mountPoint; - } - else { - if (cgroupPath.startsWith(root)) { - if (cgroupPath.length() > root.length()) { - String cgroupSubstr = cgroupPath.substring(root.length()); - path = mountPoint + cgroupSubstr; + } else { + // containers with cgroupns=host, default setting is _root==cgroup_path + if (!cgroupPath.equals(root)) { + if (!cgroupPath.equals("") && !cgroupPath.equals("/")) { + // When moved to a subgroup, between subgroups, the path suffix will change. + Path cgp = Path.of(cgroupPath); + int nameCount = cgp.getNameCount(); + for (int i=0; i < nameCount; i++) { + Path dir = Path.of(mountPoint, cgp.toString()); + if (Files.isDirectory(dir)) { + path = dir.toString(); + if (i > 0) { + System.getLogger("jdk.internal.platform").log(Level.DEBUG, String.format( + "Cgroup v1 path reduced to: %s.", cgp)); + } + break; + } + int currentNameCount = cgp.getNameCount(); + cgp = (currentNameCount > 1) ? cgp.subpath(1, currentNameCount) : Path.of(""); } } } } + this.path = path; } } diff --git a/test/hotspot/gtest/runtime/test_cgroupSubsystem_linux.cpp b/test/hotspot/gtest/runtime/test_cgroupSubsystem_linux.cpp index c8e2beff52b04..c090aa28e9a67 100644 --- a/test/hotspot/gtest/runtime/test_cgroupSubsystem_linux.cpp +++ b/test/hotspot/gtest/runtime/test_cgroupSubsystem_linux.cpp @@ -25,6 +25,7 @@ #include "runtime/os.hpp" #include "cgroupSubsystem_linux.hpp" +#include "cgroupUtil_linux.hpp" #include "cgroupV1Subsystem_linux.hpp" #include "cgroupV2Subsystem_linux.hpp" #include "unittest.hpp" @@ -432,9 +433,16 @@ TEST(cgroupTest, set_cgroupv1_subsystem_path) { "/user.slice/user-1000.slice/user@1000.service", // cgroup_path "/sys/fs/cgroup/mem" // expected_path }; - int length = 2; + TestCase container_moving_cgroup = { + "/sys/fs/cgroup/cpu,cpuacct", // mount_path + "/system.slice/garden.service/garden/good/2f57368b-0eda-4e52-64d8-af5c", // root_path + "/system.slice/garden.service/garden/bad/2f57368b-0eda-4e52-64d8-af5c", // cgroup_path + "/sys/fs/cgroup/cpu,cpuacct" // expected_path + }; + int length = 3; TestCase* testCases[] = { &host, - &container_engine }; + &container_engine, + &container_moving_cgroup }; for (int i = 0; i < length; i++) { CgroupV1Controller* ctrl = new CgroupV1Controller( (char*)testCases[i]->root_path, (char*)testCases[i]->mount_path, @@ -444,6 +452,72 @@ TEST(cgroupTest, set_cgroupv1_subsystem_path) { } } +TEST(cgroupTest, set_cgroupv1_subsystem_path_adjusted) { + TestCase memory = { + "/sys/fs/cgroup/memory", // mount_path + "/", // root_path + "../test1", // cgroup_path + "/sys/fs/cgroup/memory" // expected_path + }; + TestCase cpu = { + "/sys/fs/cgroup/cpu", // mount_path + "/", // root_path + "../../test2", // cgroup_path + "/sys/fs/cgroup/cpu" // expected_path + }; + CgroupCpuController* ccc = new CgroupV1CpuController(CgroupV1Controller((char*)cpu.root_path, + (char*)cpu.mount_path, + true /* read-only mount */)); + ccc->set_subsystem_path((char*)cpu.cgroup_path); + EXPECT_TRUE(ccc->needs_hierarchy_adjustment()); + + CgroupUtil::adjust_controller(ccc); + ASSERT_STREQ(cpu.expected_path, ccc->subsystem_path()); + EXPECT_FALSE(ccc->needs_hierarchy_adjustment()); + + CgroupMemoryController* cmc = new CgroupV1MemoryController(CgroupV1Controller((char*)memory.root_path, + (char*)memory.mount_path, + true /* read-only mount */)); + cmc->set_subsystem_path((char*)memory.cgroup_path); + EXPECT_TRUE(cmc->needs_hierarchy_adjustment()); + + CgroupUtil::adjust_controller(cmc); + ASSERT_STREQ(memory.expected_path, cmc->subsystem_path()); + EXPECT_FALSE(cmc->needs_hierarchy_adjustment()); +} + +TEST(cgroupTest, set_cgroupv2_subsystem_path_adjusted) { + TestCase memory = { + "/sys/fs/cgroup", // mount_path + "/", // root_path + "../test1", // cgroup_path + "/sys/fs/cgroup" // expected_path + }; + TestCase cpu = { + "/sys/fs/cgroup", // mount_path + "/", // root_path + "../../test2", // cgroup_path + "/sys/fs/cgroup" // expected_path + }; + CgroupCpuController* ccc = new CgroupV2CpuController(CgroupV2Controller((char*)cpu.mount_path, + (char*)cpu.cgroup_path, + true /* read-only mount */)); + EXPECT_TRUE(ccc->needs_hierarchy_adjustment()); + + CgroupUtil::adjust_controller(ccc); + ASSERT_STREQ(cpu.expected_path, ccc->subsystem_path()); + EXPECT_FALSE(ccc->needs_hierarchy_adjustment()); + + CgroupMemoryController* cmc = new CgroupV2MemoryController(CgroupV2Controller((char*)memory.mount_path, + (char*)memory.cgroup_path, + true /* read-only mount */)); + EXPECT_TRUE(cmc->needs_hierarchy_adjustment()); + + CgroupUtil::adjust_controller(cmc); + ASSERT_STREQ(memory.expected_path, cmc->subsystem_path()); + EXPECT_FALSE(cmc->needs_hierarchy_adjustment()); +} + TEST(cgroupTest, set_cgroupv2_subsystem_path) { TestCase at_mount_root = { "/sys/fs/cgroup", // mount_path diff --git a/test/hotspot/jtreg/containers/docker/TestMemoryWithSubgroups.java b/test/hotspot/jtreg/containers/docker/TestMemoryWithSubgroups.java new file mode 100644 index 0000000000000..e7989874d7a8f --- /dev/null +++ b/test/hotspot/jtreg/containers/docker/TestMemoryWithSubgroups.java @@ -0,0 +1,126 @@ +/* + * Copyright (C) 2025, BELLSOFT. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +import jdk.test.lib.containers.docker.Common; +import jdk.test.lib.containers.docker.DockerTestUtils; +import jdk.test.lib.containers.docker.DockerRunOptions; +import jdk.test.lib.process.OutputAnalyzer; +import jdk.test.lib.process.ProcessTools; +import jdk.internal.platform.Metrics; + +import java.util.ArrayList; + +import jtreg.SkippedException; + +/* + * @test + * @bug 8343191 + * @requires os.family == "linux" + * @modules java.base/jdk.internal.platform + * @library /test/lib + * @build jdk.test.whitebox.WhiteBox + * @run driver jdk.test.lib.helpers.ClassFileInstaller -jar whitebox.jar jdk.test.whitebox.WhiteBox + * @run main TestMemoryWithSubgroups + */ +public class TestMemoryWithSubgroups { + + private static final String imageName = Common.imageName("subgroup"); + + public static void main(String[] args) throws Exception { + Metrics metrics = Metrics.systemMetrics(); + if (metrics == null) { + System.out.println("Cgroup not configured."); + return; + } + if (!DockerTestUtils.canTestDocker()) { + System.out.println("Unable to run docker tests."); + return; + } + Common.prepareWhiteBox(); + DockerTestUtils.buildJdkContainerImage(imageName); + + if ("cgroupv1".equals(metrics.getProvider())) { + try { + testMemoryLimitSubgroupV1("200m", "100m", "104857600", false); + testMemoryLimitSubgroupV1("1g", "500m", "524288000", false); + testMemoryLimitSubgroupV1("200m", "100m", "104857600", true); + testMemoryLimitSubgroupV1("1g", "500m", "524288000", true); + } finally { + DockerTestUtils.removeDockerImage(imageName); + } + } else if ("cgroupv2".equals(metrics.getProvider())) { + try { + testMemoryLimitSubgroupV2("200m", "100m", "104857600", false); + testMemoryLimitSubgroupV2("1g", "500m", "524288000", false); + testMemoryLimitSubgroupV2("200m", "100m", "104857600", true); + testMemoryLimitSubgroupV2("1g", "500m", "524288000", true); + } finally { + DockerTestUtils.removeDockerImage(imageName); + } + } else { + throw new SkippedException("Metrics are from neither cgroup v1 nor v2, skipped for now."); + } + } + + private static void testMemoryLimitSubgroupV1(String containerMemorySize, String valueToSet, String expectedValue, boolean privateNamespace) + throws Exception { + + Common.logNewTestCase("Cgroup V1 subgroup memory limit: " + valueToSet); + + DockerRunOptions opts = new DockerRunOptions(imageName, "sh", "-c"); + opts.javaOpts = new ArrayList<>(); + opts.appendTestJavaOptions = false; + opts.addDockerOpts("--privileged") + .addDockerOpts("--cgroupns=" + (privateNamespace ? "private" : "host")) + .addDockerOpts("--memory", containerMemorySize); + opts.addClassOptions("mkdir -p /sys/fs/cgroup/memory/test ; " + + "echo " + valueToSet + " > /sys/fs/cgroup/memory/test/memory.limit_in_bytes ; " + + "echo $$ > /sys/fs/cgroup/memory/test/cgroup.procs ; " + + "/jdk/bin/java -Xlog:os+container=trace -version"); + + Common.run(opts) + .shouldMatch("Lowest limit was:.*" + expectedValue); + } + + private static void testMemoryLimitSubgroupV2(String containerMemorySize, String valueToSet, String expectedValue, boolean privateNamespace) + throws Exception { + + Common.logNewTestCase("Cgroup V2 subgroup memory limit: " + valueToSet); + + DockerRunOptions opts = new DockerRunOptions(imageName, "sh", "-c"); + opts.javaOpts = new ArrayList<>(); + opts.appendTestJavaOptions = false; + opts.addDockerOpts("--privileged") + .addDockerOpts("--cgroupns=" + (privateNamespace ? "private" : "host")) + .addDockerOpts("--memory", containerMemorySize); + opts.addClassOptions("mkdir -p /sys/fs/cgroup/memory/test ; " + + "echo $$ > /sys/fs/cgroup/memory/test/cgroup.procs ; " + + "echo '+memory' > /sys/fs/cgroup/cgroup.subtree_control ; " + + "echo '+memory' > /sys/fs/cgroup/memory/cgroup.subtree_control ; " + + "echo " + valueToSet + " > /sys/fs/cgroup/memory/test/memory.max ; " + + "/jdk/bin/java -Xlog:os+container=trace -version"); + + Common.run(opts) + .shouldMatch("Lowest limit was:.*" + expectedValue); + } +} diff --git a/test/jdk/jdk/internal/platform/cgroup/CgroupV1SubsystemControllerTest.java b/test/jdk/jdk/internal/platform/cgroup/CgroupV1SubsystemControllerTest.java index a97edd581feb7..3ab8b35ae0aa4 100644 --- a/test/jdk/jdk/internal/platform/cgroup/CgroupV1SubsystemControllerTest.java +++ b/test/jdk/jdk/internal/platform/cgroup/CgroupV1SubsystemControllerTest.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2022, Red Hat, Inc. + * Copyright (c) 2022, 2025, Red Hat, Inc. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -64,6 +64,9 @@ public void testCgPathNonEmptyRoot() { assertEquals(expectedPath, ctrl.path()); } + /* + * Less common cases: Containers + */ @Test public void testCgPathSubstring() { String root = "/foo/bar/baz"; @@ -71,8 +74,18 @@ public void testCgPathSubstring() { CgroupV1SubsystemController ctrl = new CgroupV1SubsystemController(root, mountPoint); String cgroupPath = "/foo/bar/baz/some"; ctrl.setPath(cgroupPath); - String expectedPath = mountPoint + "/some"; + String expectedPath = mountPoint; assertEquals(expectedPath, ctrl.path()); } + @Test + public void testCgPathToMovedPath() { + String root = "/system.slice/garden.service/garden/good/2f57368b-0eda-4e52-64d8-af5c"; + String mountPoint = "/sys/fs/cgroup/cpu,cpuacct"; + CgroupV1SubsystemController ctrl = new CgroupV1SubsystemController(root, mountPoint); + String cgroupPath = "/system.slice/garden.service/garden/bad/2f57368b-0eda-4e52-64d8-af5c"; + ctrl.setPath(cgroupPath); + String expectedPath = mountPoint; + assertEquals(expectedPath, ctrl.path()); + } } diff --git a/test/jdk/jdk/internal/platform/cgroup/TestCgroupSubsystemFactory.java b/test/jdk/jdk/internal/platform/cgroup/TestCgroupSubsystemFactory.java index ede74b5011e4c..8cf53c66e8aad 100644 --- a/test/jdk/jdk/internal/platform/cgroup/TestCgroupSubsystemFactory.java +++ b/test/jdk/jdk/internal/platform/cgroup/TestCgroupSubsystemFactory.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2020, 2022, Red Hat Inc. + * Copyright (c) 2020, 2025, Red Hat Inc. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -44,6 +44,7 @@ import jdk.internal.platform.CgroupSubsystemFactory.CgroupTypeResult; import jdk.internal.platform.CgroupV1MetricsImpl; import jdk.internal.platform.cgroupv1.CgroupV1Subsystem; +import jdk.internal.platform.cgroupv1.CgroupV1SubsystemController; import jdk.internal.platform.Metrics; import jdk.test.lib.Utils; import jdk.test.lib.util.FileUtils; @@ -75,8 +76,10 @@ public class TestCgroupSubsystemFactory { private Path cgroupv1MntInfoDoubleControllers; private Path cgroupv1MntInfoDoubleControllers2; private Path cgroupv1MntInfoColonsHierarchy; + private Path cgroupv1MntInfoNonTrivialRoot; private Path cgroupv1SelfCgroup; private Path cgroupv1SelfColons; + private Path cgroupv1SelfNonTrivialRoot; private Path cgroupv2SelfCgroup; private Path cgroupv1SelfCgroupJoinCtrl; private Path cgroupv1CgroupsOnlyCPUCtrl; @@ -175,6 +178,7 @@ public class TestCgroupSubsystemFactory { "42 30 0:38 / /sys/fs/cgroup/cpuset rw,nosuid,nodev,noexec,relatime shared:14 - cgroup none rw,seclabel,cpuset\n" + "43 30 0:39 / /sys/fs/cgroup/blkio rw,nosuid,nodev,noexec,relatime shared:15 - cgroup none rw,seclabel,blkio\n" + "44 30 0:40 / /sys/fs/cgroup/freezer rw,nosuid,nodev,noexec,relatime shared:16 - cgroup none rw,seclabel,freezer\n"; + private String mntInfoNonTrivialRoot = "2207 2196 0:43 /system.slice/garden.service/garden/good/2f57368b-0eda-4e52-64d8-af5c /sys/fs/cgroup/cpu,cpuacct ro,nosuid,nodev,noexec,relatime master:25 - cgroup cgroup rw,cpu,cpuacct\n"; private String cgroupsNonZeroHierarchy = "#subsys_name hierarchy num_cgroups enabled\n" + "cpuset 9 1 1\n" + @@ -230,6 +234,7 @@ public class TestCgroupSubsystemFactory { "2:cpu,cpuacct:/\n" + "1:name=systemd:/user.slice/user-1000.slice/user@1000.service/apps.slice/apps-org.gnome.Terminal.slice/vte-spawn-3c00b338-5b65-439f-8e97-135e183d135d.scope\n" + "0::/user.slice/user-1000.slice/user@1000.service/apps.slice/apps-org.gnome.Terminal.slice/vte-spawn-3c00b338-5b65-439f-8e97-135e183d135d.scope\n"; + private String cgroupv1SelfNTRoot = "11:cpu,cpuacct:/system.slice/garden.service/garden/bad/2f57368b-0eda-4e52-64d8-af5c\n"; private String cgroupv2SelfCgroupContent = "0::/user.slice/user-1000.slice/session-2.scope"; // We have a mix of V1 and V2 controllers, but none of the V1 controllers @@ -294,12 +299,18 @@ public void setup() { cgroupv1MntInfoColonsHierarchy = Paths.get(existingDirectory.toString(), "mountinfo_colons"); Files.writeString(cgroupv1MntInfoColonsHierarchy, mntInfoColons); + cgroupv1MntInfoNonTrivialRoot = Paths.get(existingDirectory.toString(), "mountinfo_nt_root"); + Files.writeString(cgroupv1MntInfoNonTrivialRoot, mntInfoNonTrivialRoot); + cgroupv1SelfCgroup = Paths.get(existingDirectory.toString(), "self_cgroup_cgv1"); Files.writeString(cgroupv1SelfCgroup, cgroupv1SelfCgroupContent); cgroupv1SelfColons = Paths.get(existingDirectory.toString(), "self_colons_cgv1"); Files.writeString(cgroupv1SelfColons, cgroupv1SelfColonsContent); + cgroupv1SelfNonTrivialRoot = Paths.get(existingDirectory.toString(), "self_nt_root_cgv1"); + Files.writeString(cgroupv1SelfNonTrivialRoot, cgroupv1SelfNTRoot); + cgroupv2SelfCgroup = Paths.get(existingDirectory.toString(), "self_cgroup_cgv2"); Files.writeString(cgroupv2SelfCgroup, cgroupv2SelfCgroupContent); @@ -449,6 +460,27 @@ public void testColonsCgroupsV1() throws IOException { assertEquals(memoryInfo.getMountRoot(), memoryInfo.getCgroupPath()); } + @Test + public void testMountPrefixCgroupsV1() throws IOException { + String cgroups = cgroupv1CgInfoNonZeroHierarchy.toString(); + String mountInfo = cgroupv1MntInfoNonTrivialRoot.toString(); + String selfCgroup = cgroupv1SelfNonTrivialRoot.toString(); + Optional result = CgroupSubsystemFactory.determineType(mountInfo, cgroups, selfCgroup); + + assertTrue("Expected non-empty cgroup result", result.isPresent()); + CgroupTypeResult res = result.get(); + CgroupInfo cpuInfo = res.getInfos().get("cpu"); + assertEquals(cpuInfo.getCgroupPath(), "/system.slice/garden.service/garden/bad/2f57368b-0eda-4e52-64d8-af5c"); + String expectedMountPoint = "/sys/fs/cgroup/cpu,cpuacct"; + assertEquals(expectedMountPoint, cpuInfo.getMountPoint()); + CgroupV1SubsystemController cgroupv1MemoryController = new CgroupV1SubsystemController(cpuInfo.getMountRoot(), cpuInfo.getMountPoint()); + cgroupv1MemoryController.setPath(cpuInfo.getCgroupPath()); + String actualPath = cgroupv1MemoryController.path(); + assertNotNull(actualPath); + String expectedPath = expectedMountPoint; + assertEquals("Should be equal to the mount point path", expectedPath, actualPath); + } + @Test public void testZeroHierarchyCgroupsV1() throws IOException { String cgroups = cgroupv1CgInfoZeroHierarchy.toString(); diff --git a/test/jdk/jdk/internal/platform/docker/TestDockerMemoryMetricsSubgroup.java b/test/jdk/jdk/internal/platform/docker/TestDockerMemoryMetricsSubgroup.java new file mode 100644 index 0000000000000..2ac79c173eff0 --- /dev/null +++ b/test/jdk/jdk/internal/platform/docker/TestDockerMemoryMetricsSubgroup.java @@ -0,0 +1,120 @@ +/* + * Copyright (c) 2025, BELLSOFT. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +import jdk.internal.platform.Metrics; +import jdk.test.lib.Utils; +import jdk.test.lib.containers.docker.Common; +import jdk.test.lib.containers.docker.DockerfileConfig; +import jdk.test.lib.containers.docker.DockerRunOptions; +import jdk.test.lib.containers.docker.DockerTestUtils; + +import java.util.ArrayList; + +import jtreg.SkippedException; + +/* + * @test + * @bug 8343191 + * @key cgroups + * @summary Cgroup v1 subsystem fails to set subsystem path + * @requires container.support + * @library /test/lib + * @modules java.base/jdk.internal.platform + * @build MetricsMemoryTester + * @run main TestDockerMemoryMetricsSubgroup + */ + +public class TestDockerMemoryMetricsSubgroup { + private static final String imageName = + DockerfileConfig.getBaseImageName() + ":" + + DockerfileConfig.getBaseImageVersion(); + + public static void main(String[] args) throws Exception { + Metrics metrics = Metrics.systemMetrics(); + if (metrics == null) { + System.out.println("Cgroup not configured."); + return; + } + if (!DockerTestUtils.canTestDocker()) { + System.out.println("Unable to run docker tests."); + return; + } + if ("cgroupv1".equals(metrics.getProvider())) { + testMemoryLimitSubgroupV1("200m", "400m", false); + testMemoryLimitSubgroupV1("500m", "1G", false); + testMemoryLimitSubgroupV1("200m", "400m", true); + testMemoryLimitSubgroupV1("500m", "1G", true); + } else if ("cgroupv2".equals(metrics.getProvider())) { + testMemoryLimitSubgroupV2("200m", "400m", false); + testMemoryLimitSubgroupV2("500m", "1G", false); + testMemoryLimitSubgroupV2("200m", "400m", true); + testMemoryLimitSubgroupV2("500m", "1G", true); + } else { + throw new SkippedException("Metrics are from neither cgroup v1 nor v2, skipped for now."); + } + } + + private static void testMemoryLimitSubgroupV1(String innerSize, String outerGroupMemorySize, boolean privateNamespace) throws Exception { + Common.logNewTestCase("testMemoryLimitSubgroup, innerSize = " + innerSize); + DockerRunOptions opts = + new DockerRunOptions(imageName, "sh", "-c"); + opts.javaOpts = new ArrayList<>(); + opts.appendTestJavaOptions = false; + opts.addDockerOpts("--volume", Utils.TEST_CLASSES + ":/test-classes/") + .addDockerOpts("--volume", Utils.TEST_JDK + ":/jdk") + .addDockerOpts("--privileged") + .addDockerOpts("--cgroupns=" + (privateNamespace ? "private" : "host")) + .addDockerOpts("--memory", outerGroupMemorySize); + opts.addClassOptions("mkdir -p /sys/fs/cgroup/memory/test ; " + + "echo " + innerSize + " > /sys/fs/cgroup/memory/test/memory.limit_in_bytes ; " + + "echo $$ > /sys/fs/cgroup/memory/test/cgroup.procs ; " + + "/jdk/bin/java -cp /test-classes/ " + + "--add-exports java.base/jdk.internal.platform=ALL-UNNAMED " + + "MetricsMemoryTester memory " + innerSize); + + DockerTestUtils.dockerRunJava(opts).shouldHaveExitValue(0).shouldContain("TEST PASSED!!!"); + } + + private static void testMemoryLimitSubgroupV2(String innerSize, String outerGroupMemorySize, boolean privateNamespace) throws Exception { + Common.logNewTestCase("testMemoryLimitSubgroup, innerSize = " + innerSize); + DockerRunOptions opts = + new DockerRunOptions(imageName, "sh", "-c"); + opts.javaOpts = new ArrayList<>(); + opts.appendTestJavaOptions = false; + opts.addDockerOpts("--volume", Utils.TEST_CLASSES + ":/test-classes/") + .addDockerOpts("--volume", Utils.TEST_JDK + ":/jdk") + .addDockerOpts("--privileged") + .addDockerOpts("--cgroupns=" + (privateNamespace ? "private" : "host")) + .addDockerOpts("--memory", outerGroupMemorySize); + opts.addClassOptions("mkdir -p /sys/fs/cgroup/memory/test ; " + + "echo $$ > /sys/fs/cgroup/memory/test/cgroup.procs ; " + + "echo '+memory' > /sys/fs/cgroup/cgroup.subtree_control ; " + + "echo '+memory' > /sys/fs/cgroup/memory/cgroup.subtree_control ; " + + "echo " + innerSize + " > /sys/fs/cgroup/memory/test/memory.max ; " + + "/jdk/bin/java -cp /test-classes/ " + + "--add-exports java.base/jdk.internal.platform=ALL-UNNAMED " + + "MetricsMemoryTester memory " + innerSize); + + DockerTestUtils.dockerRunJava(opts).shouldHaveExitValue(0).shouldContain("TEST PASSED!!!"); + } +} From a88e8cd0d2a444187208b41875b9da45daadad6a Mon Sep 17 00:00:00 2001 From: Matthias Baesken Date: Wed, 5 Mar 2025 12:30:09 +0000 Subject: [PATCH 08/46] 8350952: Remove some non present files from OPT_SPEED_SRC list Reviewed-by: dholmes, clanger --- make/hotspot/lib/JvmFeatures.gmk | 3 --- 1 file changed, 3 deletions(-) diff --git a/make/hotspot/lib/JvmFeatures.gmk b/make/hotspot/lib/JvmFeatures.gmk index 0efb8671da846..0a897230f835b 100644 --- a/make/hotspot/lib/JvmFeatures.gmk +++ b/make/hotspot/lib/JvmFeatures.gmk @@ -224,11 +224,9 @@ ifeq ($(call check-jvm-feature, opt-size), true) frame_ppc.cpp \ frame_s390.cpp \ frame_x86.cpp \ - genCollectedHeap.cpp \ generation.cpp \ growableArray.cpp \ handles.cpp \ - hashtable.cpp \ heap.cpp \ icache.cpp \ icache_arm.cpp \ @@ -246,7 +244,6 @@ ifeq ($(call check-jvm-feature, opt-size), true) linkResolver.cpp \ klass.cpp \ klassVtable.cpp \ - markSweep.cpp \ memRegion.cpp \ memoryPool.cpp \ method.cpp \ From 062b7c7348453e6a96c311082b112291913dc1d9 Mon Sep 17 00:00:00 2001 From: SendaoYan Date: Wed, 5 Mar 2025 12:55:33 +0000 Subject: [PATCH 09/46] 8351115: Test AOTClassLinkingVMOptions.java fails after JDK-8348322 Reviewed-by: dholmes, iklam --- .../cds/appcds/aotClassLinking/AOTClassLinkingVMOptions.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/hotspot/jtreg/runtime/cds/appcds/aotClassLinking/AOTClassLinkingVMOptions.java b/test/hotspot/jtreg/runtime/cds/appcds/aotClassLinking/AOTClassLinkingVMOptions.java index af5c619d76793..470bb699bc36f 100644 --- a/test/hotspot/jtreg/runtime/cds/appcds/aotClassLinking/AOTClassLinkingVMOptions.java +++ b/test/hotspot/jtreg/runtime/cds/appcds/aotClassLinking/AOTClassLinkingVMOptions.java @@ -82,7 +82,7 @@ public static void main(String[] args) throws Exception { // Dumping with AOTInvokeDynamicLinking disabled TestCommon.testDump(appJar, TestCommon.list("Hello"), - "-XX:+AOTClassLinking", "-XX:-AOTInvokeDynamicLinking"); + "-XX:+UnlockDiagnosticVMOptions", "-XX:+AOTClassLinking", "-XX:-AOTInvokeDynamicLinking"); testCase("Archived full module graph must be enabled at runtime (with -XX:-AOTInvokeDynamicLinking)"); TestCommon.run("-cp", appJar, "-Djdk.module.validation=1", "Hello") From caaf4098452476d981183ad4302b76b9c883a72b Mon Sep 17 00:00:00 2001 From: SendaoYan Date: Wed, 5 Mar 2025 12:57:56 +0000 Subject: [PATCH 10/46] 8350546: Several java/net/InetAddress tests fails UnknownHostException Reviewed-by: dfuchs, myankelevich --- .../InetAddress/IsReachableViaLoopbackTest.java | 15 ++++++++------- .../java/net/InetAddress/getOriginalHostName.java | 14 ++++++++++++-- 2 files changed, 20 insertions(+), 9 deletions(-) diff --git a/test/jdk/java/net/InetAddress/IsReachableViaLoopbackTest.java b/test/jdk/java/net/InetAddress/IsReachableViaLoopbackTest.java index 4d340a72fba21..2f2ad0ac7ddf4 100644 --- a/test/jdk/java/net/InetAddress/IsReachableViaLoopbackTest.java +++ b/test/jdk/java/net/InetAddress/IsReachableViaLoopbackTest.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2015, 2022, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2015, 2025, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -21,22 +21,24 @@ * questions. */ -import java.io.*; -import java.net.*; -import java.util.*; +import java.io.IOException; +import java.net.InetAddress; +import java.net.NetworkInterface; /** * @test * @bug 8135305 * @key intermittent + * @library /test/lib * @summary ensure we can't ping external hosts via loopback if + * @run main IsReachableViaLoopbackTest */ public class IsReachableViaLoopbackTest { public static void main(String[] args) { try { - InetAddress addr = InetAddress.getByName("localhost"); - InetAddress remoteAddr = InetAddress.getByName("bugs.openjdk.org"); + InetAddress addr = InetAddress.getLoopbackAddress(); + InetAddress remoteAddr = InetAddress.getByName("23.197.138.208"); // use literal address to avoid DNS checks if (!addr.isReachable(10000)) throw new RuntimeException("Localhost should always be reachable"); NetworkInterface inf = NetworkInterface.getByInetAddress(addr); @@ -54,7 +56,6 @@ public static void main(String[] args) { } else { System.out.println("inf == null"); } - } catch (IOException e) { throw new RuntimeException("Unexpected exception:" + e); } diff --git a/test/jdk/java/net/InetAddress/getOriginalHostName.java b/test/jdk/java/net/InetAddress/getOriginalHostName.java index 9f1e6e965d178..71567a7915d65 100644 --- a/test/jdk/java/net/InetAddress/getOriginalHostName.java +++ b/test/jdk/java/net/InetAddress/getOriginalHostName.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2015, 2018, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2015, 2025, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -41,7 +41,9 @@ public class getOriginalHostName { public static void main(String[] args) throws Exception { final String HOST = "dummyserver.java.net"; InetAddress ia = null; - ia = InetAddress.getByName(HOST); + ia = getInetAddress(HOST); + if (ia != null) testInetAddress(ia, HOST); + ia = InetAddress.getByAddress(HOST, new byte[] { 1, 2, 3, 4}); testInetAddress(ia, HOST); ia = InetAddress.getByName("255.255.255.0"); testInetAddress(ia, null); @@ -53,6 +55,14 @@ public static void main(String[] args) throws Exception { testInetAddress(ia, ia.getHostName()); } + private static InetAddress getInetAddress(String host) { + try { + return InetAddress.getByName(host); + } catch (java.net.UnknownHostException uhe) { + System.out.println("Skipping " + host + " due to " + uhe); + return null; + } + } private static void testInetAddress(InetAddress ia, String expected) throws Exception { From 1404e6d899b4ce31eee9c24ee5dcd53fe472beb3 Mon Sep 17 00:00:00 2001 From: Alexey Semenyuk Date: Sat, 1 Mar 2025 21:32:40 -0500 Subject: [PATCH 11/46] TestInstance: escape control characters in test description. Fail running tests if multiple tests have the same description --- .../helpers/jdk/jpackage/test/Main.java | 17 ++++++++++++----- .../helpers/jdk/jpackage/test/TestInstance.java | 15 ++++++++++++++- 2 files changed, 26 insertions(+), 6 deletions(-) diff --git a/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/Main.java b/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/Main.java index 439479a666ec0..8e16eb04232e7 100644 --- a/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/Main.java +++ b/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/Main.java @@ -23,6 +23,10 @@ package jdk.jpackage.test; +import static java.util.stream.Collectors.toCollection; +import static java.util.stream.Collectors.toMap; +import static jdk.jpackage.test.TestBuilder.CMDLINE_ARG_PREFIX; + import java.nio.file.Files; import java.nio.file.Path; import java.util.ArrayDeque; @@ -32,9 +36,7 @@ import java.util.List; import java.util.function.Function; import java.util.function.Predicate; -import static java.util.stream.Collectors.toCollection; import java.util.stream.Stream; -import static jdk.jpackage.test.TestBuilder.CMDLINE_ARG_PREFIX; public final class Main { @@ -88,9 +90,7 @@ public static void main(TestBuilder.Builder builder, String args[]) throws Throw TKit.unbox(throwable); } finally { if (!success) { - TKit.log( - String.format("Error processing parameter=[%s]", - arg)); + TKit.log(String.format("Error processing parameter=[%s]", arg)); } } } @@ -104,6 +104,13 @@ public static void main(TestBuilder.Builder builder, String args[]) throws Throw // Just list the tests orderedTests.forEach(test -> System.out.println(String.format( "%s; workDir=[%s]", test.fullName(), test.workDir()))); + } + + orderedTests.stream().collect(toMap(TestInstance::fullName, x -> x, (x, y) -> { + throw new IllegalArgumentException(String.format("Multiple tests with the same description: [%s]", x.fullName())); + })); + + if (listTests) { return; } diff --git a/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/TestInstance.java b/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/TestInstance.java index ca9523d576067..25167a93f1b5e 100644 --- a/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/TestInstance.java +++ b/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/TestInstance.java @@ -31,6 +31,7 @@ import java.util.Arrays; import java.util.Collections; import java.util.HashSet; +import java.util.HexFormat; import java.util.List; import java.util.Map; import java.util.Objects; @@ -119,12 +120,24 @@ private static String formatArgs(List values) { return String.format("%s(length=%d)", asString, Array.getLength(v)); } return String.format("%s", v); - }).collect(Collectors.joining(", ")); + }).collect(Collectors.joining(", ")).transform(str -> { + final var sb = new StringBuilder(); + for (var chr : str.toCharArray()) { + if (chr != ' ' && (Character.isWhitespace(chr) || Character.isISOControl(chr))) { + sb.append("\\u").append(ARGS_CHAR_FORMATTER.toHexDigits(chr)); + } else { + sb.append(chr); + } + } + return sb.toString(); + }); } private List ctorArgs; private List methodArgs; private Method method; + + private final static HexFormat ARGS_CHAR_FORMATTER = HexFormat.of().withUpperCase(); } static TestDesc create(Method m, Object... args) { From c50fc5c52aecb4de3608d8477f2048bd78d481bc Mon Sep 17 00:00:00 2001 From: Alexey Semenyuk Date: Thu, 6 Mar 2025 16:41:51 -0500 Subject: [PATCH 12/46] More robust msi unpack when wix4 is used --- .../helpers/jdk/jpackage/test/WindowsHelper.java | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/WindowsHelper.java b/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/WindowsHelper.java index 91705afd5feba..97431aac35e18 100644 --- a/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/WindowsHelper.java +++ b/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/WindowsHelper.java @@ -175,13 +175,19 @@ private static Path unpackMsi(JPackageCommand cmd, Path destinationDir) { Path installationSubDirectory = getInstallationSubDirectory(cmd); Path from = Path.of(extraPathComponent).resolve(installationSubDirectory); Path to = installationSubDirectory; - TKit.trace(String.format("Convert [%s] into [%s] in [%s] directory", from, to, - unpackDir)); + ThrowingRunnable.toRunnable(() -> { Files.createDirectories(unpackDir.resolve(to).getParent()); - Files.move(unpackDir.resolve(from), unpackDir.resolve(to)); - TKit.deleteDirectoryRecursive(unpackDir.resolve(extraPathComponent)); }).run(); + + // Files.move() occasionally results into java.nio.file.AccessDeniedException + Executor.tryRunMultipleTimes(ThrowingRunnable.toRunnable(() -> { + TKit.trace(String.format("Convert [%s] into [%s] in [%s] directory", from, to, unpackDir)); + final var dstDir = unpackDir.resolve(to); + TKit.deleteDirectoryRecursive(dstDir); + Files.move(unpackDir.resolve(from), dstDir); + TKit.deleteDirectoryRecursive(unpackDir.resolve(extraPathComponent)); + }), 3, 5); } } return destinationDir; From 08a2a66ef4a8cbc1257d26992ba5faf504de308e Mon Sep 17 00:00:00 2001 From: Alexey Semenyuk Date: Wed, 5 Mar 2025 14:53:44 -0500 Subject: [PATCH 13/46] Can run launcher verifier with altered environment variables --- .../helpers/jdk/jpackage/test/HelloApp.java | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/HelloApp.java b/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/HelloApp.java index 224a3f752cf3b..7ac4159c2a624 100644 --- a/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/HelloApp.java +++ b/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/HelloApp.java @@ -31,6 +31,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.Optional; import java.util.concurrent.atomic.AtomicBoolean; import java.util.function.Predicate; @@ -350,6 +351,7 @@ public static final class AppOutputVerifier { this.launcherPath = helloAppLauncher; this.outputFilePath = TKit.workDir().resolve(OUTPUT_FILENAME); this.params = new HashMap<>(); + this.env = new HashMap<>(); this.defaultLauncherArgs = new ArrayList<>(); } @@ -363,6 +365,16 @@ public AppOutputVerifier expectedExitCode(int v) { return this; } + public AppOutputVerifier addEnvironment(Map v) { + env.putAll(v); + return this; + } + + public AppOutputVerifier addEnvironmentVar(String name, String value) { + env.put(Objects.requireNonNull(name), Objects.requireNonNull(name)); + return this; + } + public AppOutputVerifier addDefaultArguments(String... v) { return addDefaultArguments(List.of(v)); } @@ -466,6 +478,10 @@ private Executor getExecutor(String...args) { .setExecutable(executablePath) .addArguments(List.of(args)); + env.forEach((envVarName, envVarValue) -> { + executor.setEnvVar(envVarName, envVarValue); + }); + return configureEnvironment(executor); } @@ -476,6 +492,7 @@ private Executor getExecutor(String...args) { private int expectedExitCode; private final List defaultLauncherArgs; private final Map params; + private final Map env; } public static AppOutputVerifier assertApp(Path helloAppLauncher) { From ce854a664b1c9e73add751e707d6781817dddc3f Mon Sep 17 00:00:00 2001 From: Alexey Semenyuk Date: Wed, 5 Mar 2025 14:52:34 -0500 Subject: [PATCH 14/46] Add Executor.setEnvVar() --- .../helpers/jdk/jpackage/test/Executor.java | 30 +++++++++++++++++-- 1 file changed, 28 insertions(+), 2 deletions(-) diff --git a/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/Executor.java b/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/Executor.java index a8a849cb9e786..cd1db30e9c9dc 100644 --- a/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/Executor.java +++ b/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/Executor.java @@ -33,8 +33,10 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; +import java.util.HashMap; import java.util.HashSet; import java.util.List; +import java.util.Map; import java.util.Objects; import java.util.Optional; import java.util.Set; @@ -92,6 +94,13 @@ public Executor setExecutable(JavaTool v) { public Executor removeEnvVar(String envVarName) { removeEnvVars.add(Objects.requireNonNull(envVarName)); + setEnvVars.remove(envVarName); + return this; + } + + public Executor setEnvVar(String envVarName, String envVarValue) { + setEnvVars.put(Objects.requireNonNull(envVarName), Objects.requireNonNull(envVarValue)); + removeEnvVars.remove(envVarName); return this; } @@ -370,9 +379,25 @@ private Result runExecutable() throws IOException, InterruptedException { builder.directory(directory.toFile()); sb.append(String.format("; in directory [%s]", directory)); } + if (!setEnvVars.isEmpty()) { + final var defaultEnv = builder.environment(); + final var envComm = Comm.compare(defaultEnv.keySet(), setEnvVars.keySet()); + envComm.unique2().forEach(envVar -> { + TKit.trace(String.format("Adding %s=[%s] to environment", envVar, setEnvVars.get(envVar))); + }); + envComm.common().forEach(envVar -> { + final var curValue = defaultEnv.get(envVar); + final var newValue = setEnvVars.get(envVar); + if (!curValue.equals(newValue)) { + TKit.trace(String.format("Setting %s=[%s] in environment", envVar, setEnvVars.get(envVar))); + } + }); + defaultEnv.putAll(setEnvVars); + } if (!removeEnvVars.isEmpty()) { - final var envComm = Comm.compare(builder.environment().keySet(), removeEnvVars); - builder.environment().keySet().removeAll(envComm.common()); + final var defaultEnv = builder.environment().keySet(); + final var envComm = Comm.compare(defaultEnv, removeEnvVars); + defaultEnv.removeAll(envComm.common()); envComm.common().forEach(envVar -> { TKit.trace(String.format("Clearing %s in environment", envVar)); }); @@ -515,6 +540,7 @@ private static void trace(String msg) { private Set saveOutputType; private Path directory; private Set removeEnvVars = new HashSet<>(); + private Map setEnvVars = new HashMap<>(); private boolean winEnglishOutput; private String winTmpDir = null; From 2ede257d1615c0d1908cd2322b613a6a77fec4c8 Mon Sep 17 00:00:00 2001 From: Alexey Semenyuk Date: Fri, 28 Feb 2025 09:05:55 -0500 Subject: [PATCH 15/46] Relax constraints on arguments for string formatting in JPackageStringBundle.cannedFormattedString() --- .../helpers/jdk/jpackage/test/JPackageStringBundle.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/JPackageStringBundle.java b/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/JPackageStringBundle.java index fff08d3541033..b9c8826b5af15 100644 --- a/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/JPackageStringBundle.java +++ b/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/JPackageStringBundle.java @@ -64,7 +64,7 @@ private String getFormattedString(String key, Object[] args) { } } - public CannedFormattedString cannedFormattedString(String key, String ... args) { + public CannedFormattedString cannedFormattedString(String key, Object ... args) { return new CannedFormattedString(this::getFormattedString, key, args); } From e74c14c888582df475c6371602d7d43b39fa33f6 Mon Sep 17 00:00:00 2001 From: Alexey Semenyuk Date: Fri, 28 Feb 2025 15:08:24 -0500 Subject: [PATCH 16/46] Make CannedFormattedString a record and add null checks. Add CannedFormattedString.CannedArgument interface to make CannedFormattedString instances provide system-independent test descriptions. --- .../jpackage/test/CannedFormattedString.java | 58 +++++++++++++++---- 1 file changed, 47 insertions(+), 11 deletions(-) diff --git a/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/CannedFormattedString.java b/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/CannedFormattedString.java index 8b28049b7b17e..b44cd80fe89b6 100644 --- a/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/CannedFormattedString.java +++ b/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/CannedFormattedString.java @@ -22,20 +22,60 @@ */ package jdk.jpackage.test; +import java.nio.file.Path; import java.util.List; +import java.util.Objects; import java.util.function.BiFunction; +import java.util.function.Supplier; +import java.util.stream.Stream; -public final class CannedFormattedString { +public record CannedFormattedString(BiFunction formatter, String key, Object[] args) { - CannedFormattedString(BiFunction formatter, - String key, Object[] args) { - this.formatter = formatter; - this.key = key; - this.args = args; + @FunctionalInterface + public interface CannedArgument { + public String value(); + } + + public static Object cannedArgument(Supplier supplier, String label) { + Objects.requireNonNull(supplier); + Objects.requireNonNull(label); + return new CannedArgument() { + + @Override + public String value() { + return supplier.get().toString(); + } + + @Override + public String toString( ) { + return label; + } + }; + } + + public static Object cannedAbsolutePath(Path v) { + return cannedArgument(() -> v.toAbsolutePath(), String.format("AbsolutePath(%s)", v)); + } + + public static Object cannedAbsolutePath(String v) { + return cannedAbsolutePath(Path.of(v)); + } + + public CannedFormattedString { + Objects.requireNonNull(formatter); + Objects.requireNonNull(key); + Objects.requireNonNull(args); + List.of(args).forEach(Objects::requireNonNull); } public String getValue() { - return formatter.apply(key, args); + return formatter.apply(key, Stream.of(args).map(arg -> { + if (arg instanceof CannedArgument cannedArg) { + return cannedArg.value(); + } else { + return arg; + } + }).toArray()); } @Override @@ -46,8 +86,4 @@ public String toString() { return String.format("%s+%s", key, List.of(args)); } } - - private final BiFunction formatter; - private final String key; - private final Object[] args; } From f4381f0750031ac413c45e674cc8ff7c01ac9cd7 Mon Sep 17 00:00:00 2001 From: Alexey Semenyuk Date: Fri, 28 Feb 2025 13:14:19 -0500 Subject: [PATCH 17/46] JPackageCommand can derive app name from jpackage command line without --name option but with --runtime-image option. Allow to use arguments with CannedFormattedString that get their values from JPackageCommand instance. Make JPackageCommand.validateOutput(CannedFormattedString) more robust and allow it to take an array of canned formatted strings. --- .../jdk/jpackage/test/JPackageCommand.java | 77 ++++++++++++++----- 1 file changed, 58 insertions(+), 19 deletions(-) diff --git a/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/JPackageCommand.java b/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/JPackageCommand.java index efcd004157916..1bcc202946754 100644 --- a/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/JPackageCommand.java +++ b/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/JPackageCommand.java @@ -218,26 +218,27 @@ public String version() { } public String name() { - String appImage = getArgumentValue("--app-image"); - if (appImage != null) { - String name = AppImageFile.load(Path.of(appImage)).mainLauncherName(); - // can be null if using foreign app-image - return ((name != null) ? name : getArgumentValue("--name")); - } - return getArgumentValue("--name", () -> getArgumentValue("--main-class")); + return nameFromAppImage().or(this::nameFromBasicArgs).or(this::nameFromRuntimeImage).orElseThrow(); } public String installerName() { verifyIsOfType(PackageType.NATIVE); - String installerName = getArgumentValue("--name", - () -> getArgumentValue("--main-class", () -> null)); - if (installerName == null) { - String appImage = getArgumentValue("--app-image"); - if (appImage != null) { - installerName = AppImageFile.load(Path.of(appImage)).mainLauncherName(); - } - } - return installerName; + return nameFromBasicArgs().or(this::nameFromAppImage).or(this::nameFromRuntimeImage).orElseThrow(); + } + + private Optional nameFromAppImage() { + return Optional.ofNullable(getArgumentValue("--app-image")) + .map(Path::of).map(AppImageFile::load).map(AppImageFile::mainLauncherName); + } + + private Optional nameFromRuntimeImage() { + return Optional.ofNullable(getArgumentValue("--runtime-image")) + .map(Path::of).map(Path::getFileName).map(Path::toString); + } + + private Optional nameFromBasicArgs() { + return Optional.ofNullable(getArgumentValue("--name")).or( + () -> Optional.ofNullable(getArgumentValue("--main-class"))); } public boolean isRuntime() { @@ -703,7 +704,7 @@ public JPackageCommand ignoreDefaultVerbose(boolean v) { } public JPackageCommand validateOutput(TKit.TextStreamVerifier validator) { - return JPackageCommand.this.validateOutput(validator::apply); + return validateOutput(validator::apply); } public JPackageCommand validateOutput(Consumer> validator) { @@ -716,8 +717,43 @@ public JPackageCommand validateOutput(Consumer> validator) { return this; } - public JPackageCommand validateOutput(CannedFormattedString str) { - return JPackageCommand.this.validateOutput(TKit.assertTextStream(str.getValue())); + @FunctionalInterface + public interface CannedArgument { + public String value(JPackageCommand cmd); + } + + public static Object cannedArgument(Function supplier, String label) { + Objects.requireNonNull(supplier); + Objects.requireNonNull(label); + return new CannedArgument() { + @Override + public String value(JPackageCommand cmd) { + return supplier.apply(cmd).toString(); + } + + @Override + public String toString( ) { + return label; + } + }; + } + + public String getValue(CannedFormattedString str) { + return new CannedFormattedString(str.formatter(), str.key(), Stream.of(str.args()).map(arg -> { + if (arg instanceof CannedArgument cannedArg) { + return cannedArg.value(this); + } else { + return arg; + } + }).toArray()).getValue(); + } + + public JPackageCommand validateOutput(CannedFormattedString... str) { + // Will look up the given errors in the order they are specified. + return validateOutput(Stream.of(str) + .map(this::getValue) + .map(TKit::assertTextStream) + .reduce(TKit.TextStreamVerifier::andThen).get()); } public boolean isWithToolProvider() { @@ -876,6 +912,9 @@ private static JPackageCommand convertFromRuntime(JPackageCommand cmd) { copy.immutable = false; copy.removeArgumentWithValue("--runtime-image"); copy.dmgInstallDir = cmd.appInstallationDirectory(); + if (!copy.hasArgument("--name")) { + copy.addArguments("--name", cmd.nameFromRuntimeImage().orElseThrow()); + } return copy; } From f2cd6514e1cd2b8c16285f3a88bb59ca24698873 Mon Sep 17 00:00:00 2001 From: Alexey Semenyuk Date: Thu, 6 Mar 2025 22:23:17 -0500 Subject: [PATCH 18/46] Create fake runtime in unique directory --- .../jpackage/helpers/jdk/jpackage/test/JPackageCommand.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/JPackageCommand.java b/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/JPackageCommand.java index 1bcc202946754..28a20f3b7d223 100644 --- a/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/JPackageCommand.java +++ b/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/JPackageCommand.java @@ -274,7 +274,7 @@ public JPackageCommand setFakeRuntime() { }; addPrerequisiteAction(cmd -> { - Path fakeRuntimeDir = TKit.workDir().resolve("fake_runtime"); + Path fakeRuntimeDir = TKit.createTempDirectory("fake_runtime"); TKit.trace(String.format("Init fake runtime in [%s] directory", fakeRuntimeDir)); From 4fc37b277aa70d8614c1c517a5fc2b00e3f4d9b9 Mon Sep 17 00:00:00 2001 From: Alexey Semenyuk Date: Thu, 6 Mar 2025 13:43:44 -0500 Subject: [PATCH 19/46] Support multiple output validators in JPackageCommand --- .../jdk/jpackage/test/JPackageCommand.java | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/JPackageCommand.java b/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/JPackageCommand.java index 28a20f3b7d223..f0035210822af 100644 --- a/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/JPackageCommand.java +++ b/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/JPackageCommand.java @@ -76,7 +76,7 @@ public JPackageCommand(JPackageCommand cmd) { prerequisiteActions = new Actions(cmd.prerequisiteActions); verifyActions = new Actions(cmd.verifyActions); appLayoutAsserts = cmd.appLayoutAsserts; - outputValidator = cmd.outputValidator; + outputValidators = cmd.outputValidators; executeInDirectory = cmd.executeInDirectory; winMsiLogFile = cmd.winMsiLogFile; } @@ -708,12 +708,9 @@ public JPackageCommand validateOutput(TKit.TextStreamVerifier validator) { } public JPackageCommand validateOutput(Consumer> validator) { - if (validator != null) { - saveConsoleOutput(true); - outputValidator = validator; - } else { - outputValidator = null; - } + Objects.requireNonNull(validator); + saveConsoleOutput(true); + outputValidators.add(validator); return this; } @@ -834,7 +831,7 @@ public Executor.Result execute(int expectedExitCode) { .createExecutor() .execute(expectedExitCode); - if (outputValidator != null) { + for (final var outputValidator: outputValidators) { outputValidator.accept(result.getOutput().stream()); } @@ -1234,7 +1231,7 @@ public void run() { private Path executeInDirectory; private Path winMsiLogFile; private Set appLayoutAsserts = Set.of(AppLayoutAssert.values()); - private Consumer> outputValidator; + private List>> outputValidators = new ArrayList<>(); private static boolean defaultWithToolProvider; private static final Map PACKAGE_TYPES = Functional.identity( From 185e38a618a0a9fc7e05d9bf04d20619e6019807 Mon Sep 17 00:00:00 2001 From: Alexey Semenyuk Date: Fri, 28 Feb 2025 15:09:19 -0500 Subject: [PATCH 20/46] Make PackageType.getName() public and rename it into PackageType.getType(); Add PackageType.isEnabled(). Make PackageType.isEnabled() and PackageType.isSupported() public. --- .../jdk/jpackage/test/PackageTestTest.java | 2 +- .../jdk/jpackage/test/JPackageCommand.java | 2 +- .../jdk/jpackage/test/PackageTest.java | 12 ++--- .../jdk/jpackage/test/PackageType.java | 49 +++++++++---------- .../jdk/jpackage/test/WindowsHelper.java | 2 +- 5 files changed, 33 insertions(+), 34 deletions(-) diff --git a/test/jdk/tools/jpackage/helpers-test/jdk/jpackage/test/PackageTestTest.java b/test/jdk/tools/jpackage/helpers-test/jdk/jpackage/test/PackageTestTest.java index 433db6fc6f69b..c582be5ac6263 100644 --- a/test/jdk/tools/jpackage/helpers-test/jdk/jpackage/test/PackageTestTest.java +++ b/test/jdk/tools/jpackage/helpers-test/jdk/jpackage/test/PackageTestTest.java @@ -376,7 +376,7 @@ public Executor.Result execute(int expectedExitCode) { }; }).setExpectedExitCode(expectedJPackageExitCode) .setExpectedInstallExitCode(handlersSpec.installExitCode) - .isPackageTypeSupported(type -> true) + .isPackageTypeEnabled(type -> true) .forTypes().packageHandlers(handlers); } diff --git a/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/JPackageCommand.java b/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/JPackageCommand.java index f0035210822af..8ffe1b68692e6 100644 --- a/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/JPackageCommand.java +++ b/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/JPackageCommand.java @@ -1238,7 +1238,7 @@ public void run() { () -> { Map reply = new HashMap<>(); for (PackageType type : PackageType.values()) { - reply.put(type.getName(), type); + reply.put(type.getType(), type); } return reply; }).get(); diff --git a/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/PackageTest.java b/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/PackageTest.java index e7b3f3e3a440b..f9c6e4da507eb 100644 --- a/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/PackageTest.java +++ b/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/PackageTest.java @@ -71,7 +71,7 @@ public final class PackageTest extends RunnablePackageTest { public PackageTest() { - isPackageTypeSupported = PackageType::isSupported; + isPackageTypeEnabled = PackageType::isEnabled; jpackageFactory = JPackageCommand::new; packageHandlers = new HashMap<>(); disabledInstallers = new HashSet<>(); @@ -102,7 +102,7 @@ public PackageTest forTypes(PackageType... types) { newTypes = Stream.of(types).collect(Collectors.toSet()); } currentTypes = newTypes.stream() - .filter(isPackageTypeSupported) + .filter(isPackageTypeEnabled) .filter(Predicate.not(excludeTypes::contains)) .collect(Collectors.toUnmodifiableSet()); return this; @@ -394,9 +394,9 @@ PackageTest packageHandlers(PackageHandlers v) { return this; } - PackageTest isPackageTypeSupported(Predicate v) { + PackageTest isPackageTypeEnabled(Predicate v) { Objects.requireNonNull(v); - isPackageTypeSupported = v; + isPackageTypeEnabled = v; return this; } @@ -505,7 +505,7 @@ public void accept(Action action) { case UNPACK -> { cmd.setUnpackedPackageLocation(null); final var unpackRootDir = TKit.createTempDirectory( - String.format("unpacked-%s", type.getName())); + String.format("unpacked-%s", type.getType())); final Path unpackDir = packageHandlers.unpack(cmd, unpackRootDir); if (!unpackDir.startsWith(TKit.workDir())) { state.deleteUnpackDirs.add(unpackDir); @@ -918,7 +918,7 @@ private static boolean isOfType(JPackageCommand cmd, Set packageTyp private final Map packageHandlers; private final Set disabledInstallers; private final Set disabledUninstallers; - private Predicate isPackageTypeSupported; + private Predicate isPackageTypeEnabled; private Supplier jpackageFactory; private boolean ignoreBundleOutputDir; private boolean createMsiLog; diff --git a/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/PackageType.java b/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/PackageType.java index 4d85f82df9b17..f6c2b84057ae0 100644 --- a/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/PackageType.java +++ b/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/PackageType.java @@ -26,6 +26,7 @@ import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; import java.util.Collections; +import java.util.Objects; import java.util.Optional; import java.util.Set; import java.util.concurrent.atomic.AtomicBoolean; @@ -48,19 +49,23 @@ public enum PackageType { TKit.isLinux() ? "jdk.jpackage.internal.LinuxRpmBundler" : null), MAC_DMG(".dmg", TKit.isOSX() ? "jdk.jpackage.internal.MacDmgBundler" : null), MAC_PKG(".pkg", TKit.isOSX() ? "jdk.jpackage.internal.MacPkgBundler" : null), - IMAGE("app-image", null, null); + IMAGE; + + PackageType() { + type = "app-image"; + suffix = null; + supported = true; + enabled = true; + } PackageType(String packageName, String bundleSuffix, String bundlerClass) { - name = packageName; - suffix = bundleSuffix; - if (bundlerClass != null && !Inner.DISABLED_PACKAGERS.contains(getName())) { - supported = isBundlerSupported(bundlerClass); - } else { - supported = false; - } + type = Objects.requireNonNull(packageName); + suffix = Objects.requireNonNull(bundleSuffix); + supported = Optional.ofNullable(bundlerClass).map(PackageType::isBundlerSupported).orElse(false); + enabled = supported && !Inner.DISABLED_PACKAGERS.contains(getType()); - if (suffix != null && supported) { - TKit.trace(String.format("Bundler %s supported", getName())); + if (suffix != null && enabled) { + TKit.trace(String.format("Bundler %s enabled", getType())); } } @@ -69,30 +74,23 @@ public enum PackageType { } void applyTo(JPackageCommand cmd) { - cmd.setArgumentValue("--type", getName()); + cmd.setArgumentValue("--type", getType()); } String getSuffix() { - return suffix; + return Optional.ofNullable(suffix).orElseThrow(UnsupportedOperationException::new); } - boolean isSupported() { + public boolean isSupported() { return supported; } - String getName() { - return name; + public boolean isEnabled() { + return supported; } - static PackageType fromSuffix(String packageFilename) { - if (packageFilename != null) { - for (PackageType v : values()) { - if (packageFilename.endsWith(v.getSuffix())) { - return v; - } - } - } - return null; + public String getType() { + return type; } private static boolean isBundlerSupportedImpl(String bundlerClass) { @@ -133,8 +131,9 @@ private static boolean isBundlerSupported(String bundlerClass) { return reply.get(); } - private final String name; + private final String type; private final String suffix; + private final boolean enabled; private final boolean supported; public static final Set LINUX = Set.of(LINUX_DEB, LINUX_RPM); diff --git a/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/WindowsHelper.java b/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/WindowsHelper.java index 97431aac35e18..68f4508c4134f 100644 --- a/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/WindowsHelper.java +++ b/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/WindowsHelper.java @@ -107,7 +107,7 @@ private static Optional configureMsiLogFile(JPackageCommand cmd, boolean c final Optional msiLogFile; if (createMsiLog) { msiLogFile = Optional.of(TKit.createTempFile(String.format("logs\\%s-msi.log", - cmd.packageType().getName()))); + cmd.packageType().getType()))); } else { msiLogFile = Optional.empty(); } From 2838a080cd4dbfe24289276504b5dde931e2e151 Mon Sep 17 00:00:00 2001 From: Alexey Semenyuk Date: Sun, 2 Mar 2025 15:31:37 -0500 Subject: [PATCH 21/46] Move token substitution in generic TokenReplace class. --- .../jpackage/internal/util/TokenReplace.java | 197 ++++++++++++ .../internal/util/TokenReplaceTest.java | 290 ++++++++++++++++++ 2 files changed, 487 insertions(+) create mode 100644 src/jdk.jpackage/share/classes/jdk/jpackage/internal/util/TokenReplace.java create mode 100644 test/jdk/tools/jpackage/junit/share/jdk.jpackage/jdk/jpackage/internal/util/TokenReplaceTest.java diff --git a/src/jdk.jpackage/share/classes/jdk/jpackage/internal/util/TokenReplace.java b/src/jdk.jpackage/share/classes/jdk/jpackage/internal/util/TokenReplace.java new file mode 100644 index 0000000000000..24c13db1aa772 --- /dev/null +++ b/src/jdk.jpackage/share/classes/jdk/jpackage/internal/util/TokenReplace.java @@ -0,0 +1,197 @@ +/* + * Copyright (c) 2025, Oracle and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. Oracle designates this + * particular file as subject to the "Classpath" exception as provided + * by Oracle in the LICENSE file that accompanied this code. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ +package jdk.jpackage.internal.util; + +import static java.util.stream.Collectors.joining; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Comparator; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Objects; +import java.util.Optional; +import java.util.function.Function; +import java.util.function.Predicate; +import java.util.function.Supplier; +import java.util.regex.Matcher; +import java.util.regex.Pattern; +import java.util.stream.IntStream; +import java.util.stream.Stream; + +public final class TokenReplace { + + private record TokenCut(String[] main, String[] sub) { + static String[] orderTokens(String... tokens) { + if (tokens.length == 0) { + throw new IllegalArgumentException("Empty token list"); + } + + final var orderedTokens = Stream.of(tokens) + .sorted(Comparator.naturalOrder().thenComparing(Comparator.comparingInt(String::length))) + .distinct() + .toArray(String[]::new); + + if (orderedTokens[0].isEmpty()) { + throw new IllegalArgumentException("Empty token in the list of tokens"); + } + + return orderedTokens; + } + + static TokenCut createFromOrderedTokens(String... tokens) { + final List subTokens = new ArrayList<>(); + + for (var i = 0; i < tokens.length - 1; ++i) { + final var x = tokens[i]; + for (var j = i + 1; j < tokens.length; ++j) { + final var y = tokens[j]; + if (y.contains(x)) { + subTokens.add(i); + } + } + } + + if (subTokens.isEmpty()) { + return new TokenCut(tokens, null); + } else { + final var main = IntStream.range(0, tokens.length) + .mapToObj(Integer::valueOf) + .filter(Predicate.not(subTokens::contains)) + .map(i -> { + return tokens[i]; + }).toArray(String[]::new); + final var sub = subTokens.stream().map(i -> { + return tokens[i]; + }).toArray(String[]::new); + return new TokenCut(main, sub); + } + } + + @Override + public String toString() { + return String.format("TokenCut(main=%s, sub=%s)", Arrays.toString(main), Arrays.toString(sub)); + } + } + + public TokenReplace(String... tokens) { + tokens = TokenCut.orderTokens(tokens); + + this.tokens = tokens; + regexps = new ArrayList<>(); + + for(;;) { + final var tokenCut = TokenCut.createFromOrderedTokens(tokens); + regexps.add(Pattern.compile(Stream.of(tokenCut.main()).map(Pattern::quote).collect(joining("|", "(", ")")))); + + if (tokenCut.sub() == null) { + break; + } + + tokens = tokenCut.sub(); + } + } + + public String applyTo(String str, Function tokenValueSupplier) { + Objects.requireNonNull(str); + Objects.requireNonNull(tokenValueSupplier); + for (final var regexp : regexps) { + str = regexp.matcher(str).replaceAll(mr -> { + final var token = mr.group(); + return Matcher.quoteReplacement(Objects.requireNonNull(tokenValueSupplier.apply(token), () -> { + return String.format("Null value for token [%s]", token); + }).toString()); + }); + } + return str; + } + + public String recursiveApplyTo(String str, Function tokenValueSupplier) { + String newStr; + int counter = tokens.length + 1; + while (!(newStr = applyTo(str, tokenValueSupplier)).equals(str)) { + str = newStr; + if (counter-- == 0) { + throw new IllegalStateException("Infinite recursion"); + } + } + return newStr; + } + + @Override + public int hashCode() { + // Auto generated code + final int prime = 31; + int result = 1; + result = prime * result + Arrays.hashCode(tokens); + return result; + } + + @Override + public boolean equals(Object obj) { + // Auto generated code + if (this == obj) + return true; + if (obj == null) + return false; + if (getClass() != obj.getClass()) + return false; + TokenReplace other = (TokenReplace) obj; + return Arrays.equals(tokens, other.tokens); + } + + @Override + public String toString() { + return "TokenReplace(" + String.join("|", tokens) + ")"; + } + + public static TokenReplace combine(TokenReplace x, TokenReplace y) { + return new TokenReplace(Stream.of(x.tokens, y.tokens).flatMap(Stream::of).toArray(String[]::new)); + } + + public static Function createCachingTokenValueSupplier(Map> tokenValueSuppliers) { + Objects.requireNonNull(tokenValueSuppliers); + final Map cache = new HashMap<>(); + return token -> { + final var value = cache.computeIfAbsent(token, k -> { + final var tokenValueSupplier = Objects.requireNonNull(tokenValueSuppliers.get(token), () -> { + return String.format("No token value supplier for token [%s]", token); + }); + return Optional.ofNullable(tokenValueSupplier.get()).orElse(NULL_SUPPLIED); + }); + + if (value == NULL_SUPPLIED) { + throw new NullPointerException(String.format("Null value for token [%s]", token)); + } + + return value; + }; + } + + private final String[] tokens; + private final transient List regexps; + private final static Object NULL_SUPPLIED = new Object(); +} diff --git a/test/jdk/tools/jpackage/junit/share/jdk.jpackage/jdk/jpackage/internal/util/TokenReplaceTest.java b/test/jdk/tools/jpackage/junit/share/jdk.jpackage/jdk/jpackage/internal/util/TokenReplaceTest.java new file mode 100644 index 0000000000000..3ead7c3d00835 --- /dev/null +++ b/test/jdk/tools/jpackage/junit/share/jdk.jpackage/jdk/jpackage/internal/util/TokenReplaceTest.java @@ -0,0 +1,290 @@ +/* + * Copyright (c) 2025, Oracle and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +package jdk.jpackage.internal.util; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; + +import java.util.HashMap; +import java.util.Map; +import java.util.Objects; +import java.util.Optional; +import java.util.function.Supplier; +import java.util.stream.Stream; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.MethodSource; + +public class TokenReplaceTest { + + public record TestSpec(String str, Optional expectedStr, Optional expectedCtorException, + Optional expectedApplyToException, Map tokenWithValues, boolean recursive) { + + public TestSpec { + Objects.requireNonNull(expectedStr); + Objects.requireNonNull(expectedCtorException); + Objects.requireNonNull(expectedApplyToException); + Objects.requireNonNull(tokenWithValues); + tokenWithValues.values().forEach(Objects::requireNonNull); + + if (expectedStr.isPresent()) { + if (!(expectedCtorException.isEmpty() && expectedApplyToException.isEmpty())) { + throw new IllegalArgumentException(); + } + } else if (expectedCtorException.isEmpty() == expectedApplyToException.isEmpty()) { + throw new IllegalArgumentException(); + } + } + + static final class Builder { + + Builder str(String v) { + str = v; + return this; + } + + Builder recursive(boolean v) { + recursive = v; + return this; + } + + Builder recursive() { + return recursive(true); + } + + Builder expect(String v) { + expectedStr = v; + return this; + } + + Builder expectCtorThrow(String v) { + expectedCtorException = new IllegalArgumentException(v); + return this; + } + + Builder expectApplyToNPE() { + expectedApplyToException = new NullPointerException(); + return this; + } + + Builder expectInfiniteRecursion() { + expectedApplyToException = new IllegalStateException("Infinite recursion"); + return this; + } + + Builder token(String token, String value) { + tokenWithValues.put(token, value); + return this; + } + + TestSpec create() { + return new TestSpec(str, expectedStr(), Optional.ofNullable(expectedCtorException), + Optional.ofNullable(expectedApplyToException), tokenWithValues, recursive); + } + + private Optional expectedStr() { + if (expectedCtorException == null && expectedApplyToException == null) { + return Optional.ofNullable(expectedStr).or(() -> Optional.of(str)); + } else { + return Optional.empty(); + } + } + + private boolean recursive; + private String str; + private String expectedStr; + private Exception expectedCtorException; + private Exception expectedApplyToException; + private final Map tokenWithValues = new HashMap<>(); + } + + void test() { + final var tokens = tokenWithValues.keySet().toArray(String[]::new); + expectedStr.ifPresent(expected -> { + final var tokenReplace = new TokenReplace(tokens); + final String actual; + if (recursive) { + actual = tokenReplace.recursiveApplyTo(str, tokenWithValues::get); + } else { + actual = tokenReplace.applyTo(str, tokenWithValues::get); + } + assertEquals(expected, actual); + }); + + expectedCtorException.ifPresent(expected -> { + final var ex = assertThrows(expected.getClass(), () -> { + new TokenReplace(tokens); + }); + assertEquals(expected.getMessage(), ex.getMessage()); + }); + + expectedApplyToException.ifPresent(expected -> { + final var tokenReplace = new TokenReplace(tokens); + final var ex = assertThrows(expected.getClass(), () -> { + if (recursive) { + tokenReplace.recursiveApplyTo(str, tokenWithValues::get); + } else { + tokenReplace.applyTo(str, tokenWithValues::get); + } + }); + assertEquals(expected.getMessage(), ex.getMessage()); + }); + } + } + + @ParameterizedTest + @MethodSource + public void test(TestSpec spec) { + spec.test(); + } + + public static Stream test() { + return Stream.of( + testSpec("foo").token("", "B").expectCtorThrow("Empty token in the list of tokens"), + testSpec("foo").expectCtorThrow("Empty token list"), + testSpec("a").expect("a").token("b", "B"), + testSpec("a").expect("A").token("a", "A"), + testSpec("aaa").expect("AAA").token("a", "A"), + testSpec("aaa").recursive().expect("{B}{B}{B}").token("a", "b").token("b", "{B}"), + testSpec("aaa").token("a", "aa").token("aa", "C").expect("Caa"), + testSpec("aaa").token("a", "aa").token("aa", "C").expect("CC").recursive(), + testSpec("aaa").expect("A2A").token("a", "A").token("aa", "A2"), + testSpec("aaa").token("a", "b").token("b", "c").token("c", "a").expect("bbb"), + testSpec("aaa").token("a", "b").token("b", "").recursive().expect(""), + testSpec("aaa").token("a", "").recursive().expect(""), + testSpec("aaa").token("a", "b").token("b", "c").token("c", "a").expectInfiniteRecursion().recursive(), + testSpec(null).token("a", "b").expectApplyToNPE(), + testSpec("abc").expect("abc").token(".", "A"), + testSpec("abc.").expect("abcD").token(".", "D") + ).map(TestSpec.Builder::create); + } + + private final static class CountingSupplier implements Supplier { + + CountingSupplier(Object value, int expectedCount) { + this.value = value; + this.expectedCount = expectedCount; + } + + @Override + public Object get() { + counter++; + return value; + } + + public Object value() { + return value; + } + + void verifyCount() { + assertEquals(expectedCount, counter); + } + + private final Object value; + private int counter; + private final int expectedCount; + } + + @Test + public void testCombine() { + final var x = new TokenReplace("a"); + final var y = new TokenReplace("aa"); + + final var xy = TokenReplace.combine(x, y); + + assertEquals(xy, new TokenReplace("aa", "a")); + assertEquals(xy, new TokenReplace("a", "aa")); + } + + @Test + public void testCombine2() { + final var x = new TokenReplace("a"); + final var y = new TokenReplace("a"); + + final var xy = TokenReplace.combine(x, y); + + assertEquals(xy, new TokenReplace("a", "a")); + assertEquals(xy, new TokenReplace("a")); + assertEquals(xy, x); + assertEquals(xy, y); + } + + @Test + public void testCombine3() { + final var x = new TokenReplace("a"); + final var y = new TokenReplace("b"); + + final var xy = TokenReplace.combine(x, y); + + assertEquals(xy, new TokenReplace("a", "b")); + assertEquals(xy, new TokenReplace("b", "a")); + } + + @Test + public void testEquals() { + final var x = new TokenReplace("x"); + final var y = new TokenReplace("y"); + final var y2 = new TokenReplace("y"); + + assertNotEquals(x, y); + assertNotEquals(x, null); + assertNotEquals(null, x); + assertNotEquals(x, "x"); + + assertEquals(y, y2); + assertEquals(y, y); + } + + @Test + public void testCreateCachingTokenValueSupplier() { + final var neverCalledSupplier = new CountingSupplier("", 0); + final var calledOnceSupplier = new CountingSupplier("foo", 1); + final var calledOnceNullSupplier = new CountingSupplier(null, 1); + + final var supplier = TokenReplace.createCachingTokenValueSupplier(Map.of( + "never", neverCalledSupplier, + "once", calledOnceSupplier, + "onceNull", calledOnceNullSupplier + )); + + for (int i = 0; i != 2; i++) { + assertEquals(calledOnceSupplier.value(), supplier.apply("once")); + + final var ex = assertThrows(NullPointerException.class, () -> supplier.apply("onceNull")); + assertEquals("Null value for token [onceNull]", ex.getMessage()); + } + + final var ex = assertThrows(NullPointerException.class, () -> supplier.apply("foo")); + assertEquals("No token value supplier for token [foo]", ex.getMessage()); + + neverCalledSupplier.verifyCount(); + calledOnceSupplier.verifyCount(); + calledOnceNullSupplier.verifyCount(); + } + + private static TestSpec.Builder testSpec(String str) { + return new TestSpec.Builder().str(str); + } +} From 939846397413c483d4d1728b2fefee6c7856bc66 Mon Sep 17 00:00:00 2001 From: Alexey Semenyuk Date: Thu, 27 Feb 2025 19:49:49 -0500 Subject: [PATCH 22/46] More negative tests --- test/jdk/tools/jpackage/share/ErrorTest.java | 659 ++++++++++++++++--- 1 file changed, 581 insertions(+), 78 deletions(-) diff --git a/test/jdk/tools/jpackage/share/ErrorTest.java b/test/jdk/tools/jpackage/share/ErrorTest.java index f28fe0a41523f..0483dc9e78d94 100644 --- a/test/jdk/tools/jpackage/share/ErrorTest.java +++ b/test/jdk/tools/jpackage/share/ErrorTest.java @@ -22,19 +22,32 @@ */ +import static java.util.stream.Collectors.toMap; +import static jdk.internal.util.OperatingSystem.LINUX; +import static jdk.internal.util.OperatingSystem.MACOS; +import static jdk.internal.util.OperatingSystem.WINDOWS; +import static jdk.jpackage.test.CannedFormattedString.cannedAbsolutePath; + +import java.nio.file.Path; +import java.util.ArrayList; import java.util.Collection; import java.util.List; +import java.util.Map; +import java.util.Objects; import java.util.Optional; +import java.util.function.Function; +import java.util.function.Supplier; +import java.util.regex.Pattern; import java.util.stream.Stream; +import jdk.jpackage.internal.util.TokenReplace; +import jdk.jpackage.test.Annotations.Parameter; import jdk.jpackage.test.Annotations.ParameterSupplier; import jdk.jpackage.test.Annotations.Test; import jdk.jpackage.test.CannedFormattedString; import jdk.jpackage.test.JPackageCommand; import jdk.jpackage.test.JPackageStringBundle; -import jdk.jpackage.test.PackageTest; -import jdk.jpackage.test.RunnablePackageTest; +import jdk.jpackage.test.PackageType; import jdk.jpackage.test.TKit; -import static jdk.internal.util.OperatingSystem.WINDOWS; /* * @test @@ -42,7 +55,7 @@ * @library /test/jdk/tools/jpackage/helpers * @build jdk.jpackage.test.* * @compile -Xlint:all -Werror ErrorTest.java - * @run main/othervm/timeout=360 -Xmx512m jdk.jpackage.test.Main + * @run main/othervm/timeout=720 -Xmx512m jdk.jpackage.test.Main * --jpt-run=ErrorTest * --jpt-before-run=jdk.jpackage.test.JPackageCommand.useExecutableByDefault */ @@ -53,101 +66,566 @@ * @library /test/jdk/tools/jpackage/helpers * @build jdk.jpackage.test.* * @compile -Xlint:all -Werror ErrorTest.java - * @run main/othervm/timeout=360 -Xmx512m jdk.jpackage.test.Main + * @run main/othervm/timeout=720 -Xmx512m jdk.jpackage.test.Main * --jpt-run=ErrorTest * --jpt-before-run=jdk.jpackage.test.JPackageCommand.useToolProviderByDefault */ public final class ErrorTest { - public static Collection input() { - return List.of(new Object[][]{ + enum Token { + JAVA_HOME(cmd -> { + return System.getProperty("java.home"); + }), + APP_IMAGE(cmd -> { + final var appImageRoot = TKit.createTempDirectory("appimage"); + + final var appImageCmd = JPackageCommand.helloAppImage() + .setFakeRuntime().setArgumentValue("--dest", appImageRoot); + + appImageCmd.execute(); + + return appImageCmd.outputBundle().toString(); + }), + ADD_LAUNCHER_PROPERTY_FILE; + + private Token() { + this.valueSupplier = Optional.empty(); + } + + private Token(Function valueSupplier) { + this.valueSupplier = Optional.of(valueSupplier); + } + + String token() { + return makeToken(name()); + } + + TokenReplace asTokenReplace() { + return tokenReplace; + } + + Optional expand(JPackageCommand cmd) { + return valueSupplier.map(func -> func.apply(cmd)); + } + + private static String makeToken(String v) { + Objects.requireNonNull(v); + return String.format("@@%s@@", v); + } + + private final Optional> valueSupplier; + private final TokenReplace tokenReplace = new TokenReplace(token()); + } + + public record TestSpec(Optional type, Optional appDesc, List addArgs, + List removeArgs, List expectedErrors) { + + static final class Builder { + + Builder type(PackageType v) { + type = v; + return this; + } + + Builder notype() { + return type(null); + } + + Builder nativeType() { + return type(NATIVE_TYPE); + } + + Builder appDesc(String v) { + appDesc = v; + return this; + } + + Builder noAppDesc() { + return appDesc(null); + } + + Builder setAddArgs(List v) { + addArgs.clear(); + addArgs.addAll(v); + return this; + } + + Builder setAddArgs(String... v) { + return setAddArgs(List.of(v)); + } + + Builder addArgs(List v) { + addArgs.addAll(v); + return this; + } + + Builder addArgs(String... v) { + return addArgs(List.of(v)); + } + + Builder setRemoveArgs(List v) { + removeArgs.clear(); + removeArgs.addAll(v); + return this; + } + + Builder setRemoveArgs(String... v) { + return setRemoveArgs(List.of(v)); + } + + Builder removeArgs(List v) { + removeArgs.addAll(v); + return this; + } + + Builder removeArgs(String... v) { + return removeArgs(List.of(v)); + } + + Builder setErrors(List v) { + expectedErrors = v; + return this; + } + + Builder setErrors(CannedFormattedString... v) { + return setErrors(List.of(v)); + } + + Builder errors(List v) { + expectedErrors.addAll(v); + return this; + } + + Builder errors(CannedFormattedString... v) { + return errors(List.of(v)); + } + + Builder error(String key, Object ... args) { + return errors(JPackageStringBundle.MAIN.cannedFormattedString(key, args)); + } + + Builder invalidTypeArg(String arg, String... otherArgs) { + return addArgs(arg).addArgs(otherArgs).error("ERR_InvalidTypeOption", arg, type.getType()); + } + + Builder unsupportedPlatformOption(String arg, String ... otherArgs) { + return addArgs(arg).addArgs(otherArgs).error("ERR_UnsupportedOption", arg); + } + + TestSpec create() { + return new TestSpec(Optional.ofNullable(type), Optional.ofNullable(appDesc), + List.copyOf(addArgs), List.copyOf(removeArgs), List.copyOf(expectedErrors)); + } + + private PackageType type = PackageType.IMAGE; + private String appDesc = DEFAULT_APP_DESC; + private List addArgs = new ArrayList<>(); + private List removeArgs = new ArrayList<>(); + private List expectedErrors = new ArrayList<>(); + } + + public TestSpec { + Objects.requireNonNull(type); + Objects.requireNonNull(appDesc); + Objects.requireNonNull(addArgs); + addArgs.forEach(Objects::requireNonNull); + Objects.requireNonNull(removeArgs); + removeArgs.forEach(Objects::requireNonNull); + if (expectedErrors.isEmpty()) { + throw new IllegalArgumentException("The list of expected errors must be non-empty"); + } + } + + void test() { + test(Map.of()); + } + + void test(Map> tokenValueSuppliers) { + final var cmd = appDesc.map(JPackageCommand::helloAppImage).orElseGet(JPackageCommand::new); + type.ifPresent(cmd::setPackageType); + + removeArgs.forEach(cmd::removeArgumentWithValue); + cmd.addArguments(addArgs); + + final var tokenValueSupplier = TokenReplace.createCachingTokenValueSupplier(Stream.of(Token.values()).collect(toMap(Token::token, token -> { + return () -> { + return token.expand(cmd).orElseGet(() -> { + final var tvs = Objects.requireNonNull(tokenValueSuppliers.get(token), () -> { + return String.format("No token value supplier for token [%s]", token); + }); + return tvs.apply(cmd); + }); + }; + }))); + + for (final var token : Token.values()) { + final var newArgs = cmd.getAllArguments().stream().map(arg -> { + return token.asTokenReplace().applyTo(arg, tokenValueSupplier); + }).toList(); + cmd.clearArguments().addArguments(newArgs); + } + + defaultInit(cmd, expectedErrors); + cmd.execute(1); + } + + @Override + public final String toString() { + final var sb = new StringBuilder(); + type.ifPresent(v -> { + sb.append(v).append("; "); + }); + appDesc.ifPresent(v -> { + sb.append("app-desc=").append(v).append("; "); + }); + if (!addArgs.isEmpty()) { + sb.append("args-add=").append(addArgs).append("; "); + } + if (!removeArgs.isEmpty()) { + sb.append("args-del=").append(removeArgs).append("; "); + } + sb.append("errors=").append(expectedErrors); + return sb.toString(); + } + + private final static String DEFAULT_APP_DESC = "Hello"; + } + + private static TestSpec.Builder testSpec() { + return new TestSpec.Builder(); + } + + public static Collection basic() { + final List testCases = new ArrayList<>(); + + testCases.addAll(Stream.of( // non-existent arg - {"Hello", - new String[]{"--no-such-argument"}, - null, - JPackageStringBundle.MAIN.cannedFormattedString("ERR_InvalidOption", "--no-such-argument")}, + testSpec().addArgs("--no-such-argument") + .error("ERR_InvalidOption", "--no-such-argument"), // no main jar - {"Hello", - null, - new String[]{"--main-jar"}, - JPackageStringBundle.MAIN.cannedFormattedString("ERR_NoEntryPoint")}, + testSpec().removeArgs("--main-jar").error("ERR_NoEntryPoint"), // no main-class - {"Hello", - null, - new String[]{"--main-class"}, - JPackageStringBundle.MAIN.cannedFormattedString("error.no-main-class-with-main-jar", "hello.jar"), - JPackageStringBundle.MAIN.cannedFormattedString("error.no-main-class-with-main-jar.advice", "hello.jar")}, + testSpec().removeArgs("--main-class") + .error("error.no-main-class-with-main-jar", "hello.jar") + .error("error.no-main-class-with-main-jar.advice", "hello.jar"), // non-existent main jar - {"Hello", - new String[]{"--main-jar", "non-existent.jar"}, - null, - JPackageStringBundle.MAIN.cannedFormattedString("error.main-jar-does-not-exist", "non-existent.jar")}, + testSpec().addArgs("--main-jar", "non-existent.jar") + .error("error.main-jar-does-not-exist", "non-existent.jar"), // non-existent runtime - {"Hello", - new String[]{"--runtime-image", "non-existent.runtime"}, - null, - JPackageStringBundle.MAIN.cannedFormattedString("message.runtime-image-dir-does-not-exist", "runtime-image", "non-existent.runtime")}, + testSpec().addArgs("--runtime-image", "non-existent.runtime") + .error("message.runtime-image-dir-does-not-exist", "runtime-image", "non-existent.runtime"), + // non-existent app image + testSpec().noAppDesc().nativeType().addArgs("--name", "foo", "--app-image", "non-existent.appimage") + .error("ERR_AppImageNotExist", "non-existent.appimage"), // non-existent resource-dir - {"Hello", - new String[]{"--resource-dir", "non-existent.dir"}, - null, - JPackageStringBundle.MAIN.cannedFormattedString("message.resource-dir-does-not-exist", "resource-dir", "non-existent.dir")}, + testSpec().addArgs("--resource-dir", "non-existent.dir") + .error("message.resource-dir-does-not-exist", "resource-dir", "non-existent.dir"), + // non-existent icon + testSpec().addArgs("--icon", "non-existent.icon") + .error("ERR_IconFileNotExit", cannedAbsolutePath("non-existent.icon")), + // non-existent license file + testSpec().nativeType().addArgs("--license-file", "non-existent.license") + .error("ERR_LicenseFileNotExit"), // invalid type - {"Hello", - new String[]{"--type", "invalid-type"}, - null, - JPackageStringBundle.MAIN.cannedFormattedString("ERR_InvalidInstallerType", "invalid-type")}, - // no --input - {"Hello", - null, - new String[]{"--input"}, - JPackageStringBundle.MAIN.cannedFormattedString("ERR_MissingArgument", "--input")}, + testSpec().addArgs("--type", "invalid-type") + .error("ERR_InvalidInstallerType", "invalid-type"), + // no --input for non-mular app + testSpec().removeArgs("--input").error("error.no-input-parameter"), // no --module-path - {"com.other/com.other.Hello", - null, - new String[]{"--module-path"}, - JPackageStringBundle.MAIN.cannedFormattedString("ERR_MissingArgument", "--runtime-image or --module-path")}, - }); + testSpec().appDesc("com.other/com.other.Hello").removeArgs("--module-path") + .error("ERR_MissingArgument", "--runtime-image or --module-path"), + // no main class in module path + testSpec().noAppDesc().addArgs("--module", "java.base", "--runtime-image", Token.JAVA_HOME.token()) + .error("ERR_NoMainClass"), + // no module in module path + testSpec().noAppDesc().addArgs("--module", "com.foo.bar", "--runtime-image", Token.JAVA_HOME.token()) + .error("error.no-module-in-path", "com.foo.bar"), + // --main-jar and --module-name + testSpec().noAppDesc().addArgs("--main-jar", "foo.jar", "--module", "foo.bar") + .error("ERR_BothMainJarAndModule"), + // non-existing argument file + testSpec().noAppDesc().notype().addArgs("@foo") + .error("ERR_CannotParseOptions", "foo"), + // invalid jlink option + testSpec().addArgs("--jlink-options", "--foo") + .error("error.jlink.failed", "Error: unknown option: --foo") + ).map(TestSpec.Builder::create).toList()); + + // forbidden jlink options + testCases.addAll(Stream.of("--output", "--add-modules", "--module-path").map(opt -> { + return testSpec().addArgs("--jlink-options", opt).error("error.blocked.option", opt); + }).map(TestSpec.Builder::create).toList()); + + // --runtime-image and --app-image are mutually-exclusive + testCases.addAll(createRuntimeMutuallyExclusive("--app-image", "app-image")); + // --runtime-image and --app-modules are mutually-exclusive + testCases.addAll(createRuntimeMutuallyExclusive("--add-modules", "foo.bar", "--module", "foo.bar")); + // --runtime-image and --jlink-options are mutually-exclusive + testCases.addAll(createRuntimeMutuallyExclusive("--jlink-options", "--bind-services", "--module", "foo.bar")); + + return toTestArgs(testCases.stream()); + } + + record ArgumentGroup(String arg, String... otherArgs) { + ArgumentGroup { + Objects.requireNonNull(arg); + List.of(otherArgs).forEach(Objects::requireNonNull); + } + + String[] asArray() { + return Stream.concat(Stream.of(arg), Stream.of(otherArgs)).toArray(String[]::new); + } + } + + private static List createRuntimeMutuallyExclusive(String arg, String... otherArgs) { + return createMutuallyExclusive( + new ArgumentGroup("--runtime-image", Token.JAVA_HOME.token()), + new ArgumentGroup(arg, otherArgs) + ).map(TestSpec.Builder::noAppDesc).map(TestSpec.Builder::nativeType).map(TestSpec.Builder::create).toList(); + } + + private static Stream createMutuallyExclusive(ArgumentGroup firstGroup, ArgumentGroup secondGroup) { + final Supplier createBuilder = () -> { + return testSpec().error("ERR_MutuallyExclusiveOptions", firstGroup.arg(), secondGroup.arg()); + }; + return Stream.of( + createBuilder.get().addArgs(firstGroup.asArray()).addArgs(secondGroup.asArray()), + createBuilder.get().addArgs(secondGroup.asArray()).addArgs(firstGroup.asArray())); + } + + public static Collection invalidAppVersion() { + return fromTestSpecBuilders(Stream.of( + // Invalid app version. Just cover all different error messages. + // Extensive testing of invalid version strings is done in DottedVersionTest unit test. + testSpec().addArgs("--app-version", "").error("error.version-string-empty"), + testSpec().addArgs("--app-version", "1.").error("error.version-string-zero-length-component", "1."), + testSpec().addArgs("--app-version", "1.b.3").error("error.version-string-invalid-component", "1.b.3", "b.3") + )); } @Test - @ParameterSupplier("input") - public static void test(String javaAppDesc, String[] jpackageArgs, - String[] removeArgs, CannedFormattedString... expectedErrors) { - // Init default jpackage test command line. - var cmd = JPackageCommand.helloAppImage(javaAppDesc); + @ParameterSupplier("basic") + @ParameterSupplier(value="testWindows", ifOS = WINDOWS) + @ParameterSupplier(value="testMac", ifOS = MACOS) + @ParameterSupplier(value="winOption", ifNotOS = WINDOWS) + @ParameterSupplier(value="linuxOption", ifNotOS = LINUX) + @ParameterSupplier(value="macOption", ifNotOS = MACOS) + @ParameterSupplier(value="invalidAppVersion", ifOS = {WINDOWS,MACOS}) + public static void test(TestSpec spec) { + spec.test(); + } - defaultInit(cmd, expectedErrors); + @Test + @Parameter({"--input", "foo"}) + @Parameter({"--module-path", "dir"}) + @Parameter({"--add-modules", "java.base"}) + @Parameter({"--main-class", "Hello"}) + @Parameter({"--arguments", "foo"}) + @Parameter({"--java-options", "-Dfoo.bar=10"}) + @Parameter({"--add-launcher", "foo=foo.properties"}) + @Parameter({"--app-content", "dir"}) + @Parameter(value="--win-console", ifOS = WINDOWS) + public static void testRuntimeInstallerInvalidOptions(String... args) { + testSpec().noAppDesc().nativeType().addArgs("--runtime-image", Token.JAVA_HOME.token()).addArgs(args) + .error("ERR_NoInstallerEntryPoint", args[0]).create().test(); + } - // Add arguments if requested. - Optional.ofNullable(jpackageArgs).ifPresent(cmd::addArguments); + @Test + @ParameterSupplier + public static void testAdditionLaunchers(TestSpec spec) { + final Path propsFile = TKit.createTempFile("add-launcher.properties"); + TKit.createPropertiesFile(propsFile, Map.of()); + spec.test(Map.of(Token.ADD_LAUNCHER_PROPERTY_FILE, cmd -> propsFile)); + } - // Remove arguments if requested. - Optional.ofNullable(removeArgs).map(List::of).ifPresent( - args -> args.forEach(cmd::removeArgumentWithValue)); + public static Collection testAdditionLaunchers() { + return fromTestSpecBuilders(Stream.of( + testSpec().addArgs("--add-launcher", Token.ADD_LAUNCHER_PROPERTY_FILE.token()) + .error("ERR_NoAddLauncherName"), + testSpec().removeArgs("--name").addArgs("--name", "foo", "--add-launcher", "foo=" + Token.ADD_LAUNCHER_PROPERTY_FILE.token()) + .error("ERR_NoUniqueName") + )); + } - cmd.execute(1); + @Test + @ParameterSupplier("invalidNames") + public static void testInvalidAppName(String name) { + testSpec().removeArgs("--name").addArgs("--name", name) + .error("ERR_InvalidAppName", adjustTextStreamVerifierArg(name)).create().test(); } - @Test(ifOS = WINDOWS) - public static void testWinService() { + @Test + @ParameterSupplier("invalidNames") + public static void testInvalidAddLauncherName(String name) { + testAdditionLaunchers(testSpec() + .addArgs("--add-launcher", name + "=" + Token.ADD_LAUNCHER_PROPERTY_FILE.token()) + .error("ERR_InvalidSLName", adjustTextStreamVerifierArg(name)) + .create()); + } - CannedFormattedString[] expectedErrors = new CannedFormattedString[] { - JPackageStringBundle.MAIN.cannedFormattedString("error.missing-service-installer"), - JPackageStringBundle.MAIN.cannedFormattedString("error.missing-service-installer.advice") - }; + public static Collection invalidNames() { + final List data = new ArrayList<>(); + data.addAll(List.of("", "foo/bar", "foo\tbar", "foo\rbar", "foo\nbar")); + if (TKit.isWindows()) { + data.add("foo\\bar"); + } + return toTestArgs(data.stream()); + } + + public static Collection testWindows() { + final List testCases = new ArrayList<>(); + + testCases.addAll(PackageType.WINDOWS.stream().map(type -> { + return Stream.of( + testSpec().type(type).addArgs("--launcher-as-service") + .error("error.missing-service-installer") + .error("error.missing-service-installer.advice"), + // The below version strings are invalid for msi and exe packaging. + // They are valid for app image packaging. + testSpec().type(type).addArgs("--app-version", "1234") + .error("error.msi-product-version-components", "1234") + .error("error.version-string-wrong-format.advice"), + testSpec().type(type).addArgs("--app-version", "1.2.3.4.5") + .error("error.msi-product-version-components", "1.2.3.4.5") + .error("error.version-string-wrong-format.advice"), + testSpec().type(type).addArgs("--app-version", "256.1") + .error("error.msi-product-version-major-out-of-range", "256.1") + .error("error.version-string-wrong-format.advice"), + testSpec().type(type).addArgs("--app-version", "1.256") + .error("error.msi-product-version-minor-out-of-range", "1.256") + .error("error.version-string-wrong-format.advice"), + testSpec().type(type).addArgs("--app-version", "1.2.65536") + .error("error.msi-product-version-build-out-of-range", "1.2.65536") + .error("error.version-string-wrong-format.advice") + ); + }).flatMap(x -> x).map(TestSpec.Builder::create).toList()); - new PackageTest().configureHelloApp() - .addInitializer(cmd -> { - defaultInit(cmd, expectedErrors); - cmd.addArgument("--launcher-as-service"); - }) - .setExpectedExitCode(1) - .run(RunnablePackageTest.Action.CREATE); + return toTestArgs(testCases.stream()); } - private static void defaultInit(JPackageCommand cmd, CannedFormattedString... expectedErrors) { + public static Collection testMac() { + final List testCases = new ArrayList<>(); + + testCases.addAll(Stream.of( + testSpec().addArgs("--app-version", "0.2") + .error("message.version-string-first-number-not-zero") + .error("error.invalid-cfbundle-version.advice"), + testSpec().addArgs("--app-version", "1.2.3.4") + .error("message.version-string-too-many-components") + .error("error.invalid-cfbundle-version.advice"), + testSpec().invalidTypeArg("--mac-installer-sign-identity", "foo"), + testSpec().type(PackageType.MAC_DMG).invalidTypeArg("--mac-installer-sign-identity", "foo"), + testSpec().invalidTypeArg("--mac-dmg-content", "foo"), + testSpec().type(PackageType.MAC_PKG).invalidTypeArg("--mac-dmg-content", "foo"), + testSpec().noAppDesc().addArgs("--app-image", Token.APP_IMAGE.token()) + .error("error.app-image.mac-sign.required"), + testSpec().type(PackageType.MAC_PKG).addArgs("--mac-package-identifier", "#1") + .error("message.invalid-identifier", "#1"), + // Bundle for mac app store should not have runtime commands + testSpec().nativeType().addArgs("--mac-app-store", "--jlink-options", "--bind-services") + .error("ERR_MissingJLinkOptMacAppStore", "--strip-native-commands"), + testSpec().nativeType().addArgs("--mac-app-store", "--runtime-image", Token.JAVA_HOME.token()) + .error("ERR_MacAppStoreRuntimeBinExists", JPackageCommand.cannedArgument(cmd -> { + return Path.of(cmd.getArgumentValue("--runtime-image")).toAbsolutePath(); + }, Token.JAVA_HOME.token())) + ).map(TestSpec.Builder::create).toList()); + + // Test a few app-image options that should not be used when signing external app image + testCases.addAll(Stream.of( + new ArgumentGroup("--app-version", "2.0"), + new ArgumentGroup("--name", "foo"), + new ArgumentGroup("--mac-app-store") + ).map(argGroup -> { + return testSpec().noAppDesc().addArgs(argGroup.asArray()).addArgs("--app-image", Token.APP_IMAGE.token()) + .error("ERR_InvalidOptionWithAppImageSigning", argGroup.arg()); + }).mapMulti((builder, acc) -> { + // It should bail out with the same error message regardless of `--mac-sign` option. + acc.accept(builder.create()); + acc.accept(builder.addArgs("--mac-sign").create()); + }).toList()); + + testCases.addAll(createMutuallyExclusive( + new ArgumentGroup("--mac-signing-key-user-name", "foo"), + new ArgumentGroup("--mac-app-image-sign-identity", "bar") + ).map(TestSpec.Builder::create).toList()); + + testCases.addAll(createMutuallyExclusive( + new ArgumentGroup("--mac-signing-key-user-name", "foo"), + new ArgumentGroup("--mac-installer-sign-identity", "bar") + ).map(TestSpec.Builder::nativeType).map(TestSpec.Builder::create).toList()); + + return toTestArgs(testCases.stream()); + } + + private record UnsupportedPlatformOption(String name, Optional value) { + UnsupportedPlatformOption { + Objects.requireNonNull(name); + Objects.requireNonNull(value); + } + + UnsupportedPlatformOption(String name) { + this(name, Optional.empty()); + } + + UnsupportedPlatformOption(String name, String value) { + this(name, Optional.of(value)); + } + + TestSpec toTestSpec() { + return value.map(v -> testSpec().unsupportedPlatformOption(name, v)).orElseGet( + () -> testSpec().unsupportedPlatformOption(name)).create(); + } + + static Collection createTestArgs(UnsupportedPlatformOption... options) { + return toTestArgs(Stream.of(options).map(UnsupportedPlatformOption::toTestSpec)); + } + } + + public static Collection winOption() { + return UnsupportedPlatformOption.createTestArgs( + new UnsupportedPlatformOption("--win-console"), + new UnsupportedPlatformOption("--win-dir-chooser"), + new UnsupportedPlatformOption("--win-help-url", "url"), + new UnsupportedPlatformOption("--win-menu"), + new UnsupportedPlatformOption("--win-menu-group", "name"), + new UnsupportedPlatformOption("--win-per-user-install"), + new UnsupportedPlatformOption("--win-shortcut"), + new UnsupportedPlatformOption("--win-shortcut-prompt"), + new UnsupportedPlatformOption("--win-update-url", "url"), + new UnsupportedPlatformOption("--win-upgrade-uuid", "uuid") + ); + } + + public static Collection linuxOption() { + return UnsupportedPlatformOption.createTestArgs( + new UnsupportedPlatformOption("--linux-package-name", "name"), + new UnsupportedPlatformOption("--linux-deb-maintainer", "email-address"), + new UnsupportedPlatformOption("--linux-menu-group", "menu-group-name"), + new UnsupportedPlatformOption("--linux-package-deps", "deps"), + new UnsupportedPlatformOption("--linux-rpm-license-type", "type"), + new UnsupportedPlatformOption("--linux-app-release", "release"), + new UnsupportedPlatformOption("--linux-app-category", "category-value"), + new UnsupportedPlatformOption("--linux-shortcut") + ); + } + + public static Collection macOption() { + return UnsupportedPlatformOption.createTestArgs( + new UnsupportedPlatformOption("--mac-package-identifier", "identifier"), + new UnsupportedPlatformOption("--mac-package-name", "name"), + new UnsupportedPlatformOption("--mac-package-signing-prefix", "prefix"), + new UnsupportedPlatformOption("--mac-sign"), + new UnsupportedPlatformOption("--mac-signing-keychain", "keychain-name"), + new UnsupportedPlatformOption("--mac-signing-key-user-name", "name"), + new UnsupportedPlatformOption("--mac-app-store"), + new UnsupportedPlatformOption("--mac-entitlements", "path"), + new UnsupportedPlatformOption("--mac-app-category", "category"), + new UnsupportedPlatformOption("--mac-dmg-content", "additional-content") + ); + } + + private static void defaultInit(JPackageCommand cmd, List expectedErrors) { // Disable default logic adding `--verbose` option // to jpackage command line. @@ -158,11 +636,36 @@ private static void defaultInit(JPackageCommand cmd, CannedFormattedString... ex // with jpackage arguments in this test. cmd.ignoreDefaultRuntime(true); - // Configure jpackage output verifier to look up the list of provided - // errors in the order they are specified. - cmd.validateOutput(Stream.of(expectedErrors) - .map(CannedFormattedString::getValue) - .map(TKit::assertTextStream) - .reduce(TKit.TextStreamVerifier::andThen).get()); + cmd.validateOutput(expectedErrors.toArray(CannedFormattedString[]::new)); + } + + private static PackageType defaultNativeType() { + if (TKit.isLinux()) { + return PackageType.LINUX.stream().filter(PackageType::isSupported).findFirst().orElseThrow(); + } else if (TKit.isOSX()) { + return PackageType.MAC_DMG; + } else if (TKit.isWindows()) { + return PackageType.WIN_MSI; + } else { + throw new UnsupportedOperationException(); + } + } + + private static Collection toTestArgs(Stream stream) { + return stream.map(v -> { + return new Object[] {v}; + }).toList(); } + + private static Collection fromTestSpecBuilders(Stream stream) { + return toTestArgs(stream.map(TestSpec.Builder::create)); + } + + private static String adjustTextStreamVerifierArg(String str) { + return LINE_SEP_REGEXP.split(str)[0]; + } + + private static final Pattern LINE_SEP_REGEXP = Pattern.compile("\\R"); + + private static final PackageType NATIVE_TYPE = defaultNativeType(); } From 2706c9052d2de5711a10970795244132ded8e582 Mon Sep 17 00:00:00 2001 From: Alexey Semenyuk Date: Thu, 27 Feb 2025 19:47:08 -0500 Subject: [PATCH 23/46] Better testing of DottedVersion: verify exception error messages --- .../jdk/jpackage/internal/DottedVersion.java | 30 ++++++-- .../jpackage/internal/DottedVersionTest.java | 68 ++++++++++++++----- 2 files changed, 73 insertions(+), 25 deletions(-) diff --git a/src/jdk.jpackage/share/classes/jdk/jpackage/internal/DottedVersion.java b/src/jdk.jpackage/share/classes/jdk/jpackage/internal/DottedVersion.java index 91c0a4915fd43..2f1f3af6d7881 100644 --- a/src/jdk.jpackage/share/classes/jdk/jpackage/internal/DottedVersion.java +++ b/src/jdk.jpackage/share/classes/jdk/jpackage/internal/DottedVersion.java @@ -57,9 +57,7 @@ private DottedVersion(String version, boolean greedy) { if (!greedy) { return null; } else { - throw new IllegalArgumentException(MessageFormat.format(I18N. - getString("error.version-string-zero-length-component"), - version)); + ds.throwException(); } } @@ -77,8 +75,7 @@ private DottedVersion(String version, boolean greedy) { }).takeWhile(Objects::nonNull).toArray(BigInteger[]::new); suffix = ds.getUnprocessedString(); if (!suffix.isEmpty() && greedy) { - throw new IllegalArgumentException(MessageFormat.format(I18N.getString( - "error.version-string-invalid-component"), version, suffix)); + ds.throwException(); } } } @@ -89,7 +86,7 @@ private static class DigitsSupplier { this.input = input; } - public String getNextDigits() { + String getNextDigits() { if (stoped) { return null; } @@ -130,10 +127,29 @@ public String getNextDigits() { return sb.toString(); } - public String getUnprocessedString() { + String getUnprocessedString() { return input.substring(cursor); } + void throwException() { + final String tail; + if (lastDotPos >= 0) { + tail = input.substring(lastDotPos + 1); + } else { + tail = getUnprocessedString(); + } + + final String errMessage; + if (tail.isEmpty()) { + errMessage = MessageFormat.format(I18N.getString( + "error.version-string-zero-length-component"), input); + } else { + errMessage = MessageFormat.format(I18N.getString( + "error.version-string-invalid-component"), input, tail); + } + throw new IllegalArgumentException(errMessage); + } + private int cursor; private int lastDotPos = -1; private boolean stoped; diff --git a/test/jdk/tools/jpackage/junit/share/jdk.jpackage/jdk/jpackage/internal/DottedVersionTest.java b/test/jdk/tools/jpackage/junit/share/jdk.jpackage/jdk/jpackage/internal/DottedVersionTest.java index c9c2b0185567e..03e6ce1390d58 100644 --- a/test/jdk/tools/jpackage/junit/share/jdk.jpackage/jdk/jpackage/internal/DottedVersionTest.java +++ b/test/jdk/tools/jpackage/junit/share/jdk.jpackage/jdk/jpackage/internal/DottedVersionTest.java @@ -22,8 +22,10 @@ */ package jdk.jpackage.internal; +import java.text.MessageFormat; import java.util.ArrayList; import java.util.List; +import java.util.Objects; import java.util.function.Function; import java.util.stream.Stream; import static org.junit.jupiter.api.Assertions.assertEquals; @@ -112,28 +114,57 @@ private static List testValid() { return data; } + record InvalidVersionTestSpec(String version, String invalidComponent) { + public InvalidVersionTestSpec { + Objects.requireNonNull(version); + Objects.requireNonNull(invalidComponent); + } + + InvalidVersionTestSpec(String version) { + this(version, ""); + } + + void run() { + final String expectedErrorMsg; + if (invalidComponent.isEmpty()) { + expectedErrorMsg = MessageFormat.format(I18N.getString("error.version-string-zero-length-component"), version); + } else { + expectedErrorMsg = MessageFormat.format(I18N.getString("error.version-string-invalid-component"), version, invalidComponent); + } + + final var ex = assertThrowsExactly(IllegalArgumentException.class, () -> new DottedVersion(version)); + + assertEquals(expectedErrorMsg, ex.getMessage()); + } + } + @ParameterizedTest @MethodSource - public void testInvalid(String str) { - assertThrowsExactly(IllegalArgumentException.class, () -> new DottedVersion(str)); + public void testInvalid(InvalidVersionTestSpec testSpec) { + testSpec.run(); } - private static Stream testInvalid() { + private static Stream testInvalid() { return Stream.of( - "1.-1", - "5.", - "4.2.", - "3..2", - "2.a", - "0a", - ".", - " ", - " 1", - "1. 2", - "+1", - "-1", - "-0", - "+0" + new InvalidVersionTestSpec("1.-1", "-1"), + new InvalidVersionTestSpec("5."), + new InvalidVersionTestSpec("4.2."), + new InvalidVersionTestSpec("3..2", ".2"), + new InvalidVersionTestSpec("3...2", "..2"), + new InvalidVersionTestSpec("2.a", "a"), + new InvalidVersionTestSpec("0a", "a"), + new InvalidVersionTestSpec("1.0a", "0a"), + new InvalidVersionTestSpec(".", "."), + new InvalidVersionTestSpec("..", ".."), + new InvalidVersionTestSpec(".a.b", ".a.b"), + new InvalidVersionTestSpec(".1.2", ".1.2"), + new InvalidVersionTestSpec(" ", " "), + new InvalidVersionTestSpec(" 1", " 1"), + new InvalidVersionTestSpec("1. 2", " 2"), + new InvalidVersionTestSpec("+1", "+1"), + new InvalidVersionTestSpec("-1", "-1"), + new InvalidVersionTestSpec("-0", "-0"), + new InvalidVersionTestSpec("+0", "+0") ); } @@ -145,7 +176,8 @@ public void testNull(Type type) { @Test public void testEmptyGreedy() { - assertThrowsExactly(IllegalArgumentException.class, () -> DottedVersion.greedy(""), "Version may not be empty string"); + final var ex = assertThrowsExactly(IllegalArgumentException.class, () -> DottedVersion.greedy("")); + assertEquals(I18N.getString("error.version-string-empty"), ex.getMessage()); } @Test From 5197f614dd761d73f5cab4c3b521058d751fdee4 Mon Sep 17 00:00:00 2001 From: Alexey Semenyuk Date: Fri, 28 Feb 2025 09:07:50 -0500 Subject: [PATCH 24/46] Better test coverage for bad app image --- .../jpackage/share/AppImagePackageTest.java | 70 ++++++++++++------- 1 file changed, 44 insertions(+), 26 deletions(-) diff --git a/test/jdk/tools/jpackage/share/AppImagePackageTest.java b/test/jdk/tools/jpackage/share/AppImagePackageTest.java index fc545cc1f5519..34a418c6f9ec9 100644 --- a/test/jdk/tools/jpackage/share/AppImagePackageTest.java +++ b/test/jdk/tools/jpackage/share/AppImagePackageTest.java @@ -25,10 +25,13 @@ import java.nio.file.Files; import java.io.IOException; import java.util.List; +import jdk.jpackage.internal.util.XmlUtils; import jdk.jpackage.test.AppImageFile; import jdk.jpackage.test.Annotations.Parameter; +import jdk.jpackage.test.CannedFormattedString; import jdk.jpackage.test.TKit; import jdk.jpackage.test.JPackageCommand; +import jdk.jpackage.test.JPackageStringBundle; import jdk.jpackage.test.PackageTest; import jdk.jpackage.test.RunnablePackageTest.Action; import jdk.jpackage.test.Annotations.Test; @@ -106,17 +109,16 @@ public static void testEmpty(boolean withIcon) throws IOException { public static void testBadAppImage() throws IOException { Path appImageDir = TKit.createTempDirectory("appimage"); Files.createFile(appImageDir.resolve("foo")); - configureAppImageWithoutJPackageXMLFile(appImageDir).addInitializer( - cmd -> { - cmd.removeArgumentWithValue("--name"); - }).run(Action.CREATE); + configureBadAppImage(appImageDir).addInitializer(cmd -> { + cmd.removeArgumentWithValue("--name"); + }).run(Action.CREATE); } @Test public static void testBadAppImage2() throws IOException { Path appImageDir = TKit.createTempDirectory("appimage"); Files.createFile(appImageDir.resolve("foo")); - configureAppImageWithoutJPackageXMLFile(appImageDir).run(Action.CREATE); + configureBadAppImage(appImageDir).run(Action.CREATE); } @Test @@ -126,29 +128,45 @@ public static void testBadAppImage3() throws IOException { JPackageCommand appImageCmd = JPackageCommand.helloAppImage(). setFakeRuntime().setArgumentValue("--dest", appImageDir); - configureAppImageWithoutJPackageXMLFile(appImageCmd.outputBundle()). - addRunOnceInitializer(() -> { - appImageCmd.execute(); - Files.delete(AppImageFile.getPathInAppImage(appImageCmd. - outputBundle())); - }).run(Action.CREATE); + configureBadAppImage(appImageCmd.outputBundle()).addRunOnceInitializer(() -> { + appImageCmd.execute(); + Files.delete(AppImageFile.getPathInAppImage(appImageCmd.outputBundle())); + }).run(Action.CREATE); } - private static PackageTest configureAppImageWithoutJPackageXMLFile( - Path appImageDir) { - return new PackageTest() - .addInitializer(cmd -> { - cmd.saveConsoleOutput(true); - cmd.addArguments("--app-image", appImageDir); - cmd.removeArgumentWithValue("--input"); - cmd.ignoreDefaultVerbose(true); // no "--verbose" option - }) - .addBundleVerifier((cmd, result) -> { - TKit.assertTextStream( - "Error: Missing .jpackage.xml file in app-image dir").apply( - result.getOutput().stream()); - }) - .setExpectedExitCode(1); + @Test + public static void testBadAppImageFile() throws IOException { + final var appImageRoot = TKit.createTempDirectory("appimage"); + + final var appImageCmd = JPackageCommand.helloAppImage(). + setFakeRuntime().setArgumentValue("--dest", appImageRoot); + + final var appImageDir = appImageCmd.outputBundle(); + + final var expectedError = JPackageStringBundle.MAIN.cannedFormattedString( + "error.invalid-app-image", appImageDir, AppImageFile.getPathInAppImage(appImageDir)); + + configureBadAppImage(appImageDir, expectedError).addRunOnceInitializer(() -> { + appImageCmd.execute(); + XmlUtils.createXml(AppImageFile.getPathInAppImage(appImageDir), xml -> { + xml.writeStartElement("jpackage-state"); + xml.writeEndElement(); + }); + }).run(Action.CREATE); + } + + private static PackageTest configureBadAppImage(Path appImageDir) { + return configureBadAppImage(appImageDir, + JPackageStringBundle.MAIN.cannedFormattedString("error.foreign-app-image", appImageDir)); + } + + private static PackageTest configureBadAppImage(Path appImageDir, CannedFormattedString expectedError) { + return new PackageTest().addInitializer(cmd -> { + cmd.addArguments("--app-image", appImageDir); + cmd.removeArgumentWithValue("--input"); + cmd.ignoreDefaultVerbose(true); // no "--verbose" option + cmd.validateOutput(expectedError); + }).setExpectedExitCode(1); } private static Path iconPath(String name) { From a4288bb36ff53552595bb85857e617bb7ec451df Mon Sep 17 00:00:00 2001 From: Alexey Semenyuk Date: Sat, 1 Mar 2025 22:13:51 -0500 Subject: [PATCH 25/46] Test error reported by jpackage when `--temp` arg points to existing non-empty directory --- test/jdk/tools/jpackage/share/BasicTest.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/test/jdk/tools/jpackage/share/BasicTest.java b/test/jdk/tools/jpackage/share/BasicTest.java index 94e0a335d18ae..fe988533a28bd 100644 --- a/test/jdk/tools/jpackage/share/BasicTest.java +++ b/test/jdk/tools/jpackage/share/BasicTest.java @@ -36,6 +36,7 @@ import java.util.stream.Stream; import jdk.jpackage.test.TKit; import jdk.jpackage.test.JPackageCommand; +import jdk.jpackage.test.JPackageStringBundle; import jdk.jpackage.test.JavaAppDesc; import jdk.jpackage.test.PackageTest; import jdk.jpackage.test.HelloApp; @@ -381,7 +382,10 @@ public void testTemp(TestTempType type) throws IOException { ); if (TestTempType.TEMPDIR_NOT_EMPTY.equals(type)) { - pkgTest.setExpectedExitCode(1).addBundleVerifier(cmd -> { + pkgTest.setExpectedExitCode(1).addInitializer(cmd -> { + cmd.validateOutput(JPackageStringBundle.MAIN.cannedFormattedString( + "ERR_BuildRootInvalid", cmd.getArgumentValue("--temp"))); + }).addBundleVerifier(cmd -> { // Check jpackage didn't use the supplied directory. Path tempDir = Path.of(cmd.getArgumentValue("--temp")); TKit.assertDirectoryContent(tempDir).match(Path.of("foo.txt")); From fefdf4d75d7682040a56be8170ffc05726416c30 Mon Sep 17 00:00:00 2001 From: Alexey Semenyuk Date: Mon, 3 Mar 2025 15:22:11 -0500 Subject: [PATCH 26/46] Remove MacAppStoreRuntimeTest.java. The positive test duplicates other tests covering `--runtime-image` option and the negative test duplicates a test in ErrorTest (the one covering ERR_MacAppStoreRuntimeBinExists) --- .../macosx/MacAppStoreRuntimeTest.java | 100 ------------------ 1 file changed, 100 deletions(-) delete mode 100644 test/jdk/tools/jpackage/macosx/MacAppStoreRuntimeTest.java diff --git a/test/jdk/tools/jpackage/macosx/MacAppStoreRuntimeTest.java b/test/jdk/tools/jpackage/macosx/MacAppStoreRuntimeTest.java deleted file mode 100644 index ab8b8590b33de..0000000000000 --- a/test/jdk/tools/jpackage/macosx/MacAppStoreRuntimeTest.java +++ /dev/null @@ -1,100 +0,0 @@ -/* - * Copyright (c) 2022, 2024, Oracle and/or its affiliates. All rights reserved. - * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. - * - * This code is free software; you can redistribute it and/or modify it - * under the terms of the GNU General Public License version 2 only, as - * published by the Free Software Foundation. - * - * This code is distributed in the hope that it will be useful, but WITHOUT - * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or - * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License - * version 2 for more details (a copy is included in the LICENSE file that - * accompanied this code). - * - * You should have received a copy of the GNU General Public License version - * 2 along with this work; if not, write to the Free Software Foundation, - * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. - * - * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA - * or visit www.oracle.com if you need additional information or have any - * questions. - */ - -import java.nio.file.Files; -import java.nio.file.Path; -import java.io.IOException; -import java.util.ArrayList; -import java.util.List; - -import jdk.jpackage.test.JPackageCommand; -import jdk.jpackage.test.Executor; -import jdk.jpackage.test.TKit; -import jdk.jpackage.test.JavaTool; -import jdk.jpackage.test.Annotations.Test; -import jdk.jpackage.test.Annotations.Parameter; - - -/** - * Tests generation of app image with --mac-app-store and --runtime-image. jpackage should able - * to generate app image if runtime image does not have "bin" folder and fail otherwise. - */ - -/* - * @test - * @summary jpackage with --mac-app-store and --runtime-image - * @library /test/jdk/tools/jpackage/helpers - * @build jdk.jpackage.test.* - * @build MacAppStoreRuntimeTest - * @requires (os.family == "mac") - * @run main/othervm -Xmx512m jdk.jpackage.test.Main - * --jpt-run=MacAppStoreRuntimeTest - */ -public class MacAppStoreRuntimeTest { - - private static String getRuntimeImage(boolean stripNativeCommands) throws IOException { - final Path workDir = TKit.createTempDirectory("runtime").resolve("data"); - final Path jlinkOutputDir = workDir.resolve("temp.runtime"); - Files.createDirectories(jlinkOutputDir.getParent()); - - // List of modules required for test app. - final var modules = new String[] { - "java.base", - "java.desktop" - }; - - List jlinkArgs = new ArrayList<>(); - jlinkArgs.add("--output"); - jlinkArgs.add(jlinkOutputDir.toString()); - jlinkArgs.add("--add-modules"); - jlinkArgs.add(String.join(",", modules)); - jlinkArgs.add("--strip-debug"); - jlinkArgs.add("--no-header-files"); - jlinkArgs.add("--no-man-pages"); - if (stripNativeCommands) { - jlinkArgs.add("--strip-native-commands"); - } - - new Executor() - .setToolProvider(JavaTool.JLINK) - .dumpOutput() - .addArguments(jlinkArgs) - .execute(); - - return jlinkOutputDir.toString(); - } - - @Test - @Parameter("true") - @Parameter("false") - public static void test(boolean stripNativeCommands) throws Exception { - JPackageCommand cmd = JPackageCommand.helloAppImage(); - cmd.addArguments("--mac-app-store", "--runtime-image", getRuntimeImage(stripNativeCommands)); - - if (stripNativeCommands) { - cmd.executeAndAssertHelloAppImageCreated(); - } else { - cmd.execute(1); - } - } -} From d85d67fdbc27acf483e9981ee0465332c9c2fdd9 Mon Sep 17 00:00:00 2001 From: Alexey Semenyuk Date: Mon, 3 Mar 2025 18:06:20 -0500 Subject: [PATCH 27/46] Remove redundant PredefinedAppImageErrorTest.java. It duplicates ErrorTest test cases: '--mac-sign option is required' -> 'error.app-image.mac-sign.required'; 'Option [--mac-app-store] is not valid' -> 'ERR_InvalidOptionWithAppImageSigning'. --- .../share/PredefinedAppImageErrorTest.java | 118 ------------------ 1 file changed, 118 deletions(-) delete mode 100644 test/jdk/tools/jpackage/share/PredefinedAppImageErrorTest.java diff --git a/test/jdk/tools/jpackage/share/PredefinedAppImageErrorTest.java b/test/jdk/tools/jpackage/share/PredefinedAppImageErrorTest.java deleted file mode 100644 index a415f0370ccec..0000000000000 --- a/test/jdk/tools/jpackage/share/PredefinedAppImageErrorTest.java +++ /dev/null @@ -1,118 +0,0 @@ -/* - * Copyright (c) 2022, 2025, Oracle and/or its affiliates. All rights reserved. - * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. - * - * This code is free software; you can redistribute it and/or modify it - * under the terms of the GNU General Public License version 2 only, as - * published by the Free Software Foundation. - * - * This code is distributed in the hope that it will be useful, but WITHOUT - * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or - * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License - * version 2 for more details (a copy is included in the LICENSE file that - * accompanied this code). - * - * You should have received a copy of the GNU General Public License version - * 2 along with this work; if not, write to the Free Software Foundation, - * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. - * - * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA - * or visit www.oracle.com if you need additional information or have any - * questions. - */ - - -import java.io.IOException; -import java.nio.file.Path; -import java.nio.file.Files; -import java.util.Collection; -import java.util.List; - -import jdk.jpackage.test.AppImageFile; -import jdk.jpackage.test.Annotations.Parameters; -import jdk.jpackage.test.Annotations.Test; -import jdk.jpackage.test.JPackageCommand; -import jdk.jpackage.test.TKit; - -/* - * @test - * @summary Test jpackage output for erroneous input with --type "app-image" and --app-image - * @library /test/jdk/tools/jpackage/helpers - * @build jdk.jpackage.test.* - * @compile -Xlint:all -Werror PredefinedAppImageErrorTest.java - * - * @run main/othervm/timeout=360 -Xmx512m jdk.jpackage.test.Main - * --jpt-run=PredefinedAppImageErrorTest - * --jpt-before-run=jdk.jpackage.test.JPackageCommand.useExecutableByDefault - * - * @run main/othervm/timeout=360 -Xmx512m jdk.jpackage.test.Main - * --jpt-run=PredefinedAppImageErrorTest - * --jpt-before-run=jdk.jpackage.test.JPackageCommand.useToolProviderByDefault - */ - -public final class PredefinedAppImageErrorTest { - - private final String expectedError; - private final JPackageCommand cmd; - - @Parameters - public static Collection input() throws IOException { - return List.of(new Object[][]{ - // --mac-sign is required - {"Hello", - null, - new String[]{"--input", "--dest", "--name", "--main-jar", "--main-class"}, - TKit.isOSX() ? - "--mac-sign option is required" : - "Option [--app-image] is not valid with type [app-image]" - }, - // --mac-app-store is required - {"Hello", - new String[]{"--mac-sign", "--mac-app-store", "--mac-app-image-sign-identity", "test"}, - new String[]{"--input", "--dest", "--name", "--main-jar", "--main-class"}, - TKit.isOSX() ? - "Option [--mac-app-store] is not valid" : - "Option [--mac-sign] is not valid on this platform" - }, - }); - } - - public PredefinedAppImageErrorTest(String javaAppDesc, String[] jpackageArgs, - String[] removeArgs, - String expectedError) { - this.expectedError = expectedError; - - cmd = JPackageCommand.helloAppImage(javaAppDesc) - .saveConsoleOutput(true).dumpOutput(true); - if (jpackageArgs != null) { - cmd.addArguments(jpackageArgs); - } - if (removeArgs != null) { - for (String arg : removeArgs) { - cmd.removeArgumentWithValue(arg); - } - } - } - - @Test - public void test() throws IOException { - getDummyAppImage(cmd); - - List output = cmd.execute(1).getOutput(); - TKit.assertNotNull(output, "output is null"); - TKit.assertTextStream(expectedError).apply(output.stream()); - } - - private void getDummyAppImage(JPackageCommand cmd) throws IOException { - Path dummyAppFolder - = TKit.createTempDirectory("DummyAppImage").toAbsolutePath(); - - Path dummyAppFile - = dummyAppFolder.resolve("DummyAppFile").toAbsolutePath(); - Files.createFile(dummyAppFile); - - cmd.addArguments("--app-image", dummyAppFolder.toString()); - new AppImageFile("PredefinedAppImageErrorTest", "Hello").save(dummyAppFolder); - } - -} From 915a15c5c08b01e284ea60d41763fc70e69714cd Mon Sep 17 00:00:00 2001 From: Alexey Semenyuk Date: Mon, 3 Mar 2025 18:08:23 -0500 Subject: [PATCH 28/46] Remove redundant MacAppStoreJlinkOptionsTest.testWithoutStripNativeCommands(). It duplicates `ERR_MissingJLinkOptMacAppStore` test case in ErrorTest --- .../macosx/MacAppStoreJlinkOptionsTest.java | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/test/jdk/tools/jpackage/macosx/MacAppStoreJlinkOptionsTest.java b/test/jdk/tools/jpackage/macosx/MacAppStoreJlinkOptionsTest.java index 1501fd15d8fdf..313e003033e05 100644 --- a/test/jdk/tools/jpackage/macosx/MacAppStoreJlinkOptionsTest.java +++ b/test/jdk/tools/jpackage/macosx/MacAppStoreJlinkOptionsTest.java @@ -25,9 +25,7 @@ import jdk.jpackage.test.Annotations.Test; /** - * Tests generation of app image with --mac-app-store and --jlink-options. jpackage should able - * to generate app image if "--strip-native-commands" is specified for --jlink-options and should - * fail if it is not specified. + * Tests generation of app image with --mac-app-store and --jlink-options. */ /* @@ -50,13 +48,4 @@ public static void testWithStripNativeCommands() throws Exception { cmd.executeAndAssertHelloAppImageCreated(); } - - @Test - public static void testWithoutStripNativeCommands() throws Exception { - JPackageCommand cmd = JPackageCommand.helloAppImage(); - cmd.addArguments("--mac-app-store", "--jlink-options", - "--strip-debug --no-man-pages --no-header-files"); - - cmd.execute(1); - } } From 2aa3de51017a6b38d5f101ac336f40eb489452d3 Mon Sep 17 00:00:00 2001 From: Alexey Semenyuk Date: Thu, 6 Mar 2025 11:26:44 -0500 Subject: [PATCH 29/46] Better negative testing in MainClassTest --- .../tools/jpackage/share/MainClassTest.java | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/test/jdk/tools/jpackage/share/MainClassTest.java b/test/jdk/tools/jpackage/share/MainClassTest.java index 7ac72c2c87b41..15d4e0999c58f 100644 --- a/test/jdk/tools/jpackage/share/MainClassTest.java +++ b/test/jdk/tools/jpackage/share/MainClassTest.java @@ -39,10 +39,13 @@ import jdk.jpackage.internal.util.function.ThrowingConsumer; import jdk.jpackage.test.Annotations.Parameters; import jdk.jpackage.test.Annotations.Test; +import jdk.jpackage.test.CannedFormattedString; +import jdk.jpackage.test.JPackageStringBundle; import jdk.jpackage.test.CfgFile; import jdk.jpackage.test.Executor; import jdk.jpackage.test.HelloApp; import jdk.jpackage.test.JPackageCommand; +import static jdk.jpackage.test.JPackageCommand.cannedArgument; import jdk.jpackage.test.JavaAppDesc; import jdk.jpackage.test.JavaTool; import jdk.jpackage.test.TKit; @@ -87,8 +90,8 @@ Script withJarMainClass(MainClassType v) { return this; } - Script expectedErrorMessage(String v) { - expectedErrorMessage = v; + Script expectedErrorMessage(String key, Object... args) { + expectedErrorMessage = JPackageStringBundle.MAIN.cannedFormattedString(key, args); return this; } @@ -131,7 +134,7 @@ public String toString() { private boolean withJLink; private MainClassType mainClass; private MainClassType jarMainClass; - private String expectedErrorMessage; + private CannedFormattedString expectedErrorMessage; } public MainClassTest(Script script) { @@ -194,11 +197,12 @@ public static Collection scripts() { if (withMainClass.contains(jarMainClass) || withMainClass.contains(mainClass)) { } else if (modular) { - script.expectedErrorMessage( - "Error: Main application class is missing"); + script.expectedErrorMessage("ERR_NoMainClass"); } else { script.expectedErrorMessage( - "A main class was not specified nor was one found in the jar"); + "error.no-main-class-with-main-jar", cannedArgument(cmd -> { + return cmd.getArgumentValue("--main-jar"); + }, "MAIN-JAR")); } scripts.add(new Script[]{script}); @@ -218,11 +222,7 @@ public void test() throws IOException { if (script.expectedErrorMessage != null) { // This is the case when main class is not found nor in jar // file nor on command line. - List output = cmd - .saveConsoleOutput(true) - .execute(1) - .getOutput(); - TKit.assertTextStream(script.expectedErrorMessage).apply(output.stream()); + cmd.validateOutput(script.expectedErrorMessage).execute(1); return; } From 0c32b276015f1f7767ae051ad0e25c47fd0f4870 Mon Sep 17 00:00:00 2001 From: Alexey Semenyuk Date: Thu, 6 Mar 2025 11:47:18 -0500 Subject: [PATCH 30/46] NonExistentTest is redundant. It duplicates functionality of ErrorTest and is less strict on analyzing error output --- .../tools/jpackage/share/NonExistentTest.java | 91 ------------------- 1 file changed, 91 deletions(-) delete mode 100644 test/jdk/tools/jpackage/share/NonExistentTest.java diff --git a/test/jdk/tools/jpackage/share/NonExistentTest.java b/test/jdk/tools/jpackage/share/NonExistentTest.java deleted file mode 100644 index 3c87d3c6411c7..0000000000000 --- a/test/jdk/tools/jpackage/share/NonExistentTest.java +++ /dev/null @@ -1,91 +0,0 @@ -/* - * Copyright (c) 2020, 2025, Oracle and/or its affiliates. All rights reserved. - * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. - * - * This code is free software; you can redistribute it and/or modify it - * under the terms of the GNU General Public License version 2 only, as - * published by the Free Software Foundation. - * - * This code is distributed in the hope that it will be useful, but WITHOUT - * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or - * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License - * version 2 for more details (a copy is included in the LICENSE file that - * accompanied this code). - * - * You should have received a copy of the GNU General Public License version - * 2 along with this work; if not, write to the Free Software Foundation, - * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. - * - * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA - * or visit www.oracle.com if you need additional information or have any - * questions. - */ - - -import java.util.Collection; -import java.util.List; -import jdk.jpackage.test.Annotations.Parameters; -import jdk.jpackage.test.Annotations.Test; -import jdk.jpackage.test.JPackageCommand; -import jdk.jpackage.test.TKit; - -/* - * @test - * @summary jpackage application version testing - * @library /test/jdk/tools/jpackage/helpers - * @build jdk.jpackage.test.* - * @compile -Xlint:all -Werror NonExistentTest.java - * @run main/othervm/timeout=360 -Xmx512m jdk.jpackage.test.Main - * --jpt-run=NonExistentTest - */ - -public final class NonExistentTest { - - private final String expectedError; - private final JPackageCommand cmd; - - @Parameters - public static Collection input() { - return List.of(new Object[][]{ - // non-existent icon - {"Hello", - new String[]{"--icon", "non-existent"}, - "Error:"}, - {"com.other/com.other.Hello", - new String[]{"--icon", "non-existent"}, - "Error:"}, - // non-existent input - {"Hello", - new String[]{"--input", "non-existent"}, - "Exception:"}, - {"com.other/com.other.Hello", - new String[]{"--input", "non-existent"}, - "Exception:"}, - // non-existent resource-dir - {"Hello", - new String[]{"--resource-dir", "non-existent"}, - "Specified resource directory"}, - {"com.other/com.other.Hello", - new String[]{"--resource-dir", "non-existent"}, - "Specified resource directory"}, - }); - } - - public NonExistentTest(String javaAppDesc, String[] jpackageArgs, - String expectedError) { - this.expectedError = expectedError; - - cmd = JPackageCommand.helloAppImage(javaAppDesc) - .saveConsoleOutput(true).dumpOutput(true); - if (jpackageArgs != null) { - cmd.addArguments(jpackageArgs); - } - } - - @Test - public void test() { - List output = cmd.execute(1).getOutput(); - TKit.assertNotNull(output, "output is null"); - TKit.assertTextStream(expectedError).apply(output.stream()); - } -} From 810aac39107b926951c1183533302f1e9386d472 Mon Sep 17 00:00:00 2001 From: Alexey Semenyuk Date: Thu, 6 Mar 2025 11:48:37 -0500 Subject: [PATCH 31/46] ModulePathTest: use CannedFormattedString to validate error output --- .../tools/jpackage/share/ModulePathTest.java | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/test/jdk/tools/jpackage/share/ModulePathTest.java b/test/jdk/tools/jpackage/share/ModulePathTest.java index 9fd07a95c230a..19dd2dbfb5186 100644 --- a/test/jdk/tools/jpackage/share/ModulePathTest.java +++ b/test/jdk/tools/jpackage/share/ModulePathTest.java @@ -30,10 +30,12 @@ import java.util.function.Function; import java.util.stream.Collectors; import java.util.stream.Stream; +import jdk.jpackage.test.CannedFormattedString; import jdk.jpackage.test.TKit; import jdk.jpackage.test.JavaAppDesc; import jdk.jpackage.test.HelloApp; import jdk.jpackage.test.JPackageCommand; +import jdk.jpackage.test.JPackageStringBundle; import jdk.jpackage.test.PackageType; import jdk.jpackage.test.Annotations.Parameter; import jdk.jpackage.test.Annotations.Parameters; @@ -121,19 +123,16 @@ public void test(String javaAppDesc) throws IOException { if (withGoodPath) { cmd.executeAndAssertHelloAppImageCreated(); } else { - final String expectedErrorMessage; + final CannedFormattedString expectedErrorMessage; if (modulePathArgs.isEmpty()) { - expectedErrorMessage = "Error: Missing argument: --runtime-image or --module-path"; + expectedErrorMessage = JPackageStringBundle.MAIN.cannedFormattedString( + "ERR_MissingArgument", "--runtime-image or --module-path"); } else { - expectedErrorMessage = String.format( - "Failed to find %s module in module path", appDesc.moduleName()); + expectedErrorMessage = JPackageStringBundle.MAIN.cannedFormattedString( + "error.no-module-in-path", appDesc.moduleName()); } - List output = cmd - .saveConsoleOutput(true) - .execute(1) - .getOutput(); - TKit.assertTextStream(expectedErrorMessage).apply(output.stream()); + cmd.validateOutput(expectedErrorMessage).execute(1); } } From 30d42dc046c0536d5425fa3a573905895effbc6d Mon Sep 17 00:00:00 2001 From: Alexey Semenyuk Date: Thu, 6 Mar 2025 12:19:39 -0500 Subject: [PATCH 32/46] FileAssociationsTest: use CannedFormattedString --- .../jpackage/share/FileAssociationsTest.java | 44 +++++++++---------- 1 file changed, 21 insertions(+), 23 deletions(-) diff --git a/test/jdk/tools/jpackage/share/FileAssociationsTest.java b/test/jdk/tools/jpackage/share/FileAssociationsTest.java index 08578ab802744..2257c5dbc6510 100644 --- a/test/jdk/tools/jpackage/share/FileAssociationsTest.java +++ b/test/jdk/tools/jpackage/share/FileAssociationsTest.java @@ -21,12 +21,14 @@ * questions. */ +import static jdk.jpackage.test.JPackageStringBundle.MAIN; + import java.nio.file.Path; import java.util.Map; - import jdk.jpackage.test.Annotations.Parameter; import jdk.jpackage.test.Annotations.Test; import jdk.jpackage.test.FileAssociations; +import jdk.jpackage.test.JPackageCommand; import jdk.jpackage.test.PackageTest; import jdk.jpackage.test.PackageType; import jdk.jpackage.test.TKit; @@ -111,22 +113,16 @@ public static void test(boolean includeDescription) { public static void testNoMime() { final Path propFile = TKit.workDir().resolve("fa.properties"); - PackageTest packageTest = new PackageTest().excludeTypes(PackageType.MAC); - - packageTest.configureHelloApp().addRunOnceInitializer(() -> { + initPackageTest().addRunOnceInitializer(() -> { TKit.createPropertiesFile(propFile, Map.of( "extension", "foo", "description", "bar" )); }).addInitializer(cmd -> { - cmd.addArguments("--file-associations", propFile).saveConsoleOutput(true); - }).setExpectedExitCode(1).addBundleVerifier((cmd, result) -> { - TKit.assertTextStream( - "No MIME types were specified for File Association number 1") - .apply(result.getOutput().stream()); - TKit.assertTextStream( - "Advice to fix: Specify MIME type for File Association number 1") - .apply(result.getOutput().stream()); + cmd.addArguments("--file-associations", propFile); + cmd.validateOutput( + MAIN.cannedFormattedString("error.no-content-types-for-file-association", 1), + MAIN.cannedFormattedString("error.no-content-types-for-file-association.advice", 1)); }).run(); } @@ -134,23 +130,25 @@ public static void testNoMime() { public static void testTooManyMimes() { final Path propFile = TKit.workDir().resolve("fa.properties"); - PackageTest packageTest = new PackageTest().excludeTypes(PackageType.MAC); - - packageTest.configureHelloApp().addRunOnceInitializer(() -> { + initPackageTest().addRunOnceInitializer(() -> { TKit.createPropertiesFile(propFile, Map.of( "mime-type", "application/x-jpackage-foo, application/x-jpackage-bar", "extension", "foo", "description", "bar" )); }).addInitializer(cmd -> { - cmd.addArguments("--file-associations", propFile).saveConsoleOutput(true); - }).setExpectedExitCode(1).addBundleVerifier((cmd, result) -> { - TKit.assertTextStream( - "More than one MIME types was specified for File Association number 1") - .apply(result.getOutput().stream()); - TKit.assertTextStream( - "Advice to fix: Specify only one MIME type for File Association number 1") - .apply(result.getOutput().stream()); + cmd.addArguments("--file-associations", propFile); + cmd.validateOutput( + MAIN.cannedFormattedString("error.too-many-content-types-for-file-association", 1), + MAIN.cannedFormattedString("error.too-many-content-types-for-file-association.advice", 1)); }).run(); } + + private static PackageTest initPackageTest() { + return new PackageTest() + .excludeTypes(PackageType.MAC) + .configureHelloApp() + .addInitializer(JPackageCommand::setFakeRuntime) + .setExpectedExitCode(1); + } } From 3810b5762b23fd7a6342ce02688e26ba803160f4 Mon Sep 17 00:00:00 2001 From: Alexey Semenyuk Date: Thu, 6 Mar 2025 15:37:53 -0500 Subject: [PATCH 33/46] Make JavaOptionsEqualsTest and JavaOptionsTest work when external fake runtime is used --- test/jdk/tools/jpackage/share/JavaOptionsEqualsTest.java | 6 +----- test/jdk/tools/jpackage/share/JavaOptionsTest.java | 3 +-- 2 files changed, 2 insertions(+), 7 deletions(-) diff --git a/test/jdk/tools/jpackage/share/JavaOptionsEqualsTest.java b/test/jdk/tools/jpackage/share/JavaOptionsEqualsTest.java index a4c2a26944a5a..71f4b1229f3eb 100644 --- a/test/jdk/tools/jpackage/share/JavaOptionsEqualsTest.java +++ b/test/jdk/tools/jpackage/share/JavaOptionsEqualsTest.java @@ -75,17 +75,13 @@ public static Collection input() { } public JavaOptionsEqualsTest(String javaAppDesc, String[] jpackageArgs) { - cmd = JPackageCommand.helloAppImage(javaAppDesc); - if (jpackageArgs != null) { - cmd.addArguments(jpackageArgs); - } + cmd = JPackageCommand.helloAppImage(javaAppDesc).addArguments(jpackageArgs).ignoreFakeRuntime(); } @Test public void test() { cmd.executeAndAssertHelloAppImageCreated(); List output = HelloApp.executeLauncher(cmd).getOutput(); - TKit.assertNotNull(output, "output is null"); TKit.assertTextStream(WARNING1).apply(output.stream()); TKit.assertTextStream(WARNING2).apply(output.stream()); } diff --git a/test/jdk/tools/jpackage/share/JavaOptionsTest.java b/test/jdk/tools/jpackage/share/JavaOptionsTest.java index e15dc02eac6e6..014a9434cad5e 100644 --- a/test/jdk/tools/jpackage/share/JavaOptionsTest.java +++ b/test/jdk/tools/jpackage/share/JavaOptionsTest.java @@ -76,7 +76,7 @@ public static Collection input() { public JavaOptionsTest(String javaAppDesc, String[] jpackageArgs, String[] expectedParams) { - cmd = JPackageCommand.helloAppImage(javaAppDesc); + cmd = JPackageCommand.helloAppImage(javaAppDesc).ignoreFakeRuntime(); if (jpackageArgs != null) { cmd.addArguments(jpackageArgs); } @@ -90,7 +90,6 @@ public void test() { // 2.) run the launcher it generated List output = HelloApp.executeLauncher(cmd).getOutput(); - TKit.assertNotNull(output, "output is null"); for (String expect : expected) { TKit.assertTextStream(expect).apply(output.stream()); } From e853e56e6a7b58fcce48091a512a573407e559bb Mon Sep 17 00:00:00 2001 From: Alexey Semenyuk Date: Thu, 6 Mar 2025 15:40:18 -0500 Subject: [PATCH 34/46] LinuxResourceTest: use CannedFormattedString --- .../jpackage/linux/LinuxResourceTest.java | 156 ++++++++++++------ 1 file changed, 106 insertions(+), 50 deletions(-) diff --git a/test/jdk/tools/jpackage/linux/LinuxResourceTest.java b/test/jdk/tools/jpackage/linux/LinuxResourceTest.java index 1dc8dee3c9768..85d2176339112 100644 --- a/test/jdk/tools/jpackage/linux/LinuxResourceTest.java +++ b/test/jdk/tools/jpackage/linux/LinuxResourceTest.java @@ -21,14 +21,19 @@ * questions. */ +import static jdk.jpackage.test.JPackageStringBundle.MAIN; + import java.io.IOException; import java.nio.file.Path; -import jdk.jpackage.test.TKit; +import java.util.List; +import java.util.Objects; +import jdk.jpackage.test.Annotations.Test; +import jdk.jpackage.test.JPackageCommand; +import jdk.jpackage.test.LinuxHelper; import jdk.jpackage.test.PackageTest; import jdk.jpackage.test.PackageType; -import jdk.jpackage.test.LinuxHelper; -import jdk.jpackage.test.Annotations.Test; -import java.util.List; +import jdk.jpackage.test.RunnablePackageTest.Action; +import jdk.jpackage.test.TKit; /* * @test @@ -55,48 +60,47 @@ public static void testHardcodedProperties() throws IOException { }) .forTypes(PackageType.LINUX_DEB) .addInitializer(cmd -> { - Path controlFile = Path.of(cmd.getArgumentValue("--resource-dir"), - "control"); + Path controlFile = Path.of(cmd.getArgumentValue("--resource-dir"), "control"); + + final var packageProp = property("Package", "dont-install-me"); + final var verProp = property("Version", "1.2.3-R2"); + final var arhProp = property("Architecture", "bar"); + TKit.createTextFile(controlFile, List.of( - "Package: dont-install-me", - "Version: 1.2.3-R2", + packageProp.format(), + verProp.format(), "Section: APPLICATION_SECTION", "Maintainer: APPLICATION_MAINTAINER", "Priority: optional", - "Architecture: bar", + arhProp.format(), "Provides: dont-install-me", "Description: APPLICATION_DESCRIPTION", "Installed-Size: APPLICATION_INSTALLED_SIZE", "Depends: PACKAGE_DEFAULT_DEPENDENCIES" )); - }) - .addBundleVerifier((cmd, result) -> { - TKit.assertTextStream("Using custom package resource [DEB control file]") - .predicate(String::contains) - .apply(result.getOutput().stream()); - TKit.assertTextStream(String.format( - "Expected value of \"Package\" property is [%s]. Actual value in output package is [dont-install-me]", - LinuxHelper.getPackageName(cmd))) - .predicate(String::contains) - .apply(result.getOutput().stream()); - TKit.assertTextStream( - "Expected value of \"Version\" property is [1.0]. Actual value in output package is [1.2.3-R2]") - .predicate(String::contains) - .apply(result.getOutput().stream()); - TKit.assertTextStream(String.format( - "Expected value of \"Architecture\" property is [%s]. Actual value in output package is [bar]", - LinuxHelper.getDefaultPackageArch(cmd.packageType()))) - .predicate(String::contains) - .apply(result.getOutput().stream()); + + cmd.validateOutput(MAIN.cannedFormattedString( + "message.using-custom-resource", + String.format("[%s]", MAIN.cannedFormattedString("resource.deb-control-file").getValue()), + controlFile.getFileName())); + + packageProp.expectedValue(LinuxHelper.getPackageName(cmd)).token("APPLICATION_PACKAGE").resourceDirFile(controlFile).validateOutput(cmd); + verProp.expectedValue(cmd.version()).token("APPLICATION_VERSION_WITH_RELEASE").resourceDirFile(controlFile).validateOutput(cmd); + arhProp.expectedValue(LinuxHelper.getDefaultPackageArch(cmd.packageType())).token("APPLICATION_ARCH").resourceDirFile(controlFile).validateOutput(cmd); }) .forTypes(PackageType.LINUX_RPM) .addInitializer(cmd -> { Path specFile = Path.of(cmd.getArgumentValue("--resource-dir"), LinuxHelper.getPackageName(cmd) + ".spec"); + + final var packageProp = property("Name", "dont-install-me"); + final var verProp = property("Version", "1.2.3"); + final var releaseProp = property("Release", "R2"); + TKit.createTextFile(specFile, List.of( - "Name: dont-install-me", - "Version: 1.2.3", - "Release: R2", + packageProp.format(), + verProp.format(), + releaseProp.format(), "Summary: APPLICATION_SUMMARY", "License: APPLICATION_LICENSE_TYPE", "Prefix: %{dirname:APPLICATION_DIRECTORY}", @@ -113,25 +117,77 @@ public static void testHardcodedProperties() throws IOException { "%files", "APPLICATION_DIRECTORY" )); + + cmd.validateOutput(MAIN.cannedFormattedString( + "message.using-custom-resource", + String.format("[%s]", MAIN.cannedFormattedString("resource.rpm-spec-file").getValue()), + specFile.getFileName())); + + packageProp.expectedValue(LinuxHelper.getPackageName(cmd)).token("APPLICATION_PACKAGE").resourceDirFile(specFile).validateOutput(cmd); + verProp.expectedValue(cmd.version()).token("APPLICATION_VERSION").resourceDirFile(specFile).validateOutput(cmd); + releaseProp.expectedValue("1").token("APPLICATION_RELEASE").resourceDirFile(specFile).validateOutput(cmd); }) - .addBundleVerifier((cmd, result) -> { - TKit.assertTextStream("Using custom package resource [RPM spec file]") - .predicate(String::contains) - .apply(result.getOutput().stream()); - TKit.assertTextStream(String.format( - "Expected value of \"Name\" property is [%s]. Actual value in output package is [dont-install-me]", - LinuxHelper.getPackageName(cmd))) - .predicate(String::contains) - .apply(result.getOutput().stream()); - TKit.assertTextStream( - "Expected value of \"Version\" property is [1.0]. Actual value in output package is [1.2.3]") - .predicate(String::contains) - .apply(result.getOutput().stream()); - TKit.assertTextStream( - "Expected value of \"Release\" property is [1]. Actual value in output package is [R2]") - .predicate(String::contains) - .apply(result.getOutput().stream()); - }) - .run(); + .run(Action.CREATE); + } + + private final static class PropertyValidator { + + PropertyValidator name(String v) { + name = v; + return this; + } + + PropertyValidator customValue(String v) { + customValue = v; + return this; + } + + PropertyValidator expectedValue(String v) { + expectedValue = v; + return this; + } + + PropertyValidator token(String v) { + token = v; + return this; + } + + PropertyValidator resourceDirFile(Path v) { + resourceDirFile = v; + return this; + } + + String format() { + Objects.requireNonNull(name); + Objects.requireNonNull(customValue); + return String.format("%s: %s", name, customValue); + } + + void validateOutput(JPackageCommand cmd) { + Objects.requireNonNull(name); + Objects.requireNonNull(customValue); + Objects.requireNonNull(expectedValue); + Objects.requireNonNull(token); + Objects.requireNonNull(resourceDirFile); + + final var customResourcePath = customResourcePath(); + cmd.validateOutput( + MAIN.cannedFormattedString("error.unexpected-package-property", name, expectedValue, customValue, customResourcePath), + MAIN.cannedFormattedString("error.unexpected-package-property.advice", token, customValue, name, customResourcePath)); + } + + private Path customResourcePath() { + return resourceDirFile.getFileName(); + } + + private String name; + private String customValue; + private String expectedValue; + private String token; + private Path resourceDirFile; + } + + private static PropertyValidator property(String name, String customValue) { + return new PropertyValidator().name(name).customValue(customValue); } } From cc03a14ba24d0480785722a546ceadc82e6f84e8 Mon Sep 17 00:00:00 2001 From: Alexey Semenyuk Date: Thu, 6 Mar 2025 17:09:58 -0500 Subject: [PATCH 35/46] Enhance RuntimePackageTest to test that jpackage can derive package name from the runtime image directory name if `--name` option is not given --- .../jpackage/share/RuntimePackageTest.java | 22 ++++++++++++++----- 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/test/jdk/tools/jpackage/share/RuntimePackageTest.java b/test/jdk/tools/jpackage/share/RuntimePackageTest.java index 7b0e88a5ae71b..9eb4f8d231e55 100644 --- a/test/jdk/tools/jpackage/share/RuntimePackageTest.java +++ b/test/jdk/tools/jpackage/share/RuntimePackageTest.java @@ -29,10 +29,12 @@ import java.util.function.Predicate; import java.util.stream.Collectors; import jdk.jpackage.test.PackageType; +import jdk.jpackage.test.RunnablePackageTest.Action; import jdk.jpackage.test.PackageTest; import jdk.jpackage.test.JPackageCommand; import jdk.jpackage.test.TKit; import jdk.jpackage.test.Annotations.Test; +import jdk.jpackage.test.Annotations.Parameter; import jdk.jpackage.test.Executor; import jdk.jpackage.test.JavaTool; import jdk.jpackage.test.LinuxHelper; @@ -83,17 +85,24 @@ public static void test() { } @Test - public static void testUsrInstallDir() { + @Parameter("/usr") + @Parameter("/usr/lib/Java") + public static void testUsrInstallDir(String installDir) { init(PackageType.LINUX) .addInitializer(cmd -> cmd.addArguments("--install-dir", "/usr")) .run(); } @Test - public static void testUsrInstallDir2() { - init(PackageType.LINUX) - .addInitializer(cmd -> cmd.addArguments("--install-dir", "/usr/lib/Java")) - .run(); + public static void testName() { + // Test that jpackage can derive package name from the path to runtime image. + init(PackageType.NATIVE) + .addInitializer(cmd -> cmd.removeArgumentWithValue("--name")) + // Don't attempt to install this package as it may have an odd name derived from + // the runtime image path. Say, on Linux for `--runtime-image foo/bar/sed` + // command line jpackage will create a package named 'sed' that will conflict + // with the default 'sed' package. + .run(Action.CREATE_AND_UNPACK); } private static PackageTest init(Set types) { @@ -157,7 +166,8 @@ private static PackageTest init(Set types) { "Check the package doesn't deliver [%s] copyright file", copyright)); } - }); + }) + .forTypes(types); } private static Set listFiles(Path root) throws IOException { From 4835cc5ed0e9cb16f6f437ef5f389f4ec8c15535 Mon Sep 17 00:00:00 2001 From: Alexey Semenyuk Date: Fri, 28 Feb 2025 13:19:27 -0500 Subject: [PATCH 36/46] Remove negative tests from AppVersionTest. ErrorTest tests invalid app versions better, it verifies error messages in jpackage output, AppVersionTest doesn't. --- .../tools/jpackage/share/AppVersionTest.java | 25 ------------------- 1 file changed, 25 deletions(-) diff --git a/test/jdk/tools/jpackage/share/AppVersionTest.java b/test/jdk/tools/jpackage/share/AppVersionTest.java index e81b1eae9cfa9..c9e66cbd7a19f 100644 --- a/test/jdk/tools/jpackage/share/AppVersionTest.java +++ b/test/jdk/tools/jpackage/share/AppVersionTest.java @@ -31,7 +31,6 @@ import jdk.jpackage.test.Annotations.Parameters; import jdk.jpackage.test.Annotations.Test; import jdk.jpackage.test.JPackageCommand; -import jdk.jpackage.test.PackageTest; import jdk.jpackage.test.TKit; /* @@ -70,19 +69,6 @@ public static Collection input() { "--app-version", "7.5.81"}} })); - // These are invalid version strings. - // Don't need to test all invalid input as this is handled in - // PlatformVersionTest unit test - if (TKit.isWindows()) { - data.addAll(List.of(new Object[][]{ - {null, "Hello", new String[]{"--app-version", "256"}} - })); - } else if (TKit.isOSX()) { - data.addAll(List.of(new Object[][]{ - {null, "Hello", new String[]{"--app-version", "0.2"}} - })); - } - return data; } @@ -95,17 +81,6 @@ public AppVersionTest(String expectedVersion, String javaAppDesc, @Test public void test() throws XPathExpressionException, IOException { - if (expectedVersion == null) { - new PackageTest() - .setExpectedExitCode(1) - .configureHelloApp(javaAppDesc) - .addInitializer(cmd -> { - cmd.addArguments(jpackageArgs); - }) - .run(); - return; - } - JPackageCommand cmd = JPackageCommand.helloAppImage(javaAppDesc); if (jpackageArgs != null) { cmd.addArguments(jpackageArgs); From 656a8a3aecb42bafbefa395d9becbfe141762edd Mon Sep 17 00:00:00 2001 From: Alexey Semenyuk Date: Fri, 28 Feb 2025 17:23:59 -0500 Subject: [PATCH 37/46] CommandLine.parse() doesn't throw `java.io.FileNotFoundException` when command file is no found. Instead it throws `java.nio.file.NoSuchFileException`. --- .../share/classes/jdk/jpackage/main/Main.java | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/src/jdk.jpackage/share/classes/jdk/jpackage/main/Main.java b/src/jdk.jpackage/share/classes/jdk/jpackage/main/Main.java index 97b1faee249ac..db4d8676fca3d 100644 --- a/src/jdk.jpackage/share/classes/jdk/jpackage/main/Main.java +++ b/src/jdk.jpackage/share/classes/jdk/jpackage/main/Main.java @@ -25,15 +25,16 @@ package jdk.jpackage.main; -import jdk.internal.opt.CommandLine; -import jdk.jpackage.internal.Arguments; -import jdk.jpackage.internal.Log; -import jdk.jpackage.internal.CLIHelp; -import java.io.PrintWriter; -import java.util.ResourceBundle; import java.io.FileNotFoundException; import java.io.IOException; +import java.io.PrintWriter; +import java.nio.file.NoSuchFileException; import java.text.MessageFormat; +import java.util.ResourceBundle; +import jdk.internal.opt.CommandLine; +import jdk.jpackage.internal.Arguments; +import jdk.jpackage.internal.CLIHelp; +import jdk.jpackage.internal.Log; public class Main { @@ -69,7 +70,7 @@ public int execute(PrintWriter out, PrintWriter err, String... args) { String[] newArgs; try { newArgs = CommandLine.parse(args); - } catch (FileNotFoundException fnfe) { + } catch (FileNotFoundException|NoSuchFileException fnfe) { Log.fatalError(MessageFormat.format(I18N.getString( "ERR_CannotParseOptions"), fnfe.getMessage())); return 1; From f810e23ca6a5306626b1b265d72c5869b08df441 Mon Sep 17 00:00:00 2001 From: Alexey Semenyuk Date: Thu, 27 Feb 2025 19:46:27 -0500 Subject: [PATCH 38/46] DeployParams: replace {"ERR_MissingArgument", "--input"} with "error.no-input-parameter" as the error is not missing argument of `--input` parameter, but missing `--input` parameter itself. --- .../share/classes/jdk/jpackage/internal/DeployParams.java | 3 +-- .../share/classes/jdk/jpackage/internal/LauncherData.java | 4 ---- 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/src/jdk.jpackage/share/classes/jdk/jpackage/internal/DeployParams.java b/src/jdk.jpackage/share/classes/jdk/jpackage/internal/DeployParams.java index 18645f17a6dfd..ceb4f684de094 100644 --- a/src/jdk.jpackage/share/classes/jdk/jpackage/internal/DeployParams.java +++ b/src/jdk.jpackage/share/classes/jdk/jpackage/internal/DeployParams.java @@ -166,8 +166,7 @@ public void validate() throws PackagerException { } } else { if (!hasInput && !hasAppImage) { - throw new PackagerException( - "ERR_MissingArgument", "--input"); + throw new PackagerException("error.no-input-parameter"); } } } else { diff --git a/src/jdk.jpackage/share/classes/jdk/jpackage/internal/LauncherData.java b/src/jdk.jpackage/share/classes/jdk/jpackage/internal/LauncherData.java index ce6813de28b1e..3f5390f7d8b56 100644 --- a/src/jdk.jpackage/share/classes/jdk/jpackage/internal/LauncherData.java +++ b/src/jdk.jpackage/share/classes/jdk/jpackage/internal/LauncherData.java @@ -185,10 +185,6 @@ private static LauncherData createNonModular( } Path mainJarDir = StandardBundlerParam.SOURCE_DIR.fetchFrom(params); - if (mainJarDir == null && launcherData.qualifiedClassName == null) { - throw new ConfigException(I18N.getString("error.no-input-parameter"), - null); - } final Path mainJarPath; if (launcherData.mainJarName != null && mainJarDir != null) { From 8e2db234b6580df14b94b29fea5de04d5b29f7d8 Mon Sep 17 00:00:00 2001 From: Alexey Semenyuk Date: Fri, 7 Mar 2025 00:20:16 -0500 Subject: [PATCH 39/46] Update copyright year --- .../share/classes/jdk/jpackage/internal/DeployParams.java | 2 +- .../share/classes/jdk/jpackage/internal/DottedVersion.java | 2 +- .../share/classes/jdk/jpackage/internal/LauncherData.java | 2 +- src/jdk.jpackage/share/classes/jdk/jpackage/main/Main.java | 2 +- .../helpers/jdk/jpackage/test/CannedFormattedString.java | 2 +- .../helpers/jdk/jpackage/test/JPackageStringBundle.java | 2 +- test/jdk/tools/jpackage/helpers/jdk/jpackage/test/Main.java | 2 +- .../tools/jpackage/helpers/jdk/jpackage/test/WindowsHelper.java | 2 +- .../jdk.jpackage/jdk/jpackage/internal/DottedVersionTest.java | 2 +- test/jdk/tools/jpackage/macosx/MacAppStoreJlinkOptionsTest.java | 2 +- 10 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/jdk.jpackage/share/classes/jdk/jpackage/internal/DeployParams.java b/src/jdk.jpackage/share/classes/jdk/jpackage/internal/DeployParams.java index ceb4f684de094..c95edbbad0a01 100644 --- a/src/jdk.jpackage/share/classes/jdk/jpackage/internal/DeployParams.java +++ b/src/jdk.jpackage/share/classes/jdk/jpackage/internal/DeployParams.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2011, 2024, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2011, 2025, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it diff --git a/src/jdk.jpackage/share/classes/jdk/jpackage/internal/DottedVersion.java b/src/jdk.jpackage/share/classes/jdk/jpackage/internal/DottedVersion.java index 2f1f3af6d7881..b188a9b552c94 100644 --- a/src/jdk.jpackage/share/classes/jdk/jpackage/internal/DottedVersion.java +++ b/src/jdk.jpackage/share/classes/jdk/jpackage/internal/DottedVersion.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2019, 2024, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2019, 2025, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it diff --git a/src/jdk.jpackage/share/classes/jdk/jpackage/internal/LauncherData.java b/src/jdk.jpackage/share/classes/jdk/jpackage/internal/LauncherData.java index 3f5390f7d8b56..c5a74ac09cc42 100644 --- a/src/jdk.jpackage/share/classes/jdk/jpackage/internal/LauncherData.java +++ b/src/jdk.jpackage/share/classes/jdk/jpackage/internal/LauncherData.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2020, 2023, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2020, 2025, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it diff --git a/src/jdk.jpackage/share/classes/jdk/jpackage/main/Main.java b/src/jdk.jpackage/share/classes/jdk/jpackage/main/Main.java index db4d8676fca3d..9ead69890cca5 100644 --- a/src/jdk.jpackage/share/classes/jdk/jpackage/main/Main.java +++ b/src/jdk.jpackage/share/classes/jdk/jpackage/main/Main.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2011, 2022, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2011, 2025, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it diff --git a/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/CannedFormattedString.java b/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/CannedFormattedString.java index b44cd80fe89b6..940dd47e06d20 100644 --- a/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/CannedFormattedString.java +++ b/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/CannedFormattedString.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2024, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2024, 2025, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it diff --git a/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/JPackageStringBundle.java b/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/JPackageStringBundle.java index b9c8826b5af15..cdf2855faef4e 100644 --- a/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/JPackageStringBundle.java +++ b/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/JPackageStringBundle.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2024, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2024, 2025, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it diff --git a/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/Main.java b/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/Main.java index 8e16eb04232e7..e7f06b0d60852 100644 --- a/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/Main.java +++ b/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/Main.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2019, 2024, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2019, 2025, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it diff --git a/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/WindowsHelper.java b/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/WindowsHelper.java index 68f4508c4134f..9f8c1725a1364 100644 --- a/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/WindowsHelper.java +++ b/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/WindowsHelper.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2019, 2024, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2019, 2025, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it diff --git a/test/jdk/tools/jpackage/junit/share/jdk.jpackage/jdk/jpackage/internal/DottedVersionTest.java b/test/jdk/tools/jpackage/junit/share/jdk.jpackage/jdk/jpackage/internal/DottedVersionTest.java index 03e6ce1390d58..8256782418972 100644 --- a/test/jdk/tools/jpackage/junit/share/jdk.jpackage/jdk/jpackage/internal/DottedVersionTest.java +++ b/test/jdk/tools/jpackage/junit/share/jdk.jpackage/jdk/jpackage/internal/DottedVersionTest.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2019, 2024, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2019, 2025, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it diff --git a/test/jdk/tools/jpackage/macosx/MacAppStoreJlinkOptionsTest.java b/test/jdk/tools/jpackage/macosx/MacAppStoreJlinkOptionsTest.java index 313e003033e05..0f7534aa88d33 100644 --- a/test/jdk/tools/jpackage/macosx/MacAppStoreJlinkOptionsTest.java +++ b/test/jdk/tools/jpackage/macosx/MacAppStoreJlinkOptionsTest.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2022, 2024, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2022, 2025, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it From dc5974fc054df21fad58174df08774648f781bc0 Mon Sep 17 00:00:00 2001 From: Alexey Semenyuk Date: Fri, 7 Mar 2025 00:23:47 -0500 Subject: [PATCH 40/46] Ran ./bin/blessed-modifier-order.sh --- .../jdk/jpackage/test/PackageTestTest.java | 14 +++++++------- .../helpers/jdk/jpackage/test/PackageTest.java | 2 +- .../helpers/jdk/jpackage/test/TestInstance.java | 2 +- .../helpers/jdk/jpackage/test/WindowsHelper.java | 2 +- .../jpackage/internal/util/TokenReplaceTest.java | 2 +- .../tools/jpackage/linux/LinuxResourceTest.java | 2 +- test/jdk/tools/jpackage/share/ErrorTest.java | 2 +- .../jpackage/share/JavaOptionsEqualsTest.java | 8 ++++---- test/jdk/tools/jpackage/share/ModulePathTest.java | 6 +++--- 9 files changed, 20 insertions(+), 20 deletions(-) diff --git a/test/jdk/tools/jpackage/helpers-test/jdk/jpackage/test/PackageTestTest.java b/test/jdk/tools/jpackage/helpers-test/jdk/jpackage/test/PackageTestTest.java index c582be5ac6263..3e14022a70e2e 100644 --- a/test/jdk/tools/jpackage/helpers-test/jdk/jpackage/test/PackageTestTest.java +++ b/test/jdk/tools/jpackage/helpers-test/jdk/jpackage/test/PackageTestTest.java @@ -97,12 +97,12 @@ CountingBundleVerifier createBundleVerifier(int jpackageExitCode) { private final int tickCount; } - private final static int ERROR_EXIT_CODE_JPACKAGE = 35; - private final static int ERROR_EXIT_CODE_INSTALL = 27; + private static final int ERROR_EXIT_CODE_JPACKAGE = 35; + private static final int ERROR_EXIT_CODE_INSTALL = 27; - private final static CallbackFactory NEVER = new CallbackFactory(0); - private final static CallbackFactory ONCE = new CallbackFactory(1); - private final static CallbackFactory TWICE = new CallbackFactory(2); + private static final CallbackFactory NEVER = new CallbackFactory(0); + private static final CallbackFactory ONCE = new CallbackFactory(1); + private static final CallbackFactory TWICE = new CallbackFactory(2); enum BundleVerifier { ONCE_SUCCESS(ONCE), @@ -238,7 +238,7 @@ public String toString() { private final int expectedJPackageExitCode; } - private final static class CountingInstaller extends TickCounter implements Function { + private static final class CountingInstaller extends TickCounter implements Function { @Override public Integer apply(JPackageCommand cmd) { @@ -439,7 +439,7 @@ private List run(Optional> customConfigure) { } } - private final static class TestSpecBuilder { + private static final class TestSpecBuilder { TestSpecBuilder type(PackageType v) { type = Objects.requireNonNull(v); diff --git a/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/PackageTest.java b/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/PackageTest.java index f9c6e4da507eb..5500b670fbcb9 100644 --- a/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/PackageTest.java +++ b/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/PackageTest.java @@ -618,7 +618,7 @@ private boolean unpackNotSupported() { return processed(Action.UNPACK) && packageHandlers.unpackHandler().isEmpty(); } - private final static class State { + private static final class State { private final Set packageActions = new HashSet<>(); private final List deleteUnpackDirs = new ArrayList<>(); } diff --git a/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/TestInstance.java b/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/TestInstance.java index 25167a93f1b5e..159a46c96c29c 100644 --- a/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/TestInstance.java +++ b/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/TestInstance.java @@ -137,7 +137,7 @@ private static String formatArgs(List values) { private List methodArgs; private Method method; - private final static HexFormat ARGS_CHAR_FORMATTER = HexFormat.of().withUpperCase(); + private static final HexFormat ARGS_CHAR_FORMATTER = HexFormat.of().withUpperCase(); } static TestDesc create(Method m, Object... args) { diff --git a/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/WindowsHelper.java b/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/WindowsHelper.java index 9f8c1725a1364..bc67f6e417de5 100644 --- a/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/WindowsHelper.java +++ b/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/WindowsHelper.java @@ -662,7 +662,7 @@ Path getPath() { private final RegValuePath reg; private final Optional alt; - private final static Map CACHE = new ConcurrentHashMap<>(); + private static final Map CACHE = new ConcurrentHashMap<>(); } private static final class ShortPathUtils { diff --git a/test/jdk/tools/jpackage/junit/share/jdk.jpackage/jdk/jpackage/internal/util/TokenReplaceTest.java b/test/jdk/tools/jpackage/junit/share/jdk.jpackage/jdk/jpackage/internal/util/TokenReplaceTest.java index 3ead7c3d00835..01d1d10ef9d5b 100644 --- a/test/jdk/tools/jpackage/junit/share/jdk.jpackage/jdk/jpackage/internal/util/TokenReplaceTest.java +++ b/test/jdk/tools/jpackage/junit/share/jdk.jpackage/jdk/jpackage/internal/util/TokenReplaceTest.java @@ -181,7 +181,7 @@ public static Stream test() { ).map(TestSpec.Builder::create); } - private final static class CountingSupplier implements Supplier { + private static final class CountingSupplier implements Supplier { CountingSupplier(Object value, int expectedCount) { this.value = value; diff --git a/test/jdk/tools/jpackage/linux/LinuxResourceTest.java b/test/jdk/tools/jpackage/linux/LinuxResourceTest.java index 85d2176339112..8371dc01e4370 100644 --- a/test/jdk/tools/jpackage/linux/LinuxResourceTest.java +++ b/test/jdk/tools/jpackage/linux/LinuxResourceTest.java @@ -130,7 +130,7 @@ public static void testHardcodedProperties() throws IOException { .run(Action.CREATE); } - private final static class PropertyValidator { + private static final class PropertyValidator { PropertyValidator name(String v) { name = v; diff --git a/test/jdk/tools/jpackage/share/ErrorTest.java b/test/jdk/tools/jpackage/share/ErrorTest.java index 0483dc9e78d94..e1a2e19998b7b 100644 --- a/test/jdk/tools/jpackage/share/ErrorTest.java +++ b/test/jdk/tools/jpackage/share/ErrorTest.java @@ -289,7 +289,7 @@ public final String toString() { return sb.toString(); } - private final static String DEFAULT_APP_DESC = "Hello"; + private static final String DEFAULT_APP_DESC = "Hello"; } private static TestSpec.Builder testSpec() { diff --git a/test/jdk/tools/jpackage/share/JavaOptionsEqualsTest.java b/test/jdk/tools/jpackage/share/JavaOptionsEqualsTest.java index 71f4b1229f3eb..220c55b5e50f8 100644 --- a/test/jdk/tools/jpackage/share/JavaOptionsEqualsTest.java +++ b/test/jdk/tools/jpackage/share/JavaOptionsEqualsTest.java @@ -54,13 +54,13 @@ public class JavaOptionsEqualsTest { - private final static String OPTION1 = + private static final String OPTION1 = "--add-exports=java.base/sun.util=me.mymodule.foo,ALL-UNNAMED"; - private final static String OPTION2 = + private static final String OPTION2 = "--add-exports=java.base/sun.security.util=other.mod.bar,ALL-UNNAMED"; - private final static String WARNING1 = + private static final String WARNING1 = "WARNING: Unknown module: me.mymodule.foo"; - private final static String WARNING2 = + private static final String WARNING2 = "WARNING: Unknown module: other.mod.bar"; private final JPackageCommand cmd; diff --git a/test/jdk/tools/jpackage/share/ModulePathTest.java b/test/jdk/tools/jpackage/share/ModulePathTest.java index 19dd2dbfb5186..06b30ec89e72a 100644 --- a/test/jdk/tools/jpackage/share/ModulePathTest.java +++ b/test/jdk/tools/jpackage/share/ModulePathTest.java @@ -138,7 +138,7 @@ public void test(String javaAppDesc) throws IOException { private final List modulePathArgs; - private final static String GOOD_PATH = "@GoodPath@"; - private final static String EMPTY_DIR = "@EmptyDir@"; - private final static String NON_EXISTING_DIR = "@NonExistingDir@"; + private static final String GOOD_PATH = "@GoodPath@"; + private static final String EMPTY_DIR = "@EmptyDir@"; + private static final String NON_EXISTING_DIR = "@NonExistingDir@"; } From 8d728128cd4d559cdd3c665095a90b82a4bb28c4 Mon Sep 17 00:00:00 2001 From: Alexey Semenyuk Date: Fri, 7 Mar 2025 00:37:19 -0500 Subject: [PATCH 41/46] Ignore default runtime if jpackage command line has `--jlink-options` option. --- .../jpackage/helpers/jdk/jpackage/test/JPackageCommand.java | 2 +- test/jdk/tools/jpackage/share/JLinkOptionsTest.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/JPackageCommand.java b/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/JPackageCommand.java index 8ffe1b68692e6..1a6fd9ac6cafc 100644 --- a/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/JPackageCommand.java +++ b/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/JPackageCommand.java @@ -1058,7 +1058,7 @@ private JPackageCommand adjustArgumentsBeforeExecution() { // to allow the jlink process to print exception stacktraces on any failure addArgument("-J-Djlink.debug=true"); } - if (!hasArgument("--runtime-image") && !hasArgument("--app-image") && DEFAULT_RUNTIME_IMAGE != null && !ignoreDefaultRuntime) { + if (!hasArgument("--runtime-image") && !hasArgument("--jlink-options") && !hasArgument("--app-image") && DEFAULT_RUNTIME_IMAGE != null && !ignoreDefaultRuntime) { addArguments("--runtime-image", DEFAULT_RUNTIME_IMAGE); } diff --git a/test/jdk/tools/jpackage/share/JLinkOptionsTest.java b/test/jdk/tools/jpackage/share/JLinkOptionsTest.java index 7c110a250148c..a882fd15f1809 100644 --- a/test/jdk/tools/jpackage/share/JLinkOptionsTest.java +++ b/test/jdk/tools/jpackage/share/JLinkOptionsTest.java @@ -154,7 +154,7 @@ public void testNoBindServicesByDefault() { } private final JPackageCommand createJPackageCommand(String javaAppDesc) { - return JPackageCommand.helloAppImage(javaAppDesc).ignoreDefaultRuntime(true); + return JPackageCommand.helloAppImage(javaAppDesc); } private final Set getModulesInRuntime(String ... jlinkOptions) { From 3fe3eed412c50b7386b7ae12a4000e377823e067 Mon Sep 17 00:00:00 2001 From: Alexey Semenyuk Date: Fri, 7 Mar 2025 14:48:13 -0500 Subject: [PATCH 42/46] Give unique descriptions to signing tests --- .../jpackage/macosx/SigningAppImageTest.java | 14 ++++++-------- .../jpackage/macosx/SigningPackageTest.java | 19 ++++++++----------- .../jpackage/macosx/base/SigningBase.java | 18 ++++++++++++++++-- 3 files changed, 30 insertions(+), 21 deletions(-) diff --git a/test/jdk/tools/jpackage/macosx/SigningAppImageTest.java b/test/jdk/tools/jpackage/macosx/SigningAppImageTest.java index 959f6d710b6ba..cebd4200f24d0 100644 --- a/test/jdk/tools/jpackage/macosx/SigningAppImageTest.java +++ b/test/jdk/tools/jpackage/macosx/SigningAppImageTest.java @@ -62,17 +62,15 @@ public class SigningAppImageTest { @Test // ({"sign or not", "signing-key or sign-identity", "certificate index"}) // Sign, signing-key and ASCII certificate - @Parameter({"true", "true", SigningBase.ASCII_INDEX}) + @Parameter({"true", "true", "ASCII_INDEX"}) // Sign, signing-key and UNICODE certificate - @Parameter({"true", "true", SigningBase.UNICODE_INDEX}) + @Parameter({"true", "true", "UNICODE_INDEX"}) // Sign, signing-indentity and UNICODE certificate - @Parameter({"true", "false", SigningBase.UNICODE_INDEX}) + @Parameter({"true", "false", "UNICODE_INDEX"}) // Unsigned - @Parameter({"false", "true", "-1"}) - public void test(String... testArgs) throws Exception { - boolean doSign = Boolean.parseBoolean(testArgs[0]); - boolean signingKey = Boolean.parseBoolean(testArgs[1]); - int certIndex = Integer.parseInt(testArgs[2]); + @Parameter({"false", "true", "INVALID_INDEX"}) + public void test(boolean doSign, boolean signingKey, SigningBase.CertIndex certEnum) throws Exception { + final var certIndex = certEnum.value(); SigningCheck.checkCertificates(certIndex); diff --git a/test/jdk/tools/jpackage/macosx/SigningPackageTest.java b/test/jdk/tools/jpackage/macosx/SigningPackageTest.java index 46a3dad2af8c8..2cf5211d52588 100644 --- a/test/jdk/tools/jpackage/macosx/SigningPackageTest.java +++ b/test/jdk/tools/jpackage/macosx/SigningPackageTest.java @@ -110,27 +110,24 @@ private static int getCertIndex(JPackageCommand cmd) { return SigningBase.getDevNameIndex(devName); } else { // Signing-indentity - return Integer.valueOf(SigningBase.UNICODE_INDEX); + return SigningBase.CertIndex.UNICODE_INDEX.value(); } } @Test // ("signing-key or sign-identity", "sign app-image", "sign pkg", "certificate index"}) // Signing-key and ASCII certificate - @Parameter({"true", "true", "true", SigningBase.ASCII_INDEX}) + @Parameter({"true", "true", "true", "ASCII_INDEX"}) // Signing-key and UNICODE certificate - @Parameter({"true", "true", "true", SigningBase.UNICODE_INDEX}) + @Parameter({"true", "true", "true", "UNICODE_INDEX"}) // Signing-indentity and UNICODE certificate - @Parameter({"false", "true", "true", SigningBase.UNICODE_INDEX}) + @Parameter({"false", "true", "true", "UNICODE_INDEX"}) // Signing-indentity, but sign app-image only and UNICODE certificate - @Parameter({"false", "true", "false", SigningBase.UNICODE_INDEX}) + @Parameter({"false", "true", "false", "UNICODE_INDEX"}) // Signing-indentity, but sign pkg only and UNICODE certificate - @Parameter({"false", "false", "true", SigningBase.UNICODE_INDEX}) - public static void test(String... testArgs) throws Exception { - boolean signingKey = Boolean.parseBoolean(testArgs[0]); - boolean signAppImage = Boolean.parseBoolean(testArgs[1]); - boolean signPKG = Boolean.parseBoolean(testArgs[2]); - int certIndex = Integer.parseInt(testArgs[3]); + @Parameter({"false", "false", "true", "UNICODE_INDEX"}) + public static void test(boolean signingKey, boolean signAppImage, boolean signPKG, SigningBase.CertIndex certEnum) throws Exception { + final var certIndex = certEnum.value(); SigningCheck.checkCertificates(certIndex); diff --git a/test/jdk/tools/jpackage/macosx/base/SigningBase.java b/test/jdk/tools/jpackage/macosx/base/SigningBase.java index 254aa306b522c..ae4439294e53d 100644 --- a/test/jdk/tools/jpackage/macosx/base/SigningBase.java +++ b/test/jdk/tools/jpackage/macosx/base/SigningBase.java @@ -32,9 +32,23 @@ public class SigningBase { + enum CertIndex { + ASCII_INDEX(0), + UNICODE_INDEX(0), + INVALID_INDEX(-1); + + CertIndex(int value) { + this.value = value; + } + + int value() { + return value; + } + + private final int value; + } + public static int DEFAULT_INDEX = 0; - public static final String ASCII_INDEX = "0"; - public static final String UNICODE_INDEX = "0"; private static String [] DEV_NAMES = { "jpackage.openjdk.java.net", "jpackage.openjdk.java.net (รถ)", From eda0efec3cb26a0719632ff55864b963e9711c5a Mon Sep 17 00:00:00 2001 From: Alexey Semenyuk Date: Fri, 7 Mar 2025 14:49:40 -0500 Subject: [PATCH 43/46] Update copyright year --- test/jdk/tools/jpackage/macosx/SigningAppImageTest.java | 2 +- test/jdk/tools/jpackage/macosx/SigningPackageTest.java | 2 +- test/jdk/tools/jpackage/macosx/base/SigningBase.java | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/test/jdk/tools/jpackage/macosx/SigningAppImageTest.java b/test/jdk/tools/jpackage/macosx/SigningAppImageTest.java index cebd4200f24d0..e27f6c8729be4 100644 --- a/test/jdk/tools/jpackage/macosx/SigningAppImageTest.java +++ b/test/jdk/tools/jpackage/macosx/SigningAppImageTest.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2019, 2024, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2019, 2025, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it diff --git a/test/jdk/tools/jpackage/macosx/SigningPackageTest.java b/test/jdk/tools/jpackage/macosx/SigningPackageTest.java index 2cf5211d52588..c56f01e192ef7 100644 --- a/test/jdk/tools/jpackage/macosx/SigningPackageTest.java +++ b/test/jdk/tools/jpackage/macosx/SigningPackageTest.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2019, 2024, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2019, 2025, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it diff --git a/test/jdk/tools/jpackage/macosx/base/SigningBase.java b/test/jdk/tools/jpackage/macosx/base/SigningBase.java index ae4439294e53d..2d84c90584845 100644 --- a/test/jdk/tools/jpackage/macosx/base/SigningBase.java +++ b/test/jdk/tools/jpackage/macosx/base/SigningBase.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2019, 2023, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2019, 2025, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it From e6ad500c150fa052e60e163ea03693f6ab6e87e0 Mon Sep 17 00:00:00 2001 From: Alexey Semenyuk Date: Mon, 10 Mar 2025 11:13:07 -0400 Subject: [PATCH 44/46] Fix Unicode signing certificate index --- test/jdk/tools/jpackage/macosx/base/SigningBase.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/jdk/tools/jpackage/macosx/base/SigningBase.java b/test/jdk/tools/jpackage/macosx/base/SigningBase.java index 2d84c90584845..021a19bb82f38 100644 --- a/test/jdk/tools/jpackage/macosx/base/SigningBase.java +++ b/test/jdk/tools/jpackage/macosx/base/SigningBase.java @@ -34,7 +34,7 @@ public class SigningBase { enum CertIndex { ASCII_INDEX(0), - UNICODE_INDEX(0), + UNICODE_INDEX(1), INVALID_INDEX(-1); CertIndex(int value) { From 545851473d38368e9301f1c754a58b447bfd9edf Mon Sep 17 00:00:00 2001 From: Alexey Semenyuk Date: Mon, 10 Mar 2025 11:13:16 -0400 Subject: [PATCH 45/46] Fix typo --- test/jdk/tools/jpackage/share/ErrorTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/jdk/tools/jpackage/share/ErrorTest.java b/test/jdk/tools/jpackage/share/ErrorTest.java index e1a2e19998b7b..5260dceae3277 100644 --- a/test/jdk/tools/jpackage/share/ErrorTest.java +++ b/test/jdk/tools/jpackage/share/ErrorTest.java @@ -330,7 +330,7 @@ public static Collection basic() { // invalid type testSpec().addArgs("--type", "invalid-type") .error("ERR_InvalidInstallerType", "invalid-type"), - // no --input for non-mular app + // no --input for non-mudular app testSpec().removeArgs("--input").error("error.no-input-parameter"), // no --module-path testSpec().appDesc("com.other/com.other.Hello").removeArgs("--module-path") From 3fdf653e537f542ae35fee93d2f80181a59ba2bb Mon Sep 17 00:00:00 2001 From: Alexey Semenyuk Date: Mon, 10 Mar 2025 16:42:46 -0400 Subject: [PATCH 46/46] Use Executor.trace() instead of TKit.trace() --- .../tools/jpackage/helpers/jdk/jpackage/test/Executor.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/Executor.java b/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/Executor.java index cd1db30e9c9dc..66cd422203b2a 100644 --- a/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/Executor.java +++ b/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/Executor.java @@ -383,13 +383,13 @@ private Result runExecutable() throws IOException, InterruptedException { final var defaultEnv = builder.environment(); final var envComm = Comm.compare(defaultEnv.keySet(), setEnvVars.keySet()); envComm.unique2().forEach(envVar -> { - TKit.trace(String.format("Adding %s=[%s] to environment", envVar, setEnvVars.get(envVar))); + trace(String.format("Adding %s=[%s] to environment", envVar, setEnvVars.get(envVar))); }); envComm.common().forEach(envVar -> { final var curValue = defaultEnv.get(envVar); final var newValue = setEnvVars.get(envVar); if (!curValue.equals(newValue)) { - TKit.trace(String.format("Setting %s=[%s] in environment", envVar, setEnvVars.get(envVar))); + trace(String.format("Setting %s=[%s] in environment", envVar, setEnvVars.get(envVar))); } }); defaultEnv.putAll(setEnvVars); @@ -399,7 +399,7 @@ private Result runExecutable() throws IOException, InterruptedException { final var envComm = Comm.compare(defaultEnv, removeEnvVars); defaultEnv.removeAll(envComm.common()); envComm.common().forEach(envVar -> { - TKit.trace(String.format("Clearing %s in environment", envVar)); + trace(String.format("Clearing %s in environment", envVar)); }); }