Skip to content

Commit 4dfe7bb

Browse files
committed
Drop dead CLI flags and simplify version/diagnostic helpers
1 parent 90bb5c1 commit 4dfe7bb

6 files changed

Lines changed: 30 additions & 150 deletions

File tree

scip-java/src/main/scala/com/sourcegraph/scip_java/buildtools/GradleBuildTool.scala

Lines changed: 14 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ import java.nio.charset.StandardCharsets
44
import java.nio.file._
55

66
import scala.collection.mutable.ListBuffer
7-
import scala.jdk.CollectionConverters._
87
import scala.util.Properties
98
import scala.util.Try
109

@@ -42,19 +41,16 @@ class GradleBuildTool(index: IndexCommand) extends BuildTool("Gradle", index) {
4241
* error pointing at the two known causes.
4342
*/
4443
private def reportMissingSemanticdbOutput(): Unit = {
45-
val semanticdbFileCount = countSemanticdbFiles(targetroot)
46-
if (semanticdbFileCount > 0)
44+
if (containsFileWithSuffix(targetroot, ".semanticdb"))
4745
return
48-
val classFileCount = countClassFiles(index.workingDirectory)
49-
if (classFileCount == 0)
50-
// Project has no compiled JVM output — nothing to index, stay quiet.
46+
if (!containsFileWithSuffix(index.workingDirectory, ".class"))
47+
// Project produced no compiled JVM output — nothing to index, stay quiet.
5148
return
5249
index
5350
.app
5451
.reporter
5552
.error(
56-
s"""scip-java: Gradle finished successfully but produced no SemanticDB output.
57-
|Compiled $classFileCount .class file(s); expected at least one .semanticdb file in $targetroot.
53+
s"""scip-java: Gradle finished successfully but produced no SemanticDB output in $targetroot.
5854
|
5955
|This means our SemanticDB compiler plugin was not attached to one or more JavaCompile tasks. Two known causes:
6056
|
@@ -72,53 +68,17 @@ class GradleBuildTool(index: IndexCommand) extends BuildTool("Gradle", index) {
7268
)
7369
}
7470

75-
private def countSemanticdbFiles(dir: Path): Long = {
76-
if (!Files.isDirectory(dir))
77-
return 0L
78-
Try {
79-
val stream = Files.walk(dir)
80-
try stream
81-
.filter(p =>
82-
Files.isRegularFile(p) && p.getFileName.toString.endsWith(".semanticdb")
83-
)
84-
.count()
85-
finally stream.close()
86-
}.getOrElse(0L)
87-
}
88-
89-
private def countClassFiles(workingDirectory: Path): Long = {
90-
// Gradle places compiled classes under <project>/build/classes/**. Walk
91-
// every `build/classes` directory we can find (top-level + immediate
92-
// subprojects) and count `.class` files. Bounded so this stays cheap even
93-
// on large multi-project builds.
94-
val rootBuildClasses = workingDirectory.resolve("build").resolve("classes")
95-
val subprojectBuildClasses = Try {
96-
val stream = Files.list(workingDirectory)
97-
try stream
98-
.iterator()
99-
.asScala
100-
.filter(Files.isDirectory(_))
101-
.map(_.resolve("build").resolve("classes"))
102-
.toList
103-
finally stream.close()
104-
}.getOrElse(Nil)
105-
(rootBuildClasses :: subprojectBuildClasses)
106-
.filter(Files.isDirectory(_))
107-
.iterator
108-
.map(countClassFilesIn)
109-
.sum
110-
}
111-
112-
private def countClassFilesIn(dir: Path): Long =
113-
Try {
114-
val stream = Files.walk(dir)
115-
try stream
116-
.filter(p =>
117-
Files.isRegularFile(p) && p.getFileName.toString.endsWith(".class")
118-
)
119-
.count()
71+
private def containsFileWithSuffix(root: Path, suffix: String): Boolean =
72+
Files.isDirectory(root) && Try {
73+
val stream = Files.find(
74+
root,
75+
Integer.MAX_VALUE,
76+
(p, attrs) =>
77+
attrs.isRegularFile && p.getFileName.toString.endsWith(suffix)
78+
)
79+
try stream.findFirst().isPresent
12080
finally stream.close()
121-
}.getOrElse(0L)
81+
}.getOrElse(false)
12282

12383
def targetroot: Path = index.finalTargetroot(defaultTargetroot)
12484

scip-java/src/main/scala/com/sourcegraph/scip_java/buildtools/MavenBuildTool.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ class MavenBuildTool(index: IndexCommand) extends BuildTool("Maven", index) {
4444
index.workingDirectory,
4545
index.finalTargetroot(defaultTargetroot),
4646
tmp,
47-
SystemJavaVersion.isAtLeast(SystemJavaVersion.detect(), "11")
47+
SystemJavaVersion.detectMajor() >= 11
4848
)
4949
buildCommand ++=
5050
List(

scip-java/src/main/scala/com/sourcegraph/scip_java/buildtools/SystemJavaVersion.scala

Lines changed: 15 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -2,20 +2,23 @@ package com.sourcegraph.scip_java.buildtools
22

33
import java.nio.file.Files
44

5-
import scala.annotation.tailrec
6-
75
import com.sourcegraph.scip_java.Embedded
86

97
object SystemJavaVersion {
10-
// Returns the output of `System.getProperty("java.version")` from a fresh JVM
11-
// instance using the system JVM installation. We can't use this process since
12-
// it may be running on a separate JVM process (that's the case when we run
13-
// `sbt test` in this build at least).
14-
def detect(): String = {
8+
// Returns the major version of the JVM that `java` on PATH resolves to. We
9+
// can't just use `Properties.javaSpecVersion` because the current process
10+
// may be a different JVM (e.g. when running `sbt test`).
11+
//
12+
// Examples:
13+
// "1.8.0_372" => 8 (Java 8 reports as "1.8.x")
14+
// "11.0.2" => 11
15+
// "17.0.5" => 17
16+
def detectMajor(): Int = {
1517
val tmp = Files.createTempDirectory("java-version")
1618
val jar = Embedded.semanticdbJar(tmp)
1719
try {
18-
os.proc(
20+
val version = os
21+
.proc(
1922
"java",
2023
"-cp",
2124
jar.toString(),
@@ -25,69 +28,12 @@ object SystemJavaVersion {
2528
.out
2629
.text()
2730
.trim()
31+
if (version.startsWith("1."))
32+
8
33+
else
34+
version.takeWhile(_.isDigit).toInt
2835
} finally {
2936
Files.deleteIfExists(jar)
3037
}
3138
}
32-
33-
// Copy-pasted from scala.util.Properties.isJavaAtLeast but makes the actual
34-
// version a parameterizeable instead of being hard-coded to
35-
// `Properties.javaVersionSpec`.
36-
def isAtLeast(actualVersion: String, comparisonVersion: String): Boolean = {
37-
def versionOf(s: String, depth: Int): (Int, String) =
38-
s.indexOf('.') match {
39-
case 0 =>
40-
(-2, s.substring(1))
41-
case 1 if depth == 0 && s.charAt(0) == '1' =>
42-
val r0 = s.substring(2)
43-
val (v, r) = versionOf(r0, 1)
44-
val n =
45-
if (v > 8 || r0.isEmpty)
46-
-2
47-
else
48-
v // accept 1.8, not 1.9 or 1.
49-
(n, r)
50-
case -1 =>
51-
val n =
52-
if (!s.isEmpty)
53-
s.toInt
54-
else if (depth == 0)
55-
-2
56-
else
57-
0
58-
(n, "")
59-
case i =>
60-
val r = s.substring(i + 1)
61-
val n =
62-
if (depth < 2 && r.isEmpty)
63-
-2
64-
else
65-
s.substring(0, i).toInt
66-
(n, r)
67-
}
68-
@tailrec
69-
def compareVersions(s: String, v: String, depth: Int): Int = {
70-
if (depth >= 3)
71-
0
72-
else {
73-
val (sn, srest) = versionOf(s, depth)
74-
val (vn, vrest) = versionOf(v, depth)
75-
if (vn < 0)
76-
-2
77-
else if (sn < vn)
78-
-1
79-
else if (sn > vn)
80-
1
81-
else
82-
compareVersions(srest, vrest, depth + 1)
83-
}
84-
}
85-
86-
compareVersions(actualVersion, comparisonVersion, 0) match {
87-
case -2 =>
88-
throw new NumberFormatException(s"Not a version: $comparisonVersion")
89-
case i =>
90-
i >= 0
91-
}
92-
}
9339
}

scip-java/src/main/scala/com/sourcegraph/scip_java/commands/IndexCommand.scala

Lines changed: 0 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -34,14 +34,6 @@ case class IndexCommand(
3434
"For example, the default value for Gradle is 'build/semanticdb-targetroot' and for Maven it's 'target/semanticdb-targetroot'"
3535
)
3636
targetroot: Option[Path] = None,
37-
@Description(
38-
"Whether to enable the -verbose flag in the SemanticDB compiler plugin."
39-
)
40-
verbose: Boolean = false,
41-
@Description(
42-
"Whether to enable the -text:on flag in the SemanticDB compiler plugin."
43-
)
44-
text: Boolean = false,
4537
@Description(
4638
"Explicitly specify which build tool to use. " +
4739
"By default, the build tool is automatically detected. " +
@@ -125,17 +117,6 @@ case class IndexCommand(
125117
)
126118
}
127119

128-
def textFlag: String =
129-
if (text)
130-
"-text:on"
131-
else
132-
""
133-
def verboseFlag: String =
134-
if (verbose)
135-
"-verbose:on"
136-
else
137-
""
138-
139120
def workingDirectory: Path = AbsolutePath.of(app.env.workingDirectory)
140121
def finalTargetroot(default: Path): Path = AbsolutePath.of(
141122
targetroot.getOrElse(default),

scip-java/src/main/scala/com/sourcegraph/scip_java/commands/IndexSemanticdbCommand.scala

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -28,10 +28,6 @@ final case class IndexSemanticdbCommand(
2828
output: Path = Paths.get("index.scip"),
2929
@Description("Whether to process the SemanticDB files in parallel")
3030
parallel: Boolean = true,
31-
@Description(
32-
"Whether to infer the location of SemanticDB files based as produced by Bazel"
33-
)
34-
bazel: Boolean = true,
3531
@Description(
3632
"Whether to emit parent->child relationships for 'Find references' and 'Find implementations'. " +
3733
"This flag exists as a workaround for the issue https://github.com/sourcegraph/sourcegraph/issues/50927"

tests/buildTools/src/test/scala/tests/BaseBuildToolSuite.scala

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -113,9 +113,6 @@ abstract class BaseBuildToolSuite extends MopedSuite(ScipJava.app) {
113113
targetroot.toString
114114
) ++ extraArguments
115115
val exit = app().run(arguments)
116-
if (extraArguments.contains("--verbose")) {
117-
println(app.capturedOutput)
118-
}
119116
expectedError match {
120117
case Some(fn) =>
121118
assert(clue(exit) != 0, clues(app.capturedOutput))

0 commit comments

Comments
 (0)