-
Notifications
You must be signed in to change notification settings - Fork 1.4k
feat: Add support for macOS layered icons #5451
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
base: master
Are you sure you want to change the base?
Changes from all commits
b608da2
6878a0a
27e4a6b
3cce19c
f6d1291
8c45518
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,6 +6,7 @@ | |
| package org.jetbrains.compose.desktop.application.dsl | ||
|
|
||
| import org.gradle.api.Action | ||
| import org.gradle.api.file.DirectoryProperty | ||
| import org.gradle.api.file.RegularFileProperty | ||
| import org.gradle.api.model.ObjectFactory | ||
| import java.io.File | ||
|
|
@@ -35,6 +36,7 @@ abstract class AbstractMacOSPlatformSettings : AbstractPlatformSettings() { | |
| var dmgPackageBuildVersion: String? = null | ||
| var appCategory: String? = null | ||
| var minimumSystemVersion: String? = null | ||
| var layeredIconDir: DirectoryProperty = objects.directoryProperty() | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we support it for
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please add a link to the example (
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We need to describe necessary changes required in the documentation. I suggest to just do it in the PR: Documentation changes needed |
||
|
|
||
|
|
||
| /** | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,99 @@ | ||||||
| package org.jetbrains.compose.desktop.application.internal | ||||||
|
|
||||||
| import org.gradle.api.logging.Logger | ||||||
| import org.jetbrains.compose.internal.utils.MacUtils | ||||||
| import java.io.File | ||||||
|
|
||||||
| internal class MacAssetsTool(private val runTool: ExternalToolRunner, private val logger: Logger) { | ||||||
|
|
||||||
| fun compileAssets(iconDir: File, workingDir: File, minimumSystemVersion: String?): File { | ||||||
| val toolVersion = checkAssetsToolVersion() | ||||||
| logger.info("compile mac assets is starting, supported actool version:$toolVersion") | ||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
|
|
||||||
| val result = runTool( | ||||||
| tool = MacUtils.xcrun, | ||||||
| args = listOf( | ||||||
| "actool", | ||||||
| iconDir.absolutePath, // Input asset catalog | ||||||
| "--compile", workingDir.absolutePath, | ||||||
| "--app-icon", iconDir.name.removeSuffix(".icon"), | ||||||
| "--enable-on-demand-resources", "NO", | ||||||
| "--development-region", "en", | ||||||
| "--target-device", "mac", | ||||||
| "--platform", "macosx", | ||||||
| "--enable-icon-stack-fallback-generation=disabled", | ||||||
| "--include-all-app-icons", | ||||||
| "--minimum-deployment-target", minimumSystemVersion ?: "10.13", | ||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Better to pass a non-null parameter to not have "10.13" in multiple places. We seems can do that, by changing one of the calling code to |
||||||
| "--output-partial-info-plist", "/dev/null" | ||||||
| ), | ||||||
| ) | ||||||
|
|
||||||
| if (result.exitValue != 0) { | ||||||
| error("Could not compile the layered icons directory into Assets.car.") | ||||||
| } | ||||||
| if (!assetsFile(workingDir).exists()) { | ||||||
| error("Could not find Assets.car in the working directory.") | ||||||
| } | ||||||
| return workingDir.resolve("Assets.car") | ||||||
| } | ||||||
|
|
||||||
| fun assetsFile(workingDir: File): File = workingDir.resolve("Assets.car") | ||||||
|
|
||||||
| private fun checkAssetsToolVersion(): String { | ||||||
| val requiredVersion = 26.0 | ||||||
| var outputContent = "" | ||||||
| val result = runTool( | ||||||
| tool = MacUtils.xcrun, | ||||||
| args = listOf("actool", "--version"), | ||||||
| processStdout = { outputContent = it }, | ||||||
| ) | ||||||
|
|
||||||
| if (result.exitValue != 0) { | ||||||
| error("Could not get actool version: Command `xcrun actool -version` exited with code ${result.exitValue}\nStdOut: $outputContent\n") | ||||||
| } | ||||||
|
|
||||||
| val versionString: String? = try { | ||||||
| var versionContent = "" | ||||||
| runTool( | ||||||
| tool = MacUtils.plutil, | ||||||
| args = listOf( | ||||||
| "-extract", | ||||||
| "com\\.apple\\.actool\\.version.short-bundle-version", | ||||||
| "raw", | ||||||
| "-expect", | ||||||
| "string", | ||||||
| "-o", | ||||||
| "-", | ||||||
| "-" | ||||||
| ), | ||||||
| stdinStr = outputContent, | ||||||
| processStdout = { | ||||||
| versionContent = it | ||||||
| } | ||||||
| ) | ||||||
| versionContent | ||||||
| } catch (e: Exception) { | ||||||
| error("Could not check actool version. Error: ${e.message}") | ||||||
| } | ||||||
|
|
||||||
| if (versionString.isNullOrBlank()) { | ||||||
| error("Could not extract short-bundle-version from actool output: '$outputContent'. Assuming it meets requirements.") | ||||||
| } | ||||||
|
|
||||||
| val majorVersion = versionString | ||||||
| .split(".") | ||||||
| .firstOrNull() | ||||||
| ?.toIntOrNull() | ||||||
| ?: error("Could not get actool major version from version string '$versionString' . Output was: '$outputContent'. Assuming it meets requirements.") | ||||||
|
|
||||||
| if (majorVersion < requiredVersion) { | ||||||
| error( | ||||||
| "Unsupported actool version: $versionString. " + | ||||||
| "Version $requiredVersion or higher is required. " + | ||||||
| "Please update your Xcode Command Line Tools." | ||||||
| ) | ||||||
| } else { | ||||||
| return versionString | ||||||
| } | ||||||
| } | ||||||
| } | ||||||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -37,6 +37,7 @@ import org.jetbrains.compose.desktop.application.internal.InfoPlistBuilder.InfoP | |||||
| import org.jetbrains.compose.desktop.application.internal.InfoPlistBuilder.InfoPlistValue.InfoPlistMapValue | ||||||
| import org.jetbrains.compose.desktop.application.internal.InfoPlistBuilder.InfoPlistValue.InfoPlistStringValue | ||||||
| import org.jetbrains.compose.desktop.application.internal.JvmRuntimeProperties | ||||||
| import org.jetbrains.compose.desktop.application.internal.MacAssetsTool | ||||||
| import org.jetbrains.compose.desktop.application.internal.MacSigner | ||||||
| import org.jetbrains.compose.desktop.application.internal.MacSignerImpl | ||||||
| import org.jetbrains.compose.desktop.application.internal.NoCertificateSigner | ||||||
|
|
@@ -293,7 +294,11 @@ abstract class AbstractJPackageTask @Inject constructor( | |||||
|
|
||||||
| @get:Input | ||||||
| internal val fileAssociations: SetProperty<FileAssociation> = objects.setProperty(FileAssociation::class.java) | ||||||
|
|
||||||
|
|
||||||
| @get:InputDirectory | ||||||
| @get:Optional | ||||||
| internal val macLayeredIcons: DirectoryProperty = objects.directoryProperty() | ||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
to be consistent with the public property (if we still decide to keep it after this) |
||||||
|
|
||||||
| private val iconMapping by lazy { | ||||||
| val icons = fileAssociations.get().mapNotNull { it.iconFile }.distinct() | ||||||
| if (icons.isEmpty()) return@lazy emptyMap() | ||||||
|
|
@@ -344,6 +349,8 @@ abstract class AbstractJPackageTask @Inject constructor( | |||||
| } else null | ||||||
| } | ||||||
|
|
||||||
| private val macAssetsTool by lazy { MacAssetsTool(runExternalTool, logger) } | ||||||
|
|
||||||
| @get:LocalState | ||||||
| protected val signDir: Provider<Directory> = project.layout.buildDirectory.dir("compose/tmp/sign") | ||||||
|
|
||||||
|
|
@@ -600,12 +607,28 @@ abstract class AbstractJPackageTask @Inject constructor( | |||||
|
|
||||||
| fileOperations.clearDirs(jpackageResources) | ||||||
| if (currentOS == OS.MacOS) { | ||||||
| val systemVersion = macMinimumSystemVersion.orNull ?: "10.13" | ||||||
|
|
||||||
| macLayeredIcons.ioFileOrNull?.let { layeredIcon -> | ||||||
| if (layeredIcon.exists()) { | ||||||
| try { | ||||||
| macAssetsTool.compileAssets( | ||||||
| layeredIcon, | ||||||
| workingDir.ioFile, | ||||||
| systemVersion | ||||||
| ) | ||||||
| } catch (e: Exception) { | ||||||
| logger.warn("Can not compile layered icon: ${e.message}") | ||||||
| } | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| InfoPlistBuilder(macExtraPlistKeysRawXml.orNull) | ||||||
| .also { setInfoPlistValues(it) } | ||||||
| .writeToFile(jpackageResources.ioFile.resolve("Info.plist")) | ||||||
|
|
||||||
| if (macAppStore.orNull == true) { | ||||||
| val systemVersion = macMinimumSystemVersion.orNull ?: "10.13" | ||||||
|
|
||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
| val productDefPlistXml = """ | ||||||
| <key>os</key> | ||||||
| <array> | ||||||
|
|
@@ -642,6 +665,12 @@ abstract class AbstractJPackageTask @Inject constructor( | |||||
| val appDir = destinationDir.ioFile.resolve("${packageName.get()}.app") | ||||||
| val runtimeDir = appDir.resolve("Contents/runtime") | ||||||
|
|
||||||
| macAssetsTool.assetsFile(workingDir.ioFile).apply { | ||||||
| if (exists()) { | ||||||
| copyTo(appDir.resolve("Contents/Resources/Assets.car")) | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| // Add the provisioning profile | ||||||
| macRuntimeProvisioningProfile.ioFileOrNull?.copyTo( | ||||||
| target = runtimeDir.resolve("Contents/embedded.provisionprofile"), | ||||||
|
|
@@ -739,6 +768,10 @@ abstract class AbstractJPackageTask @Inject constructor( | |||||
| ) | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| if (macAssetsTool.assetsFile(workingDir.ioFile).exists()) { | ||||||
| macLayeredIcons.orNull?.let { plist[PlistKeys.CFBundleIconName] = it.asFile.name.removeSuffix(".icon") } | ||||||
| } | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
|
|
||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we override the icon defined in
iconFileby defining this property and packaging it on the new macOs/Xcode? Is this icon supported on the oldest OS, because it is compiled into backward-compatible assets?If so, it is better to reuse the existing
iconFileinstead of adding a new property and check that if it is a directory ending with ".icon" then we should use the layeredIcon logic.Reasoning:
iconFile,layeredIconDirchange one thing (the application icon) and differ only in implementationsfileAssociation