Skip to content

Commit 5d06ac0

Browse files
GoeLinGoeLin
committed
8281682: Redundant .ico files in Windows app-image cause unnecessary bloat
Reviewed-by: mdoerr Backport-of: 45180633d34b6cbb679bae0753d9f422e76d6297
1 parent ff97f22 commit 5d06ac0

File tree

7 files changed

+159
-17
lines changed

7 files changed

+159
-17
lines changed

‎src/jdk.jpackage/windows/classes/jdk/jpackage/internal/WindowsAppImageBuilder.java‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2015, 2020, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2015, 2022, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* This code is free software; you can redistribute it and/or modify it
@@ -117,8 +117,9 @@ private void createLauncherForEntryPoint(Map<String, ? super Object> params,
117117
mainParams);
118118
Path iconTarget = null;
119119
if (iconResource != null) {
120-
iconTarget = appLayout.destktopIntegrationDirectory().resolve(
121-
APP_NAME.fetchFrom(params) + ".ico");
120+
Path iconDir = StandardBundlerParam.TEMP_ROOT.fetchFrom(params).resolve(
121+
"icons");
122+
iconTarget = iconDir.resolve(APP_NAME.fetchFrom(params) + ".ico");
122123
if (null == iconResource.saveToFile(iconTarget)) {
123124
iconTarget = null;
124125
}

‎src/jdk.jpackage/windows/classes/jdk/jpackage/internal/WixAppImageFragmentBuilder.java‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -443,17 +443,11 @@ private String addShortcutComponent(XMLStreamWriter xml, Path launcherPath,
443443

444444
Path shortcutPath = folder.getPath(this).resolve(launcherBasename);
445445
return addComponent(xml, shortcutPath, Component.Shortcut, unused -> {
446-
final Path icoFile = IOUtils.addSuffix(
447-
installedAppImage.destktopIntegrationDirectory().resolve(
448-
launcherBasename), ".ico");
449-
450446
xml.writeAttribute("Name", launcherBasename);
451447
xml.writeAttribute("WorkingDirectory", INSTALLDIR.toString());
452448
xml.writeAttribute("Advertise", "no");
453-
xml.writeAttribute("IconIndex", "0");
454449
xml.writeAttribute("Target", String.format("[#%s]",
455450
Component.File.idOf(launcherPath)));
456-
xml.writeAttribute("Icon", Id.Icon.of(icoFile));
457451
});
458452
}
459453

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2019, 2021, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2019, 2022, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* This code is free software; you can redistribute it and/or modify it
@@ -149,7 +149,8 @@ public ExceptionBox(Throwable throwable) {
149149
}
150150

151151
@SuppressWarnings("unchecked")
152-
public static void rethrowUnchecked(Throwable throwable) throws ExceptionBox {
152+
public static RuntimeException rethrowUnchecked(Throwable throwable) throws
153+
ExceptionBox {
153154
if (throwable instanceof RuntimeException) {
154155
throw (RuntimeException)throwable;
155156
}

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

Lines changed: 140 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2019, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2019, 2022, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* This code is free software; you can redistribute it and/or modify it
@@ -24,8 +24,12 @@
2424
package jdk.jpackage.test;
2525

2626
import java.io.IOException;
27+
import java.lang.reflect.InvocationTargetException;
28+
import java.lang.reflect.Method;
2729
import java.nio.file.Files;
2830
import java.nio.file.Path;
31+
import java.util.List;
32+
import java.util.Optional;
2933

3034
public final class LauncherIconVerifier {
3135
public LauncherIconVerifier() {
@@ -60,7 +64,11 @@ public void applyTo(JPackageCommand cmd) throws IOException {
6064
Path iconPath = cmd.appLayout().destktopIntegrationDirectory().resolve(
6165
curLauncherName + TKit.ICON_SUFFIX);
6266

63-
if (expectedDefault) {
67+
if (TKit.isWindows()) {
68+
TKit.assertPathExists(iconPath, false);
69+
WinIconVerifier.instance.verifyLauncherIcon(cmd, launcherName,
70+
expectedIcon, expectedDefault);
71+
} else if (expectedDefault) {
6472
TKit.assertPathExists(iconPath, true);
6573
} else if (expectedIcon == null) {
6674
TKit.assertPathExists(iconPath, false);
@@ -73,6 +81,136 @@ public void applyTo(JPackageCommand cmd) throws IOException {
7381
}
7482
}
7583

84+
private static class WinIconVerifier {
85+
86+
void verifyLauncherIcon(JPackageCommand cmd, String launcherName,
87+
Path expectedIcon, boolean expectedDefault) {
88+
TKit.withTempDirectory("icons", tmpDir -> {
89+
Path launcher = cmd.appLauncherPath(launcherName);
90+
Path iconWorkDir = tmpDir.resolve(launcher.getFileName());
91+
Path iconContainer = iconWorkDir.resolve("container.exe");
92+
Files.createDirectories(iconContainer.getParent());
93+
Files.copy(getDefaultAppLauncher(expectedIcon == null
94+
&& !expectedDefault), iconContainer);
95+
if (expectedIcon != null) {
96+
setIcon(expectedIcon, iconContainer);
97+
}
98+
99+
Path extractedExpectedIcon = extractIconFromExecutable(
100+
iconWorkDir, iconContainer, "expected");
101+
Path extractedActualIcon = extractIconFromExecutable(iconWorkDir,
102+
launcher, "actual");
103+
TKit.assertTrue(-1 == Files.mismatch(extractedExpectedIcon,
104+
extractedActualIcon),
105+
String.format(
106+
"Check icon file [%s] of %s launcher is a copy of source icon file [%s]",
107+
extractedActualIcon,
108+
Optional.ofNullable(launcherName).orElse("main"),
109+
extractedExpectedIcon));
110+
});
111+
}
112+
113+
private WinIconVerifier() {
114+
try {
115+
executableRebranderClass = Class.forName(
116+
"jdk.jpackage.internal.ExecutableRebrander");
117+
118+
lockResource = executableRebranderClass.getDeclaredMethod(
119+
"lockResource", String.class);
120+
// Note: this reflection call requires
121+
// --add-opens jdk.jpackage/jdk.jpackage.internal=ALL-UNNAMED
122+
lockResource.setAccessible(true);
123+
124+
unlockResource = executableRebranderClass.getDeclaredMethod(
125+
"unlockResource", long.class);
126+
unlockResource.setAccessible(true);
127+
128+
iconSwap = executableRebranderClass.getDeclaredMethod("iconSwap",
129+
long.class, String.class);
130+
iconSwap.setAccessible(true);
131+
} catch (ClassNotFoundException | NoSuchMethodException
132+
| SecurityException ex) {
133+
throw Functional.rethrowUnchecked(ex);
134+
}
135+
}
136+
137+
private Path extractIconFromExecutable(Path outputDir, Path executable,
138+
String label) {
139+
Path psScript = outputDir.resolve(label + ".ps1");
140+
Path extractedIcon = outputDir.resolve(label + ".bmp");
141+
TKit.createTextFile(psScript, List.of(
142+
"[System.Reflection.Assembly]::LoadWithPartialName('System.Drawing')",
143+
String.format(
144+
"[System.Drawing.Icon]::ExtractAssociatedIcon(\"%s\").ToBitmap().Save(\"%s\", [System.Drawing.Imaging.ImageFormat]::Bmp)",
145+
executable.toAbsolutePath().normalize(),
146+
extractedIcon.toAbsolutePath().normalize()),
147+
"exit 0"));
148+
149+
Executor.of("powershell", "-NoLogo", "-NoProfile", "-File",
150+
psScript.toAbsolutePath().normalize().toString()).execute();
151+
152+
return extractedIcon;
153+
}
154+
155+
private Path getDefaultAppLauncher(boolean noIcon) {
156+
// Create app image with the sole purpose to get the default app launcher
157+
Path defaultAppOutputDir = TKit.workDir().resolve(String.format(
158+
"out-%d", ProcessHandle.current().pid()));
159+
JPackageCommand cmd = JPackageCommand.helloAppImage().setFakeRuntime().setArgumentValue(
160+
"--dest", defaultAppOutputDir);
161+
162+
String launcherName;
163+
if (noIcon) {
164+
launcherName = "no-icon";
165+
new AdditionalLauncher(launcherName).setNoIcon().applyTo(cmd);
166+
} else {
167+
launcherName = null;
168+
}
169+
170+
if (!Files.isExecutable(cmd.appLauncherPath(launcherName))) {
171+
cmd.execute();
172+
}
173+
return cmd.appLauncherPath(launcherName);
174+
}
175+
176+
private void setIcon(Path iconPath, Path launcherPath) {
177+
TKit.trace(String.format("Set icon of [%s] launcher to [%s] file",
178+
launcherPath, iconPath));
179+
try {
180+
launcherPath.toFile().setWritable(true, true);
181+
try {
182+
long lock = 0;
183+
try {
184+
lock = (Long) lockResource.invoke(null, new Object[]{
185+
launcherPath.toAbsolutePath().normalize().toString()});
186+
if (lock == 0) {
187+
throw new RuntimeException(String.format(
188+
"Failed to lock [%s] executable",
189+
launcherPath));
190+
}
191+
iconSwap.invoke(null, new Object[]{lock,
192+
iconPath.toAbsolutePath().normalize().toString()});
193+
} finally {
194+
if (lock != 0) {
195+
unlockResource.invoke(null, new Object[]{lock});
196+
}
197+
}
198+
} catch (IllegalAccessException | InvocationTargetException ex) {
199+
throw Functional.rethrowUnchecked(ex);
200+
}
201+
} finally {
202+
launcherPath.toFile().setWritable(false, true);
203+
}
204+
}
205+
206+
final static WinIconVerifier instance = new WinIconVerifier();
207+
208+
private final Class executableRebranderClass;
209+
private final Method lockResource;
210+
private final Method unlockResource;
211+
private final Method iconSwap;
212+
}
213+
76214
private String launcherName;
77215
private Path expectedIcon;
78216
private boolean expectedDefault;

‎test/jdk/tools/jpackage/share/AddLauncherTest.java‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,9 @@
5151
* @library /test/jdk/tools/jpackage/helpers
5252
* @build jdk.jpackage.test.*
5353
* @compile AddLauncherTest.java
54-
* @run main/othervm/timeout=360 -Xmx512m jdk.jpackage.test.Main
54+
* @run main/othervm/timeout=360 -Xmx512m
55+
* --add-opens jdk.jpackage/jdk.jpackage.internal=ALL-UNNAMED
56+
* jdk.jpackage.test.Main
5557
* --jpt-run=AddLauncherTest.test
5658
*/
5759

@@ -63,7 +65,9 @@
6365
* @library /test/jdk/tools/jpackage/helpers
6466
* @build jdk.jpackage.test.*
6567
* @compile AddLauncherTest.java
66-
* @run main/othervm/timeout=540 -Xmx512m jdk.jpackage.test.Main
68+
* @run main/othervm/timeout=540 -Xmx512m
69+
* --add-opens jdk.jpackage/jdk.jpackage.internal=ALL-UNNAMED
70+
* jdk.jpackage.test.Main
6771
* --jpt-run=AddLauncherTest
6872
*/
6973

‎test/jdk/tools/jpackage/share/IconTest.java‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,9 @@
5252
* @library /test/jdk/tools/jpackage/helpers
5353
* @build jdk.jpackage.test.*
5454
* @compile IconTest.java
55-
* @run main/othervm/timeout=540 -Xmx512m jdk.jpackage.test.Main
55+
* @run main/othervm/timeout=540 -Xmx512m
56+
* --add-opens jdk.jpackage/jdk.jpackage.internal=ALL-UNNAMED
57+
* jdk.jpackage.test.Main
5658
* --jpt-run=IconTest
5759
*/
5860

‎test/jdk/tools/jpackage/share/MultiLauncherTwoPhaseTest.java‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,9 @@
4747
* @key jpackagePlatformPackage
4848
* @build jdk.jpackage.test.*
4949
* @compile MultiLauncherTwoPhaseTest.java
50-
* @run main/othervm/timeout=360 -Xmx512m jdk.jpackage.test.Main
50+
* @run main/othervm/timeout=360 -Xmx512m
51+
* --add-opens jdk.jpackage/jdk.jpackage.internal=ALL-UNNAMED
52+
* jdk.jpackage.test.Main
5153
* --jpt-run=MultiLauncherTwoPhaseTest
5254
*/
5355

0 commit comments

Comments
 (0)