-
Notifications
You must be signed in to change notification settings - Fork 69
Add flag to use cquery #178
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,12 +3,13 @@ package com.bazel_diff.bazel | |
import com.bazel_diff.log.Logger | ||
import com.bazel_diff.process.Redirect | ||
import com.bazel_diff.process.process | ||
import com.google.devtools.build.lib.analysis.AnalysisProtosV2 | ||
import com.google.devtools.build.lib.query2.proto.proto2api.Build | ||
import kotlinx.coroutines.ExperimentalCoroutinesApi | ||
import kotlinx.coroutines.runBlocking | ||
import org.koin.core.component.KoinComponent | ||
import org.koin.core.component.inject | ||
import java.nio.charset.StandardCharsets | ||
import java.io.File | ||
import java.nio.file.Files | ||
import java.nio.file.Path | ||
|
||
|
@@ -17,16 +18,53 @@ class BazelQueryService( | |
private val bazelPath: Path, | ||
private val startupOptions: List<String>, | ||
private val commandOptions: List<String>, | ||
private val keepGoing: Boolean?, | ||
private val cqueryOptions: List<String>, | ||
private val keepGoing: Boolean, | ||
private val noBazelrc: Boolean, | ||
) : KoinComponent { | ||
private val logger: Logger by inject() | ||
|
||
suspend fun query(query: String, useCquery: Boolean = false): List<Build.Target> { | ||
// Unfortunately, there is still no direct way to tell if a target is compatible or not with the proto output | ||
// by itself. So we do an extra cquery with the trick at | ||
// https://bazel.build/extending/platforms#cquery-incompatible-target-detection to first find all compatible | ||
// targets. | ||
val compatibleTargetSet = | ||
if (useCquery) { | ||
runQuery(query, useCquery = true, outputCompatibleTargets = true).useLines { | ||
it.filter { it.isNotBlank() }.toSet() | ||
} | ||
} else { | ||
emptySet() | ||
} | ||
val outputFile = runQuery(query, useCquery) | ||
|
||
val targets = outputFile.inputStream().buffered().use { proto -> | ||
if (useCquery) { | ||
val cqueryResult = AnalysisProtosV2.CqueryResult.parseFrom(proto) | ||
cqueryResult.resultsList.filter { it.target.rule.name in compatibleTargetSet }.map { it.target } | ||
} else { | ||
mutableListOf<Build.Target>().apply { | ||
while (true) { | ||
val target = Build.Target.parseDelimitedFrom(proto) ?: break | ||
// EOF | ||
add(target) | ||
} | ||
} | ||
} | ||
} | ||
|
||
return targets | ||
} | ||
|
||
@OptIn(ExperimentalCoroutinesApi::class) | ||
suspend fun query(query: String): List<Build.Target> { | ||
val tempFile = Files.createTempFile(null, ".txt") | ||
val outputFile = Files.createTempFile(null, ".bin") | ||
Files.write(tempFile, query.toByteArray(StandardCharsets.UTF_8)) | ||
private suspend fun runQuery(query: String, useCquery: Boolean, outputCompatibleTargets: Boolean = false): File { | ||
val queryFile = Files.createTempFile(null, ".txt").toFile() | ||
queryFile.deleteOnExit() | ||
val outputFile = Files.createTempFile(null, ".bin").toFile() | ||
outputFile.deleteOnExit() | ||
|
||
queryFile.writeText(query) | ||
logger.i { "Executing Query: $query" } | ||
|
||
val cmd: MutableList<String> = ArrayList<String>().apply { | ||
|
@@ -35,41 +73,69 @@ class BazelQueryService( | |
add("--bazelrc=/dev/null") | ||
} | ||
addAll(startupOptions) | ||
add("query") | ||
if (useCquery) { | ||
add("cquery") | ||
add("--transitions=lite") | ||
} else { | ||
add("query") | ||
} | ||
add("--output") | ||
add("streamed_proto") | ||
add("--order_output=no") | ||
if (keepGoing != null && keepGoing) { | ||
if (useCquery) { | ||
if (outputCompatibleTargets) { | ||
add("starlark") | ||
add("--starlark:file") | ||
val cqueryOutputFile = Files.createTempFile(null, ".cquery").toFile() | ||
cqueryOutputFile.deleteOnExit() | ||
cqueryOutputFile.writeText(""" | ||
def format(target): | ||
if providers(target) == None: | ||
# skip printing non-target results. That is, source files and generated files won't be | ||
# printed | ||
return "" | ||
if "IncompatiblePlatformProvider" not in providers(target): | ||
label = str(target.label) | ||
if label.startswith("@//"): | ||
# normalize label to be consistent with content inside proto | ||
return label[1:] | ||
else: | ||
return label | ||
return "" | ||
""".trimIndent()) | ||
add(cqueryOutputFile.toString()) | ||
} else { | ||
// Unfortunately, cquery does not support streamed_proto yet. | ||
// See https://github.com/bazelbuild/bazel/issues/17743. This poses an issue for large monorepos. | ||
add("proto") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bummer here but seems unavoidable for now There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah :(. BTW, I realized there is a bug with the first commit so I have pushed another update with one more test covering the basic source code change case. It seems that the implementation can be cleaner if source code hashing and rule target hashing are done uniformly with a |
||
} | ||
} else { | ||
add("streamed_proto") | ||
} | ||
if (!useCquery) { | ||
add("--order_output=no") | ||
} | ||
if (keepGoing) { | ||
add("--keep_going") | ||
} | ||
addAll(commandOptions) | ||
if (useCquery) { | ||
addAll(cqueryOptions) | ||
} else { | ||
addAll(commandOptions) | ||
} | ||
add("--query_file") | ||
add(tempFile.toString()) | ||
add(queryFile.toString()) | ||
} | ||
|
||
val result = runBlocking { | ||
process( | ||
*cmd.toTypedArray(), | ||
stdout = Redirect.ToFile(outputFile.toFile()), | ||
stdout = Redirect.ToFile(outputFile), | ||
workingDirectory = workingDirectory.toFile(), | ||
stderr = Redirect.PRINT, | ||
destroyForcibly = true, | ||
) | ||
} | ||
|
||
if(result.resultCode != 0) throw RuntimeException("Bazel query failed, exit code ${result.resultCode}") | ||
|
||
val targets = mutableListOf<Build.Target>() | ||
outputFile.toFile().inputStream().buffered().use {stream -> | ||
while (true) { | ||
val target = Build.Target.parseDelimitedFrom(stream) ?: break | ||
// EOF | ||
targets.add(target) | ||
} | ||
} | ||
|
||
Files.delete(tempFile) | ||
Files.delete(outputFile) | ||
return targets | ||
if (result.resultCode != 0) throw RuntimeException("Bazel query failed, exit code ${result.resultCode}") | ||
return outputFile | ||
} | ||
} |
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What was the reason removing
//external:all-targets
fromquery
branch? Without those repo generation targets, how would "coarse grained" hashes get computed?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#263 is a related change where I removed rule input transformation for
query
.