8000 Clean up code for review · Arthurm1/scip-java@928102e · GitHub
[go: up one dir, main page]
Skip to content
< 10000 header class="prc-PageLayout-Header-mQXK1" style="--spacing:var(--spacing-none)">

Commit 928102e

Browse files
olafurpgolafurpg
committed
Clean up code for review
1 parent ea47c1a commit 928102e

File tree

7 files changed

+73
-32
lines changed

7 files changed

+73
-32
lines changed

‎build.sbt

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -167,13 +167,19 @@ lazy val javacPlugin = project
167167
// referenced from META-INF/services/com.sun.source.util.Plugin
168168
"com.sourcegraph.semanticdb_javac.SemanticdbPlugin" ->
169169
"com.sourcegraph.semanticdb_javac.SemanticdbPlugin",
170+
// Don't rename PrintJavaVersion because we load it via FQN to
171+
// detect the Java of a JVM installation.
170172
"com.sourcegraph.semanticdb_javac.PrintJavaVersion" ->
171173
"com.sourcegraph.semanticdb_javac.PrintJavaVersion",
174+
// Don't rename InjectSemanticdbOptions because w 8000 e load it via FQN to
175+
// process a list of Java compiler options.
172176
"com.sourcegraph.semanticdb_javac.InjectSemanticdbOptions" ->
173177
"com.sourcegraph.semanticdb_javac.InjectSemanticdbOptions",
174178
"com.google.**" -> "com.sourcegraph.shaded.com.google.@1",
175-
// Need to shade the semanticdb-javac compiler plugin in order to be
176-
// able to index the plugin code itself.
179+
// Shade everything else in the semanticdb-javac compiler plugin in
180+
// order to be able to index the plugin code itself. Without this step,
181+
// we can't add the plugin to the classpath while compiling the source
182+
// code of the plugin itself because it results in cryptic compile errors.
177183
"com.sourcegraph.**" -> "com.sourcegraph.shaded.com.sourcegraph.@1",
178184
"google.**" -> "com.sourcegraph.shaded.google.@1",
179185
"org.relaxng.**" -> "com.sourcegraph.shaded.relaxng.@1"

‎scip-java/src/main/scala/com/sourcegraph/scip_java/Embedded.scala

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,12 @@ object Embedded {
112112
}
113113
}
114114

