Skip to content

8351073: [macos] jpackage produces invalid Java runtime DMG bundles #26173

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.

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,9 @@ record CodesignConfig(Optional<SigningIdentity> identity, Optional<String> ident
Objects.requireNonNull(keychain);

if (identity.isPresent() != identifierPrefix.isPresent()) {
throw new IllegalArgumentException("Signing identity and identifier prefix mismatch");
throw new IllegalArgumentException(
"Signing identity (" + identity + ") and identifier prefix (" +
identifierPrefix + ") mismatch");
}

identifierPrefix.ifPresent(v -> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ private String validatedBundleName() throws ConfigException {
return value;
}

private String validatedBundleIdentifier() throws ConfigException {
public String validatedBundleIdentifier() throws ConfigException {

Choose a reason for hiding this comment

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

This method is private on purpose. It should not be used outside of the MacApplicationBuilder class. If you need to get the valid bundle identifier, create MacApplication instance and call MacApplication.bundleIdentifier() on it.

final var value = Optional.ofNullable(bundleIdentifier).orElseGet(() -> {
return app.mainLauncher()
.flatMap(Launcher::startupInfo)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,17 @@

import static jdk.jpackage.internal.StandardBundlerParam.PREDEFINED_APP_IMAGE;
import static jdk.jpackage.internal.StandardBundlerParam.PREDEFINED_APP_IMAGE_FILE;
import static jdk.jpackage.internal.StandardBundlerParam.PREDEFINED_RUNTIME_IMAGE;
import static jdk.jpackage.internal.StandardBundlerParam.SIGN_BUNDLE;

import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.text.MessageFormat;
import java.util.Map;
import java.util.NoSuchElementException;
import java.util.Optional;
import java.util.stream.Stream;
import jdk.jpackage.internal.model.ConfigException;

public abstract class MacBaseInstallerBundler extends AbstractBundler {
Expand Down Expand Up @@ -63,6 +67,23 @@ protected void validateAppImageAndBundeler(
"warning.unsigned.app.image"), getID()));
}
}
} else if (StandardBundlerParam.isRuntimeInstaller(params)) {
// Call appImageBundler.validate(params); to validate signing
// requirements.
appImageBundler.validate(params);

Path runtimeImage = PREDEFINED_RUNTIME_IMAGE.fetchFrom(params);

// Make sure we have valid runtime image.
if (!isRuntimeImageJDKBundle(runtimeImage)
&& !isRuntimeImageJDKImage(runtimeImage)) {
throw new ConfigException(
Copy link
Member

@alexeysemenyukoracle alexeysemenyukoracle Jul 8, 2025

Choose a reason for hiding this comment

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

  1. Can be simplified:
throw new ConfigException(
                    I18N.format("message.runtime-image-invalid", runtimeImage),
                    I18N.getString("message.runtime-image-invalid.advice"));
  1. This validation should be a part of MacPackageBuilder.create().

MessageFormat.format(I18N.getString(
"message.runtime-image-invalid"),
runtimeImage.toString()),
I18N.getString(
"message.runtime-image-invalid.advice"));
}
} else {
appImageBundler.validate(params);
}
Expand All @@ -73,5 +94,33 @@ public String getBundleType() {
return "INSTALLER";
}

// JDK bundle: "Contents/Home", "Contents/MacOS/libjli.dylib"
// and "Contents/Info.plist"
private static boolean isRuntimeImageJDKBundle(Path runtimeImage) {
Path path1 = runtimeImage.resolve("Contents/Home");
Path path2 = runtimeImage.resolve("Contents/MacOS/libjli.dylib");
Path path3 = runtimeImage.resolve("Contents/Info.plist");
return IOUtils.exists(path1)
&& path1.toFile().list() != null
&& path1.toFile().list().length > 0
&& IOUtils.exists(path2)
&& IOUtils.exists(path3);
}

// JDK image: "lib/*/libjli.dylib"
static boolean isRuntimeImageJDKImage(Path runtimeImage) {
final Path jliName = Path.of("libjli.dylib");
try (Stream<Path> walk = Files.walk(runtimeImage.resolve("lib"))) {
final Path jli = walk
.filter(file -> file.getFileName().equals(jliName))
.findFirst()
.get();
return IOUtils.exists(jli);
} catch (IOException | NoSuchElementException ex) {
Log.verbose(ex);

Choose a reason for hiding this comment

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

So if jpackage doesn't know if the given path is a JDK image or a JDK bundle, it will proceed with packaging anyway. This doesn't look right. In general, most, if not all, of the existing constructions like:

} catch (Exception ex) {
    Log.verbose(ex);
    ...
}

are wrong. Let's not add new ones.

return false;
}
}

private final Bundler appImageBundler;
}
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
import static jdk.jpackage.internal.model.StandardPackageType.MAC_DMG;
import static jdk.jpackage.internal.model.StandardPackageType.MAC_PKG;
import static jdk.jpackage.internal.util.function.ThrowingFunction.toFunction;
import static jdk.jpackage.internal.StandardBundlerParam.isRuntimeInstaller;

