Skip to content

Commit

Permalink
gen/genbzl: general improvements
Browse files Browse the repository at this point in the history
This change does a few things:

 * It reworks the queries in terms of eachother in-memory. This is better than
   the previous iteration whereby it'd generate the results and then rely on
   the output of that query. Instead, we just build up bigger query expressions
   and pass them to bazel using the --query_file flag.
 * It avoids exploring the pkg/ui directory (and the pkg/gen directory) because
   those can cause problems. The pkg/ui directory ends up bringing in npm,
   which hurts.
 * It stops rewriting the files before executing the queries. It no longer
   needs to rewrite them up front because they aren't referenced by later
   queries.
 * It removes the excluded target which was problematic because those files
   weren't properly visible.

Fixes cockroachdb#76521
Fixes cockroachdb#76503

Release note: None
  • Loading branch information
ajwerner authored and RajivTS committed Mar 6, 2022
1 parent 8b082f6 commit 116fb5e
Show file tree
Hide file tree
Showing 7 changed files with 252 additions and 675 deletions.
13 changes: 1 addition & 12 deletions pkg/gen/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -1,15 +1,4 @@
load(":gen.bzl", "EXPLICIT_SRCS", "docs", "execgen", "gen", "go_proto", "gomock", "misc", "optgen", "stringer")
load(":excluded.bzl", "EXCLUDED_SRCS")

filegroup(
name = "explicitly_generated",
srcs = EXPLICIT_SRCS,
)

filegroup(
name = "excluded",
srcs = EXCLUDED_SRCS,
)
load(":gen.bzl", "docs", "execgen", "gen", "go_proto", "gomock", "misc", "optgen", "stringer")

execgen()

Expand Down
515 changes: 0 additions & 515 deletions pkg/gen/excluded.bzl

This file was deleted.

9 changes: 1 addition & 8 deletions pkg/gen/gen.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,6 @@ load(":optgen.bzl", "OPTGEN_SRCS")
load(":misc.bzl", "MISC_SRCS")
load(":docs.bzl", "DOCS_SRCS")


# TODO(ajwerner): Use the all variable combined with genquery to construct
# a test to show that all of the generated files in the repo are represented
# here. Of course, this will rely on actually representing all of the generated
# file here.
EXPLICIT_SRCS = PROTOBUF_SRCS + GOMOCK_SRCS + STRINGER_SRCS + EXECGEN_SRCS + OPTGEN_SRCS + DOCS_SRCS

