Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

8279995: jpackage --add-launcher option should allow overriding description #7399

Closed
wants to merge 6 commits into from
Closed

8279995: jpackage --add-launcher option should allow overriding description #7399

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
a764be5
8279995: jpackage --add-launcher option should allow overriding descr…
sashamatveev Feb 9, 2022
dbb3690
8279995: jpackage --add-launcher option should allow overriding descr…
sashamatveev Feb 11, 2022
c5fea59
8279995: jpackage --add-launcher option should allow overriding descr…
sashamatveev Feb 14, 2022
6d4b4d3
8279995: jpackage --add-launcher option should allow overriding descr…
sashamatveev Feb 17, 2022
401db24
Merge master
sashamatveev Feb 17, 2022
e4893e5
8279995: jpackage --add-launcher option should allow overriding descr…
sashamatveev Feb 25, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion src/jdk.jpackage/share/classes/jdk/jpackage/internal/AddLauncherArguments.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2018, 2021, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2018, 2022, 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
Expand Down Expand Up @@ -54,6 +54,7 @@
* The add-launcher properties file may have any of:
*
* appVersion
* description
* module
* main-jar
* main-class
Expand Down Expand Up @@ -111,6 +112,9 @@ private void initLauncherMap() {
Arguments.putUnlessNull(bundleParams, CLIOptions.VERSION.getId(),
getOptionValue(CLIOptions.VERSION));

Arguments.putUnlessNull(bundleParams, CLIOptions.DESCRIPTION.getId(),
getOptionValue(CLIOptions.DESCRIPTION));

Arguments.putUnlessNull(bundleParams, CLIOptions.RELEASE.getId(),
getOptionValue(CLIOptions.RELEASE));

Expand Down
4 changes: 2 additions & 2 deletions src/jdk.jpackage/share/classes/jdk/jpackage/internal/resources/HelpResources.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
#
# Copyright (c) 2017, 2021, Oracle and/or its affiliates. All rights reserved.
# Copyright (c) 2017, 2022, 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
Expand Down Expand Up @@ -138,7 +138,7 @@ Generic Options:\n\
\ Name of launcher, and a path to a Properties file that contains\n\
\ a list of key, value pairs\n\
\ (absolute path or relative to the current directory)\n\
\ The keys "module", "main-jar", "main-class",\n\
\ The keys "module", "main-jar", "main-class", "description",\n\
\ "arguments", "java-options", "app-version", "icon",\n\
\ "win-console", "win-shortcut", "win-menu",\n\
\ "linux-app-category", and "linux-shortcut" can be used.\n\
Expand Down
4 changes: 2 additions & 2 deletions src/jdk.jpackage/share/classes/jdk/jpackage/internal/resources/HelpResources_ja.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
#
# Copyright (c) 2017, 2021, Oracle and/or its affiliates. All rights reserved.
# Copyright (c) 2017, 2022, 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
Expand Down Expand Up @@ -138,7 +138,7 @@ Generic Options:\n\
\ Name of launcher, and a path to a Properties file that contains\n\
\ a list of key, value pairs\n\
\ (absolute path or relative to the current directory)\n\
\ The keys "module", "main-jar", "main-class",\n\
\ The keys "module", "main-jar", "main-class", "description",\n\
\ "arguments", "java-options", "app-version", "icon",\n\
\ "win-console", "win-shortcut", "win-menu",\n\
\ "linux-app-category", and "linux-shortcut" can be used.\n\
Expand Down
4 changes: 2 additions & 2 deletions ...jdk.jpackage/share/classes/jdk/jpackage/internal/resources/HelpResources_zh_CN.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
#
# Copyright (c) 2017, 2021, Oracle and/or its affiliates. All rights reserved.
# Copyright (c) 2017, 2022, 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
Expand Down Expand Up @@ -138,7 +138,7 @@ Generic Options:\n\
\ Name of launcher, and a path to a Properties file that contains\n\
\ a list of key, value pairs\n\
\ (absolute path or relative to the current directory)\n\
\ The keys "module", "main-jar", "main-class",\n\
\ The keys "module", "main-jar", "main-class", "description",\n\
\ "arguments", "java-options", "app-version", "icon",\n\
\ "win-console", "win-shortcut", "win-menu",\n\
\ "linux-app-category", and "linux-shortcut" can be used.\n\
Expand Down
36 changes: 35 additions & 1 deletion test/jdk/tools/jpackage/helpers/jdk/jpackage/test/AdditionalLauncher.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2019, 2021, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2019, 2022, 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
Expand Down Expand Up @@ -27,10 +27,12 @@
import java.nio.file.Path;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.function.BiConsumer;
import java.util.function.Supplier;
import java.util.stream.Stream;
import jdk.jpackage.internal.ApplicationLayout;
import jdk.jpackage.test.Functional.ThrowingBiConsumer;
Expand Down Expand Up @@ -81,6 +83,17 @@ public AdditionalLauncher addRawProperties(
return this;
}

public String getRawPropertyValue(String key, Supplier<String> getDefault) {
return rawProperties.stream()
.filter(item -> item.getKey().equals(key))
.map(e -> e.getValue()).findAny().orElseGet(getDefault);
}

private String getDesciption(JPackageCommand cmd) {
return getRawPropertyValue("description", () -> cmd.getArgumentValue(
"--description", unused -> cmd.name()));
}

public AdditionalLauncher setShortcuts(boolean menu, boolean shortcut) {
withMenuShortcut = menu;
withShortcut = shortcut;
Expand Down Expand Up @@ -242,9 +255,30 @@ private void verifyShortcuts(JPackageCommand cmd) throws IOException {
}
}

private void verifyDescription(JPackageCommand cmd) throws IOException {
if (TKit.isWindows()) {
String expectedDescription = getDesciption(cmd);
Path launcherPath = cmd.appLauncherPath(name);
String actualDescription =
WindowsHelper.getExecutableDesciption(launcherPath);
TKit.assertEquals(expectedDescription, actualDescription,
String.format("Check file description of [%s]", launcherPath));
} else if (TKit.isLinux() && !cmd.isImagePackageType()) {
String expectedDescription = getDesciption(cmd);
Path desktopFile = LinuxHelper.getDesktopFile(cmd, name);
if (Files.exists(desktopFile)) {
TKit.assertTextStream("Comment=" + expectedDescription)
.label(String.format("[%s] file", desktopFile))
.predicate(String::equals)
.apply(Files.readAllLines(desktopFile).stream());
}
}
}

private void verify(JPackageCommand cmd) throws IOException {
verifyIcon(cmd);
verifyShortcuts(cmd);
verifyDescription(cmd);

Path launcherPath = cmd.appLauncherPath(name);

Expand Down
27 changes: 26 additions & 1 deletion test/jdk/tools/jpackage/helpers/jdk/jpackage/test/WindowsHelper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2019, 2020, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2019, 2022, 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
Expand Down Expand Up @@ -139,6 +139,31 @@ public static String getMsiProperty(JPackageCommand cmd, String propertyName) {
.executeAndGetOutput().stream().collect(Collectors.joining("\n"));
}

public static String getExecutableDesciption(Path pathToExeFile) {
String description = null;
Executor exec = Executor.of("powershell",
"-NoLogo",
"-NoProfile",
"-Command",
"(Get-Item \\\""
+ pathToExeFile.toAbsolutePath()
+ "\\\").VersionInfo | select FileDescription");
List<String> lines = exec.executeAndGetOutput();
for (int i = 0; i < lines.size(); i++) {
if (lines.get(i).trim().equals("FileDescription")) {
i += 2; // Skip "---------------" and move to description
description = lines.get(i).trim();
}
}

if (description == null) {
throw new RuntimeException(String.format(
"Failed to get file description of [%s]", pathToExeFile));
}

return description;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about this:

public static String getExecutableDesciption(Path pathToExeFile) {
        Executor exec = Executor.of("powershell",
                "-NoLogo",
                "-NoProfile",
                "-Command",
                "(Get-Item \\\""
                + pathToExeFile.toAbsolutePath()
                + "\\\").VersionInfo | select FileDescription");

        var lineIt = exec.dumpOutput().executeAndGetOutput().iterator();
        while (lineIt.hasNext()) {
            var line = lineIt.next();
            if (line.trim().equals("FileDescription")) {
                // Skip "---------------" and move to the description value
                lineIt.next();
                var description = lineIt.next().trim();
                if (lineIt.hasNext()) {
                    throw new RuntimeException("Unexpected input");
                }
                return description;
            }
        }

        throw new RuntimeException(String.format(
                "Failed to get file description of [%s]", pathToExeFile));
    }

Added Executor.dumpOutput() call to save the output of powershell command in the test log for easier debugging.
No null initialized description variable. No iteration over the list using the index. Check for the end of the output.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed. Except I removed if (lineIt.hasNext()) { throw new RuntimeException("Unexpected input"); }, since there are empty lines after description in output and this check triggers exception.


private static boolean isUserLocalInstall(JPackageCommand cmd) {
return cmd.hasArgument("--win-per-user-install");
}
Expand Down
10 changes: 7 additions & 3 deletions test/jdk/tools/jpackage/share/AddLauncherTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2018, 2021, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2018, 2022, 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
Expand Down Expand Up @@ -40,7 +40,7 @@
* AddLauncherTest*.* installer. The output installer should provide the
* same functionality as the default installer (see description of the default
* installer in SimplePackageTest.java) plus install three extra application
* launchers.
* launchers with unique description ("LauncherName Description").
*/

/*
Expand Down Expand Up @@ -80,7 +80,8 @@ public void test() {
PackageTest packageTest = new PackageTest().configureHelloApp();
packageTest.addInitializer(cmd -> {
cmd.addArguments("--arguments", "Duke", "--arguments", "is",
"--arguments", "the", "--arguments", "King");
"--arguments", "the", "--arguments", "King",
"--description", "AddLauncherTest Description");
});

new FileAssociations(
Expand All @@ -89,14 +90,17 @@ public void test() {

new AdditionalLauncher("Baz2")
.setDefaultArguments()
.addRawProperties(Map.entry("description", "Baz2 Description"))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How expected description is verified in the test?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not think that we have ability to check description on executable files. I added it for manual verification.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Powershell can be used to extract the description from an executable. E.g.:

$ powershell -NoLogo -NoProfile -Command "(Get-Item build\\windows-x64\\images\\jdk\\bin\\java.exe).VersionInfo | select FileDescription"

FileDescription
---------------
Java(TM) Platform SE binary

You can create jdk.jpackage.test.AdditionalLauncher.verifyDescription() function that will call this script and call this function from jdk.jpackage.test.AdditionalLauncher.verify().

.applyTo(packageTest);

new AdditionalLauncher("foo")
.setDefaultArguments("yep!")
.addRawProperties(Map.entry("description", "foo Description"))
.applyTo(packageTest);

new AdditionalLauncher("Bar")
.setDefaultArguments("one", "two", "three")
.addRawProperties(Map.entry("description", "Bar Description"))
.setIcon(GOLDEN_ICON)
.applyTo(packageTest);

Expand Down