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.

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

Only pull compile dependencies during compilation #4028

Merged
merged 7 commits into from
Nov 28, 2024

Conversation

alexarchambault
Copy link
Collaborator

@alexarchambault alexarchambault commented Nov 25, 2024

Maven offers a way to pull some dependencies only at runtime, via Maven scopes.

Roughly speaking, in Maven, dependencies can be added

  • only for compilation (provided scope)
  • for compilation and runtime (compile scope - compile is for both compile-time and runtime)
  • only at runtime (runtime scope)

This matters during dependency management: coursier can pull runtime dependencies or not, for example.

Mill already knows about compile-time versus runtime, with compileIvyDeps / ivyDeps / runIvyDeps, which are equivalent to Maven scopes provided / compile / runtime.

The PR here asks coursier not to pull runtime dependencies in Mill class paths made for compile-time, like compileClasspath.

Maven offers to add dependencies only at runtime, with its runtime scope.
This tries to respect that, by not pulling purely runtime dependencies
during compilation.
@alexarchambault

This comment was marked as outdated.

@alexarchambault

This comment was marked as outdated.

@alexarchambault alexarchambault marked this pull request as ready for review November 27, 2024 15:24
@alexarchambault

This comment was marked as outdated.

@lihaoyi
Copy link
Member

lihaoyi commented Nov 27, 2024

@alexarchambault could you elaborate a bit on the problem in the PR description and how you are solving it? Some of the code changes seem pretty non-obvious to me