115+
/**
116+
* Returns the string contents of the scip_java.bzl file on disk.
117+
*/
115118
def bazelAspectFile(tmpDir: Path): String = {
119+
// We could in theory load the resource straight into a string but it was
120+
// easier to copy it to a file and read it from there.
116121
val tmpFile = copyFile(tmpDir, "scip-java/scip_java.bzl")
117122
val contents =
118123
new String(Files.readAllBytes(tmpFile), StandardCharsets.UTF_8)

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

Lines changed: 31 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -39,12 +39,11 @@ class BazelBuildTool(index: IndexCommand) extends BuildTool("Bazel", index) {
3939
// to the provided targetroot directory.
4040
buildCommand += "--spawn_strategy=local"
4141

42-
val targetSpecs = ListBuffer.empty[String]
43-
if (index.buildCommand.isEmpty) {
44-
targetSpecs += "//..."
45-
} else {
46-
targetSpecs ++= index.buildCommand
47-
}
42+
val targetSpecs =
43+
if (index.buildCommand.isEmpty)
44+
List("//...")
45+
else
46+
index.buildCommand
4847
buildCommand ++= targetSpecs
4948

5049
buildCommand += "--aspects"
@@ -86,11 +85,17 @@ class BazelBuildTool(index: IndexCommand) extends BuildTool("Bazel", index) {
8685
.process(buildCommand.toList)
8786
.call(check = false, stderr = Readlines(sandbox))
8887
if (commandResult.exitCode != 0) {
89-
// Automatically run the sandbox command to help the user debug the problem.
90-
index
91-
.app
92-
.process("bash", "-c", sandbox.commandLines().mkString("\n"))
93-
.call(check = false, stdout = os.Inherit, stderr = os.Inherit)
88+
if (index.bazelAutorunSandboxCommand) {
89+
index
90+
.app
91+
.info(
92+
"Automatically re-running sandbox command to help debug the problem."
93+
)
94+
index
95+
.app
96+
.process("bash", "-c", sandbox.commandLines().mkString("\n"))
97+
.call(check = false, stdout = os.Inherit, stderr = os.Inherit)
98+
}
9499
index
95100
.app
96101
.error(
@@ -105,6 +110,7 @@ class BazelBuildTool(index: IndexCommand) extends BuildTool("Bazel", index) {
105110
)
106111
return commandResult.exitCode
107112
}
113+
108114
// Final step after running the aspect: aggregate all the generated `*.scip` files into a single index.scip file.
109115
// We have to do this step outside of Bazel because Bazel does not allow actions to generate outputs outside
110116
// of the bazel-out directory. Ideally, we should be able to implement the aggregation step inside Bazel
@@ -114,10 +120,20 @@ class BazelBuildTool(index: IndexCommand) extends BuildTool("Bazel", index) {
114120
Files.deleteIfExists(index.finalOutput)
115121
Files.createDirectories(index.finalOutput.getParent)
116122
val scipPattern = FileSystems.getDefault.getPathMatcher("glob:**.scip")
117-
val binDirectory = Files
118-
.readSymbolicLink(index.workingDirectory.resolve("bazel-bin"))
123+
val bazelOut = index.workingDirectory.resolve("bazel-out")
124+
if (!Files.exists(bazelOut)) {
125+
index
126+
.app
127+
.error(
128+
s"doing nothing, the directory $bazelOut does not exist. " +
129+
s"The most likely cause for this problem is that there are no Java targets in this Bazel workspace. " +
130+
s"Please report an issue to the scip-java issue tracker if the command `bazel query 'kind(java_*, //...)'` returns non-empty output."
131+
)
132+
return 0
133+
}
134+
val bazelOutLink = Files.readSymbolicLink(bazelOut)
119135
Files.walkFileTree(
120-
binDirectory,
136+
bazelOutLink,
121137
new SimpleFileVisitor[Path] {
122138
override def visitFile(
123139
file: Path,
@@ -153,11 +169,10 @@ class BazelBuildTool(index: IndexCommand) extends BuildTool("Bazel", index) {
153169
private var isSandboxCommandPrinting = false
154170
private val lines = ListBuffer.empty[String]
155171
def commandLines(): List[String] = lines.toList
156-
def command(): List[String] = List("bash", "-c", lines.mkString("\n"))
157172
override def apply(line: String): Unit = {
158173
if (
159174
!isSandboxCommandPrinting && line.startsWith("ERROR:") &&
160-
line.endsWith("error executing command ")
175+
line.contains("error executing command")
161176
) {
162177
isSandboxCommandPrinting = true
163178
} else if (isSandboxCommandPrinting && !line.startsWith(" ")) {

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

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -528,16 +528,13 @@ class ScipBuildTool(index: IndexCommand) extends BuildTool("SCIP", index) {
528528
arguments += processors.mkString(",")
529529
}
530530
arguments ++=
531-
config
532-
.javacOptions
533-
.filterNot(option =>
534-
option.startsWith("-Xep") ||
535-
option
536-
.startsWith("-Xplugin:semanticdb") || option.startsWith("-XD") ||
537-
index
538-
.scipIgnoredJavacOptionPrefixes
539-
.exists(prefix => option.startsWith(prefix))
540-
)
531+
fixJavacOptions(config.javacOptions).filterNot(option =>
532+
option.startsWith("-Xep") || option.startsWith("-Xplugin:semanticdb") ||
533+
option.startsWith("-XD") ||
534+
index
535+
.scipIgnoredJavacOptionPrefixes
536+
.exists(prefix => option.startsWith(prefix))
537+
)
541538
if (config.kind == "jdk" && moduleInfos.nonEmpty) {
542539
moduleInfos.foreach { module =>
543540
arguments += "--module"
@@ -577,6 +574,16 @@ class ScipBuildTool(index: IndexCommand) extends BuildTool("SCIP", index) {
577574
Failure(SubprocessException(result))
578575
}
579576

577+
private def fixJavacOptions(options: List[String]): List[String] =
578+
options match {
579+
case "--release" :: _ :: rest =>
580+
fixJavacOptions(rest)
581+
case option :: rest =>
582+
option :: fixJavacOptions(rest)
583+
case Nil =>
584+
Nil
585+
}
586+
580587
private def javacPath(config: Config, tmp: Path): Path = {
581588
config.javaHome match {
582589
case Some(home) =>

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,9 @@ case class IndexCommand(
7575
@Description(
7676
"If true, overwrites the existing Bazel aspect file (if any)"
7777
) bazelOverwriteAspectFile: Boolean = false,
78+
@Description(
79+
"If true, automatically tries to extract the printed out sandbox command and re-run the command to reveal the underlying problem."
80+
) bazelAutorunSandboxCommand: Boolean = true,
7881
@Description(
7982
"Optional. The build command to use to compile all sources. " +
8083
"Defaults to a build-specific command. For example, the default command for Maven command is 'clean verify -DskipTests'." +

‎scip-semanticdb/src/main/java/com/sourcegraph/scip_semanticdb/BazelBuildTool.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -63,9 +63,9 @@ public boolean hasErrors() {
6363
ScipOutputFormat.TYPED_PROTOBUF,
6464
options.parallel,
6565
mavenPackages,
66-
"",
67-
true,
68-
true);
66+
/* buildKind */ "",
67+
/* emitInverseRelationships */ true,
68+
/* allowEmptyIndex */ true);
6969
ScipSemanticdb.run(scipOptions);
7070

7171
if (!scipOptions.reporter.hasErrors()) {
@@ -109,7 +109,7 @@ public static List<MavenPackage> mavenPackages(BazelOptions options)
109109
String[] parts = tag.substring("maven_coordinates=".length()).split(":");
110110
if (parts.length == 3) {
111111
// The jar part is populated via `withJar()` below.
112-
basePackage = new MavenPackage(/* jar = */ null, parts[0], parts[1], parts[2]);
112+
basePackage = new MavenPackage(/* jar= */ null, parts[0], parts[1], parts[2]);
113113
}
114114
}
115115
}

‎scip-semanticdb/src/main/java/com/sourcegraph/scip_semanticdb/MessageOnlyException.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,10 @@
11
package com.sourcegraph.scip_semanticdb;
22

3+
/**
4+
* Exception that doesn't fill out the stack trace, it only prints out the message.
5+
*
6+
* <p>Use this exception if you want prettier console output without stack trace noise.
7+
*/
38
public class MessageOnlyException extends Throwable {
49
public MessageOnlyException(String message) {
510
super(message);

0 commit comments

Comments
 (0)
0