From 41610c785267a9fd449c7a12ddc94176bce1b9ef Mon Sep 17 00:00:00 2001 From: jupblb Date: Fri, 29 May 2026 14:08:39 +0200 Subject: [PATCH] Derive JDK version in stdlib symbols from bytecode target PackageTable previously baked the indexer's runtime JDK version into every stdlib SCIP symbol (e.g. 'jdk 11 java/lang/String#'), so the embedded version varied with whichever JVM happened to run scip-java in CI. Source it instead from the project's bytecode target: - JdkPackage gains fromClassfile/fromDirectory/fromJar/fromPath, all going through a single Files.walk traversal (jars are mounted as a NIO FileSystem). META-INF/versions/ entries are skipped so the result reflects the base bytecode target of multi-release jars. - ScipSemanticdbOptions gets a jdkPackage field; PackageTable uses it in place of JdkPackage.forRuntime(). - IndexSemanticdbCommand resolves the value from any .class under a project-owned classes dir (the -d entry parsed from javacopts.txt, which Maven and Gradle emit through scip-java's wrappers), then the sibling 'classes/' of the targetroot's parent (sbt/Maven layout), falling back to JdkPackage.forRuntime() when nothing is found. - BazelBuildTool autodetects from its output jars. - The scip.jdk.version system-property escape hatch and the test JVM pin in build.sbt/SaveSnapshots are gone; snapshot tests now autodetect jdk 11 from the minimized fixture's classfiles under JDK 11/17/21. --- build.sbt | 3 - .../commands/IndexSemanticdbCommand.scala | 37 +++++++- .../scip_semanticdb/BazelBuildTool.java | 13 ++- .../scip_semanticdb/JdkPackage.java | 92 ++++++++++++++++-- .../scip_semanticdb/PackageTable.java | 2 +- .../ScipSemanticdbOptions.java | 5 +- .../src/main/scala/tests/SaveSnapshots.scala | 4 - .../test/scala/tests/JdkPackageSuite.scala | 94 +++++++++++++++++++ 8 files changed, 229 insertions(+), 21 deletions(-) diff --git a/build.sbt b/build.sbt index 9430dd38..52adc341 100644 --- a/build.sbt +++ b/build.sbt @@ -666,9 +666,6 @@ val testSettings = List( // javac via reflection (e.g. JavacClassesDirectorySuite, TestCompiler). // On JDK 17+ this is required or the reflective access fails. Test / javaOptions ++= javacModuleOptions.map(_.stripPrefix("-J")), - // Pin the JDK version embedded in stdlib SCIP symbols (e.g. `jdk 11 - // java/lang/String#`) so snapshots are stable across JDK 11/17/21. - Test / javaOptions += "-Dscip.jdk.version=11", testFrameworks := List(TestFrameworks.MUnit), testOptions ++= { if (!(Test / testForkedParallel).value) diff --git a/scip-java/src/main/scala/com/sourcegraph/scip_java/commands/IndexSemanticdbCommand.scala b/scip-java/src/main/scala/com/sourcegraph/scip_java/commands/IndexSemanticdbCommand.scala index 194c1a85..e5e08970 100644 --- a/scip-java/src/main/scala/com/sourcegraph/scip_java/commands/IndexSemanticdbCommand.scala +++ b/scip-java/src/main/scala/com/sourcegraph/scip_java/commands/IndexSemanticdbCommand.scala @@ -1,5 +1,6 @@ package com.sourcegraph.scip_java.commands +import java.nio.file.Files import java.nio.file.Path import java.nio.file.Paths import java.util.concurrent.TimeUnit @@ -11,6 +12,7 @@ import com.sourcegraph.io.AbsolutePath import com.sourcegraph.scip_java.BuildInfo import com.sourcegraph.scip_java.buildtools.ClasspathEntry import com.sourcegraph.scip_semanticdb.ConsoleScipSemanticdbReporter +import com.sourcegraph.scip_semanticdb.JdkPackage import com.sourcegraph.scip_semanticdb.ScipOutputFormat import com.sourcegraph.scip_semanticdb.ScipSemanticdb import com.sourcegraph.scip_semanticdb.ScipSemanticdbOptions @@ -89,6 +91,7 @@ final case class IndexSemanticdbCommand( ) .distinct .toList + val resolvedJdkPackage = resolveJdkPackage(packages) val options = new ScipSemanticdbOptions( absoluteTargetroots.asJava, @@ -106,7 +109,8 @@ final case class IndexSemanticdbCommand( packages.map(_.toPackageInformation).asJava, emitInverseRelationships, allowEmptyIndex, - allowExportingGlobalSymbolsFromDirectoryEntries + allowExportingGlobalSymbolsFromDirectoryEntries, + resolvedJdkPackage ) ScipSemanticdb.run(options) postPackages(packages) @@ -116,6 +120,37 @@ final case class IndexSemanticdbCommand( app.reporter.exitCode() } + /** + * Resolves the JDK version to embed in stdlib SCIP symbols by reading the + * bytecode major version of a produced classfile. Tries, in order: + * + * 1. Any `.class` under a project-owned classes directory (the `-d` + * directory parsed from `javacopts.txt`). This is the path Maven and + * Gradle take when driven by `scip-java index`. Dependency jars on + * the classpath are intentionally skipped — their bytecode target is + * unrelated to the project's. + * 2. Any `.class` under a sibling `classes/` directory of the + * targetroot's parent (a common filesystem convention for sbt and + * Maven projects: `target/classes` next to `target/meta` or + * `target/semanticdb-targetroot`). + * 3. `JdkPackage.forRuntime()` — the running JVM's version. + */ + private def resolveJdkPackage( + packages: List[ClasspathEntry] + ): JdkPackage = { + val classesDirs = + packages.iterator.map(_.entry).filter(Files.isDirectory(_)) + val siblingClassesDirs = + absoluteTargetroots + .iterator + .flatMap(t => Option(t.getParent).iterator) + .map(_.resolve("classes")) + (classesDirs ++ siblingClassesDirs) + .map(JdkPackage.fromPath) + .collectFirst { case opt if opt.isPresent => opt.get } + .getOrElse(JdkPackage.forRuntime()) + } + /** * If the PackageHub URL is configured, sends an HTTP POST request to register * the packages that are used in this codebase. diff --git a/scip-semanticdb/src/main/java/com/sourcegraph/scip_semanticdb/BazelBuildTool.java b/scip-semanticdb/src/main/java/com/sourcegraph/scip_semanticdb/BazelBuildTool.java index e5fb161f..515c31ca 100644 --- a/scip-semanticdb/src/main/java/com/sourcegraph/scip_semanticdb/BazelBuildTool.java +++ b/scip-semanticdb/src/main/java/com/sourcegraph/scip_semanticdb/BazelBuildTool.java @@ -52,6 +52,15 @@ public boolean hasErrors() { return this.hasErrors; } }; + JdkPackage jdkPackage = + options + .targetroots + .stream() + .map(JdkPackage::fromPath) + .filter(Optional::isPresent) + .map(Optional::get) + .findFirst() + .orElseGet(JdkPackage::forRuntime); ScipSemanticdbOptions scipOptions = new ScipSemanticdbOptions( options.targetroots, @@ -64,8 +73,8 @@ public boolean hasErrors() { mavenPackages, /* emitInverseRelationships */ true, /* allowEmptyIndex */ true, - /* indexDirectoryEntries */ false // because Bazel only compiles to jar files. - ); + /* indexDirectoryEntries */ false, // because Bazel only compiles to jar files. + jdkPackage); ScipSemanticdb.run(scipOptions); if (!scipOptions.reporter.hasErrors()) { diff --git a/scip-semanticdb/src/main/java/com/sourcegraph/scip_semanticdb/JdkPackage.java b/scip-semanticdb/src/main/java/com/sourcegraph/scip_semanticdb/JdkPackage.java index 93b1e3bb..88c8542e 100644 --- a/scip-semanticdb/src/main/java/com/sourcegraph/scip_semanticdb/JdkPackage.java +++ b/scip-semanticdb/src/main/java/com/sourcegraph/scip_semanticdb/JdkPackage.java @@ -1,5 +1,17 @@ package com.sourcegraph.scip_semanticdb; +import java.io.IOException; +import java.io.InputStream; +import java.net.URI; +import java.nio.ByteBuffer; +import java.nio.file.FileSystem; +import java.nio.file.FileSystems; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.Collections; +import java.util.Optional; +import java.util.stream.Stream; + public class JdkPackage extends Package { public final String version; @@ -7,16 +19,8 @@ public JdkPackage(String version) { this.version = version; } - /** - * Returns a {@link JdkPackage} for the currently running JVM, or the value of the {@code - * scip.jdk.version} system property if set. The override exists so that tests can produce - * JDK-independent snapshots. - */ + /** Returns a {@link JdkPackage} for the currently running JVM. */ public static JdkPackage forRuntime() { - String override = System.getProperty("scip.jdk.version"); - if (override != null && !override.isEmpty()) { - return new JdkPackage(override); - } return new JdkPackage(Integer.toString(Runtime.version().feature())); } @@ -26,6 +30,76 @@ public static JdkPackage parse(String version) { return new JdkPackage(dot < 0 ? version : version.substring(0, dot)); } + /** + * Reads the JDK version from the major version field of a classfile (bytes 6-7, big endian) + * mapped via {@code major - 44}. Returns empty if the file is not a valid classfile. + */ + public static Optional fromClassfile(Path classfile) { + try (InputStream in = Files.newInputStream(classfile)) { + return fromHeader(in.readNBytes(8)); + } catch (IOException e) { + return Optional.empty(); + } + } + + /** + * Walks {@code directory} (depth-bounded) for any {@code .class} file and returns the JDK + * version inferred from its major version field. Skips multi-release classfiles under + * {@code META-INF/versions/} so the result reflects the base bytecode target when walking + * a jar mounted as a {@link FileSystem}. Returns empty if the directory does not exist or + * contains no valid classfile. + */ + public static Optional fromDirectory(Path directory, int maxDepth) { + if (directory == null || !Files.isDirectory(directory)) return Optional.empty(); + try (Stream stream = Files.walk(directory, maxDepth)) { + return stream + .filter(p -> p.getFileName() != null && p.getFileName().toString().endsWith(".class")) + .filter(p -> !p.toString().replace('\\', '/').contains("/META-INF/versions/")) + .map(JdkPackage::fromClassfile) + .flatMap(Optional::stream) + .findFirst(); + } catch (IOException e) { + return Optional.empty(); + } + } + + /** + * Reads the JDK version from the first regular {@code .class} entry of a jar file by mounting + * it as a {@link FileSystem} and reusing {@link #fromDirectory}. + */ + public static Optional fromJar(Path jar) { + if (jar == null || !Files.isRegularFile(jar)) return Optional.empty(); + URI uri = URI.create("jar:" + jar.toUri()); + try (FileSystem fs = FileSystems.newFileSystem(uri, Collections.emptyMap())) { + return fromDirectory(fs.getPath("/"), Integer.MAX_VALUE); + } catch (IOException e) { + return Optional.empty(); + } + } + + /** + * Convenience dispatch: detects whether {@code path} is a jar, classfile, or directory and + * delegates to the appropriate reader. + */ + public static Optional fromPath(Path path) { + if (path == null) return Optional.empty(); + if (Files.isDirectory(path)) return fromDirectory(path, 64); + if (!Files.isRegularFile(path)) return Optional.empty(); + String name = path.getFileName().toString(); + if (name.endsWith(".jar")) return fromJar(path); + if (name.endsWith(".class")) return fromClassfile(path); + return Optional.empty(); + } + + private static Optional fromHeader(byte[] header) { + if (header.length < 8) return Optional.empty(); + ByteBuffer buf = ByteBuffer.wrap(header); + if (buf.getInt(0) != 0xCAFEBABE) return Optional.empty(); + int major = Short.toUnsignedInt(buf.getShort(6)); + if (major < 45) return Optional.empty(); + return Optional.of(new JdkPackage(Integer.toString(major - 44))); + } + @Override public String repoName() { return "jdk"; diff --git a/scip-semanticdb/src/main/java/com/sourcegraph/scip_semanticdb/PackageTable.java b/scip-semanticdb/src/main/java/com/sourcegraph/scip_semanticdb/PackageTable.java index 0e2d3347..736289d9 100644 --- a/scip-semanticdb/src/main/java/com/sourcegraph/scip_semanticdb/PackageTable.java +++ b/scip-semanticdb/src/main/java/com/sourcegraph/scip_semanticdb/PackageTable.java @@ -26,7 +26,7 @@ public class PackageTable { FileSystems.getDefault().getPathMatcher("glob:**.jar"); public PackageTable(ScipSemanticdbOptions options) throws IOException { - this.jdkPackage = JdkPackage.forRuntime(); + this.jdkPackage = options.jdkPackage; this.indexDirectoryEntries = options.allowExportingGlobalSymbolsFromDirectoryEntries; // NOTE: it's important that we index the JDK before maven packages. Some maven packages // redefine classes from the JDK and we want those maven packages to take precedence over diff --git a/scip-semanticdb/src/main/java/com/sourcegraph/scip_semanticdb/ScipSemanticdbOptions.java b/scip-semanticdb/src/main/java/com/sourcegraph/scip_semanticdb/ScipSemanticdbOptions.java index e598fe49..6055d255 100644 --- a/scip-semanticdb/src/main/java/com/sourcegraph/scip_semanticdb/ScipSemanticdbOptions.java +++ b/scip-semanticdb/src/main/java/com/sourcegraph/scip_semanticdb/ScipSemanticdbOptions.java @@ -18,6 +18,7 @@ public class ScipSemanticdbOptions { public final boolean emitInverseRelationships; public final boolean allowEmptyIndex; public final boolean allowExportingGlobalSymbolsFromDirectoryEntries; + public final JdkPackage jdkPackage; public ScipSemanticdbOptions( List targetroots, @@ -30,7 +31,8 @@ public ScipSemanticdbOptions( List packages, boolean emitInverseRelationships, boolean allowEmptyIndex, - boolean allowExportingGlobalSymbolsFromDirectoryEntries) { + boolean allowExportingGlobalSymbolsFromDirectoryEntries, + JdkPackage jdkPackage) { this.targetroots = targetroots; this.output = output; this.sourceroot = sourceroot; @@ -43,5 +45,6 @@ public ScipSemanticdbOptions( this.allowEmptyIndex = allowEmptyIndex; this.allowExportingGlobalSymbolsFromDirectoryEntries = allowExportingGlobalSymbolsFromDirectoryEntries; + this.jdkPackage = jdkPackage; } } diff --git a/tests/snapshots/src/main/scala/tests/SaveSnapshots.scala b/tests/snapshots/src/main/scala/tests/SaveSnapshots.scala index 3b1b3fdd..c24b7fb5 100644 --- a/tests/snapshots/src/main/scala/tests/SaveSnapshots.scala +++ b/tests/snapshots/src/main/scala/tests/SaveSnapshots.scala @@ -2,10 +2,6 @@ package tests object SaveSnapshots { def main(args: Array[String]): Unit = { - // Keep regenerated goldens stable across JDK 11/17/21 by pinning the - // JDK version embedded in stdlib SCIP symbols. Matches the - // `-Dscip.jdk.version=11` set on the test JVM in build.sbt. - System.setProperty("scip.jdk.version", "11") val context = SnapshotContext( tests.snapshots.BuildInfo.snapshotDirectory.toPath ) diff --git a/tests/unit/src/test/scala/tests/JdkPackageSuite.scala b/tests/unit/src/test/scala/tests/JdkPackageSuite.scala index 45bdabdf..2f69a4ab 100644 --- a/tests/unit/src/test/scala/tests/JdkPackageSuite.scala +++ b/tests/unit/src/test/scala/tests/JdkPackageSuite.scala @@ -1,5 +1,11 @@ package tests +import java.io.DataOutputStream +import java.nio.file.Files +import java.nio.file.Path +import java.util.jar.JarEntry +import java.util.jar.JarOutputStream + import com.sourcegraph.scip_semanticdb.JdkPackage import munit.FunSuite import munit.TestOptions @@ -15,4 +21,92 @@ class JdkPackageSuite extends FunSuite { checkVersion("11.0.9", "11") checkVersion("17.0.5", "17") + private def writeFakeClassfile(file: Path, major: Int): Path = { + Files.createDirectories(file.getParent) + val out = new DataOutputStream(Files.newOutputStream(file)) + try { + out.writeInt(0xCAFEBABE) + out.writeShort(0) // minor + out.writeShort(major) // major + } finally out.close() + file + } + + test("fromClassfile reads bytecode major version and maps major - 44") { + val tmp = Files.createTempDirectory("jdk-package-test") + try { + val classfile = writeFakeClassfile(tmp.resolve("Foo.class"), 55) + val pkg = JdkPackage.fromClassfile(classfile) + assert(pkg.isPresent, "expected a JdkPackage") + assertNoDiff(pkg.get.version, "11") + } finally Files.walk(tmp).sorted(java.util.Comparator.reverseOrder()).forEach(Files.delete) + } + + test("fromClassfile returns empty for non-classfile bytes") { + val tmp = Files.createTempDirectory("jdk-package-test") + try { + val bogus = tmp.resolve("Bogus.class") + Files.write(bogus, "not a classfile".getBytes()) + assert(!JdkPackage.fromClassfile(bogus).isPresent) + } finally Files.walk(tmp).sorted(java.util.Comparator.reverseOrder()).forEach(Files.delete) + } + + test("fromDirectory walks recursively for any classfile") { + val tmp = Files.createTempDirectory("jdk-package-test") + try { + writeFakeClassfile(tmp.resolve("a/b/c/Deep.class"), 61) // 61 - 44 = 17 + val pkg = JdkPackage.fromDirectory(tmp, 64) + assert(pkg.isPresent) + assertNoDiff(pkg.get.version, "17") + } finally Files.walk(tmp).sorted(java.util.Comparator.reverseOrder()).forEach(Files.delete) + } + + test("fromDirectory returns empty when no classfiles exist") { + val tmp = Files.createTempDirectory("jdk-package-test") + try { + Files.write(tmp.resolve("README.md"), "not a classfile".getBytes()) + assert(!JdkPackage.fromDirectory(tmp, 64).isPresent) + } finally Files.walk(tmp).sorted(java.util.Comparator.reverseOrder()).forEach(Files.delete) + } + + test("fromJar reads version from first regular classfile entry") { + val tmp = Files.createTempDirectory("jdk-package-test") + try { + val jar = tmp.resolve("test.jar") + val out = new JarOutputStream(Files.newOutputStream(jar)) + try { + // Multi-release entry under META-INF/versions should be skipped. + out.putNextEntry(new JarEntry("META-INF/versions/17/com/Newer.class")) + val mr = new DataOutputStream(out) + mr.writeInt(0xCAFEBABE) + mr.writeShort(0) + mr.writeShort(61) // would map to 17 + out.closeEntry() + + out.putNextEntry(new JarEntry("com/Base.class")) + val base = new DataOutputStream(out) + base.writeInt(0xCAFEBABE) + base.writeShort(0) + base.writeShort(55) // maps to 11 + out.closeEntry() + } finally out.close() + val pkg = JdkPackage.fromJar(jar) + assert(pkg.isPresent) + assertNoDiff(pkg.get.version, "11") + } finally Files.walk(tmp).sorted(java.util.Comparator.reverseOrder()).forEach(Files.delete) + } + + test("fromPath dispatches by file type") { + val tmp = Files.createTempDirectory("jdk-package-test") + try { + writeFakeClassfile(tmp.resolve("Foo.class"), 52) // 52 - 44 = 8 + val direct = JdkPackage.fromPath(tmp.resolve("Foo.class")) + assert(direct.isPresent) + assertNoDiff(direct.get.version, "8") + val viaDir = JdkPackage.fromPath(tmp) + assert(viaDir.isPresent) + assertNoDiff(viaDir.get.version, "8") + } finally Files.walk(tmp).sorted(java.util.Comparator.reverseOrder()).forEach(Files.delete) + } + }