import java.io.IOException;
import java.nio.file.Files;
Expand Down Expand Up @@ -76,7 +77,8 @@ private static MacApplication createMacApplication(
Map<String, ? super Object> params) throws ConfigException, IOException {

final var predefinedRuntimeLayout = PREDEFINED_RUNTIME_IMAGE.findIn(params).map(predefinedRuntimeImage -> {
if (Files.isDirectory(RUNTIME_PACKAGE_LAYOUT.resolveAt(predefinedRuntimeImage).runtimeDirectory())) {
if (!isRuntimeInstaller(params) &&
Files.isDirectory(RUNTIME_PACKAGE_LAYOUT.resolveAt(predefinedRuntimeImage).runtimeDirectory())) {
return RUNTIME_PACKAGE_LAYOUT;
} else {
return RuntimeLayout.DEFAULT;
Expand Down Expand Up @@ -147,7 +149,15 @@ private static MacApplication createMacApplication(
signingBuilder.entitlementsResourceName("sandbox.plist");
}

app.mainLauncher().flatMap(Launcher::startupInfo).ifPresent(signingBuilder::signingIdentifierPrefix);
final var bundleIdentifier = appBuilder.validatedBundleIdentifier();
app.mainLauncher().flatMap(Launcher::startupInfo).ifPresentOrElse(
signingBuilder::signingIdentifierPrefix,
() -> {
// Runtime installer does not have main launcher, so use
// 'bundleIdentifier' as prefix by default.
signingBuilder.signingIdentifierPrefix(
bundleIdentifier + ".");
});
SIGN_IDENTIFIER_PREFIX.copyInto(params, signingBuilder::signingIdentifierPrefix);

ENTITLEMENTS.copyInto(params, signingBuilder::entitlements);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
import static jdk.jpackage.internal.util.XmlUtils.toXmlConsumer;
import static jdk.jpackage.internal.util.function.ThrowingBiConsumer.toBiConsumer;
import static jdk.jpackage.internal.util.function.ThrowingSupplier.toSupplier;
import static jdk.jpackage.internal.model.MacPackage.RUNTIME_PACKAGE_LAYOUT;

import java.io.IOException;
import java.io.StringWriter;
Expand Down Expand Up @@ -71,6 +72,7 @@
import jdk.jpackage.internal.model.MacPackage;
import jdk.jpackage.internal.model.Package;
import jdk.jpackage.internal.model.PackageType;
import jdk.jpackage.internal.model.RuntimeLayout;
import jdk.jpackage.internal.model.PackagerException;
import jdk.jpackage.internal.util.PathUtils;
import jdk.jpackage.internal.util.function.ThrowingConsumer;
Expand All @@ -91,12 +93,22 @@ enum MacBuildApplicationTaskID implements TaskID {
enum MacCopyAppImageTaskID implements TaskID {
COPY_PACKAGE_FILE,
COPY_RUNTIME_INFO_PLIST,
COPY_RUNTIME_JLILIB,
REPLACE_APP_IMAGE_FILE,
COPY_SIGN
COPY_SIGN,
SIGN_RUNTIME_BUNDLE
}

static AppImageLayout packagingLayout(Package pkg) {
return pkg.appImageLayout().resolveAt(pkg.relativeInstallDir().getFileName());
if (pkg.isRuntimeInstaller()) {
if (((MacPackage)pkg).isRuntimeImageJDKImage()) {
return RUNTIME_PACKAGE_LAYOUT.resolveAt(pkg.relativeInstallDir().getFileName());
} else {
return RuntimeLayout.DEFAULT.resolveAt(pkg.relativeInstallDir().getFileName());
}
} else {
return pkg.appImageLayout().resolveAt(pkg.relativeInstallDir().getFileName());
}
}

static PackagingPipeline.Builder build(Optional<Package> pkg) {
Expand All @@ -118,7 +130,7 @@ static PackagingPipeline.Builder build(Optional<Package> pkg) {
.applicationAction(MacPackagingPipeline::writeApplicationRuntimeInfoPlist)
.addDependent(BuildApplicationTaskID.CONTENT).add()
.task(MacBuildApplicationTaskID.COPY_JLILIB)
.applicationAction(MacPackagingPipeline::copyJliLib)
.applicationAction(MacPackagingPipeline::copyApplicationRuntimeJliLib)
.addDependency(BuildApplicationTaskID.RUNTIME)
.addDependent(BuildApplicationTaskID.CONTENT).add()
.task(MacBuildApplicationTaskID.APP_ICON)
Expand All @@ -140,6 +152,12 @@ static PackagingPipeline.Builder build(Optional<Package> pkg) {
.task(MacCopyAppImageTaskID.COPY_RUNTIME_INFO_PLIST)
.addDependencies(CopyAppImageTaskID.COPY)
.addDependents(PrimaryTaskID.COPY_APP_IMAGE).add()
.task(MacCopyAppImageTaskID.COPY_RUNTIME_JLILIB)
.addDependencies(CopyAppImageTaskID.COPY)
.addDependents(PrimaryTaskID.COPY_APP_IMAGE).add()
.task(MacCopyAppImageTaskID.SIGN_RUNTIME_BUNDLE)
.addDependencies(CopyAppImageTaskID.COPY)
.addDependents(PrimaryTaskID.COPY_APP_IMAGE).add()
.task(MacBuildApplicationTaskID.FA_ICONS)
.applicationAction(MacPackagingPipeline::writeFileAssociationIcons)
.addDependent(BuildApplicationTaskID.CONTENT).add()
Expand All @@ -148,13 +166,13 @@ static PackagingPipeline.Builder build(Optional<Package> pkg) {
.addDependent(BuildApplicationTaskID.CONTENT).add();

builder.task(MacBuildApplicationTaskID.SIGN)
.appImageAction(MacPackagingPipeline::sign)
.appImageAction(MacPackagingPipeline::signApplicationBundle)
.addDependencies(builder.taskGraphSnapshot().getAllTailsOf(PrimaryTaskID.BUILD_APPLICATION_IMAGE))
.addDependent(PrimaryTaskID.BUILD_APPLICATION_IMAGE)
.add();

builder.task(MacCopyAppImageTaskID.COPY_SIGN)
.appImageAction(MacPackagingPipeline::sign)
.appImageAction(MacPackagingPipeline::signApplicationBundle)
.addDependencies(builder.taskGraphSnapshot().getAllTailsOf(PrimaryTaskID.COPY_APP_IMAGE))
.addDependent(PrimaryTaskID.COPY_APP_IMAGE)
.add();
Expand All @@ -179,9 +197,20 @@ static PackagingPipeline.Builder build(Optional<Package> pkg) {
// don't create ".package" file and don't sign it.
disabledTasks.add(MacCopyAppImageTaskID.COPY_PACKAGE_FILE);
disabledTasks.add(MacCopyAppImageTaskID.COPY_SIGN);
// if (p.isRuntimeInstaller()) {
// builder.task(MacCopyAppImageTaskID.COPY_RUNTIME_INFO_PLIST).packageAction(MacPackagingPipeline::writeRuntimeRuntimeInfoPlist).add();
// }
if (((MacPackage)p).isRuntimeImageJDKImage()) {
builder.task(MacCopyAppImageTaskID.COPY_RUNTIME_INFO_PLIST)
.packageAction(MacPackagingPipeline::writeRuntimeRuntimeInfoPlist)
.add();
builder.task(MacCopyAppImageTaskID.COPY_RUNTIME_JLILIB)
.packageAction(MacPackagingPipeline::copyRuntimeRuntimeJliLib)
.add();
}

if (((MacPackage)p).isRuntimeJDKBundleNeedSigning()) {
builder.task(MacCopyAppImageTaskID.SIGN_RUNTIME_BUNDLE)
.packageAction(MacPackagingPipeline::signRuntimeBundle)
.add();
}
}

for (final var taskId : disabledTasks) {
Expand Down Expand Up @@ -211,14 +240,24 @@ private static void copyAppImage(MacPackage pkg, AppImageDesc srcAppImage,
PackagingPipeline.copyAppImage(srcAppImage, dstAppImage, !pkg.predefinedAppImageSigned().orElse(false));
}

private static void copyJliLib(
private static void copyRuntimeRuntimeJliLib(PackageBuildEnv<MacPackage, AppImageLayout> env) throws IOException {
copyJliLib(env.resolvedLayout().rootDirectory(),
env.resolvedLayout().rootDirectory().resolve("Contents/Home"));
}

private static void copyApplicationRuntimeJliLib(
AppImageBuildEnv<MacApplication, MacApplicationLayout> env) throws IOException {
copyJliLib(env.resolvedLayout().runtimeRootDirectory(),
env.resolvedLayout().runtimeDirectory());
}

private static void copyJliLib(Path runtimeRootDirectory, Path runtimeLibRoot) throws IOException {

final var runtimeMacOSDir = env.resolvedLayout().runtimeRootDirectory().resolve("Contents/MacOS");
final var runtimeMacOSDir = runtimeRootDirectory.resolve("Contents/MacOS");

final var jliName = Path.of("libjli.dylib");

try (var walk = Files.walk(env.resolvedLayout().runtimeDirectory().resolve("lib"))) {
try (var walk = Files.walk(runtimeLibRoot.resolve("lib"))) {
final var jli = walk
.filter(file -> file.getFileName().equals(jliName))
.findFirst()
Expand Down Expand Up @@ -248,7 +287,7 @@ private static void writePkgInfoFile(
}

private static void writeRuntimeRuntimeInfoPlist(PackageBuildEnv<MacPackage, AppImageLayout> env) throws IOException {
writeRuntimeInfoPlist(env.pkg().app(), env.env(), env.resolvedLayout().rootDirectory());
writeRuntimeBundleInfoPlist(env.pkg().app(), env.env(), env.resolvedLayout().rootDirectory());
}

private static void writeApplicationRuntimeInfoPlist(
Expand All @@ -271,6 +310,22 @@ private static void writeRuntimeInfoPlist(MacApplication app, BuildEnv env, Path
.saveToFile(runtimeRootDirectory.resolve("Contents/Info.plist"));
}

private static void writeRuntimeBundleInfoPlist(MacApplication app, BuildEnv env, Path runtimeRootDirectory) throws IOException {

Choose a reason for hiding this comment

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

This function is almost a duplicate of writeRuntimeInfoPlist(). They can be merged together with branching:

if (app.asApplicationLayout().isPresent()) {
    // This is application bundle
} else {
    // This is runtime bundle
}


Map<String, String> data = new HashMap<>();
data.put("CF_BUNDLE_IDENTIFIER", app.bundleIdentifier());
data.put("CF_BUNDLE_NAME", app.bundleName());
data.put("CF_BUNDLE_VERSION", app.version());
data.put("CF_BUNDLE_SHORT_VERSION_STRING", app.shortVersion().toString());
data.put("CF_BUNDLE_VENDOR", app.vendor());

env.createResource("RuntimeBundle-Info.plist.template")
.setPublicName("RuntimeBundle-Info.plist")
.setCategory(I18N.getString("resource.runtime-bundle-info-plist"))
.setSubstitutionData(data)
.saveToFile(runtimeRootDirectory.resolve("Contents/Info.plist"));
}

private static void writeAppInfoPlist(
AppImageBuildEnv<MacApplication, MacApplicationLayout> env) throws IOException {

Expand Down Expand Up @@ -308,9 +363,16 @@ private static void writeAppInfoPlist(
.saveToFile(infoPlistFile);
}

private static void sign(AppImageBuildEnv<MacApplication, MacApplicationLayout> env) throws IOException {

Choose a reason for hiding this comment

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

I don't understand why individual signing functions are needed for the runtime and application bundles. They look identical to me.

private static void signRuntimeBundle(PackageBuildEnv<MacPackage, AppImageLayout> env) throws IOException {
sign(env.pkg().app(), env.env(), env.resolvedLayout().rootDirectory());
}

final var app = env.app();
private static void signApplicationBundle(AppImageBuildEnv<MacApplication, MacApplicationLayout> env) throws IOException {
sign(env.app(), env.env(), env.resolvedLayout().rootDirectory());
}

private static void sign(final MacApplication app,
final BuildEnv env, final Path appImageDir) throws IOException {

final var codesignConfigBuilder = CodesignConfig.build();
app.signingConfig().ifPresent(codesignConfigBuilder::from);
Expand All @@ -319,17 +381,16 @@ private static void sign(AppImageBuildEnv<MacApplication, MacApplicationLayout>
final var entitlementsDefaultResource = app.signingConfig().map(
AppImageSigningConfig::entitlementsResourceName).orElseThrow();

final var entitlementsFile = env.env().configDir().resolve(app.name() + ".entitlements");
final var entitlementsFile = env.configDir().resolve(app.name() + ".entitlements");

env.env().createResource(entitlementsDefaultResource)
env.createResource(entitlementsDefaultResource)
.setCategory(I18N.getString("resource.entitlements"))
.saveToFile(entitlementsFile);

codesignConfigBuilder.entitlements(entitlementsFile);
}

final Runnable signAction = () -> {
final var appImageDir = env.resolvedLayout().rootDirectory();
AppImageSigner.createSigner(app, codesignConfigBuilder.create()).accept(appImageDir);
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,11 @@ default DottedVersion shortVersion() {
@Override
default Path appImageDirName() {
if (isRuntime()) {
return Application.super.appImageDirName();
if (Application.super.appImageDirName().toString().endsWith(".jdk")) {
return Application.super.appImageDirName();
} else {
return Path.of(Application.super.appImageDirName().toString() + ".jdk");
}
} else {
return Path.of(Application.super.appImageDirName().toString() + ".app");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
package jdk.jpackage.internal.model;

import java.nio.file.Path;
import java.nio.file.Files;
import jdk.jpackage.internal.util.CompositeProxy;

public interface MacPackage extends Package, MacPackageMixin {
Expand All @@ -34,7 +35,7 @@ public interface MacPackage extends Package, MacPackageMixin {
@Override
default AppImageLayout appImageLayout() {
if (isRuntimeInstaller()) {
return RUNTIME_PACKAGE_LAYOUT;
return RuntimeLayout.DEFAULT;
Comment on lines -37 to +38

Choose a reason for hiding this comment

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

Why is this change? It looks wrong. The layout of the output bundle should have the "Contents" folder. This is how it was before the change.

} else {
return Package.super.appImageLayout();
}
Expand All @@ -44,6 +45,36 @@ default Path installDir() {
return Path.of("/").resolve(relativeInstallDir());
}

default boolean isRuntimeImageJDKImage() {
if (isRuntimeInstaller()) {
Path runtimeImage = app().runtimeImage().orElseThrow();
Path p = runtimeImage.resolve("Contents/Home");
return !Files.exists(p);
}

return false;
}

// Returns true if signing is requested or JDK bundle is not signed
// or JDK image is provided.
default boolean isRuntimeJDKBundleNeedSigning() {
if (!isRuntimeInstaller()) {
return false;
}

if (app().sign()) {
return true;
}

if (isRuntimeImageJDKImage()) {
return true;
} else {
Path runtimeImage = app().runtimeImage().orElseThrow();
Path p = runtimeImage.resolve("Contents/_CodeSignature");
return !Files.exists(p);
}
}
Comment on lines +48 to +76

Choose a reason for hiding this comment

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

Default interface methods should operate on values returned by other interface methods. These new functions involve filesystem operations. They probably don't belong to the interface.


public static MacPackage create(Package pkg, MacPackageMixin mixin) {
return CompositeProxy.create(MacPackage.class, pkg, mixin);
}
Expand Down
Loading