- 
                Notifications
    You must be signed in to change notification settings 
- Fork 237
Add support for Kotlin 2.2+ #1387
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: master
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 | 
|---|---|---|
|  | @@ -25,6 +25,9 @@ kt_bootstrap_library( | |
| "k2/checker/declaration/*.kt", | ||
| "k2/checker/expression/*.kt", | ||
| ]), | ||
| kotlinc_opts = { | ||
| 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. Should we just convert this to an array type so that we aren't messing around with dict key,values like this? 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. I don't have any strong opinion about that | ||
| "-Xcontext-parameters": "", | ||
| }, | ||
| visibility = ["//src:__subpackages__"], | ||
| deps = [ | ||
| "//kotlin/compiler:kotlin-compiler", | ||
|  | ||
| Original file line number | Diff line number | Diff line change | 
|---|---|---|
|  | @@ -16,23 +16,69 @@ abstract class BaseJdepsGenExtension( | |
| ) { | ||
| val directDeps = configuration.getList(JdepsGenConfigurationKeys.DIRECT_DEPENDENCIES) | ||
| val targetLabel = configuration.getNotNull(JdepsGenConfigurationKeys.TARGET_LABEL) | ||
| val explicitDeps = createDepsMap(explicitClassesCanonicalPaths) | ||
| val fullClasspath = configuration.getList(JdepsGenConfigurationKeys.FULL_CLASSPATH) | ||
|  | ||
| doWriteJdeps(directDeps, targetLabel, explicitDeps, implicitClassesCanonicalPaths) | ||
| // Create mapping from canonical paths to original classpath paths | ||
| val canonicalToClasspath = createCanonicalToClasspathMap(fullClasspath) | ||
|  | ||
| val explicitDeps = createDepsMap(explicitClassesCanonicalPaths, canonicalToClasspath) | ||
|  | ||
| doWriteJdeps( | ||
| directDeps, | ||
| targetLabel, | ||
| explicitDeps, | ||
| implicitClassesCanonicalPaths, | ||
| canonicalToClasspath, | ||
| ) | ||
|  | ||
| doStrictDeps(configuration, targetLabel, directDeps, explicitDeps) | ||
| } | ||
|  | ||
| /** | ||
| * Creates a mapping from canonical filesystem paths to original classpath paths. | ||
| * This ensures jdeps uses the same paths that were on the compiler's classpath. | ||
| * When multiple classpath entries point to the same canonical file, prefer runfiles paths. | ||
| */ | ||
| private fun createCanonicalToClasspathMap(classpathEntries: List<String>): Map<String, String> { | ||
| val mapping = mutableMapOf<String, String>() | ||
| 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. Are these jdeps changes related to this supporting Kotlin 2.2 or is this fixing something independent? 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. Supporting kotlin 2.2. I don't understand what exactly changed on kotlinc's side, but without these changes the tests fail | ||
|  | ||
| // Create canonical -> original mapping | ||
| // Process entries and prefer runfiles paths over execroot paths | ||
| classpathEntries.forEach { classpathPath -> | ||
| try { | ||
| val canonicalPath = File(classpathPath).canonicalPath | ||
| val existing = mapping[canonicalPath] | ||
| // Only update if no existing entry, or if current path is a runfiles path | ||
| // and existing is not | ||
| if (existing == null || | ||
| (classpathPath.contains("/runfiles/") && !existing.contains("/runfiles/")) | ||
| ) { | ||
| mapping[canonicalPath] = classpathPath | ||
| } | ||
| } catch (e: Exception) { | ||
| // If we can't resolve, just use the original path for both | ||
| mapping[classpathPath] = classpathPath | ||
| } | ||
| } | ||
| return mapping | ||
| } | ||
|  | ||
| /** | ||
| * Returns a map of jars to classes loaded from those jars. | ||
| * Uses the canonical-to-classpath mapping to preserve original classpath paths. | ||
| */ | ||
| private fun createDepsMap(classes: Set<String>): Map<String, List<String>> { | ||
| private fun createDepsMap( | ||
| classes: Set<String>, | ||
| canonicalToClasspath: Map<String, String>, | ||
| ): Map<String, List<String>> { | ||
| val jarsToClasses = mutableMapOf<String, MutableList<String>>() | ||
| classes.forEach { | ||
| val parts = it.split("!/") | ||
| val jarPath = parts[0] | ||
| if (jarPath.endsWith(".jar")) { | ||
| jarsToClasses.computeIfAbsent(jarPath) { ArrayList() }.add(parts[1]) | ||
| val canonicalJarPath = parts[0] | ||
| if (canonicalJarPath.endsWith(".jar")) { | ||
| // Map back to original classpath path | ||
| val classpathJarPath = canonicalToClasspath[canonicalJarPath] ?: canonicalJarPath | ||
| jarsToClasses.computeIfAbsent(classpathJarPath) { ArrayList() }.add(parts[1]) | ||
| } | ||
| } | ||
| return jarsToClasses | ||
|  | @@ -43,8 +89,9 @@ abstract class BaseJdepsGenExtension( | |
| targetLabel: String, | ||
| explicitDeps: Map<String, List<String>>, | ||
| implicitClassesCanonicalPaths: Set<String>, | ||
| canonicalToClasspath: Map<String, String>, | ||
| ) { | ||
| val implicitDeps = createDepsMap(implicitClassesCanonicalPaths) | ||
| val implicitDeps = createDepsMap(implicitClassesCanonicalPaths, canonicalToClasspath) | ||
|  | ||
| // Build and write out deps.proto | ||
| val jdepsOutput = configuration.getNotNull(JdepsGenConfigurationKeys.OUTPUT_JDEPS) | ||
|  | ||
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.
What's the reasoning behind this change?