-
Notifications
You must be signed in to change notification settings - Fork 122
Use single skiko-awt-runtime dependency on Desktop #2806
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: jb-main
Are you sure you want to change the base?
Changes from all commits
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 |
|---|---|---|
| @@ -0,0 +1,60 @@ | ||
| /* | ||
| * Copyright 2026 The Android Open Source Project | ||
| * | ||
| * Licensed under the Apache License, Version 2.0 (the "License"); | ||
| * you may not use this file except in compliance with the License. | ||
| * You may obtain a copy of the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, software | ||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| * See the License for the specific language governing permissions and | ||
| * limitations under the License. | ||
| */ | ||
|
|
||
| package org.jetbrains.androidx.build | ||
|
|
||
| import org.gradle.api.Project | ||
| import org.gradle.api.artifacts.DependencySubstitution | ||
| import org.gradle.api.artifacts.component.ModuleComponentSelector | ||
| import org.gradle.nativeplatform.MachineArchitecture | ||
| import org.gradle.nativeplatform.OperatingSystemFamily | ||
| import org.gradle.nativeplatform.platform.internal.DefaultNativePlatform | ||
|
|
||
| fun Project.configureSkikoAwtRuntimeConstraints() { | ||
| val currentOs = DefaultNativePlatform.getCurrentOperatingSystem() | ||
| val currentArch = DefaultNativePlatform.getCurrentArchitecture() | ||
|
|
||
| val platformSuffix = when { | ||
| currentOs.isMacOsX && currentArch.isArm64 -> "macos-arm64" | ||
| currentOs.isMacOsX && currentArch.isAmd64 -> "macos-x64" | ||
| currentOs.isLinux && currentArch.isArm64 -> "linux-arm64" | ||
| currentOs.isLinux && currentArch.isAmd64 -> "linux-x64" | ||
| currentOs.isWindows && currentArch.isArm64 -> "windows-arm64" | ||
| currentOs.isWindows && currentArch.isAmd64 -> "windows-x64" | ||
| else -> error("Unsupported platform: OS=${currentOs.name}, Arch=${currentArch.name}") | ||
| } | ||
|
|
||
| // Use dependency substitution to replace the universal dependency with platform-specific one | ||
| // during resolution, without affecting what gets published | ||
| project.configurations.configureEach { configuration -> | ||
| if (configuration.isCanBeResolved) { | ||
| configuration.resolutionStrategy.dependencySubstitution { | ||
|
Member
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.
|
||
| it.all { substitution -> | ||
| val requested = substitution.requested | ||
| if (requested is ModuleComponentSelector && | ||
| requested.group == "org.jetbrains.skiko" && | ||
| requested.module == "skiko-awt-runtime") { | ||
| // Keep the same version, just change the module name to platform-specific | ||
| substitution.useTarget( | ||
| "${requested.group}:skiko-awt-runtime-${platformSuffix}:${requested.version}", | ||
| "Platform-specific variant selection based on current machine" | ||
| ) | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
This file was deleted.
|
Member
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. Direct |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -33,10 +33,9 @@ kotlin { | |
| resources.srcDirs += "src/jvmMain/res" | ||
|
|
||
| dependencies { | ||
| implementation(libs.skikoCurrentOs) | ||
| implementation(project(":collection:collection")) | ||
| implementation(project(":collection:collection")) | ||
| implementation(project(":compose:desktop:desktop")) | ||
|
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 rename this module to
Member
Author
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. Could you please elaborate which exactly module you are proposing to rename?
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.
Member
Author
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. Does that mean that every library would have it as dependency, I've assumed we don't want this.
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.
Probably we want, because the
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. Because of that, maybe we even don't need extra dependencies in Compose, and just make
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 don't require users to depend on this explicitly, and add transitive dependency So, when users use the CMP plugin and |
||
|
|
||
| implementation(project(":compose:material:material")) | ||
| implementation("org.jetbrains.compose.material:material-icons-core:1.7.3") { | ||
| // exclude dependencies, because they override local projects when we build 0.0.0-* version | ||
| // (see https://repo1.maven.org/maven2/org/jetbrains/compose/material/material-icons-core-desktop/1.6.11/material-icons-core-desktop-1.6.11.module) | ||
|
|
||
This file was deleted.
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.
Can we use
dependencyConstraintsas in the CMP plugin to be consistent in the logic?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.
Unfortunately, not.
Dependency constraints are propagated to published artifacts when configuration is consumable, and it is.
This means that all users will get
linux/x86requirement, if we are building on such a machine.That's the best solution I've come up so far.
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.
Does this mean that we also need to use
dependencySubstitututionin the plugin, otherwise libraries built with "Compose Multiplatform plugin" have this constraint?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.
These constraints are only applied to
skiko-awt-runtimemodule, and it is only added as dependency in applications, it is not included in libraries' dependencies. This works so far. And there is a property to disable this behavior completely.If we rework dependency as suggested in another comment, add
skiko-awt-runtimewill be added to all libraries, than it might be an issue.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.
I thought about it a little, and I see that if want to implement hiding this dependency from users (I see that it is beneficial), we can:
dependencySubstitututionin the Gradle pluginskiko-awt-runtime, and we can apply dependency substitution on theskikoartifact itself? If this is true, we can remove this artifact from Skiko publication?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.
It looks like from user perspective it works the same? I.e. it allows running applications, and doesn't add anything to the library publications?
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.
If we rely on
dependencySubstitution, we introduce hard dependency on Compose Gradle plugin.This will limit us in the future in supporting other build systems.
Current proposed approach with
skiko-awt-runtimeis less limiting, as the required attributes can be provided manually. Well, after writing this, I realize that there is still a limitation and it is only marginally better.