feat: Add support for macOS layered icons#5451
feat: Add support for macOS layered icons#5451Sanlorng wants to merge 6 commits intoJetBrains:masterfrom
Conversation
|
Hi, thank you for the contribution! |
|
Thanks again for creating the related YouTrack issue – it’s been really helpful! I just wanted to gently follow up on this PR – would you have a moment to review it when you get a chance? Let me know if I can clarify anything to make it easier! No rush at all, appreciate your help! |
freelife4850-netizen
left a comment
There was a problem hiding this comment.
Sounds like a good idea to try 🙂
kropp
left a comment
There was a problem hiding this comment.
Sorry for a long wait, could you please take a look at comments and also rebase the branch to the latest version?
...pose/src/main/kotlin/org/jetbrains/compose/desktop/application/tasks/AbstractJPackageTask.kt
Outdated
Show resolved
Hide resolved
...mpose/src/test/kotlin/org/jetbrains/compose/test/tests/integration/DesktopApplicationTest.kt
Outdated
Show resolved
Hide resolved
gradle-plugins/compose/src/test/test-projects/application/macLayeredIcon/build.gradle.kts
Outdated
Show resolved
Hide resolved
.../compose/src/main/kotlin/org/jetbrains/compose/desktop/application/internal/MacAssetsTool.kt
Outdated
Show resolved
Hide resolved
This commit introduces support for using macOS layered icons (`.icon` directory) for both JVM and native application packaging. - A new `layeredIconDir` property is added to the `compose.desktop.application.nativeDistributions.macOS` and `compose.nativeApplication.macOS` DSLs. - The `actool` from Xcode Command Line Tools is used to compile the `.icon` directory into an `Assets.car` file. - The `Assets.car` file is included in the `.app` bundle resources. - The `Info.plist` is updated with `CFBundleIconName` to reference the compiled asset. - A check for a supported `actool` version is performed before compilation. - New integration tests (`testMacLayeredIcon`, `testMacLayeredIconRemove`) are added to verify the functionality for both JVM and native targets.
The `macAssetsDir` property in `AbstractJPackageTask` was unused and has been removed.
- Deleted the `Expected-Info.plist` which verified now-removed properties. - Removed `extraInfoPlistKeys` and file association configurations from the test's `build.gradle.kts`.
Updated the `macLayeredIcon` test project to use `COMPOSE_VERSION_PLACEHOLDER` for Compose dependency versions. This makes the test configuration more flexible and easier to maintain.
This commit refactors the `actool` version parsing logic to use the `plutil` command-line tool instead of a manual XML parser. This change simplifies the code and improves robustness. - Replaced the XML parsing implementation in `MacAssetsTool` with a call to `plutil -extract`. - Updated `try`/`catch` blocks to handle exceptions from the tool execution. - Added a `plutil` file definition to `MacUtils`. - Integration tests for macOS layered icons (`testMacLayeredIcon`, `testMacLayeredIconRemove`) are now skipped if the `actool` version is less than 26, ensuring they run only on supported environments.
6fe432f to
f6d1291
Compare
|
@kropp All the changes you mentioned that needed to be made have already been completed. |
|
@igordmn requesting second review as it adds new public API in Gradle plugin |
PierreVieira
left a comment
There was a problem hiding this comment.
Simple comments. Nice addition!
.../compose/src/main/kotlin/org/jetbrains/compose/desktop/application/internal/MacAssetsTool.kt
Outdated
Show resolved
Hide resolved
...plugins/compose/src/test/test-projects/application/macLayeredIcon/src/jvmMain/kotlin/main.kt
Outdated
Show resolved
Hide resolved
...ugins/compose/src/test/test-projects/application/macLayeredIcon/src/macosMain/kotlin/main.kt
Outdated
Show resolved
Hide resolved
...pose/src/test/test-projects/application/macLayeredIcon/subdir/kotlin_icon_big.icon/icon.json
Outdated
Show resolved
Hide resolved
gradle-plugins/compose/src/test/test-projects/application/macLayeredIcon/build.gradle.kts
Outdated
Show resolved
Hide resolved
gradle-plugins/compose/src/test/test-projects/application/macLayeredIcon/gradle.properties
Outdated
Show resolved
Hide resolved
gradle-plugins/compose/src/test/test-projects/application/macLayeredIcon/settings.gradle.kts
Outdated
Show resolved
Hide resolved
Co-authored-by: Pierre Vieira <pierrevieiraggg@gmail.com>
igordmn
left a comment
There was a problem hiding this comment.
Thank you and sorry for more waiting time. After we resolve the new comments, it should be good to merge.
| var dmgPackageBuildVersion: String? = null | ||
| var appCategory: String? = null | ||
| var minimumSystemVersion: String? = null | ||
| var layeredIconDir: DirectoryProperty = objects.directoryProperty() |
There was a problem hiding this comment.
Do we override the icon defined in iconFile by 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 iconFile instead 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 implementations- it is similar to using different extensions, just more unusual
- we might need to support it in
fileAssociation
| var dmgPackageBuildVersion: String? = null | ||
| var appCategory: String? = null | ||
| var minimumSystemVersion: String? = null | ||
| var layeredIconDir: DirectoryProperty = objects.directoryProperty() |
There was a problem hiding this comment.
Can we support it for fileAssociation?:
compose.desktop {
application {
nativeDistributions {
macOS {
fileAssociation(
"text/kotlin",
"kott",
"Kotlin Source File2",
project.file("subdir/kotlin_icon_big.icon"),
)
| var dmgPackageBuildVersion: String? = null | ||
| var appCategory: String? = null | ||
| var minimumSystemVersion: String? = null | ||
| var layeredIconDir: DirectoryProperty = objects.directoryProperty() |
There was a problem hiding this comment.
Please add a link to the example (gradle-plugins/compose/src/test/test-projects/application/macLayeredIcon/subdir/kotlin_icon_big.icon) in the PR description, near the snippet and change the path in the snippet to subdir/kotlin_icon_big.icon
| var dmgPackageBuildVersion: String? = null | ||
| var appCategory: String? = null | ||
| var minimumSystemVersion: String? = null | ||
| var layeredIconDir: DirectoryProperty = objects.directoryProperty() |
There was a problem hiding this comment.
We need to describe necessary changes required in the documentation.
I suggest to just do it in the PR:
Documentation changes needed
.icns files or .icon directory ([example](gradle-plugins/compose/src/test/test-projects/application/macLayeredIcon/subdir/kotlin_icon_big.icon)) for macOS
There was a problem hiding this comment.
Are the root icons not used? If so, let's delete.
|
|
||
| if (macAppStore.orNull == true) { | ||
| val systemVersion = macMinimumSystemVersion.orNull ?: "10.13" | ||
|
|
| "--platform", "macosx", | ||
| "--enable-icon-stack-fallback-generation=disabled", | ||
| "--include-all-app-icons", | ||
| "--minimum-deployment-target", minimumSystemVersion ?: "10.13", |
There was a problem hiding this comment.
| "--minimum-deployment-target", minimumSystemVersion ?: "10.13", | |
| "--minimum-deployment-target", minimumSystemVersion, |
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 minimumSystemVersion.getOrElse(KOTLIN_NATIVE_MIN_SUPPORTED_MAC_OS)!!
|
|
||
| @get:InputDirectory | ||
| @get:Optional | ||
| internal val macLayeredIcons: DirectoryProperty = objects.directoryProperty() |
There was a problem hiding this comment.
| internal val macLayeredIcons: DirectoryProperty = objects.directoryProperty() | |
| internal val layeredIconDir: DirectoryProperty = objects.directoryProperty() |
to be consistent with the public property (if we still decide to keep it after this)
|
|
||
| fun compileAssets(iconDir: File, workingDir: File, minimumSystemVersion: String?): File { | ||
| val toolVersion = checkAssetsToolVersion() | ||
| logger.info("compile mac assets is starting, supported actool version:$toolVersion") |
There was a problem hiding this comment.
| logger.info("compile mac assets is starting, supported actool version:$toolVersion") | |
| logger.info("Compilation of mac assets is starting, actool version: $toolVersion") |
Port of JetBrains/compose-multiplatform#5451. Adds a `layeredIconDir` DSL property that compiles a .icon directory into Assets.car via xcrun actool and embeds it in the .app bundle for both JVM and native Mac targets. Requires actool >= 26.0.
Support for using macOS layered icons (
.icondirectory) for both JVM and native application packaging.layeredIconDirproperty is added to thecompose.desktop.application.nativeDistributions.macOSandcompose.nativeApplication.macOSDSLs.actoolfrom Xcode Command Line Tools is used to compile the.icondirectory into anAssets.carfile.Assets.carfile is included in the.appbundle resources.Info.plistis updated withCFBundleIconNameto reference the compiled asset.actoolversion is performed before compilation.testMacLayeredIcon,testMacLayeredIconRemove) are added to verify the functionality for both JVM and native targets.Example
compose.desktop { application { nativeDistributions { macOS { layeredIconDir.set("Icon Path") } } } }Testing
Only verify the task behavior: compose:test-Gradle(9.2.0)-Agp(8.12.3) --tests org.jetbrains.compose.test.tests.integration.DesktopApplicationTest.testMacLayeredIcon
Only verify the task behavior: compose:test-Gradle(9.2.0)-Agp(8.12.3) --tests org.jetbrains.compose.test.tests.integration.DesktopApplicationTest.testMacLayeredIconRemove
This should be tested by QA
Tickets
CMP-9083 Support macOS 26 layered icons
Release Notes
Features - Gradle Plugin
.icondirectory) for both JVM and native application packaging.