@@ -210,7 +210,7 @@ trait KotlinJsModule extends KotlinModule { outer =>
outputMode = binaryKindToOutputMode(kotlinJsBinaryKind()),
irClasspath = Some(compile().classes),
allKotlinSourceFiles = Seq.empty,
librariesClasspath = compileClasspath(),
librariesClasspath = upstreamAssemblyClasspath(),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

e.g. what does this have to do with compile time deps?

Copy link
Collaborator Author

@alexarchambault alexarchambault Nov 27, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just opened #4039 with solely this change, and more comments / explanations. Basically, the PR here aims at bringing less JARs in class paths made for compilation (like compileClasspath), while the same JARs as currently should be around at runtime.

The linker needs a class path for runtime, so we pass upstreamAssemblyClasspath instead (that contains all dependencies JARs)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we could have a new dedicated target, like runDependencyClasspath - with all dependency JARs, including JARs made for runtime, but excluding the current module classes. Or maybe we could just rename upstreamAssemblyClasspath, given it can have a broader uses.

@alexarchambault
Copy link
Collaborator Author

alexarchambault commented Nov 27, 2024

To expand the PR description, Android dependencies tend to pull more dependencies at runtime, for example.

Compile-time:

$ cs resolve androidx.compose.animation:animation-core:1.1.1 androidx.compose.ui:ui:1.1.1 -r google -c compile
androidx.annotation:annotation:1.2.0:compile
androidx.compose.animation:animation-core:1.1.1:compile
androidx.compose.runtime:runtime:1.1.1:compile
androidx.compose.runtime:runtime-saveable:1.1.1:compile
androidx.compose.ui:ui:1.1.1:compile
androidx.compose.ui:ui-geometry:1.1.1:compile
androidx.compose.ui:ui-graphics:1.1.1:compile
androidx.compose.ui:ui-text:1.1.1:compile
androidx.compose.ui:ui-unit:1.1.1:compile
org.jetbrains:annotations:13.0:compile
org.jetbrains.kotlin:kotlin-stdlib:1.5.30:compile
org.jetbrains.kotlin:kotlin-stdlib-common:1.5.30:compile
org.jetbrains.kotlin:kotlin-stdlib-jdk7:1.5.30:compile
org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.5.30:compile
org.jetbrains.kotlinx:kotlinx-coroutines-android:1.5.2:compile
org.jetbrains.kotlinx:kotlinx-coroutines-core:1.5.2:compile
org.jetbrains.kotlinx:kotlinx-coroutines-core-jvm:1.5.2:compile

Runtime:

$ cs resolve androidx.compose.animation:animation-core:1.1.1 androidx.compose.ui:ui:1.1.1 -r google
androidx.annotation:annotation:1.2.0:default
androidx.arch.core:core-common:2.1.0:default
androidx.arch.core:core-runtime:2.1.0:default
androidx.autofill:autofill:1.0.0:default
androidx.collection:collection:1.1.0:default
androidx.compose.animation:animation-core:1.1.1:default
androidx.compose.runtime:runtime:1.1.1:default
androidx.compose.runtime:runtime-saveable:1.1.1:default
androidx.compose.ui:ui:1.1.1:default
androidx.compose.ui:ui-geometry:1.1.1:default
androidx.compose.ui:ui-graphics:1.1.1:default
androidx.compose.ui:ui-text:1.1.1:default
androidx.compose.ui:ui-unit:1.1.1:default
androidx.compose.ui:ui-util:1.1.1:default
androidx.core:core:1.5.0:default
androidx.lifecycle:lifecycle-common:2.3.0:default
androidx.lifecycle:lifecycle-common-java8:2.3.0:default
androidx.lifecycle:lifecycle-runtime:2.3.0:default
androidx.lifecycle:lifecycle-viewmodel:2.3.0:default
androidx.profileinstaller:profileinstaller:1.1.0:default
androidx.savedstate:savedstate:1.1.0:default
androidx.startup:startup-runtime:1.0.0:default
androidx.tracing:tracing:1.0.0:default
androidx.versionedparcelable:versionedparcelable:1.1.1:default
org.jetbrains:annotations:13.0:default
org.jetbrains.kotlin:kotlin-stdlib:1.6.10:default
org.jetbrains.kotlin:kotlin-stdlib-common:1.6.10:default
org.jetbrains.kotlin:kotlin-stdlib-jdk7:1.5.30:default
org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.5.30:default
org.jetbrains.kotlinx:kotlinx-coroutines-android:1.5.2:default
org.jetbrains.kotlinx:kotlinx-coroutines-core:1.5.2:default
org.jetbrains.kotlinx:kotlinx-coroutines-core-jvm:1.5.2:default

The PR here only passes the first set of dependencies to javac / kotlinc

lihaoyi pushed a commit that referenced this pull request Nov 28, 2024
The current code passes a class path made for compile time to the Kotlin
JS linker. While it's not a problem with the current main branch that
doesn't really make a difference between compile and runtime class
paths, this will be a problem with
#4028, that can bring less JARs
in `compileClasspath` (while all JARs will be around in runtime targets,
like `resolvedRunIvyDeps`, like currently)
@lihaoyi lihaoyi merged commit 5f57498 into com-lihaoyi:main Nov 28, 2024
27 checks passed
@lefou lefou added this to the 0.12.4 milestone Nov 28, 2024
jodersky pushed a commit to jodersky/mill that referenced this pull request Jan 14, 2025
The current code passes a class path made for compile time to the Kotlin
JS linker. While it's not a problem with the current main branch that
doesn't really make a difference between compile and runtime class
paths, this will be a problem with
com-lihaoyi#4028, that can bring less JARs
in `compileClasspath` (while all JARs will be around in runtime targets,
like `resolvedRunIvyDeps`, like currently)
jodersky pushed a commit to jodersky/mill that referenced this pull request Jan 14, 2025
Maven offers a way to pull some dependencies only at runtime, via [Maven
scopes](https://maven.apache.org/guides/introduction/introduction-to-dependency-mechanism.html#Dependency_Scope).

Roughly speaking, in Maven, dependencies can be added
- only for compilation (`provided` scope)
- for compilation and runtime (`compile` scope - `compile` is for both
compile-time and runtime)
- only at runtime (`runtime` scope)

This matters during dependency management: coursier can pull runtime
dependencies or not, for example.

Mill already knows about compile-time versus runtime, with
`compileIvyDeps` / `ivyDeps` / `runIvyDeps`, which are equivalent to
Maven scopes `provided` / `compile` / `runtime`.

The PR here asks coursier not to pull runtime dependencies in Mill class
paths made for compile-time, like `compileClasspath`.

---------

Co-authored-by: Li Haoyi <haoyi.sg@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants