Skip to content
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.

Sign up for GitHub

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

Jump to bottom

jvmTarget value specified using jvmToolchain is ignored and overwritten with "1.8" #2511

Closed
ksiarhey opened this issue Nov 28, 2022 · 3 comments · Fixed by #2515
Closed

jvmTarget value specified using jvmToolchain is ignored and overwritten with "1.8" #2511

ksiarhey opened this issue Nov 28, 2022 · 3 comments · Fixed by #2515
Assignees
@AlexeyTsvetkov @dima-avdeev-jb
Labels
bug Something isn't working desktop gradle Gradle plugin problems

Comments

@ksiarhey
Copy link

compose version=1.2.1
kotlin version=1.7.20

Specify jvmTarget value with kotlinOptions.jvmTarget parameter and compile

kotlin {
    jvm {
        compilations.all {
            kotlinOptions.jvmTarget = "11"
        }
        withJava()
    }
}

jvmTarget value is equal to "11".

Replace jvmTarget with jvmToolchain and recompile, jvmTarget value is equal to "1.8" but not to "11"

kotlin {
    jvm {
        jvmToolchain(11)
        withJava()
    }
}
@ksiarhey
Copy link
Author

compose-single-desktop.zip

@dima-avdeev-jb dima-avdeev-jb self-assigned this Nov 28, 2022
@dima-avdeev-jb dima-avdeev-jb added the bug Something isn't working label Nov 28, 2022
@dima-avdeev-jb
Copy link
Contributor

Simple reproduce with command:
./gradlew build && javap -verbose build.classes.kotlin.jvm.main.MainKt | grep "major version:"

@dima-avdeev-jb dima-avdeev-jb added the gradle Gradle plugin problems label Nov 28, 2022
@dima-avdeev-jb
Copy link
Contributor

dima-avdeev-jb commented Nov 28, 2022 •
edited

@ksiarhey Thanks a lot!
Yes, we have problem in our Compose Multiplatform Gradle plugin.
Plugin sets default kotlinOptions.jvmTarget = "1.8"
We will fix it!

AlexeyTsvetkov added a commit that referenced this issue Nov 29, 2022
@AlexeyTsvetkov
Remove JVM target version override
c14021f 
Previously, we were setting kotlin.jvmTarget version
to 1.8 if it was null or < 1.8.
As an unintended consequence we were also overriding
a version set by the jvmToolchain property.
So while users expected the jvmToolchain property
to set both jdk home & jdk target, we were quietly
overriding jdk target.

At the same time, Kotlin 1.7 sets the minimum
target version to 1.8 anyway, so our override
does not make sense with Kotlin 1.7+.

This commit removes overriding altogether.

