Skip to content

Commit df24b90

Browse files
author
Alexey Semenyuk
committed
8360571: Description of launchers is lost in two phase packaging
Reviewed-by: almatvee
1 parent c2ea75b commit df24b90

File tree

6 files changed

+89
-27
lines changed

6 files changed

+89
-27
lines changed

‎src/jdk.jpackage/share/classes/jdk/jpackage/internal/AppImageFile.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
import static java.util.stream.Collectors.toUnmodifiableSet;
3030
import static jdk.jpackage.internal.cli.StandardAppImageFileOption.APP_VERSION;
3131
import static jdk.jpackage.internal.cli.StandardAppImageFileOption.LAUNCHER_AS_SERVICE;
32+
import static jdk.jpackage.internal.cli.StandardAppImageFileOption.DESCRIPTION;
3233
import static jdk.jpackage.internal.cli.StandardAppImageFileOption.LAUNCHER_NAME;
3334
import static jdk.jpackage.internal.util.function.ThrowingFunction.toFunction;
3435

@@ -322,6 +323,7 @@ private static Map<String, String> properties(Launcher launcher) {
322323
if (launcher.isService()) {
323324
standardProps.add(Map.entry(LAUNCHER_AS_SERVICE.getName(), Boolean.TRUE.toString()));
324325
}
326+
standardProps.add(Map.entry(DESCRIPTION.getName(), launcher.description()));
325327

326328
return Stream.concat(
327329
standardProps.stream(),

‎src/jdk.jpackage/share/classes/jdk/jpackage/internal/OptionsTransformer.java‎

Lines changed: 2 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@
2626

2727
import static jdk.jpackage.internal.cli.StandardOption.ADDITIONAL_LAUNCHERS;
2828
import static jdk.jpackage.internal.cli.StandardOption.APP_VERSION;
29-
import static jdk.jpackage.internal.cli.StandardOption.DESCRIPTION;
3029
import static jdk.jpackage.internal.cli.StandardOption.ICON;
3130
import static jdk.jpackage.internal.cli.StandardOption.NAME;
3231
import static jdk.jpackage.internal.cli.StandardOption.PREDEFINED_APP_IMAGE;
@@ -60,18 +59,8 @@ Options appOptions() {
6059
NAME, li.name(),
6160
// This should prevent the code building the Launcher instance
6261
// from the Options object from trying to create a startup info object.
63-
PREDEFINED_APP_IMAGE, PREDEFINED_APP_IMAGE.getFrom(mainOptions),
64-
//
65-
// For backward compatibility, descriptions of the additional
66-
// launchers in the predefined app image will be set to
67-
// the application description, if available, or to the name
68-
// of the main launcher in the predefined app image.
69-
//
70-
// All launchers in the predefined app image will have the same description.
71-
// This is wrong and should be revised.
72-
//
73-
DESCRIPTION, DESCRIPTION.findIn(mainOptions).orElseGet(ea::appName)
74-
)));
62+
PREDEFINED_APP_IMAGE, PREDEFINED_APP_IMAGE.getFrom(mainOptions)
63+
)));
7564
}).toList()
7665
);
7766
return Options.concat(

‎src/jdk.jpackage/share/classes/jdk/jpackage/internal/cli/StandardAppImageFileOption.java‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,14 @@ private enum MandatoryOption implements OptionScope {
9999
.inScope(AppImageFileOptionScope.LAUNCHER)
100100
.toOptionValueBuilder().id(StandardOption.LAUNCHER_AS_SERVICE.id()).create();
101101

102+
/**
103+
* The description of a launcher.
104+
*/
105+
public static final OptionValue<String> DESCRIPTION = stringOption("description")
106+
.inScope(AppImageFileOptionScope.LAUNCHER)
107+
.inScope(MandatoryOption.VALUE)
108+
.toOptionValueBuilder().id(StandardOption.DESCRIPTION.id()).create();
109+
102110

103111
//
104112
// Linux-specific

‎test/jdk/tools/jpackage/helpers/jdk/jpackage/test/AppImageFile.java‎

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,9 @@
2323
package jdk.jpackage.test;
2424

2525
import static java.util.stream.Collectors.toMap;
26+
import static jdk.jpackage.internal.util.function.ThrowingConsumer.toConsumer;
2627
import static jdk.jpackage.internal.util.function.ThrowingFunction.toFunction;
2728
import static jdk.jpackage.internal.util.function.ThrowingSupplier.toSupplier;
28-
import static jdk.jpackage.internal.util.function.ThrowingConsumer.toConsumer;
2929

3030
import java.io.IOException;
3131
import java.nio.file.Files;
@@ -37,6 +37,8 @@
3737
import java.util.Objects;
3838
import java.util.Optional;
3939
import java.util.stream.Stream;
40+
import javax.xml.stream.XMLStreamException;
41+
import javax.xml.stream.XMLStreamWriter;
4042
import javax.xml.xpath.XPath;
4143
import javax.xml.xpath.XPathExpressionException;
4244
import javax.xml.xpath.XPathFactory;
@@ -75,8 +77,8 @@ public AppImageFile(String mainLauncherName, String mainLauncherClassName) {
7577
public Map<String, Map<String, String>> addLaunchers() {
7678
var map = launchers.entrySet().stream().filter(e -> {
7779
return !e.getKey().equals(mainLauncherName);
78-
}).collect(toMap(Map.Entry::getKey, Map.Entry::getValue, (k, _) -> {
79-
throw new IllegalStateException(String.format("Duplicate key %s", k));
80+
}).collect(toMap(Map.Entry::getKey, Map.Entry::getValue, (v, _) -> {
81+
throw new IllegalStateException(String.format("Duplicate value [%s]", v));
8082
}, LinkedHashMap::new));
8183
return Collections.unmodifiableMap(map);
8284
}
@@ -93,6 +95,7 @@ public void save(Path appImageDir) throws IOException {
9395

9496
xml.writeStartElement("main-launcher");
9597
xml.writeAttribute("name", mainLauncherName);
98+
writeLauncherDescription(xml, mainLauncherName);
9699
xml.writeEndElement();
97100

98101
mainLauncherClassName.ifPresent(toConsumer(v -> {
@@ -113,6 +116,9 @@ public void save(Path appImageDir) throws IOException {
113116
xml.writeStartElement("add-launcher");
114117
xml.writeAttribute("name", al);
115118
var props = launchers.get(al);
119+
if (!props.containsKey("description")) {
120+
writeLauncherDescription(xml, al);
121+
}
116122
for (var prop : props.keySet().stream().sorted().toList()) {
117123
xml.writeStartElement(prop);
118124
xml.writeCharacters(props.get(prop));
@@ -158,8 +164,8 @@ public static AppImageFile load(Path appImageDir) {
158164
}, launcherProps -> {
159165
launcherProps.remove("name");
160166
return Collections.unmodifiableMap(launcherProps);
161-
}, (k, _) -> {
162-
throw new IllegalStateException(String.format("Duplicate key %s", k));
167+
}, (v, _) -> {
168+
throw new IllegalStateException(String.format("Duplicate value [%s]", v));
163169
}, LinkedHashMap::new));
164170

165171
return new AppImageFile(
@@ -173,6 +179,12 @@ public static AppImageFile load(Path appImageDir) {
173179
}).get();
174180
}
175181

182+
private static void writeLauncherDescription(XMLStreamWriter xml, String description) throws XMLStreamException {
183+
xml.writeStartElement("description");
184+
xml.writeCharacters(Objects.requireNonNull(description));
185+
xml.writeEndElement();
186+
}
187+
176188
private static HashMap<String, String> readLauncherProperties(XPath xPath, Element launcherElement) throws XPathExpressionException {
177189
HashMap<String, String> launcherProps = new HashMap<>();
178190

‎test/jdk/tools/jpackage/helpers/jdk/jpackage/test/LauncherVerifier.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -444,7 +444,7 @@ private static String launcherDescription(
444444
return PropertyFinder.findLauncherProperty(cmd, launcherName,
445445
PropertyFinder.cmdlineOptionWithValue("--description"),
446446
PropertyFinder.launcherPropertyFile("description"),
447-
PropertyFinder.nop()
447+
PropertyFinder.appImageFileLauncher(cmd, launcherName, "description")
448448
).orElseGet(() -> {
449449
if (cmd.isMainLauncher(launcherName)) {
450450
return cmd.mainLauncherName();

‎test/jdk/tools/jpackage/junit/share/jdk.jpackage/jdk/jpackage/internal/AppImageFileTest.java‎

Lines changed: 59 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
package jdk.jpackage.internal;
2525

2626
import static java.util.stream.Collectors.toMap;
27+
import static jdk.jpackage.internal.cli.StandardAppImageFileOption.DESCRIPTION;
2728
import static jdk.jpackage.internal.cli.StandardAppImageFileOption.LAUNCHER_AS_SERVICE;
2829
import static jdk.jpackage.internal.cli.StandardAppImageFileOption.LAUNCHER_NAME;
2930
import static jdk.jpackage.internal.cli.StandardAppImageFileOption.LINUX_LAUNCHER_SHORTCUT;
@@ -258,6 +259,11 @@ LauncherBuilder service(boolean v) {
258259
return this;
259260
}
260261

262+
LauncherBuilder description(String v) {
263+
description = v;
264+
return this;
265+
}
266+
261267
LauncherBuilder addExtra(Map<String, String> v) {
262268
extra.add(v);
263269
return this;
@@ -281,7 +287,7 @@ private Launcher createLauncher() {
281287
Optional.empty(),
282288
List.of(),
283289
service,
284-
null,
290+
description(),
285291
Optional.empty(),
286292
null,
287293
extra.asStringValues());
@@ -295,6 +301,10 @@ private String name() {
295301
}
296302
}
297303

304+
private String description() {
305+
return Optional.ofNullable(description).orElseGet(this::name);
306+
}
307+
298308
private boolean isMainLauncher() {
299309
return name.isEmpty();
300310
}
@@ -305,11 +315,13 @@ private LauncherInfo createLauncherInfo() {
305315
allProps.add(LAUNCHER_AS_SERVICE, Boolean.valueOf(service));
306316
}
307317
allProps.add(LAUNCHER_NAME, name());
318+
allProps.add(DESCRIPTION, description());
308319
return LauncherInfo.create(allProps.asObjectValues());
309320
}
310321

311322
private final Optional<String> name;
312323
private boolean service;
324+
private String description;
313325
private final ExtraPropertyBuilder extra = new ExtraPropertyBuilder();
314326
}
315327

@@ -435,9 +447,29 @@ private static List<List<String>> testInavlidXml() {
435447
createWithHeader(AppImageFile.getPlatform(os), AppImageFile.getVersion(), () -> {
436448
// Missing 'app-version' element.
437449
return List.of(
438-
"<main-launcher name='D'/>",
450+
"<main-launcher name='D'>",
451+
" <description>Foo</description>",
452+
"</main-launcher>",
439453
"<main-class>Hello</main-class>"
440454
);
455+
}),
456+
createWithHeader(AppImageFile.getPlatform(os), AppImageFile.getVersion(), () -> {
457+
// Missing 'description' element in the main launcher.
458+
return List.of(
459+
"<app-version>321</app-version>",
460+
"<main-launcher name='B'/>"
461+
);
462+
}),
463+
createWithHeader(AppImageFile.getPlatform(os), AppImageFile.getVersion(), () -> {
464+
// Missing 'description' element in the additional launcher.
465+
return List.of(
466+
"<app-version>123</app-version>",
467+
"<main-launcher name='B'>",
468+
" <description>Foo</description>",
469+
"</main-launcher>",
470+
"<main-class>Hello</main-class>",
471+
"<add-launcher name='C'/>"
472+
);
441473
})
442474
));
443475

@@ -451,7 +483,9 @@ private static List<List<String>> testInavlidXml() {
451483
private static List<String> createValidBodyWithHeader(String platform, String version) {
452484
return createWithHeader(platform, version, () -> {
453485
return List.of(
454-
"<main-launcher name='D'/>",
486+
"<main-launcher name='D'>",
487+
" <description>Blah-Blah-Blah</description>",
488+
"</main-launcher>",
455489
"<app-version>100</app-version>",
456490
"<main-class>Hello</main-class>"
457491
);
@@ -483,16 +517,24 @@ private static Collection<ReadTestSpec> platformSpecificProperties() {
483517
"<signed>true</signed>",
484518
"<app-store>False</app-store>",
485519
"<add-launcher name='add-launcher'>",
520+
" <description>Quick brown fox</description>",
486521
" <service>true</service>",
487522
" <linux-shortcut>true</linux-shortcut>",
488523
" <win-shortcut>false</win-shortcut>",
489524
" <win-menu>app-dir</win-menu>",
490525
"</add-launcher>",
491-
"<main-launcher name='Bar'/>"
526+
"<main-launcher name='Bar'>",
527+
" <description>Bar launcher description</description>",
528+
"</main-launcher>"
492529
);
493530

494531
Supplier<AppBuilder.LauncherBuilder> appBuilder = () -> {
495-
return build().mainClass("Foo").version("1.34").appName("Bar").addlauncher("add-launcher").service(true);
532+
return build()
533+
.mainClass("Foo")
534+
.version("1.34")
535+
.appName("Bar")
536+
.mainlauncher().description("Bar launcher description").commit()
537+
.addlauncher("add-launcher").service(true).description("Quick brown fox");
496538
};
497539

498540
List<ReadTestSpec> testCases = new ArrayList<>();
@@ -513,19 +555,24 @@ private static Collection<ReadTestSpec> platformSpecificProperties() {
513555
private static Stream<ReadTestSpec> testValidXml() {
514556
return Stream.concat(platformSpecificProperties().stream(), Stream.of(
515557
ReadTestSpec.build().expect(
516-
build().version("72").appName("Y").mainClass("main.Class")
558+
build().version("72").mainlauncher().description("Blah-Blah-Blah").commit().appName("Y").mainClass("main.Class")
517559
).xml(
518-
"<main-launcher name='Y'/>",
560+
"<main-launcher name='Y'>",
561+
" <description>Blah-Blah-Blah</description>",
562+
"</main-launcher>",
519563
"<app-version>72</app-version>",
520564
"<main-class>main.Class</main-class>"
521565
),
522566
ReadTestSpec.build().os(OperatingSystem.LINUX).expect(
523567
build()
568+
.mainlauncher().description("Main launcher description").commit()
524569
.addlauncher("another-launcher")
525570
.addExtra(LINUX_LAUNCHER_SHORTCUT, new LauncherShortcut(LauncherShortcutStartupDirectory.APP_DIR))
571+
.description("another-launcher description")
526572
.commit()
527573
.addlauncher("service-launcher")
528574
.service(true)
575+
.description("service-launcher description")
529576
.commit()
530577
).xml(
531578
"<app-version>1.2</app-version>",
@@ -536,13 +583,17 @@ private static Stream<ReadTestSpec> testValidXml() {
536583
"<signed>true</signed>",
537584
"<add-launcher name='service-launcher' service='true'>",
538585
" <linux-shortcut><nested>foo</nested></linux-shortcut>",
586+
" <description>service-launcher description</description>",
539587
"</add-launcher>",
540588
"<add-launcher name='another-launcher'>",
541589
" <linux-shortcut>true</linux-shortcut>",
542590
" <linux-shortcut>app-<!-- This is a comment -->dir</linux-shortcut>",
591+
" <description>another-launcher description</description>",
543592
"</add-launcher>",
544593
"<main-launcher name='Bar'/>",
545-
"<main-launcher name='Foo'/>"
594+
"<main-launcher name='Foo'>",
595+
" <description>Main launcher description</description>",
596+
"</main-launcher>"
546597
)
547598
).map(ReadTestSpec.Builder::create));
548599
}

0 commit comments

Comments
 (0)