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
Remove embedded-kotlin-repo and pin Kotlin dependencies with constraints #10701
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
bamboo
approved these changes
Sep 12, 2019
.../integTest/kotlin/org/gradle/kotlin/dsl/integration/ProjectSchemaAccessorsIntegrationTest.kt
Show resolved
Hide resolved
eskatos
force-pushed
the
eskatos/kotlin-dsl/no-embedded-kotlin-repo
branch
from
September 13, 2019 17:22
3b29a3a
to
757757a
Compare
bamboo
reviewed
Sep 13, 2019
subprojects/kotlin-dsl/src/main/kotlin/org/gradle/kotlin/dsl/support/EmbeddedKotlinProvider.kt
Outdated
Show resolved
Hide resolved
this is an opportunistic change Signed-off-by: Paul Merlin <paul@gradle.com>
It was based on ClientModule dependencies that caused more troubles than anything. The main one being that the artificial dependency graph needed to be kept up to date with each Kotlin version. Moreover since several Kotlin versions, declaring a repository is necessary in any case. This change simplifies the code and should make the runtime a bit faster. However, the Kotlin dependencies when applying the `kotlin-dsl` plugin are now required to be downloaded. This isn't a big change and should only impact people using the `kotlin-dsl` plugin without depending on Kotlin in other parts of their build. This could be alleviated in the future if dependency resolution considers the Gradle install/distro as a source of artifacts in the same way it uses maven local. Along the way, the pinning of Kotlin dependencies to the embedded version for the build scripts classpath has been moved from a resolution rule to proper, faster, dependency constraints. Signed-off-by: Paul Merlin <paul@gradle.com>
Signed-off-by: Paul Merlin <paul@gradle.com>
The `embedded-kotlin` plugin now requires a repository to be declared. Signed-off-by: Paul Merlin <paul@gradle.com>
It was missing the repo declaration already required with the `kotlin-dsl` plugin. Signed-off-by: Paul Merlin <paul@gradle.com>
Signed-off-by: Paul Merlin <paul@gradle.com>
Signed-off-by: Paul Merlin <paul@gradle.com>
Signed-off-by: Paul Merlin <paul@gradle.com>
Signed-off-by: Paul Merlin <paul@gradle.com>
Signed-off-by: Paul Merlin <paul@gradle.com>
Signed-off-by: Paul Merlin <paul@gradle.com>
Signed-off-by: Paul Merlin <paul@gradle.com>
Signed-off-by: Paul Merlin <paul@gradle.com>
eskatos
force-pushed
the
eskatos/kotlin-dsl/no-embedded-kotlin-repo
branch
from
September 14, 2019 09:43
757757a
to
698c307
Compare
Signed-off-by: Paul Merlin <paul@gradle.com>
eskatos
force-pushed
the
eskatos/kotlin-dsl/no-embedded-kotlin-repo
branch
from
September 14, 2019 09:47
698c307
to
bad68ed
Compare
+1 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The embedded repo was created without metadata, this is incorrect and may cause hard to troubleshoot issues. This PR simply removes it, meaning that builds using either the
embedded-kotlin
or thekotlin-dsl
plugin will now have to resolvekotlin-stdlib-jdk8
,kotlin-reflect
and their transitive dependencies from a remote repository. That makes for ~4MB.Removing that repo which included all the kotin dependencies from the distro was writing ~40MB on first use. This can now be skipped. The extra ~36MB, mostly the embedded kotlin compiler, allowed to avoid downloading it when the
kotlin-jvm
Gradle plugin was applied with the exact same version as embedded. But providing wrong artifact metadata. This PR makes things more correct at the expense of the extra download in some conditions. The extra download could be avoided in the future with #10744This PR also changes the way
kotlin-stdlib-jdk8
,kotlin-reflect
and their transitive dependencies, are pinned to the embedded kotlin version on build scripts classpath, from using a resolution rule to constraints that are more easy to reason with and can explain the reason when facing a conflict.See #10745