Skip to content

Commit

Permalink
Merge #58019
Browse files Browse the repository at this point in the history
58019: lang{,gen},bazel: generate more files within the sandbox r=irfansharif a=irfansharif

Part of #57801. Generating these files within the sandbox was a bit
tricky seeing as how the optgen language compiler uses itself to
generate its AST expressions. This is a form of compiler
bootstrapping [1], and needed some manual overrides to fit into how Bazel
wants things to be.

To this end, we define a second `go_library` target [2] that sources
pre-generated `og.go` files that are already checked into the source
tree. These files are then used to compile `langgen`, which is in turn
used to generate newer revisions of those same generated files. All
dependants of [3] will need to depend on the og.go files generated in
the Bazel sandbox, which is achieved by [4]. Conversely, to instruct
gazelle/Bazel to resolve langgen's import of [3] appropriately, we added
a custom resolve directive.

We'll eventually want to be able to implant the sandbox generated files
back into the source tree. This will be necessary for IDEs, but also for
a sane workflow for the engineers working on langgen. If they were only
using Bazel today, langgen dependent builds would misbehave as we
wouldn't be updating the checked-in og.go files used to build langgen
itself (assuming any diffs there would result in different
code/compilation behaviour). See
#58018.

```
[1]: https://en.wikipedia.org/wiki/Bootstrapping_(compilers)
[2]: //pkg/sql/opt/optgen/lang:bootstrap
[3]: github.com/cockroachdb/cockroach/pkg/sql/opt/optgen/lang
[4]: See the "gazelle:resolve" directive in the top-level BUILD.bazel file
```
Release note: None

Co-authored-by: irfan sharif <[email protected]>
  • Loading branch information
craig[bot] and irfansharif committed Dec 17, 2020
2 parents f9e7709 + 4f66e24 commit bdc14ea
Show file tree
Hide file tree
Showing 4 changed files with 80 additions and 10 deletions.
8 changes: 6 additions & 2 deletions BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -13,17 +13,21 @@ load("@bazel_gazelle//:def.bzl", "gazelle")
# gazelle:prefix github.com/cockroachdb/cockroach
# gazelle:build_file_name BUILD.bazel

# We disable protobuf generation for our dependencies
# We disable protobuf generation for our dependencies.
#
# gazelle:proto disable_global

# Gazelle is unable to resolve this specific package.
# Gazelle needs the following resolve hints.
#
# TODO(irfansharif): I'm not sure why this is. Is it because it's a proto only
# package?
#
# gazelle:resolve go github.com/grpc-ecosystem/grpc-gateway/internal //vendor/github.com/grpc-ecosystem/grpc-gateway/internal

# See pkg/sql/opt/optgen/cmd/langgen/BUILD.bazel for more details.
#
# gazelle:resolve go github.com/cockroachdb/cockroach/pkg/sql/opt/optgen/lang //pkg/sql/opt/optgen/lang

# We exclude a few things from gazelle consideration:
# - The protobuf C dependency, it's are already bazel-ified, lest we overwrite
# those build files
Expand Down
5 changes: 0 additions & 5 deletions pkg/sql/lexbase/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -1,10 +1,5 @@
load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test")

# TODO(irfansharif): For now we've pinned the auto-generated
# reserved_keywords.go file. We'll want to eventually teach bazel to generate
# it within it's sandbox. What this means for now is that Bazel expects the
# autogenerated file to already have been generated from elsewhere (`make
# buildshort`).
go_library(
name = "lex",
srcs = [
Expand Down
28 changes: 27 additions & 1 deletion pkg/sql/opt/optgen/cmd/langgen/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -1,5 +1,31 @@
load("@io_bazel_rules_go//go:def.bzl", "go_binary", "go_library")

# The Optgen language compiler uses itself to generate its AST expressions.
# This is a form of compiler bootstrapping[1], and needs some manual overrides
# to fit into how Bazel wants things to be. We define a second go_library
# target[2], that sources pre-generated og.go files that are already checked
# into the source tree. These files are used to compile langgen, which is then
# used to generate newer revisions of those same generated files. All
# dependants of [3] will need to depend on the og.go files generated in the
# Bazel sandbox, which is achieved by [4]. Conversely, to instruct
# gazelle/Bazel to resolve langgen's import of [3] appropriately, we add the
# resolve directive below.
#
# TODO(irfansharif): We'll eventually want to be able to implant the sandbox
# generated files back into the source tree. This will be necessary for IDEs,
# but also for a sane workflow for the engineers working on langgen. If they
# were only using Bazel today, langgen dependent builds would misbehave as we
# wouldn't be updating the checked-in og.go files used to build langgen itself
# (assuming any diffs there would result in different code/compilation
# behavior). See https://github.com/cockroachdb/cockroach/issues/58018.
#
# [1]: https://en.wikipedia.org/wiki/Bootstrapping_(compilers)
# [2]: //pkg/sql/opt/optgen/lang:bootstrap
# [3]: github.com/cockroachdb/cockroach/pkg/sql/opt/optgen/lang
# [4]: See the "gazelle:resolve" directive in the top-level BUILD.bazel file
#
# gazelle:resolve go github.com/cockroachdb/cockroach/pkg/sql/opt/optgen/lang //pkg/sql/opt/optgen/lang:bootstrap

go_library(
name = "langgen_lib",
srcs = [
Expand All @@ -10,7 +36,7 @@ go_library(
importpath = "github.com/cockroachdb/cockroach/pkg/sql/opt/optgen/cmd/langgen",
visibility = ["//visibility:private"],
deps = [
"//pkg/sql/opt/optgen/lang",
"//pkg/sql/opt/optgen/lang:bootstrap", # keep
"//vendor/github.com/cockroachdb/errors",
],
)
Expand Down
49 changes: 47 additions & 2 deletions pkg/sql/opt/optgen/lang/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,32 @@ go_library(
"data_type.go",
"doc.go",
"expr.go",
"expr.og.go", # keep
"operator.og.go", # keep
"operator_string.go",
"parser.go",
"scanner.go",
"token_string.go",
":gen-expr", # keep
":gen-operator", # keep
],
importpath = "github.com/cockroachdb/cockroach/pkg/sql/opt/optgen/lang",
visibility = ["//visibility:public"],
deps = ["//vendor/github.com/cockroachdb/errors"],
)

# Langgen is a bootstrapping compiler. We need to partition the dependencies
# here to avoid cyclical structures. See the BUILD.bazel file for langgen for
# more details.
#
# keep
go_library(
name = "bootstrap",
srcs = [
"compiler.go",
"data_type.go",
"doc.go",
"expr.go",
"expr.og.go",
"operator.og.go",
"operator_string.go",
"parser.go",
"scanner.go",
Expand All @@ -33,3 +57,24 @@ go_test(
"//vendor/github.com/cockroachdb/errors",
],
)

# Define the generator for the expression definitions and functions.
genrule(
name = "gen-expr",
srcs = ["lang.opt"],
outs = ["expr-gen.og.go"],
cmd = """
$(location //pkg/sql/opt/optgen/cmd/langgen) -out $@ exprs $(location lang.opt)
""",
tools = ["//pkg/sql/opt/optgen/cmd/langgen"],
)

genrule(
name = "gen-operator",
srcs = ["lang.opt"],
outs = ["operator-gen.og.go"],
cmd = """
$(location //pkg/sql/opt/optgen/cmd/langgen) -out $@ ops $(location lang.opt)
""",
tools = ["//pkg/sql/opt/optgen/cmd/langgen"],
)

0 comments on commit bdc14ea

Please sign in to comment.