Fixes #2511
@AlexeyTsvetkov AlexeyTsvetkov mentioned this issue Nov 29, 2022
Merged
AlexeyTsvetkov added a commit that referenced this issue Nov 29, 2022
@AlexeyTsvetkov
Remove JVM target version override (#2515)
7e574a0 
Previously, we were setting kotlin.jvmTarget version
to 1.8 if it was null or < 1.8.
As an unintended consequence we were also overriding
a version set by the jvmToolchain property.
So while users expected the jvmToolchain property
to set both jdk home & jdk target, we were quietly
overriding jdk target.

At the same time, Kotlin 1.7 sets the minimum
target version to 1.8 anyway, so our override
does not make sense with Kotlin 1.7+.

This commit removes overriding altogether.

Fixes #2511
igordmn pushed a commit that referenced this issue Dec 5, 2022
@AlexeyTsvetkov @igordmn
Remove JVM target version override (#2515)
e75e9cf 
Previously, we were setting kotlin.jvmTarget version
to 1.8 if it was null or < 1.8.
As an unintended consequence we were also overriding
a version set by the jvmToolchain property.
So while users expected the jvmToolchain property
to set both jdk home & jdk target, we were quietly
overriding jdk target.

At the same time, Kotlin 1.7 sets the minimum
target version to 1.8 anyway, so our override
does not make sense with Kotlin 1.7+.

This commit removes overriding altogether.

Fixes #2511
igordmn added a commit that referenced this issue Dec 15, 2022
@igordmn @AlexeyTsvetkov
Release 1.2.2 (#2561)
be0c1c8 
* Compose 1.2.1-rc01

* Fix Web build for Kotlin 1.7.20

* Use 1.3.2.1-rc02 in Gradle plugin

* Fix Gradle Plugin tests

* Fix Gradle Plugin tests

* Compose 1.2.1-rc03

* Update CHANGELOG.md

* Update CHANGELOG.md

* Compose Compiler 1.3.2.1

* Compose 1.2.1

* Update VERSIONING.md

* Update gradle.properties

* Fix custom JDK tests on Linux

* Remove JVM target version override (#2515)

Previously, we were setting kotlin.jvmTarget version
to 1.8 if it was null or < 1.8.
As an unintended consequence we were also overriding
a version set by the jvmToolchain property.
So while users expected the jvmToolchain property
to set both jdk home & jdk target, we were quietly
overriding jdk target.

At the same time, Kotlin 1.7 sets the minimum
target version to 1.8 anyway, so our override
does not make sense with Kotlin 1.7+.

This commit removes overriding altogether.

Fixes #2511

* Update CHANGELOG.md

* Update CHANGELOG.md

* Update CHANGELOG.md

* Update Compose

* Update default ProGuard rules with changes from main branch

* Test Gradle plugin on relevant PRs (#2509)

* Update Gradle used in tooling subprojects

* Update Kotlin in Compose Gradle plugin

* Decrease verbosity of Gradle plugin tests

* Disable mac sign test

* Add workflow to test Gradle plugin

* Fix custom jdk tests on Linux

* Make Compose Gradle plugin build compatible with Configuration cache

* Print tests summary

* Remove unused code

* Refactor tests configuration

* Turn off parallel execution

* Try adding windows runner

* Turn off fail fast

* Fix Windows test issues

#2368

* Adjust default proguard rules

The following rule is needed to fix tests on Windows:
```
-dontwarn org.graalvm.compiler.core.aarch64.AArch64NodeMatchRules_MatchStatementSet*
```

Other rules are just to make builds less noisy.
Kotlin's `*.internal` packages often contain
bytecode, which triggers ProGuard's notes.
However, these notes are not actionable for
most users, so we can ignore notes by default.

#2393
# Conflicts:
#	gradle-plugins/gradle/wrapper/gradle-wrapper.properties

* Improve DSL for setting a custom Compose Plugin (#2527)

* Improve DSL for setting a custom Compose Plugin

Fixes #2459

Readme: #2526

1. Add `dependencies: Dependencies` extension that is accessible in `compose { }` block
2. Add `Dependencies.compiler` property that can return versions of Compose compiler used by the plugin:
```
compose {
    kotlinCompilerPlugin.set(dependencies.compiler.forKotlin("1.7.20"))
    //kotlinCompilerPlugin.set(dependencies.compiler.auto) // determined by applied version of Kotlin. It is a default.
}
```

3. Add ability to set arguments for Compose Compiler. Now we can write:
```
compose {
    kotlinCompilerPlugin.set(dependencies.compiler.forKotlin("1.7.20"))
    kotlinCompilerPluginArgs.add("suppressKotlinVersionCompatibilityCheck=1.7.21")
}
```

4. Remove checks for different targets

We had a separate check for JS, when we released 1.2.0. It doesn't support Kotlin 1.7.20 at that moment.

It is hard to refactor this feature in the new code, so I removed it. It is not needed now and it had an ugly code. When we will need it again, we'll write it again.

5. Remove the `compose.tests.androidx.compiler.version` property from gradle.properties and remove `defaultAndroidxCompilerEnvironment`

Because they are used only in one test, and it seems there is no reason to use it in another place in the future

* Discussions

* Update ComposeCompilerCompatability.kt (#2557)

* Update CHANGELOG.md

* 1.2.2-rc01

* Update Compose

* Update CHANGELOG.md

* Compose 1.2.2

* Remove shared.podspec

* Remove usages of deprecated Intellij APIs

Co-authored-by: Alexey Tsvetkov <alexey.tsvetkov@jetbrains.com>
Co-authored-by: Alexey Tsvetkov <654232+AlexeyTsvetkov@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@AlexeyTsvetkov AlexeyTsvetkov

@dima-avdeev-jb dima-avdeev-jb

Labels
bug Something isn't working desktop gradle Gradle plugin problems
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Remove JVM target version override JetBrains/compose-multiplatform
3 participants
@AlexeyTsvetkov @ksiarhey @dima-avdeev-jb