Enable cross-repository navigation between Gradle/Maven codebases #616
Conversation
Previously, scip-java only allowed cross-index (or cross-repo) navigation between symbols that are compiled to classfiles inside jar files. This behavior mixed an intentional design decision with an accidental implementation detail. The intentional design decision was that the ideal cross-repo navigation experience is from repositories to packages. To achieve this behavior, scip-java didn't assign a package name and package version to symbols that are compiled to directory entries on the classpath. The accidental implementation detail was that Gradle, Maven, and sbt compile to directories while packages only have jar files. This commit changes the behavior to treat symbols the same regardless if they are compiled to jar files or directories. The accidental implemenation detail was a hack, and this behavior has come up as a surprise in at least twice cases of trying to integrate scip-java with big propriatery codebases. To get the default old behavior, the flag `--allow-exporting-global-symbols-from-directory-entries` can be set to false.
olafurpg
changed the title
scip-java/src/main/scala/com/sourcegraph/scip_java/commands/IndexSemanticdbCommand.scala
Outdated
Show resolved
Hide resolved
| /* emitInverseRelationships */ true, | ||
| /* allowEmptyIndex */ true); | ||
| /* allowEmptyIndex */ true, | ||
| /* indexDirectoryEntries */ false // because Bazel only compiles to jar files. |
There was a problem hiding this comment.
nit (for future refactor): this many boolean params in a row is a bit annoying, and we can't use named parameters with java class
At some point I think we should generate a builder for this options class to make things clearer.
There was a problem hiding this comment.
Agreed. Builders are great when using Java.
There was a problem hiding this comment.
We can't used named arguments from Scala if the definition is in Java.
There was a problem hiding this comment.
The problem with builders is that we can't make the fields final, but it's not a problem here. We could simplify a lot by just providing sensible default values
semanticdb-gradle-plugin/src/main/scala/SemanticdbGradlePlugin.scala
Outdated
Show resolved
Hide resolved
scip-java/src/main/scala/com/sourcegraph/scip_java/buildtools/ClasspathEntry.scala
Show resolved
Hide resolved
olafurpg
force-pushed
the
olafurpg/classes-directory
branch
from
3b76e7a to
e313c2e
Compare
| classesDirectory <- project | ||
| .getExtensions() | ||
| .getByType(classOf[SourceSetContainer]) | ||
| .getByName("main") |
There was a problem hiding this comment.
I guess to handle test sources here we'd need to modify the artifact id or something, given that the descriptor doesn't have a notion of classifier like maven dependencies have
There was a problem hiding this comment.
Is it common to publish test sources? We can support the test scope as long as there is a reliable way to know the artifact ID of the test config
There was a problem hiding this comment.
I've seen set ups where a library publishes a testkit from its tests, but now that I think about it - it's frequently discouraged and doesn't work with the rest of tooling: https://github.com/scalacenter/bloop/pulls?q=is%3Apr+author%3Akeynmol+is%3Aclosed+test-jar
So let's ignore this for now until there's a clear need.
There was a problem hiding this comment.
We can probably support it if there's demand and there's support to extract the right information
| @tailrec | ||
| def loop(dir: Path): Option[ClasspathEntry] = { | ||
| if (dir == null || !dir.startsWith(sourceroot)) | ||
| return None |
There was a problem hiding this comment.
nit: this could easily be if (...) None else ... match
Previously, scip-java only allowed cross-index (or cross-repo) navigation between symbols that are compiled to classfiles inside jar files. The practical implication of this was that cross-repo navigation never worked between Gradle/Maven projects even if one of the projects defined symbols that the other project used.
This behavior mixed an intentional design decision with an accidental implementation detail. The intentional design decision was that the ideal cross-repo navigation experience is from repositories to packages. To achieve this behavior, scip-java didn't assign a package name and package version to symbols that are compiled to directory entries on the classpath. The accidental implementation detail was that Gradle, Maven, and sbt compile to directories while packages only have jar files.
This commit changes the behavior to treat symbols the same regardless if they are compiled to jar files or directories. The accidental implemenation detail was a hack, and this behavior has come up as a surprise in at least twice cases of trying to integrate scip-java with big propriatery codebases. To get the default old behavior, the flag
--allow-exporting-global-symbols-from-directory-entriescan be set to false.Test plan
See updated Gradle and Maven tests.