Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
76073: c-deps: disable autoreconf warnings for krb5 r=andreimatei a=nicktrav

`autoconf` in versions >= 2.70 [enables warnings for obsolete
commands][1]. This clogs the console output when building `libkrb5`,
either using `make` or with Bazel, when run on a system with a more
recent version of `autoconf`.

Disable the `autoconf` warnings for `krb5` with `-Wno-obsolete`, as
suggested in the release notes.

Release note: None

[1]: https://lwn.net/Articles/839395/

76341: DOC-2561: Remove trailing zero from dates in release-notes.py r=rafiss a=nickvigilante

Release note: None

76351: roachpb: stop using reflection to generate file r=ajwerner a=ajwerner

This reflection ended up causing serious build pain and represented a non-zero
portion of the critical path time of our build. AST inspection works just as
well as reflection and means we now only depend on the generated proto files.

Release note: None

Co-authored-by: Nick Travers <[email protected]>
Co-authored-by: Nick Vigilante <[email protected]>
Co-authored-by: Andrew Werner <[email protected]>
  • Loading branch information
4 people committed Feb 10, 2022
4 parents 45d1c9e + 63e1fbc + b949539 + 2f51653 commit de283bf
Show file tree
Hide file tree
Showing 12 changed files with 158 additions and 120 deletions.
1 change: 1 addition & 0 deletions .github/CODEOWNERS
Validating CODEOWNERS rules …
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,7 @@
/pkg/roachpb/dep_test.go @cockroachdb/dev-inf
/pkg/roachpb/error* @cockroachdb/kv-prs
/pkg/roachpb/gen @cockroachdb/dev-inf
/pkg/roachpb/gen.bzl @cockroachdb/dev-inf
/pkg/roachpb/app* @cockroachdb/sql-observability
/pkg/roachpb/index* @cockroachdb/sql-observability
/pkg/roachpb/internal* @cockroachdb/kv-prs
Expand Down
10 changes: 0 additions & 10 deletions BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -57,16 +57,6 @@ exports_files([
# gazelle:resolve go google.golang.org/genproto/googleapis/pubsub/v1 @org_golang_google_genproto//googleapis/pubsub/v1:pubsub
# gazelle:resolve go google.golang.org/genproto/googleapis/cloud/kms/v1 @org_golang_google_genproto//googleapis/cloud/kms/v1:kms

# These packages use github.com/golang/mock to generate mocks. The target
# generating mocks for a given package necessarily depends on the package's
# go_library target, which then prevents us from including those mock symbols
# as part of the package's go_library target. We consequently export a second
# target, embedding the first but also including the mock symbols. We need to
# tell gazelle to resolve to this second target instead. See
# pkg/kv/kvclient/rangefeed/BUILD.bazel for an annotated example.
#
# gazelle:resolve go github.com/cockroachdb/cockroach/pkg/roachpb //pkg/roachpb

# See pkg/roachpb/gen/BUILD.bazel for more details.
#
# gazelle:resolve proto go roachpb/api.proto //pkg/roachpb
Expand Down
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -619,7 +619,7 @@ $(JEMALLOC_DIR)/Makefile: $(C_DEPS_DIR)/jemalloc-rebuild $(JEMALLOC_SRC_DIR)/con
$(KRB5_SRC_DIR)/src/configure.in: | bin/.submodules-initialized

$(KRB5_SRC_DIR)/src/configure: $(KRB5_SRC_DIR)/src/configure.in
cd $(KRB5_SRC_DIR)/src && autoreconf
cd $(KRB5_SRC_DIR)/src && autoreconf -Wno-obsolete

$(KRB5_DIR)/Makefile: $(C_DEPS_DIR)/krb5-rebuild $(KRB5_SRC_DIR)/src/configure
rm -rf $(KRB5_DIR)
Expand Down
2 changes: 1 addition & 1 deletion build/bazelutil/check.sh
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ pkg/kv/kvclient/rangefeed/rangefeed.go://go:generate mockgen -destination=mocks_
pkg/kv/kvserver/concurrency/lock_table.go://go:generate ../../../util/interval/generic/gen.sh *lockState concurrency
pkg/kv/kvserver/spanlatch/manager.go://go:generate ../../../util/interval/generic/gen.sh *latch spanlatch
pkg/roachpb/api.go://go:generate mockgen -package=roachpbmock -destination=roachpbmock/mocks_generated.go . InternalClient,Internal_RangeFeedClient
pkg/roachpb/batch.go://go:generate go run -tags gen-batch gen/main.go
pkg/roachpb/batch.go://go:generate go run gen/main.go --filename batch_generated.go *.pb.go
pkg/security/certmgr/cert.go://go:generate mockgen -package=certmgr -destination=mocks_generated_test.go . Cert
pkg/security/securitytest/securitytest.go://go:generate go-bindata -mode 0600 -modtime 1400000000 -pkg securitytest -o embedded.go -ignore README.md -ignore regenerate.sh test_certs
pkg/security/securitytest/securitytest.go://go:generate gofmt -s -w embedded.go
Expand Down
3 changes: 3 additions & 0 deletions c-deps/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,9 @@ configure_make(
name = "libkrb5",
autoreconf = True,
autoreconf_directory = "src",
autoreconf_options = [
"-Wno-obsolete",
],
configure_command = "src/configure",
configure_in_place = True,
configure_options = [
Expand Down
2 changes: 1 addition & 1 deletion pkg/cmd/bazci/bazci.go
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,7 @@ func getBuildInfo(args parsedArgs) (buildInfo, error) {
switch targetKind {
case "cmake":
ret.cmakeTargets = append(ret.cmakeTargets, fullTarget)
case "genrule":
case "genrule", "batch_gen":
ret.genruleTargets = append(ret.genruleTargets, fullTarget)
case "go_binary":
ret.goBinaries = append(ret.goBinaries, fullTarget)
Expand Down
69 changes: 4 additions & 65 deletions pkg/roachpb/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,8 @@ load("@rules_proto//proto:defs.bzl", "proto_library")
load("@io_bazel_rules_go//proto:def.bzl", "go_proto_library")
load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test")
load("//build:STRINGER.bzl", "stringer")
load(":gen.bzl", "batch_gen")

# NB: If you've come here to resolve a build failure due to a missing
# dependency, the dependency may be missing from the "bootstrap" target rather
# than the "roachpb" target. Some input source files are duplicated between the
# two targets, and Gazelle only infers dependencies for "roachpb" rather than
# "bootstrap". See pkg/roachpb/gen/BUILD.bazel for more detail on why the
# "bootstrap" target exists.
go_library(
name = "roachpb",
srcs = [
Expand Down Expand Up @@ -64,59 +59,6 @@ go_library(
],
)

# See the BUILD.bazel file in pkg/roachpb/gen for more details. We need to
# partition the dependencies here to avoid cyclical structures.
#
# keep
go_library(
name = "bootstrap",
srcs = [
"api.go",
"app_stats.go",
"batch.go",
"batch_generated.go",
"data.go",
"errors.go",
"index_usage_stats.go",
"metadata.go",
"metadata_replicas.go",
"method.go",
"span_config.go",
"tenant.go",
"version.go",
],
embed = [":roachpb_go_proto"],
importpath = "github.com/cockroachdb/cockroach/pkg/roachpb",
visibility = [":__subpackages__"],
deps = [
"//pkg/geo",
"//pkg/geo/geopb",
"//pkg/kv/kvserver/concurrency/lock",
"//pkg/storage/enginepb",
"//pkg/util",
"//pkg/util/bitarray",
"//pkg/util/caller",
"//pkg/util/duration",
"//pkg/util/encoding",
"//pkg/util/hlc",
"//pkg/util/humanizeutil",
"//pkg/util/interval",
"//pkg/util/log",
"//pkg/util/protoutil",
"//pkg/util/timetz",
"//pkg/util/tracing",
"//pkg/util/uuid",
"@com_github_aws_aws_sdk_go//aws",
"@com_github_aws_aws_sdk_go//aws/credentials",
"@com_github_cockroachdb_apd_v3//:apd",
"@com_github_cockroachdb_errors//:errors",
"@com_github_cockroachdb_errors//errorspb",
"@com_github_cockroachdb_errors//extgrpc",
"@com_github_cockroachdb_redact//:redact",
"@io_etcd_go_etcd_raft_v3//raftpb",
],
)

go_test(
name = "roachpb_test",
size = "small",
Expand Down Expand Up @@ -259,11 +201,8 @@ stringer(
typ = "ErrorDetailType",
)

genrule(
batch_gen(
name = "gen-batch-generated",
outs = ["batch_generated-gen.go"],
cmd = """
$(location //pkg/roachpb/gen) --filename=$(location batch_generated-gen.go)
""",
exec_tools = ["//pkg/roachpb/gen"],
src = ":roachpb_go_proto",
out = "batch_generated-gen.go",
)
2 changes: 1 addition & 1 deletion pkg/roachpb/batch.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import (
"github.com/cockroachdb/redact"
)

//go:generate go run -tags gen-batch gen/main.go
//go:generate go run gen/main.go --filename batch_generated.go *.pb.go

// WriteTimestamp returns the timestamps at which this request is writing. For
// non-transactional requests, this is the same as the read timestamp. For
Expand Down
21 changes: 21 additions & 0 deletions pkg/roachpb/gen.bzl
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
load("@io_bazel_rules_go//go:def.bzl", "GoSource")

def _batch_gen_impl(ctx):
srcs = [src for src in ctx.attr.src[GoSource].srcs]
print(ctx.outputs.out)
ctx.actions.run(
outputs = [ctx.outputs.out],
inputs = srcs,
executable = ctx.executable._tool,
arguments = ["--filename", ctx.outputs.out.path] + [src.path for src in srcs],
)
return [DefaultInfo(files = depset([ctx.outputs.out])),]

batch_gen = rule(
implementation = _batch_gen_impl,
attrs = {
"out": attr.output(mandatory = True),
"src": attr.label(providers = [GoSource]),
"_tool": attr.label(default = "//pkg/roachpb/gen", executable = True, cfg = "exec"),
},
)
22 changes: 1 addition & 21 deletions pkg/roachpb/gen/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -1,31 +1,11 @@
load("@io_bazel_rules_go//go:def.bzl", "go_binary", "go_library")

# gen, as written, depends on symbols defined within //pkg/roachpb. gen is used
# to generated a file[1] that in turn is depended on to compile //pkg/roachpb.
# To break this circular structure, we define a separate target[2] within
# pkg/roachpb that enlists the pre-generated file in the source tree[3] in
# order to generate newer revisions within the sandbox. For another example of
# this pattern, see what we do in langgen[4].
#
# All dependents of //pkg/roachpb will need to depend on the file generated
# within the Bazel sandbox, which is achieved by [5]. Conversely, to instruct
# gazelle/Bazel to resolve gen's import of //pkg/roachpb appropriately, we add
# the resolve directive below.
#
# [1]: //pkg/roachpb/gen-batch-generated
# [2]: //pkg/roachpb:bootstrap
# [3]: //pkg/roachpb:batch_generated.go
# [4]: //pkg/sql/opt/optgen/lang:bootstrap
# [5]: See the "gazelle:resolve" directive in the top-level BUILD.bazel.
#
# gazelle:resolve go github.com/cockroachdb/cockroach/pkg/roachpb //pkg/roachpb:bootstrap

go_library(
name = "gen_lib",
srcs = ["main.go"],
importpath = "github.com/cockroachdb/cockroach/pkg/roachpb/gen",
visibility = ["//visibility:private"],
deps = ["//pkg/roachpb:bootstrap"],
deps = ["@org_golang_x_tools//go/ast/inspector"],
)

go_binary(
Expand Down
Loading

0 comments on commit de283bf

Please sign in to comment.