Drop functionality related to Coursier#888
Merged
Merged
Conversation
This command indexed an individual Maven artifact by fetching its sources via Coursier, unzipping the -sources.jar, and running the standard indexer over the extracted tree. It is no longer needed. Removed: - IndexDependencyCommand (and its registration in ScipJava) - LibrarySnapshotGenerator / LibrarySnapshotSuite, the only caller of the command, along with the saved exposed-core snapshot output under tests/snapshots/src/main/generated/org/jetbrains/... - JavaVersion.classfileJvmVersion, roundToNearestStableRelease, classfileMajorVersion, JAVA17_VERSION, JAVA0_MAJOR_VERSION, CLASS_PATTERN, JAR_PATTERN — only used by IndexDependencyCommand. The JavaVersion class itself is kept for PackageTable and JavaVersionSuite. - CONTRIBUTING.md entry for LibrarySnapshotSuite.
…enerator With only MinimizedSnapshotScipGenerator left, the multi-generator abstractions are no longer earning their keep. Removed: - SnapshotGenerator trait (single implementation, no abstract consumer) - AggregateSnapshotGenerator (wrapped a one-element list) - SemanticdbJavacSnapshotGenerator (defined but never referenced) - SnapshotHandler.withoutFinishedEvent and the `self =>` alias (only used by the aggregate) - MinimizedSnapshotScipGenerator.onFinished (dead empty method) Collapsed: - SnapshotSuite abstract base + MinimizedSnapshotScipSuite subclass into a single concrete MinimizedSnapshotScipSuite (file renamed to match) - SaveSnapshots Map-based dispatch into a direct generator call, with handler.onFinished(context) invoked explicitly to preserve the stale-snapshot cleanup that the aggregate used to drive.
ScipBuildTool no longer fetches anything from the network.
- Maven dependency resolution: remove Dependencies.scala and the
'dependencies' field handling from compile(). Callers must
pre-resolve and populate the 'classpath' field; passing
non-empty 'dependencies' produces a hard error explaining
the migration path.
- Kotlin MPP -common JAR fetch: removed (Kotlin MPP indexing now
produces only the JVM half).
- JDK provisioning: javacPathViaCoursier deleted. javacPath now
requires either 'javaHome' in scip-java.json or JAVA_HOME in
the environment and raises a clear error otherwise.
- build.sbt: drop io.get-coursier:coursier{,-jvm} from scip-java
and the test-side coursier dep. scala-xml stays (still used
by ClasspathEntry).
- Tests: replace the two coursier-env-var tests and the two
resolution-dependent checkBuild cases with two focused tests:
rejects-dependencies-field and compiles-with-empty-classpath.
Bazel, Maven, and Gradle indexing paths are unaffected.
jupblb
force-pushed
the
michal/drop-coursier
branch
from
9e7245d to
4c40d59
Compare
The minimized17 and minimized21 subprojects compiled the same shared sources under JDK 17/21 via JavaToolchainPlugin, which shelled out to 'cs java-home' to obtain those JDKs on demand. The CI matrix (Tests (11)/(17)/(21)) already exercises the plugin under each JDK, so these subprojects were redundant. Removing them eliminates the build's last dependency on coursier.
JavaToolchainPlugin (now removed) used to set fork := true globally,
which made unit tests run in a forked JVM. Without forking, the unit
tests' TestCompiler reads System.getProperty("java.class.path") from
sbt's JVM, which doesn't include the semanticdb-javac plugin, so
compileSemanticdb produces no document and Symtab(null) NPEs.
Add Test / fork := true to the shared testSettings so unit and
snapshots projects fork like buildTools already does.
jupblb
force-pushed
the
michal/drop-coursier
branch
3 times, most recently
from
61fec1c to
2b14b77
Compare
JavaToolchainPlugin (now removed) used to mask three JDK-17+ issues in
the javacPlugin project by pinning its compile JDK to 11, stripping flags
from the doc task, and forcing --release 11 on all Java code. With the
plugin gone, all three surface:
1. JDK 14 added Plugin.autoStart(), so JDK 17/21 javac eagerly enumerates
ServiceLoader<Plugin> providers from the processor path (or, if absent,
the compile classpath). During incremental compilation our own
META-INF/services/com.sun.source.util.Plugin descriptor sits on the
classpath but SemanticdbPlugin.class isn't built yet, so ServiceLoader
throws 'Provider com.sourcegraph.semanticdb_javac.SemanticdbPlugin not
found'.
Fix: pass an explicit empty -processorpath to javac so it doesn't scan
our own output directory for Plugin providers. The descriptor stays in
src/main/resources/ so that internal sbt consumers (e.g. the
'minimized' test project that depends on javacPlugin via dependsOn)
and the Bazel build can still find it.
2. javadoc rejects the '-g' flag that was previously added via
'javacOptions += "-g"'. The old plugin worked around this with
'(doc / javacOptions) --= List("-g")'.
Fix: scope '-g' (and the empty -processorpath from #1) to
'Compile / compile / javacOptions' so neither flag reaches the doc
task. (Plain 'Compile / javacOptions' is inherited by 'Compile / doc'
as well.)
3. sbt-assembly's shader ships an older ASM that cannot read class major
version 61 (JDK 17). When sbt runs under JDK 17/21 the fat jar ends up
with SemanticdbVisitor (and other classes that hit protobuf types)
silently un-shaded, so at runtime javac fails with
'NoClassDefFoundError: com/google/protobuf/ProtocolMessageEnum'.
Fix: add 'Compile / javacOptions ++= Seq("--release", "11")' to
javaOnlySettings so Java code always emits class version 55, which the
shader handles, regardless of which JDK runs sbt.
jupblb
force-pushed
the
michal/drop-coursier
branch
from
2b14b77 to
1453de7
Compare
After dropping JavaToolchainPlugin the Tests matrix (11/17/21) started to actually exercise the matrix JDK instead of silently pinning every compile to JDK 11. Three things needed real fixes: 1. In-process javac tests (JavacClassesDirectorySuite, TestCompiler) reflect into com.sun.tools.javac.api.BasicJavacTask#getContext, which is closed on JDK 17+ unless the host JVM is launched with --add-exports. Add the existing javacModuleOptions (sans -J) to Test / javaOptions in the shared testSettings so every forked test JVM gets the required exports. 2. SemanticdbVisitor#scan inherits TreePathScanner's traversal, which on JDK 17+ recurses into the identifiers of 'package X.Y;' and emits an extra self-reference for the package declaration. JDK 11 does not. Override visitPackage to stop at the package node, so semanticdb output is stable across JDKs and matches the long-standing JDK 8/11 behavior. 3. The runtime JDK leaks into indexer output as 'jdk N' inside SCIP symbols (JdkPackage.forRuntime). Normalize 'jdk \d+' -> 'jdk N' in the snapshot assertion + save handlers and the inline expected string in SnapshotCommandSuite, then bulk-replace the existing 'jdk 11' literals in committed snapshots. Snapshots now match regardless of the indexer's runtime JDK. Also pin Java bytecode to release 11 in semanticdbKotlinc — its protobuf-generated Java code feeds sbt-assembly's shader which can't parse class major 61 emitted by JDK 17 javac and was silently skipping shading, breaking the shaded fat jar consumed by snapshot tests.
jupblb
force-pushed
the
michal/drop-coursier
branch
from
6ca8486 to
425e7c4
Compare
byte-buddy 1.11.9 (2021) predates JDK 21 support, so the SemanticdbAgent silently fails to instrument GradleDefaultJvmLanguageCompileSpec and GradleJavaCompilerArgumentsBuilder under JDK 21, causing 6 buildTools tests to fail. Bumping both byte-buddy and byte-buddy-agent to 1.15.11 (matching versions) fixes the failures and aligns the runtime byte-buddy with the already-newer byte-buddy-agent.
A single dot reads more naturally as a placeholder ('any value') than a
literal 'N', without looking like a template variable.
…operty Replaces the regex-based 'jdk N' / 'jdk .' snapshot normalization with a proper plumbing knob: JdkPackage.forRuntime() now honors the scip.jdk.version system property before falling back to Runtime.version().feature(). The unit and snapshots test projects set -Dscip.jdk.version=11, so: - regenerated goldens contain a real 'jdk 11' instead of a placeholder - the on-disk snapshots match across JDK 11/17/21 without any comparison-time normalization - SnapshotNormalizer and the save/assert-side mutation can be deleted
It was accidentally committed in the previous change; it's intended to stay as a local-only note.
- Move -Dscip.jdk.version=11 from unit/snapshots project blocks into testSettings; snapshots/run sets the same property programmatically in SaveSnapshots.main instead of via run/fork + run/javaOptions. - Trim verbose multi-line comments in javacPlugin (-g + empty processorpath), javaOnlySettings (--release 11), and semanticdbKotlinc (--release 11) to the essential one-line rationale.
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
No description provided.