# GeneratedFileInfo provides two pieces of information to the _hoist_files
# rule. It provides the set of files to be hoisted via the generated_files
# field and it provides a list of commands to run to clean up potentially
Expand Down Expand Up @@ -147,7 +140,7 @@ chmod 0644 {dst}\
set -euo pipefail
{cleanup_tasks}
{cmds}\
{cmds}
"""
cleanup_tasks = []
cmds = []
Expand Down
6 changes: 5 additions & 1 deletion pkg/gen/genbzl/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,11 @@ load("@io_bazel_rules_go//go:def.bzl", "go_binary", "go_library")

go_library(
name = "genbzl_lib",
srcs = ["main.go"],
srcs = [
"main.go",
"target.go",
"targets.go",
],
importpath = "github.com/cockroachdb/cockroach/pkg/gen/genbzl",
visibility = ["//visibility:private"],
deps = [
Expand Down
187 changes: 48 additions & 139 deletions pkg/gen/genbzl/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,30 +12,29 @@
// the gen package's BUILD.bazel to facilitate hoisting these generated files
// back into the source tree.
//
// It's all a bit meta. The flow is that we invoke this binary inside the
// bazelutil/bazel-generate.sh script which writes out some bzl files with
// lists of targets which are then depended on in gen.bzl.
// The flow is that we invoke this binary inside the
// bazelutil/bazel-generate.sh script which writes out some bzl files
// defining lists of targets for generation and hoisting.
//
// The program assumes it will be run from the root of the cockroach workspace.
// If it's not, errors will occur.
package main

import (
"bufio"
"bytes"
"flag"
"fmt"
"io"
"os"
"os/exec"
"path/filepath"
"sort"
"strings"
"text/template"

"github.com/cockroachdb/cockroach/pkg/cli/exit"
"github.com/cockroachdb/errors"
)

var (
outDir = flag.String("out-dir", "", "directory in which to place the generated files")
outDir = flag.String(
"out-dir", "",
"directory in which to place the generated files",
)
)

func main() {
Expand All @@ -46,147 +45,57 @@ func main() {
}
}

var tmpl = template.Must(template.New("file").Parse(
`# Generated by genbzl
{{ .Variable }} = [{{ range .Targets }}
"{{ . }}",{{end}}
]
`))

type templateData struct {
Variable string
Targets []string
}

var targets = []*target{
{
filename: "protobuf.bzl", variable: "PROTOBUF_SRCS",
query: "kind(go_proto_library, //pkg/...)",
},
{
filename: "gomock.bzl", variable: "GOMOCK_SRCS",
query: `labels("out", kind("_gomock_prog_exec rule", //pkg/...:*))`,
},
{
filename: "stringer.bzl",
variable: "STRINGER_SRCS",
query: `labels("outs", filter("-stringer$", kind("genrule rule", //pkg/...:*)))`,
},
{
filename: "execgen.bzl", variable: "EXECGEN_SRCS",
query: `
let genrules =
kind("genrule rule", //pkg/...:*)
in labels("outs", attr("tools", "execgen", $genrules)
+ attr("exec_tools", "execgen", $genrules))`,
},
{
filename: "optgen.bzl", variable: "OPTGEN_SRCS",
query: `
let targets = attr("exec_tools", "(opt|lang)gen", kind("genrule rule", //pkg/...:*))
in let og = labels("outs", $targets)
in $og - filter(".*:.*(-gen|gen-).*", $og)`,
},
{
filename: "docs.bzl", variable: "DOCS_SRCS",
query: `
kind("generated file", //docs/...:*)
- labels("outs", //docs/generated/sql/bnf:svg)`,
},
{
filename: "excluded.bzl", variable: "EXCLUDED_SRCS",
query: `
let all = kind("generated file", //...:*)
in ($all ^ //pkg/ui/...:*)
+ ($all ^ labels("out", kind("_gomock_prog_gen rule", //pkg/...:*)))
+ filter(".*:.*(-gen|gen-).*", $all)
+ //pkg/testutils/lint/passes/errcheck:errcheck_excludes.txt
+ //build/bazelutil:test_stamping.txt
+ labels("outs", //docs/generated/sql/bnf:svg)`,
},
// MISC_SRCS is a special beast to capture everything which is not excluded
// or included explicitly in one of the other targets. By defining this
// target in this way, we ensure that we don't forget to generate anything.
// One note is that this needs to be last. The definition relies on the
// existence of the above definitions (indirectly via the fact that these
// referenced gen labels refer to variables whose data we're populating
// here).
{
filename: "misc.bzl", variable: "MISC_SRCS",
query: `
kind("generated file", //...:*)
- labels("srcs", //pkg/gen:explicitly_generated)
- labels("srcs", //pkg/gen:excluded)
`,
},
}

type target struct {
filename string
variable string
query string
}

func execQuery(q string) (results []string, _ error) {
cmd := exec.Command("bazel", "query", q)
var stdout bytes.Buffer
var stderr strings.Builder
cmd.Stdout = &stdout
cmd.Stderr = &stderr
if err := cmd.Run(); err != nil {
return nil, errors.Wrapf(err,
"failed to run %s: (stderr)\n%s", cmd, &stderr)
}
for sc := bufio.NewScanner(&stdout); sc.Scan(); {
results = append(results, sc.Text())
}
sort.Strings(results)
return results, nil
}

func generate(outDir string) error {
// Go through once and rewrite all the generated variables to be empty sets.
// The reason to do this is that the values stored in these variables are
// part of the build graph and can affect `bazel query`. If any of the
// targets were to have rotted and contain values which don't exist in the
// graph, we'd see errors. Clearing out the lists and then building them up
// again from empty helps avoid such issues.
for _, t := range targets {
if err := t.write(outDir, nil); err != nil {
return err
}
qd, err := getQueryData()
if err != nil {
return err
}
// Now go and populate the lists based on the results of the queries.
for _, t := range targets {
out, err := execQuery(t.query)
for _, q := range targets {
if q.doNotGenerate {
continue
}
out, err := q.target.execQuery(qd)
if err != nil {
return err
}
if err := t.write(outDir, out); err != nil {
if err := q.target.write(outDir, out); err != nil {
return err
}
}
return nil
}

func (t *target) write(outDir string, out []string) error {
var buf bytes.Buffer
if err := tmpl.Execute(&buf, templateData{
Variable: t.variable,
Targets: out,
}); err != nil {
return errors.Wrapf(err, "failed to execute template for %s", t.filename)
}
f, err := os.Create(filepath.Join(outDir, t.filename))
// getQueryData gets a bazel query expression that attempt to capture all of
// the files and targets we're interested in. Importantly, it excludes certain
// directories such as this here gen directory and pkg/ui. It excludes the
// current directory, because the currently generated files may have errors.
// It excludes the pkg/ui directory because including it will lead to an
// invocation of npm, which can be slow and painful on platforms without good
// npm support.
//
// The targets should be thought of as the following expression, constructed
// additively in code.
//
// build/...:* + //docs/...:* + //pkg/...:* - //pkg//ui/...:* - //pkg/gen/...:*
//
func getQueryData() (*queryData, error) {
dirs := []string{"build", "docs"}
ents, err := os.ReadDir("pkg")
if err != nil {
return errors.Wrapf(err, "failed to open file for %s", t.filename)
return nil, err
}
if _, err := io.Copy(f, &buf); err != nil {
return errors.Wrapf(err, "failed to write file for %s", t.filename)
toSkip := map[string]struct{}{"ui": {}, "gen": {}}
for _, e := range ents {
if _, shouldSkip := toSkip[e.Name()]; shouldSkip || !e.IsDir() {
continue
}
dirs = append(dirs, filepath.Join("pkg", e.Name()))
}
if err := f.Close(); err != nil {
return errors.Wrapf(err, "failed to write file for %s", t.filename)
exprs := make([]string, 0, len(dirs))
for _, dir := range dirs {
exprs = append(exprs, "//"+dir+"/...:*")
}
return nil
return &queryData{
All: "(" + strings.Join(exprs, " + ") + ")",
}, nil
}
102 changes: 102 additions & 0 deletions pkg/gen/genbzl/target.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
// Copyright 2022 The Cockroach Authors.
//
// Use of this software is governed by the Business Source License
// included in the file licenses/BSL.txt.
//
// As of the Change Date specified in that file, in accordance with
// the Business Source License, use of this software will be governed
// by the Apache License, Version 2.0, included in the file
// licenses/APL.txt.

package main

import (
"bufio"
"bytes"
"io"
"io/ioutil"
"os"
"os/exec"
"path/filepath"
"sort"
"strings"
"text/template"

"github.com/cockroachdb/errors"
)

type target string

func (t target) query() *template.Template {
return queryTemplates.Lookup(string(t))
}

func (t target) execQuery(qd *queryData) (results []string, _ error) {
f, err := ioutil.TempFile("", "")
if err != nil {
return nil, err
}
defer func() { _ = os.Remove(f.Name()) }()
if err := t.query().Execute(f, qd); err != nil {
return nil, err
}
if err := f.Close(); err != nil {
return nil, err
}
cmd := exec.Command("bazel", "query", "--query_file", f.Name())
var stdout bytes.Buffer
var stderr strings.Builder
cmd.Stdout = &stdout
cmd.Stderr = &stderr
if err := cmd.Run(); err != nil {
return nil, errors.Wrapf(err,
"failed to run %s: (stderr)\n%s", cmd, &stderr)
}
for sc := bufio.NewScanner(&stdout); sc.Scan(); {
results = append(results, sc.Text())
}
sort.Strings(results)
return results, nil
}

func (t target) filename() string {
return string(t) + ".bzl"
}

func (t target) variable() string {
return strings.ToUpper(string(t)) + "_SRCS"
}

func (t target) write(outDir string, out []string) error {
var buf bytes.Buffer
if err := outputVariableTemplate.Execute(&buf, variableData{
Variable: t.variable(),
Targets: out,
}); err != nil {
return errors.Wrapf(err, "failed to execute template for %s", t)
}
f, err := os.Create(filepath.Join(outDir, t.filename()))
if err != nil {
return errors.Wrapf(err, "failed to open file for %s", t)
}
if _, err := io.Copy(f, &buf); err != nil {
return errors.Wrapf(err, "failed to write file for %s", t)
}
if err := f.Close(); err != nil {
return errors.Wrapf(err, "failed to write file for %s", t)
}
return nil
}

var outputVariableTemplate = template.Must(template.New("file").Parse(
`# Generated by genbzl
{{ .Variable }} = [{{ range .Targets }}
"{{ . }}",{{end}}
]
`))

type variableData struct {
Variable string
Targets []string
}
Loading

0 comments on commit 116fb5e

Please sign in to comment.