Skip to content
This repository has been archived by the owner on Sep 30, 2024. It is now read-only.

Commit

Permalink
bazel: add utility macro for wrapping single-file tools (#62930)
Browse files Browse the repository at this point in the history
Currently, we provide single-file tools such as `ctags`, `gsutil` etc via an `sh_binary` wrapper, to have a single target to reference that automatically does platform selection of the underlying tool. 
Due to some [unfortunate reason](bazelbuild/bazel#11820), the underlying srcs (which is [a single file](https://bazel.build/reference/be/shell#sh_binary.srcs)) of an `sh_binary` are also exposed as outputs (rather than just as typical runfiles) alongside the script that wraps. This is _sometimes_ problematic when doing location expansion (e.g. `$(location ...)`) due to these only allowing a single output (dont ask why this works in some contexts but not others, I dont know). 
To address this, we create a wrapper macro + rule to replicate what we want from `sh_binary` (automatic platform selection + tool naming), while only exposing a singular file.

See example of currently required approach to consuming a tool: [BUILD.bazel](https://github.com/sourcegraph/sourcegraph/pull/62801/files#diff-e2a562c2e13908933b2ee24f0ac596829b38a5325cc69a4aee05c383aaa2e494R8) & [main_test.go](https://github.com/sourcegraph/sourcegraph/pull/62801/files#diff-7a91cb5143064bfc8993ef97baf68b718ef49747ccc1d3c5e1150d4696b88305R66).

With this change, `rlocationpath` (singular) can be used instead (or any of the other singular nouns in different contexts), as well as no `strings.Split/strings.Fields` being required

## Test plan

`bazel cquery --output=files //dev/tools:dropdb` yields 1 vs 2 files.
Also updated the rule behind `//internal/database:generate_schemas` due to the workaround in it for the fact that the underlying srcs was also exposed. The correctness is verified by running said target (locally + CI)
  • Loading branch information
Strum355 authored May 27, 2024
1 parent 56f2f92 commit 7009f1d
Show file tree
Hide file tree
Showing 3 changed files with 72 additions and 42 deletions.
15 changes: 3 additions & 12 deletions dev/migrations.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -41,20 +41,12 @@ fi
# Dumping the schema requires running the squash operation first, as it reuses the database, so we do all of those operations
# in a single step.
def _generate_schemas_impl(ctx):
# for every entry in pgutils filegroup, there's two files:
# - one in external e.g. external/createdb-linux-amd64/file/downloaded
# - one in output base e.g. bazel-out/k8-opt-exec-ST-13d3ddad9198/bin/dev/tools/createdb
# only the one in output base can be picked up by-name in PATH, so we need to filter out the ones in external.
pgutils_path = ":".join([
f.path.rpartition("/")[0]
for f in ctx.attr._pg_utils[DefaultInfo].default_runfiles.files.to_list()
if not f.path.startswith("external")
])

pgutils_path = ctx.attr._pg_utils[DefaultInfo].files.to_list()[0].path.rpartition("/")[0]
runfiles = depset(direct = ctx.attr._sg[DefaultInfo].default_runfiles.files.to_list() + ctx.attr._pg_utils[DefaultInfo].default_runfiles.files.to_list())

ctx.actions.run_shell(
inputs = ctx.files.srcs,
tools = runfiles,
outputs = [
ctx.outputs.out_frontend_squash,
ctx.outputs.out_codeintel_squash,
Expand All @@ -78,7 +70,7 @@ def _generate_schemas_impl(ctx):
},
command = """{cmd_preamble}
export PATH="{pgutils_path}:$PATH"
export PATH="{pgutils_path}"
trap "dropdb --if-exists sg-squasher-frontend && echo 'temp db sg-squasher-frontend dropped'" EXIT
trap "dropdb --if-exists sg-squasher-codeintel && echo 'temp db sg-squasher-codeintel dropped'" EXIT
Expand Down Expand Up @@ -110,7 +102,6 @@ def _generate_schemas_impl(ctx):
out_codeintel_schema_md = ctx.outputs.out_codeintel_schema_md.path,
out_codeinsights_schema_md = ctx.outputs.out_codeinsights_schema_md.path,
),
tools = runfiles,
)

return [
Expand Down
52 changes: 22 additions & 30 deletions dev/tools/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -1,41 +1,39 @@
sh_binary(
load(":tool.bzl", "tool")

tool(
name = "docsite",
srcs = select({
src = select({
"@bazel_tools//src/conditions:darwin_x86_64": ["@docsite_darwin_amd64//file:downloaded"],
"@bazel_tools//src/conditions:darwin_arm64": ["@docsite_darwin_arm64//file:downloaded"],
"@bazel_tools//src/conditions:linux_x86_64": ["@docsite_linux_amd64//file:downloaded"],
}),
visibility = ["//visibility:public"],
)

sh_binary(
tool(
name = "src-cli",
srcs = select({
src = select({
"@bazel_tools//src/conditions:darwin_x86_64": ["@src-cli-darwin-amd64//:src-cli-darwin-amd64"],
"@bazel_tools//src/conditions:darwin_arm64": ["@src-cli-darwin-arm64//:src-cli-darwin-arm64"],
"@bazel_tools//src/conditions:linux_x86_64": ["@src-cli-linux-amd64//:src-cli-linux-amd64"],
}),
visibility = ["//visibility:public"],
)

sh_binary(
tool(
name = "universal-ctags",
srcs = select({
src = select({
"@bazel_tools//src/conditions:darwin_x86_64": ["@universal-ctags-darwin-amd64//file:downloaded"],
"@bazel_tools//src/conditions:darwin_arm64": ["@universal-ctags-darwin-arm64//file:downloaded"],
"@bazel_tools//src/conditions:linux_x86_64": ["@universal-ctags-linux-amd64//file:downloaded"],
}),
visibility = ["//visibility:public"],
)

sh_binary(
tool(
name = "gcloud",
srcs = select({
src = select({
"@bazel_tools//src/conditions:darwin_x86_64": ["@gcloud-darwin-amd64//:gcloud"],
"@bazel_tools//src/conditions:darwin_arm64": ["@gcloud-darwin-arm64//:gcloud"],
"@bazel_tools//src/conditions:linux_x86_64": ["@gcloud-linux-amd64//:gcloud"],
}),
visibility = ["//visibility:public"],
)

sh_binary(
Expand All @@ -48,64 +46,58 @@ sh_binary(
visibility = ["//visibility:public"],
)

sh_binary(
tool(
name = "packer",
srcs = select({
src = select({
"@bazel_tools//src/conditions:darwin_x86_64": ["@packer-darwin-amd64//:packer-darwin-amd64"],
"@bazel_tools//src/conditions:darwin_arm64": ["@packer-darwin-arm64//:packer-darwin-arm64"],
"@bazel_tools//src/conditions:linux_x86_64": ["@packer-linux-amd64//:packer-linux-amd64"],
}),
visibility = ["//visibility:public"],
)

sh_binary(
tool(
name = "p4-fusion",
srcs = select({
src = select({
"@bazel_tools//src/conditions:darwin_x86_64": ["@p4-fusion-darwin-amd64//file:downloaded"],
"@bazel_tools//src/conditions:darwin_arm64": ["@p4-fusion-darwin-arm64//file:downloaded"],
"@bazel_tools//src/conditions:linux_x86_64": ["@p4-fusion-linux-amd64//file:downloaded"],
}),
visibility = ["//visibility:public"],
)

sh_binary(
tool(
name = "gh",
srcs = select({
src = select({
"@bazel_tools//src/conditions:darwin_x86_64": ["@gh_darwin-amd64//:gh"],
"@bazel_tools//src/conditions:darwin_arm64": ["@gh_darwin-arm64//:gh"],
"@bazel_tools//src/conditions:linux_x86_64": ["@gh_linux-amd64//:gh"],
}),
visibility = ["//visibility:public"],
)

sh_binary(
tool(
name = "pg_dump",
srcs = select({
src = select({
"@bazel_tools//src/conditions:linux_x86_64": ["@pg_dump-linux-amd64//file:downloaded"],
"@bazel_tools//src/conditions:darwin_x86_64": ["@pg_dump-darwin-amd64//file:downloaded"],
"@bazel_tools//src/conditions:darwin_arm64": ["@pg_dump-darwin-arm64//file:downloaded"],
}),
visibility = ["//visibility:public"],
)

sh_binary(
tool(
name = "createdb",
srcs = select({
src = select({
"@bazel_tools//src/conditions:linux_x86_64": ["@createdb-linux-amd64//file:downloaded"],
"@bazel_tools//src/conditions:darwin_x86_64": ["@createdb-darwin-amd64//file:downloaded"],
"@bazel_tools//src/conditions:darwin_arm64": ["@createdb-darwin-arm64//file:downloaded"],
}),
visibility = ["//visibility:public"],
)

sh_binary(
tool(
name = "dropdb",
srcs = select({
src = select({
"@bazel_tools//src/conditions:linux_x86_64": ["@dropdb-linux-amd64//file:downloaded"],
"@bazel_tools//src/conditions:darwin_x86_64": ["@dropdb-darwin-amd64//file:downloaded"],
"@bazel_tools//src/conditions:darwin_arm64": ["@dropdb-darwin-arm64//file:downloaded"],
}),
visibility = ["//visibility:public"],
)

alias(
Expand Down
47 changes: 47 additions & 0 deletions dev/tools/tool.bzl
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
load("@aspect_bazel_lib//lib:output_files.bzl", "make_output_files")

# Convenience wrapper for rules that provide a single executable (binary/script/etc) that
# we want to rename. For example, files produced by http_file have the name `downloaded`,
# we may want to rename them with sh_binary, however this has a weird quirk whereby both
# the sh_binary wrapper "script" and the input binary/script itself are included in the outputs.
# This results in some less ergonomic usage when trying to use them in e.g. go tests, having to
# use $(rlocationpaths ...) and then filepath.Dir(runfiles.Rlocation(strings.Split(..., " ")[0])) in
# order to get the path.
# With this macro, its slightly simplified by being able to use $(rlocationpath ...) (singular) and
# not having to strings.Split(..., " ") as a result.
# See more: https://github.com/bazelbuild/bazel/issues/11820
def tool(name, src, visibility = ["//visibility:public"]):
native.sh_binary(
name = name + "_sh",
srcs = src,
)
make_bin_and_deps_available(
name = name,
out = name,
data = make_output_files(
name = name + "_out",
target = name + "_sh",
paths = [native.package_name() + "/" + name + "_sh"],
),
visibility = visibility,
)

def _make_bin_and_deps_available_impl(ctx):
symlink = ctx.actions.declare_file(ctx.attr.out)
ctx.actions.symlink(output = symlink, target_file = ctx.file.data)
return [
DefaultInfo(
executable = symlink,
files = depset(direct = [symlink]),
runfiles = ctx.runfiles(files = ctx.files.data),
),
]

make_bin_and_deps_available = rule(
_make_bin_and_deps_available_impl,
executable = True,
attrs = {
"out": attr.string(mandatory = True),
"data": attr.label(mandatory = True, allow_single_file = True),
},
)

0 comments on commit 7009f1d

Please sign in to comment.