Skip to content

Commit e690621

Browse files
committed
Address review comments
1 parent 928102e commit e690621

File tree

4 files changed

+189
-144
lines changed

4 files changed

+189
-144
lines changed

docs/getting-started.md

Lines changed: 38 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,8 @@ title: Getting started
44
---
55

66
By following the instructions on this page, you should be able to generate a
7-
[SCIP](https://github.com/sourcegraph/scip)
8-
index of your Java codebase using Gradle or Maven. See
7+
[SCIP](https://github.com/sourcegraph/scip) index of your Java codebase using
8+
Gradle, Maven, sbt, or Bazel. See
99
[Supported build tools](#supported-build-tools) for an overview of other build
1010
tools that we're planning to support in the future.
1111

@@ -328,13 +328,41 @@ projects, with the following caveats:
328328

329329
### Bazel
330330

331-
Bazel is supported by scip-java, but it requires custom configuration to work
332-
correctly. The `scip-java index` command does not automatically index Bazel builds.
331+
There are two approaches to index Bazel/Java codebases: automatic and manual.
333332

334-
The Bazel integration for scip-java is specifically designed to be compatible
335-
with the Bazel build cache to enable incremental indexing. To achieve this,
336-
scip-java must be configured in `WORKSPACE` and `BUILD` files. The scip-java
337-
repository contains an example for how to configure everything.
333+
Don't hesitate to open an issue in the
334+
[scip-java repository](https://github.com/sourcegraph/scip-java) if you have any
335+
questions about using scip-java with Bazel builds.
336+
337+
#### Automatic - `aspect`
338+
339+
Since scip-java v0.8.24, it's possible to automatically index Bazel/Java
340+
codebases via `scip-java index`.
341+
342+
```sh
343+
scip-java index "--bazel-scip-java-binary=$(which scip-java)"
344+
```
345+
346+
When using this approach, indexing happens mostly inside the Bazel action graph,
347+
benefitting from parallel compilation and the Bazel build cache.
348+
349+
The `--bazel-scip-java-binary` argument is required due to implementation
350+
details, scip-java runs an [aspect](https://bazel.build/extending/aspects) that
351+
requires the absolute path to the `scip-java` binary.
352+
353+
> The current solution for automatic indexing step is not yet 100% hermetic and,
354+
> therefore, relies on `--spawn_strategy=local` under the hood. Depending on
355+
> your use-case, this might be OK or not. If there is demand for it, it's should
356+
> be possible to make the indexing fully hermetic and compatible with Bazel's
357+
> sandbox with some extra work.
358+
359+
#### Manual - `select`
360+
361+
It's possible to index Bazel codebases by integrating scip-java directly into
362+
the build configuration. To achieve this, scip-java must be configured in
363+
`WORKSPACE` and `BUILD` files. The scip-java repository contains an example for
364+
how to configure everything, including how to build scip-java itself from
365+
source.
338366

339367
- [WORKSPACE](https://github.com/sourcegraph/scip-java/blob/main/examples/bazel-example/WORKSPACE):
340368
adds the required dependencies to be able to run scip-java itself.
@@ -343,6 +371,7 @@ repository contains an example for how to configure everything.
343371
scip-java.
344372

345373
Once configured, build the codebase with the SemanticDB compiler plugin.
374+
346375
```sh
347376
bazel build //... --@scip_java//semanticdb-javac:enabled=true
348377
```
@@ -356,7 +385,7 @@ bazel run @scip_java//scip-semanticdb:bazel -- --sourceroot $PWD
356385
# The command below works for the `examples/bazel-example` directory in the sourcegraph/scip-java repository.
357386
❯ jar tf bazel-bin/src/main/java/example/libexample.jar | grep semanticdb$
358387
META-INF/semanticdb/src/main/java/example/Example.java.semanticdb
359-
```
388+
```
360389

361390
Finally, run the following commands to upload the SCIP index to Sourcegraph.
362391

@@ -372,8 +401,3 @@ src login # validate the token authenticates correctly
372401
# 3. Upload SCIP index to Sourcegraph
373402
src code-intel upload # requires index.scip file to exist
374403
```
375-
376-
377-
Don't hesitate to open an issue in the
378-
[scip-java repository](https://github.com/sourcegraph/scip-java) if you have any
379-
questions about using scip-java with Bazel builds.

scip-java/src/main/resources/scip-java/scip_java.bzl

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,7 @@ def _scip_java(target, ctx):
7979
"processors": processors,
8080
"processorpath": processorpath,
8181
"bootclasspath": bootclasspath,
82+
"reportWarningOnEmptyIndex": False,
8283
})
8384
build_config_path = ctx.actions.declare_file(ctx.label.name + ".scip.json")
8485

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

Lines changed: 127 additions & 119 deletions
Original file line numberDiff line numberDiff line change
@@ -25,134 +25,142 @@ class BazelBuildTool(index: IndexCommand) extends BuildTool("Bazel", index) {
2525
Files.isRegularFile(index.workingDirectory.resolve("WORKSPACE"))
2626
}
2727

28+
private def targetSpecs =
29+
if (index.buildCommand.isEmpty)
30+
List("//...")
31+
else
32+
index.buildCommand
33+
2834
override def generateScip(): Int = {
29-
this.generateAspectFile() match {
30-
case None =>
31-
1
32-
case Some(aspectLabel) =>
33-
val buildCommand = ListBuffer.empty[String]
34-
buildCommand += "bazel"
35-
buildCommand += "build"
36-
buildCommand += "--noshow_progress"
35+
val aspectLabel = this.generateAspectFile().getOrElse("")
36+
if (aspectLabel.isEmpty) {
37+
return 1
38+
}
3739

38-
// The local strategy is required for now because we write SemanticDB and SCIP files
39-
// to the provided targetroot directory.
40-
buildCommand += "--spawn_strategy=local"
40+
val scipJavaBinary = index.bazelScipJavaBinary.getOrElse("")
41+
if (scipJavaBinary.isEmpty) {
42+
index
43+
.app
44+
.error(
45+
"the flag --bazel-scip-java-binary is required to index Bazel codebases. To fix this problem, run scip-java index again with the flag --scip-java-binary=/path/to/scip-java"
46+
)
47+
return 1
48+
}
4149

42-
val targetSpecs =
43-
if (index.buildCommand.isEmpty)
44-
List("//...")
45-
else
46-
index.buildCommand
47-
buildCommand ++= targetSpecs
50+
val javaHome = index.app.env.environmentVariables.getOrElse("JAVA_HOME", "")
51+
if (javaHome.isEmpty) {
52+
index
53+
.app
54+
.error(
55+
"environment variable JAVA_HOME is not set. " +
56+
"To fix this problem run `export JAVA_HOME=/path/to/java` and run scip-java index again."
57+
)
58+
return 1
59+
}
4860

49-
buildCommand += "--aspects"
50-
buildCommand += s"$aspectLabel%scip_java_aspect"
51-
buildCommand += "--output_groups=scip"
52-
buildCommand += s"--define=sourceroot=${index.workingDirectory}"
61+
val buildCommand =
62+
List(
63+
"bazel",
64+
"build",
65+
"--noshow_progress",
66+
// The local strategy is required for now because we write SemanticDB and SCIP files
67+
// to the provided targetroot directory.
68+
"--spawn_strategy=local",
69+
"--aspects",
70+
s"$aspectLabel%scip_java_aspect",
71+
"--output_groups=scip",
72+
s"--define=sourceroot=${index.workingDirectory}",
73+
s"--define=java_home=$javaHome",
74+
s"--define=scip_java_binary=$scipJavaBinary"
75+
) ++ targetSpecs
76+
77+
val buildExitCode = runBazelBuild(buildCommand)
78+
if (buildExitCode != 0) {
79+
buildExitCode
80+
} else {
81+
aggregateScipFiles()
82+
0
83+
}
84+
}
5385

54-
val javaHome = index
86+
private def runBazelBuild(buildCommand: List[String]): Int = {
87+
val sandbox = new SandboxCommandExtractor(index.app)
88+
index.app.info(buildCommand.mkString(" "))
89+
val commandResult = index
90+
.app
91+
.process(buildCommand)
92+
.call(check = false, stderr = Readlines(sandbox))
93+
if (commandResult.exitCode != 0) {
94+
if (index.bazelAutorunSandboxCommand && sandbox.commandLines().nonEmpty) {
95+
index
5596
.app
56-
.env
57-
.environmentVariables
58-
.getOrElse("JAVA_HOME", "")
59-
if (javaHome.isEmpty) {
60-
index
61-
.app
62-
.error(
63-
"environment variable JAVA_HOME is not set. " +
64-
"To fix this problem run `export JAVA_HOME=/path/to/java` and run scip-java index again."
65-
)
66-
return 1
67-
}
68-
buildCommand += s"--define=java_home=$javaHome"
69-
70-
val scipJavaBinary = index.bazelScipJavaBinary.getOrElse("")
71-
if (scipJavaBinary.isEmpty) {
72-
index
73-
.app
74-
.error(
75-
"the flag --bazel-scip-java-binary is required to index Bazel codebases. To fix this problem, run scip-java index again with the flag --scip-java-binary=/path/to/scip-java"
76-
)
77-
return 1
78-
}
79-
buildCommand += s"--define=scip_java_binary=$scipJavaBinary"
80-
81-
val sandbox = new SandboxCommandExtractor(index.app)
82-
index.app.info(buildCommand.mkString(" "))
83-
val commandResult = index
97+
.info(
98+
"Automatically re-running sandbox command to help debug the problem."
99+
)
100+
index
84101
.app
85-
.process(buildCommand.toList)
86-
.call(check = false, stderr = Readlines(sandbox))
87-
if (commandResult.exitCode != 0) {
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-
}
99-
index
100-
.app
101-
.error(
102-
s"""To reproduce the failed Bazel command without scip-java, run the following command:
103-
|
104-
| bazel build ${targetSpecs.mkString(" ")}
105-
|
106-
|To narrow the set of targets to index or pass additional flags to Bazel, include extra arguments index after -- like below:
107-
|
108-
| scip-java index --bazel-scip-java-binary=... -- //custom/target --sandbox_debug
109-
|""".stripMargin
110-
)
111-
return commandResult.exitCode
112-
}
102+
.process("bash", "-c", sandbox.commandLines().mkString("\n"))
103+
.call(check = false, stdout = os.Inherit, stderr = os.Inherit)
104+
}
105+
index
106+
.app
107+
.error(
108+
s"""To reproduce the failed Bazel command without scip-java, run the following command:
109+
|
110+
| bazel build ${targetSpecs.mkString(" ")}
111+
|
112+
|To narrow the set of targets to index or pass additional flags to Bazel, include extra arguments index after -- like below:
113+
|
114+
| scip-java index --bazel-scip-java-binary=... -- //custom/target --sandbox_debug
115+
|""".stripMargin
116+
)
117+
commandResult.exitCode
118+
} else {
119+
0
120+
}
121+
}
113122

114-
// Final step after running the aspect: aggregate all the generated `*.scip` files into a single index.scip file.
115-
// We have to do this step outside of Bazel because Bazel does not allow actions to generate outputs outside
116-
// of the bazel-out directory. Ideally, we should be able to implement the aggregation step inside Bazel
117-
// and only copy the resulting index.scip file into the root of the workspace. However, to begin with, we
118-
// walk the bazel-bin/ directory to concatenate the `*.scip` files even if this is not the idiomatic way to
119-
// do it (in general, scanning the bazel-bin/ directory is a big no no).
120-
Files.deleteIfExists(index.finalOutput)
121-
Files.createDirectories(index.finalOutput.getParent)
122-
val scipPattern = FileSystems.getDefault.getPathMatcher("glob:**.scip")
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)
135-
Files.walkFileTree(
136-
bazelOutLink,
137-
new SimpleFileVisitor[Path] {
138-
override def visitFile(
139-
file: Path,
140-
attrs: BasicFileAttributes
141-
): FileVisitResult = {
142-
if (scipPattern.matches(file)) {
143-
val bytes = Files.readAllBytes(file)
144-
Files.write(
145-
index.finalOutput,
146-
bytes,
147-
StandardOpenOption.APPEND,
148-
StandardOpenOption.CREATE
149-
)
150-
}
151-
super.visitFile(file, attrs)
123+
private def aggregateScipFiles(): Unit = {
124+
// Final step after running the aspect: aggregate all the generated `*.scip` files into a single index.scip file.
125+
// We have to do this step outside of Bazel because Bazel does not allow actions to generate outputs outside
126+
// of the bazel-out directory. Ideally, we should be able to implement the aggregation step inside Bazel
127+
// and only copy the resulting index.scip file into the root of the workspace. However, to begin with, we
128+
// walk the bazel-bin/ directory to concatenate the `*.scip` files even if this is not the idiomatic way to
129+
// do it (in general, scanning the bazel-bin/ directory is a big no no).
130+
Files.deleteIfExists(index.finalOutput)
131+
Files.createDirectories(index.finalOutput.getParent)
132+
val scipPattern = FileSystems.getDefault.getPathMatcher("glob:**.scip")
133+
val bazelOut = index.workingDirectory.resolve("bazel-out")
134+
if (!Files.exists(bazelOut)) {
135+
index
136+
.app
137+
.error(
138+
s"doing nothing, the directory $bazelOut does not exist. " +
139+
s"The most likely cause for this problem is that there are no Java targets in this Bazel workspace. " +
140+
s"Please report an issue to the scip-java issue tracker if the command `bazel query 'kind(java_*, //...)'` returns non-empty output."
141+
)
142+
} else {
143+
val bazelOutLink = Files.readSymbolicLink(bazelOut)
144+
Files.walkFileTree(
145+
bazelOutLink,
146+
new SimpleFileVisitor[Path] {
147+
override def visitFile(
148+
file: Path,
149+
attrs: BasicFileAttributes
150+
): FileVisitResult = {
151+
if (scipPattern.matches(file)) {
152+
val bytes = Files.readAllBytes(file)
153+
Files.write(
154+
index.finalOutput,
155+
bytes,
156+
StandardOpenOption.APPEND,
157+
StandardOpenOption.CREATE
158+
)
152159
}
160+
super.visitFile(file, attrs)
153161
}
154-
)
155-
0
162+
}
163+
)
156164
}
157165
}
158166

@@ -214,7 +222,7 @@ class BazelBuildTool(index: IndexCommand) extends BuildTool("Bazel", index) {
214222
.app
215223
.reporter
216224
.error(
217-
"path $aspectPath already exists and is not a file. To fix this problem, remove this path and try again."
225+
s"path $aspectPath already exists and is not a file. To fix this problem, remove this path and try again."
218226
)
219227
return None
220228
}

0 commit comments

Comments
 (0)