-
Notifications
You must be signed in to change notification settings - Fork 0
fix: compute longest common prefix for sibling packages in empty source roots #3
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: develop
Are you sure you want to change the base?
Changes from all commits
805c303
fed430e
9921802
931ef78
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 |
|---|---|---|
|
|
@@ -358,6 +358,8 @@ public CompileAndRuntimeClasspath compute() throws CoreException { | |
| var libraryArtifact = new LibraryArtifact(artifact, classJar, srcJars); | ||
| var targetKey = targetLabel != null ? TargetKey.forPlainTarget(targetLabel) : null; | ||
| library = new BlazeJarLibrary(libraryArtifact, targetKey); | ||
| // Register back to avoid repeated fallback for the same jar across multiple targets | ||
| aspectsInfo.registerFallbackLibrary(artifact.getRelativePath(), library); | ||
|
Comment on lines
+361
to
+362
Owner
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. After discovering a library through fallback logic (filesystem scan), it's registered back via registerFallbackLibrary() so subsequent lookups for the same jar hit the cache instead of repeating the expensive fallback. Why: Multiple targets may reference the same jar in their jdeps. Without caching, each occurrence triggers a fresh fallback scan, causing redundant computation. |
||
| } | ||
| var entry = resolveLibrary(library); | ||
| if (entry != null) { | ||
|
|
@@ -375,9 +377,14 @@ public CompileAndRuntimeClasspath compute() throws CoreException { | |
| classpathBuilder.addCompileEntry(entry); | ||
| } else { | ||
| entry.getAccessRules().add(new AccessRule(PATTERN_EVERYTHING, IAccessRule.K_ACCESSIBLE)); | ||
| // Export EXPLICIT jdeps entries so downstream projects can resolve types | ||
| // referenced in this project's public API. This addresses ECJ vs javac differences: | ||
| // ECJ aggressively resolves all types in the API chain, while javac may not | ||
| // record them in jdeps of downstream targets. | ||
| entry.setExported(true); | ||
|
Comment on lines
+378
to
+382
Owner
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. Why: Eclipse uses ECJ (Eclipse Compiler for Java), which differs from javac in type resolution behavior. ECJ aggressively resolves all types in the public API chain — if project A's public method returns a type from project B, ECJ requires B on the classpath when compiling project C (which depends on A). However, javac may not record B as a dependency in C's jdeps. By marking explicit jdeps entries as exported, downstream projects can transitively see these dependencies, resolving ECJ compilation errors. |
||
| classpathBuilder.addCompileEntry(entry); | ||
| } | ||
| } else if (LOG.isDebugEnabled()) { | ||
| } else { | ||
| LOG.warn("Unable to resolve compile jar: {}", jdepsDependency); | ||
| } | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -261,6 +261,18 @@ private void addLibrary(BlazeJarLibrary library) { | |
| var classJar = libraryArtifact.getClassJar(); | ||
| if (classJar != null) { | ||
| libraryByJdepsRootRelativePath.put(classJar.getRelativePath(), library); | ||
|
|
||
| // When there is no interfaceJar (e.g., libraries discovered from runtime classpath), | ||
| // also index under potential ijar/hjar paths so that jdeps lookups can find them. | ||
| // jdeps files typically reference ijar/hjar paths, not class jar paths. | ||
| if (interfaceJar == null) { | ||
| var classPath = classJar.getRelativePath(); | ||
| if (classPath.endsWith(".jar")) { | ||
| var base = classPath.substring(0, classPath.length() - ".jar".length()); | ||
| libraryByJdepsRootRelativePath.putIfAbsent(base + "-ijar.jar", library); | ||
| libraryByJdepsRootRelativePath.putIfAbsent(base + "-hjar.jar", library); | ||
| } | ||
| } | ||
|
Comment on lines
+264
to
+275
Owner
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. When registering a library that has only a classJar (no interfaceJar), the system now also indexes it under -ijar.jar and -hjar.jar path variants. Why: Bazel's jdeps files typically reference ijar/hjar paths, not raw class jar paths. Libraries discovered from runtime classpath often lack interface jars, causing jdeps lookups to miss. Pre-indexing these variants ensures direct hits without fallback. |
||
| } | ||
| } | ||
|
|
||
|
|
@@ -276,6 +288,14 @@ public BlazeJarLibrary getLibraryByJdepsRootRelativePath(String relativePath) { | |
| return libraryByJdepsRootRelativePath.get(relativePath); | ||
| } | ||
|
|
||
| /** | ||
| * Registers a fallback library discovered during jdeps resolution so that subsequent lookups for the same path | ||
| * don't need to repeat the fallback logic. | ||
| */ | ||
| public void registerFallbackLibrary(String relativePath, BlazeJarLibrary library) { | ||
| libraryByJdepsRootRelativePath.putIfAbsent(relativePath, library); | ||
| } | ||
|
|
||
| /** | ||
| * Checks if a JAR file comes from an external Bazel repository. | ||
| * <p> | ||
|
|
||
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.
Longest common prefix computation: Previously, when detected Java packages were siblings (e.g., com/foo/bar/dto and com/foo/bar/merge) rather than nested, the method returned null, causing source root detection to fail. The fix computes the longest common path prefix (e.g., com/foo/bar) to correctly infer the source root.
Why: Non-standard Bazel structures may have sources spread across sibling packages. The old logic only handled parent-child relationships, not siblings, causing Eclipse to fail at identifying source directories.