RFR: JDK-8269387: jpackage --add-launcher should have option to not create shortcuts for additional launchers
JDK-8269387: jpackage --add-launcher should have option to not create shortcuts for additional launchers ------------- Commit messages: - JDK-8269387: jpackage --add-launcher should have option to not create shortcuts for additional launchers - JDK-8269387: jpackage --add-launcher should have option to not create shortcuts for additional launchers - JDK-8269387: jpackage --add-launcher should have option to not create shortcuts for additional launchers - JDK-8269387: jpackage --add-launcher should have option to not create shortcuts for additional launchers Changes: https://git.openjdk.java.net/jdk/pull/4730/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=4730&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8269387 Stats: 298 lines in 11 files changed: 240 ins; 5 del; 53 mod Patch: https://git.openjdk.java.net/jdk/pull/4730.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4730/head:pull/4730 PR: https://git.openjdk.java.net/jdk/pull/4730
On Thu, 8 Jul 2021 19:25:33 GMT, Andy Herrick <herrick@openjdk.org> wrote:
JDK-8269387: jpackage --add-launcher should have option to not create shortcuts for additional launchers
This will need a CSR (so you or someone with a Reviewer role should indicate this with the `/csr` command). Also, can you summarize the changes in this PR, including the added command line options? ------------- PR: https://git.openjdk.java.net/jdk/pull/4730
On Thu, 8 Jul 2021 19:25:33 GMT, Andy Herrick <herrick@openjdk.org> wrote:
JDK-8269387: jpackage --add-launcher should have option to not create shortcuts for additional launchers
src/jdk.jpackage/share/classes/jdk/jpackage/internal/AddLauncherArguments.java line 39:
37: 38: /* 39: * AddLauncherArguments
Class comment needs to be updated. Currently it says: * The add-launcher properties file may have any of: * * appVersion * module * main-jar * main-class * icon * arguments * java-options * win-console * linux-app-category src/jdk.jpackage/share/classes/jdk/jpackage/internal/AddLauncherArguments.java line 125:
123: (value == null) ? null : Path.of(value)); 124: 125: Arguments.putUnlessNull(bundleParams, SHORTCUT_HINT.getID(),
I think it would be better to add platform-specific options only if jpackage runs on that platform: if (Platform.isWindows()) { Arguments.putUnlessNull(bundleParams, SHORTCUT_HINT.getID(), getOptionValue(CLIOptions.WIN_SHORTCUT_HINT)); Arguments.putUnlessNull(bundleParams, MENU_HINT.getID(), getOptionValue(CLIOptions.WIN_MENU_HINT)); } if (Platform.isLinux()) { Arguments.putUnlessNull(bundleParams, SHORTCUT_HINT.getID(), getOptionValue(CLIOptions.LINUX_SHORTCUT_HINT)); } ------------- PR: https://git.openjdk.java.net/jdk/pull/4730
On Thu, 8 Jul 2021 19:25:33 GMT, Andy Herrick <herrick@openjdk.org> wrote:
JDK-8269387: jpackage --add-launcher should have option to not create shortcuts for additional launchers
src/jdk.jpackage/share/classes/jdk/jpackage/internal/AppImageFile.java line 259:
257: for (var name : names) { 258: launchers.add(new LauncherInfo(name, true, true)); 259: }
This block of code can be simplified down to: `ADD_LAUNCHERS.fetchFrom(params).stream().map(APP_NAME::fetchFrom).map(name -> new LauncherInfo(name, true, true)).forEach(launchers::add);` No need in intermediate `names` list. ------------- PR: https://git.openjdk.java.net/jdk/pull/4730
On Thu, 8 Jul 2021 19:25:33 GMT, Andy Herrick <herrick@openjdk.org> wrote:
JDK-8269387: jpackage --add-launcher should have option to not create shortcuts for additional launchers
test/jdk/tools/jpackage/share/AddLShortcutTest.java line 2:
1: /* 2: * Copyright (c) 2018, 2020, Oracle and/or its affiliates. All rights reserved.
Copyright year should be 2021 ------------- PR: https://git.openjdk.java.net/jdk/pull/4730
On Thu, 8 Jul 2021 19:25:33 GMT, Andy Herrick <herrick@openjdk.org> wrote:
JDK-8269387: jpackage --add-launcher should have option to not create shortcuts for additional launchers
Review test/jdk/tools/jpackage/share/AddLShortcutTest.java line 46:
44: */ 45: 46: /*
Why do you need two jtreg @test-s if there is only one test method in the test class? They are 100% duplicates. test/jdk/tools/jpackage/share/AddLShortcutTest.java line 72:
70: */ 71: 72: public class AddLShortcutTest {
How does the test tests shortcuts? ------------- Changes requested by asemenyuk (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/4730
JDK-8269387: jpackage --add-launcher should have option to not create shortcuts for additional launchers
Andy Herrick has updated the pull request incrementally with one additional commit since the last revision: JDK-8269387: jpackage --add-launcher should have option to not create shortcuts for additional launchers ------------- Changes: - all: https://git.openjdk.java.net/jdk/pull/4730/files - new: https://git.openjdk.java.net/jdk/pull/4730/files/8b6402c6..3fe30059 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=4730&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=4730&range=00-01 Stats: 62 lines in 7 files changed: 12 ins; 30 del; 20 mod Patch: https://git.openjdk.java.net/jdk/pull/4730.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4730/head:pull/4730 PR: https://git.openjdk.java.net/jdk/pull/4730
On Fri, 9 Jul 2021 17:59:35 GMT, Andy Herrick <herrick@openjdk.org> wrote:
JDK-8269387: jpackage --add-launcher should have option to not create shortcuts for additional launchers
Andy Herrick has updated the pull request incrementally with one additional commit since the last revision:
JDK-8269387: jpackage --add-launcher should have option to not create shortcuts for additional launchers
Changes requested by asemenyuk (Reviewer). test/jdk/tools/jpackage/helpers/jdk/jpackage/test/AdditionalLauncher.java line 84:
82: } 83: 84: public AdditionalLauncher setShortcuts(boolean menu, boolean desktop) {
I'd expect verifyShortcuts() method added to this class and called from verify() similar to how verifyIcon() is called. This would make AddLShortcutTest.java do some automatic testing of shortcuts. Something like this: private void verify(JPackageCommand cmd) throws IOException { verifyIcon(cmd); verifyShortcuts(cmd); ... } public AdditionalLauncher setShortcuts(boolean menu, boolean desktop) { withMenuShortcut = menu; withDesktopShortcut = desktop; return this; } private void verifyShortcuts(JPackageCommand cmd) throws IOException { if (TKit.isLinux() && !cmd.isImagePackageType() && withMenuShortcut != null) { Path desktopFile = LinuxHelper.getDesktopFile(cmd, name); if (withMenuShortcut) { TKit.assertFileExists(desktopFile); } else { TKit.assertPathExists(desktopFile, false); } } } private Boolean withMenuShortcut; private Boolean withDesktopShortcut; ------------- PR: https://git.openjdk.java.net/jdk/pull/4730
JDK-8269387: jpackage --add-launcher should have option to not create shortcuts for additional launchers
Andy Herrick has updated the pull request incrementally with one additional commit since the last revision: JDK-8269387: jpackage --add-launcher should have option to not create shortcuts for additional launchers ------------- Changes: - all: https://git.openjdk.java.net/jdk/pull/4730/files - new: https://git.openjdk.java.net/jdk/pull/4730/files/3fe30059..d88b1dc7 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=4730&range=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=4730&range=01-02 Stats: 45 lines in 2 files changed: 22 ins; 0 del; 23 mod Patch: https://git.openjdk.java.net/jdk/pull/4730.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4730/head:pull/4730 PR: https://git.openjdk.java.net/jdk/pull/4730
On Tue, 13 Jul 2021 16:39:01 GMT, Andy Herrick <herrick@openjdk.org> wrote:
JDK-8269387: jpackage --add-launcher should have option to not create shortcuts for additional launchers
Andy Herrick has updated the pull request incrementally with one additional commit since the last revision:
JDK-8269387: jpackage --add-launcher should have option to not create shortcuts for additional launchers
Changes requested by asemenyuk (Reviewer). test/jdk/tools/jpackage/helpers/jdk/jpackage/test/AdditionalLauncher.java line 87:
85: withMenuShortcut = menu; 86: withShortcut = shortcut; 87: if (TKit.isLinux()) {
I'd move add `addRawProperties()` calls to `initialize()` method. This will prevent multiple `addRawProperties()` calls for "linux-shortcut" and "win-shortcut" properties in case `setShortcuts()` method is called multiple times. ------------- PR: https://git.openjdk.java.net/jdk/pull/4730
JDK-8269387: jpackage --add-launcher should have option to not create shortcuts for additional launchers
Andy Herrick has updated the pull request incrementally with one additional commit since the last revision: JDK-8269387: jpackage --add-launcher should have option to not create shortcuts for additional launchers ------------- Changes: - all: https://git.openjdk.java.net/jdk/pull/4730/files - new: https://git.openjdk.java.net/jdk/pull/4730/files/d88b1dc7..96758fe8 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=4730&range=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=4730&range=02-03 Stats: 18 lines in 1 file changed: 12 ins; 6 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/4730.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4730/head:pull/4730 PR: https://git.openjdk.java.net/jdk/pull/4730
On Wed, 14 Jul 2021 16:50:39 GMT, Andy Herrick <herrick@openjdk.org> wrote:
JDK-8269387: jpackage --add-launcher should have option to not create shortcuts for additional launchers
Andy Herrick has updated the pull request incrementally with one additional commit since the last revision:
JDK-8269387: jpackage --add-launcher should have option to not create shortcuts for additional launchers
Marked as reviewed by asemenyuk (Reviewer). ------------- PR: https://git.openjdk.java.net/jdk/pull/4730
On Wed, 14 Jul 2021 16:50:39 GMT, Andy Herrick <herrick@openjdk.org> wrote:
JDK-8269387: jpackage --add-launcher should have option to not create shortcuts for additional launchers
Andy Herrick has updated the pull request incrementally with one additional commit since the last revision:
JDK-8269387: jpackage --add-launcher should have option to not create shortcuts for additional launchers
Marked as reviewed by almatvee (Reviewer). ------------- PR: https://git.openjdk.java.net/jdk/pull/4730
On Thu, 8 Jul 2021 19:25:33 GMT, Andy Herrick <herrick@openjdk.org> wrote:
JDK-8269387: jpackage --add-launcher should have option to not create shortcuts for additional launchers
This pull request has now been integrated. Changeset: 057992f2 Author: Andy Herrick <herrick@openjdk.org> URL: https://git.openjdk.java.net/jdk/commit/057992f206d48d0f6152f6fdece229e2ff56... Stats: 341 lines in 11 files changed: 258 ins; 13 del; 70 mod 8269387: jpackage --add-launcher should have option to not create shortcuts for additional launchers Reviewed-by: asemenyuk, almatvee ------------- PR: https://git.openjdk.java.net/jdk/pull/4730
participants (4)
-
Alexander Matveev
-
Alexey Semenyuk
-
Andy Herrick
-
Kevin Rushforth