From c44cb5532ade159520ac1b0cd0a07936033ecd32 Mon Sep 17 00:00:00 2001 From: healthy-pod Date: Thu, 11 May 2023 12:54:09 -0700 Subject: [PATCH 1/2] dev,generate-cgo: externalize cgo files gen into a seperate binary Previously, we only needed to gen cgo files using `dev` so that was were that logic lived. Now, we need to gen cgo files outside dev to fix an issue with gcassert [see #65485]. This code change externalizes the code into a separate binary and let's `dev gen cgo` point to it. Release note: None Epic: None Informs #65485 --- .github/CODEOWNERS | 1 + pkg/BUILD.bazel | 3 + pkg/cmd/dev/generate.go | 81 ++----------- pkg/cmd/generate-cgo/BUILD.bazel | 21 ++++ pkg/cmd/generate-cgo/main.go | 191 +++++++++++++++++++++++++++++++ 5 files changed, 225 insertions(+), 72 deletions(-) create mode 100644 pkg/cmd/generate-cgo/BUILD.bazel create mode 100644 pkg/cmd/generate-cgo/main.go diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index 1479f7f11477..b3eab65bd451 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -417,6 +417,7 @@ /pkg/cmd/generate-spatial-ref-sys/ @cockroachdb/spatial /pkg/cmd/generate-bazel-extra/ @cockroachdb/dev-inf /pkg/cmd/generate-staticcheck/ @cockroachdb/dev-inf +/pkg/cmd/generate-cgo/ @cockroachdb/dev-inf /pkg/cmd/geoviz/ @cockroachdb/spatial /pkg/cmd/github-post/ @cockroachdb/test-eng /pkg/cmd/github-pull-request-make/ @cockroachdb/dev-inf diff --git a/pkg/BUILD.bazel b/pkg/BUILD.bazel index 8dc33a325faa..134cb112e862 100644 --- a/pkg/BUILD.bazel +++ b/pkg/BUILD.bazel @@ -994,6 +994,8 @@ GO_TARGETS = [ "//pkg/cmd/generate-bazel-extra:generate-bazel-extra_test", "//pkg/cmd/generate-binary:generate-binary", "//pkg/cmd/generate-binary:generate-binary_lib", + "//pkg/cmd/generate-cgo:generate-cgo", + "//pkg/cmd/generate-cgo:generate-cgo_lib", "//pkg/cmd/generate-distdir:generate-distdir", "//pkg/cmd/generate-distdir:generate-distdir_lib", "//pkg/cmd/generate-logictest:generate-logictest", @@ -2597,6 +2599,7 @@ GET_X_DATA_TARGETS = [ "//pkg/cmd/generate-acceptance-tests:get_x_data", "//pkg/cmd/generate-bazel-extra:get_x_data", "//pkg/cmd/generate-binary:get_x_data", + "//pkg/cmd/generate-cgo:get_x_data", "//pkg/cmd/generate-distdir:get_x_data", "//pkg/cmd/generate-logictest:get_x_data", "//pkg/cmd/generate-metadata-tables:get_x_data", diff --git a/pkg/cmd/dev/generate.go b/pkg/cmd/dev/generate.go index 2bd80e5b6f67..cf1187ed8864 100644 --- a/pkg/cmd/dev/generate.go +++ b/pkg/cmd/dev/generate.go @@ -15,8 +15,6 @@ import ( "fmt" "os" "path/filepath" - "runtime" - "text/template" "github.com/spf13/cobra" ) @@ -272,14 +270,6 @@ func (d *dev) generateTarget(ctx context.Context, target string) error { func (d *dev) generateCgo(cmd *cobra.Command) error { ctx := cmd.Context() - args := []string{"build", "//build/bazelutil:test_force_build_cdeps", "//c-deps:libjemalloc", "//c-deps:libproj"} - if runtime.GOOS == "linux" { - args = append(args, "//c-deps:libkrb5") - } - logCommand("bazel", args...) - if err := d.exec.CommandContextInheritingStdStreams(ctx, "bazel", args...); err != nil { - return err - } workspace, err := d.getWorkspace(ctx) if err != nil { return err @@ -288,70 +278,17 @@ func (d *dev) generateCgo(cmd *cobra.Command) error { if err != nil { return err } - - const cgoTmpl = `// GENERATED FILE DO NOT EDIT - -package {{ .Package }} - -// #cgo CPPFLAGS: {{ .CPPFlags }} -// #cgo LDFLAGS: {{ .LDFlags }} -import "C" -` - - tpl := template.Must(template.New("source").Parse(cgoTmpl)) - archived, err := d.getArchivedCdepString(bazelBin) - if err != nil { - return err - } - // Figure out where to find the c-deps libraries. - var jemallocDir, projDir, krbDir string - if archived != "" { - execRoot, err := d.getExecutionRoot(ctx) - if err != nil { - return err - } - jemallocDir = filepath.Join(execRoot, "external", fmt.Sprintf("archived_cdep_libjemalloc_%s", archived)) - projDir = filepath.Join(execRoot, "external", fmt.Sprintf("archived_cdep_libproj_%s", archived)) - if runtime.GOOS == "linux" { - krbDir = filepath.Join(execRoot, "external", fmt.Sprintf("archived_cdep_libkrb5_%s", archived)) - } - } else { - jemallocDir = filepath.Join(bazelBin, "c-deps/libjemalloc_foreign") - projDir = filepath.Join(bazelBin, "c-deps/libproj_foreign") - if runtime.GOOS == "linux" { - krbDir = filepath.Join(bazelBin, "c-deps/libkrb5_foreign") - } - } - cppFlags := fmt.Sprintf("-I%s", filepath.Join(jemallocDir, "include")) - ldFlags := fmt.Sprintf("-L%s -L%s", filepath.Join(jemallocDir, "lib"), filepath.Join(projDir, "lib")) - if krbDir != "" { - cppFlags += fmt.Sprintf(" -I%s", filepath.Join(krbDir, "include")) - ldFlags += fmt.Sprintf(" -L%s", filepath.Join(krbDir, "lib")) - } - - cgoPkgs := []string{ - "pkg/cli", - "pkg/cli/clisqlshell", - "pkg/server/status", - "pkg/ccl/gssapiccl", - "pkg/geo/geoproj", + args := []string{ + "run", + "//pkg/cmd/generate-cgo:generate-cgo", + "--", + "--workspace", workspace, + "--bazel-bin", bazelBin, } - - for _, cgoPkg := range cgoPkgs { - out, err := os.Create(filepath.Join(workspace, cgoPkg, "zcgo_flags.go")) - if err != nil { - return err - } - err = tpl.Execute(out, struct { - Package string - CPPFlags string - LDFlags string - }{Package: filepath.Base(cgoPkg), CPPFlags: cppFlags, LDFlags: ldFlags}) - if err != nil { - return err - } + logCommand("bazel", args...) + if err := d.exec.CommandContextInheritingStdStreams(ctx, "bazel", args...); err != nil { + return fmt.Errorf("generating cgo: %w", err) } - return nil } diff --git a/pkg/cmd/generate-cgo/BUILD.bazel b/pkg/cmd/generate-cgo/BUILD.bazel new file mode 100644 index 000000000000..f44f5b0302e5 --- /dev/null +++ b/pkg/cmd/generate-cgo/BUILD.bazel @@ -0,0 +1,21 @@ +load("//build/bazelutil/unused_checker:unused.bzl", "get_x_data") +load("@io_bazel_rules_go//go:def.bzl", "go_binary", "go_library") + +go_library( + name = "generate-cgo_lib", + srcs = ["main.go"], + importpath = "github.com/cockroachdb/cockroach/pkg/cmd/generate-cgo", + visibility = ["//visibility:private"], + deps = [ + "@com_github_alessio_shellescape//:shellescape", + "@com_github_cockroachdb_errors//:errors", + ], +) + +go_binary( + name = "generate-cgo", + embed = [":generate-cgo_lib"], + visibility = ["//visibility:public"], +) + +get_x_data(name = "get_x_data") diff --git a/pkg/cmd/generate-cgo/main.go b/pkg/cmd/generate-cgo/main.go new file mode 100644 index 000000000000..8cfeb84df488 --- /dev/null +++ b/pkg/cmd/generate-cgo/main.go @@ -0,0 +1,191 @@ +// Copyright 2023 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 ( + "flag" + "fmt" + "log" + "os" + "os/exec" + "path/filepath" + "runtime" + "strings" + "text/template" + + "github.com/cockroachdb/errors" + + "github.com/alessio/shellescape" +) + +type configuration struct { + Os string + Arch string +} + +var ( + workspace string + bazelBin string + archivedCdepConfigurations = []configuration{ + {"linux", "amd64"}, + {"linux", "arm64"}, + {"darwin", "amd64"}, + {"darwin", "arm64"}, + {"windows", "amd64"}, + } +) + +func init() { + flag.StringVar(&workspace, "workspace", "", "workspace path") + flag.StringVar(&bazelBin, "bazel-bin", "", "path to bazel/bin") +} + +func logCommand(cmd string, args ...string) { + var fullArgs []string + fullArgs = append(fullArgs, cmd) + fullArgs = append(fullArgs, args...) + log.Printf("$ %s", shellescape.QuoteCommand(fullArgs)) +} + +// getArchivedCdepString returns a non-empty string iff the force_build_cdeps +// config is not being used. This string is the name of the cross config used to +// build the pre-built c-deps, minus the "cross" prefix. This can be used to +// locate the pre-built c-dep in +// $EXECUTION_ROOT/external/archived_cdep_{LIB}_{ARCHIVED_CDEP_STRING}. +// If the returned string is empty then force_build_cdeps is set in which case +// the (non-pre-built) libraries can be found in $BAZEL_BIN/c-deps/{LIB}_foreign. +// +// You MUST build //build/bazelutil:test_force_build_cdeps before calling this +// function. +func getArchivedCdepString(bazelBin string) (string, error) { + var ret string + // If force_build_cdeps is set then the prebuilt libraries won't be in + // the archived location anyway. + forceBuildCdeps, err := os.ReadFile(filepath.Join(bazelBin, "build", "bazelutil", "test_force_build_cdeps.txt")) + if err != nil { + return "", err + } + // force_build_cdeps is activated if the length of this file is not 0. + if len(forceBuildCdeps) == 0 { + for _, config := range archivedCdepConfigurations { + if config.Os == runtime.GOOS && config.Arch == runtime.GOARCH { + ret = config.Os + if ret == "darwin" { + ret = "macos" + } + if config.Arch == "arm64" { + ret += "arm" + } + break + } + } + } + return ret, nil +} + +func getExecutionRoot() (string, error) { + args := []string{"info", "execution_root", "--color=no"} + logCommand("bazel", args...) + cmd := exec.Command("bazel", args...) + cmd.Dir = workspace + if output, err := cmd.CombinedOutput(); err != nil { + return "", errors.Wrapf(err, "Output: %s", string(output)) + } else { + return strings.TrimSpace(string(output)), nil + } +} + +func generateCgo() error { + if workspace == "" || bazelBin == "" { + return errors.New("--workspace and --bazel-bin are required") + } + args := []string{"build", "//build/bazelutil:test_force_build_cdeps", "//c-deps:libjemalloc", "//c-deps:libproj"} + if runtime.GOOS == "linux" { + args = append(args, "//c-deps:libkrb5") + } + logCommand("bazel", args...) + cmd := exec.Command("bazel", args...) + cmd.Dir = workspace + if output, err := cmd.CombinedOutput(); err != nil { + errors.Wrapf(err, "Output: %s", string(output)) + } + + const cgoTmpl = `// GENERATED FILE DO NOT EDIT + + package {{ .Package }} + + // #cgo CPPFLAGS: {{ .CPPFlags }} + // #cgo LDFLAGS: {{ .LDFlags }} + import "C" + ` + + tpl := template.Must(template.New("source").Parse(cgoTmpl)) + archived, err := getArchivedCdepString(bazelBin) + if err != nil { + return err + } + // Figure out where to find the c-deps libraries. + var jemallocDir, projDir, krbDir string + if archived != "" { + execRoot, err := getExecutionRoot() + if err != nil { + return err + } + jemallocDir = filepath.Join(execRoot, "external", fmt.Sprintf("archived_cdep_libjemalloc_%s", archived)) + projDir = filepath.Join(execRoot, "external", fmt.Sprintf("archived_cdep_libproj_%s", archived)) + if runtime.GOOS == "linux" { + krbDir = filepath.Join(execRoot, "external", fmt.Sprintf("archived_cdep_libkrb5_%s", archived)) + } + } else { + jemallocDir = filepath.Join(bazelBin, "c-deps/libjemalloc_foreign") + projDir = filepath.Join(bazelBin, "c-deps/libproj_foreign") + if runtime.GOOS == "linux" { + krbDir = filepath.Join(bazelBin, "c-deps/libkrb5_foreign") + } + } + cppFlags := fmt.Sprintf("-I%s", filepath.Join(jemallocDir, "include")) + ldFlags := fmt.Sprintf("-L%s -L%s", filepath.Join(jemallocDir, "lib"), filepath.Join(projDir, "lib")) + if krbDir != "" { + cppFlags += fmt.Sprintf(" -I%s", filepath.Join(krbDir, "include")) + ldFlags += fmt.Sprintf(" -L%s", filepath.Join(krbDir, "lib")) + } + + cgoPkgs := []string{ + "pkg/cli", + "pkg/cli/clisqlshell", + "pkg/server/status", + "pkg/ccl/gssapiccl", + "pkg/geo/geoproj", + } + + for _, cgoPkg := range cgoPkgs { + out, err := os.Create(filepath.Join(workspace, cgoPkg, "zcgo_flags.go")) + if err != nil { + return err + } + err = tpl.Execute(out, struct { + Package string + CPPFlags string + LDFlags string + }{Package: filepath.Base(cgoPkg), CPPFlags: cppFlags, LDFlags: ldFlags}) + if err != nil { + return err + } + } + return nil +} + +func main() { + flag.Parse() + if err := generateCgo(); err != nil { + panic(err) + } +} From de5e2c097039cd0a4e0a317671734c2d79036d2b Mon Sep 17 00:00:00 2001 From: healthy-pod Date: Thu, 11 May 2023 14:39:40 -0700 Subject: [PATCH 2/2] gcassert: upgrade gcassert and generate cgo files before running it Release note: None Epic: none Closes #65485 --- DEPS.bzl | 6 +++--- build/bazelutil/distdir_files.bzl | 2 +- build/teamcity/cockroach/ci/tests/gcassert_impl.sh | 5 +++++ go.mod | 2 +- go.sum | 4 ++-- 5 files changed, 12 insertions(+), 7 deletions(-) diff --git a/DEPS.bzl b/DEPS.bzl index 774b74b28311..f5a4ce3ab040 100644 --- a/DEPS.bzl +++ b/DEPS.bzl @@ -5059,10 +5059,10 @@ def go_deps(): name = "com_github_jordanlewis_gcassert", build_file_proto_mode = "disable_global", importpath = "github.com/jordanlewis/gcassert", - sha256 = "4e6d2be78096ee4158d921af5d388f76bbf6d2638cee052ca628ba70da911704", - strip_prefix = "github.com/jordanlewis/gcassert@v0.0.0-20221027203946-81f097ad35a0", + sha256 = "3919384e0288d9ce93da816ef227aec2c61b978eb761bd45a7fded957f792dcf", + strip_prefix = "github.com/jordanlewis/gcassert@v0.0.0-20230505190637-fed79d91cd5f", urls = [ - "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/jordanlewis/gcassert/com_github_jordanlewis_gcassert-v0.0.0-20221027203946-81f097ad35a0.zip", + "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/jordanlewis/gcassert/com_github_jordanlewis_gcassert-v0.0.0-20230505190637-fed79d91cd5f.zip", ], ) go_repository( diff --git a/build/bazelutil/distdir_files.bzl b/build/bazelutil/distdir_files.bzl index 3097d683b27b..06f6f95b1561 100644 --- a/build/bazelutil/distdir_files.bzl +++ b/build/bazelutil/distdir_files.bzl @@ -654,7 +654,7 @@ DISTDIR_FILES = { "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/joho/godotenv/com_github_joho_godotenv-v1.3.0.zip": "acef5a394fbd1193f52d0d19690b0bfe82728d18dd3bf67730dc5031c22d563f", "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/jonboulle/clockwork/com_github_jonboulle_clockwork-v0.1.0.zip": "930d355d1ced60a668bcbca6154bb5671120ba11a34119505d1c0677f7bbbf97", "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/jordan-wright/email/com_github_jordan_wright_email-v4.0.1-0.20210109023952-943e75fe5223+incompatible.zip": "6d35fa83ea02cfacd0e1ba9c9061381b963215cef84c8bf83ad5944cb304c390", - "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/jordanlewis/gcassert/com_github_jordanlewis_gcassert-v0.0.0-20221027203946-81f097ad35a0.zip": "4e6d2be78096ee4158d921af5d388f76bbf6d2638cee052ca628ba70da911704", + "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/jordanlewis/gcassert/com_github_jordanlewis_gcassert-v0.0.0-20230505190637-fed79d91cd5f.zip": "3919384e0288d9ce93da816ef227aec2c61b978eb761bd45a7fded957f792dcf", "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/josharian/intern/com_github_josharian_intern-v1.0.0.zip": "5679bfd11c14adccdb45bd1a0f9cf4b445b95caeed6fb507ba96ecced11c248d", "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/jpillora/backoff/com_github_jpillora_backoff-v1.0.0.zip": "f856692c725143c49b9cceabfbca8bc93d3dbde84a0aaa53fb26ed3774c220cc", "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/json-iterator/go/com_github_json_iterator_go-v1.1.12.zip": "d001ea57081afd0e378467c8f4a9b6a51259996bb8bb763f78107eaf12f99501", diff --git a/build/teamcity/cockroach/ci/tests/gcassert_impl.sh b/build/teamcity/cockroach/ci/tests/gcassert_impl.sh index 2834a2f77374..46e319b2a1a2 100755 --- a/build/teamcity/cockroach/ci/tests/gcassert_impl.sh +++ b/build/teamcity/cockroach/ci/tests/gcassert_impl.sh @@ -4,6 +4,10 @@ set -xeuo pipefail bazel build @com_github_jordanlewis_gcassert//cmd/gcassert:gcassert --config=ci bazel run //pkg/gen:code +WORKSPACE=$(bazel info workspace --color=no) +BAZEL_BIN=$(bazel info bazel-bin --color=no) +bazel run //pkg/gen:code +bazel run //pkg/cmd/generate-cgo:generate-cgo -- --workspace $WORKSPACE --bazel-bin $BAZEL_BIN GODIR=$(dirname $(bazel run @go_sdk//:bin/go --run_under=realpath)) echo "##teamcity[testStarted name='GcAssert' captureStandardOutput='true']" exit_status=0 @@ -12,4 +16,5 @@ if [ "$exit_status" -ne 0 ]; then echo "##teamcity[testFailed name='GcAssert']" fi echo "##teamcity[testFinished name='GcAssert']" +cp /tmp/gcassert* $PWD/artifacts diff --git a/go.mod b/go.mod index c1f53f128c51..f49c368165b7 100644 --- a/go.mod +++ b/go.mod @@ -159,7 +159,7 @@ require ( github.com/jackc/pgx/v5 v5.3.1 github.com/jaegertracing/jaeger v1.18.1 github.com/jordan-wright/email v4.0.1-0.20210109023952-943e75fe5223+incompatible - github.com/jordanlewis/gcassert v0.0.0-20221027203946-81f097ad35a0 + github.com/jordanlewis/gcassert v0.0.0-20230505190637-fed79d91cd5f github.com/kevinburke/go-bindata v3.13.0+incompatible github.com/kisielk/errcheck v1.6.1-0.20210625163953-8ddee489636a github.com/kisielk/gotool v1.0.0 diff --git a/go.sum b/go.sum index a7cb7043cba1..91e4e260905a 100644 --- a/go.sum +++ b/go.sum @@ -1438,8 +1438,8 @@ github.com/joho/godotenv v1.3.0/go.mod h1:7hK45KPybAkOC6peb+G5yklZfMxEjkZhHbwpqx github.com/jonboulle/clockwork v0.1.0/go.mod h1:Ii8DK3G1RaLaWxj9trq07+26W01tbo22gdxWY5EU2bo= github.com/jordan-wright/email v4.0.1-0.20210109023952-943e75fe5223+incompatible h1:jdpOPRN1zP63Td1hDQbZW73xKmzDvZHzVdNYxhnTMDA= github.com/jordan-wright/email v4.0.1-0.20210109023952-943e75fe5223+incompatible/go.mod h1:1c7szIrayyPPB/987hsnvNzLushdWf4o/79s3P08L8A= -github.com/jordanlewis/gcassert v0.0.0-20221027203946-81f097ad35a0 h1:y9VRjxb2hZlTElaaUH50QkZnPKrvQR5NhPza66vh+9w= -github.com/jordanlewis/gcassert v0.0.0-20221027203946-81f097ad35a0/go.mod h1:FsDIOYX6N9TFyHD602Yu/Zp04+KySTH00okDq+9IPqo= +github.com/jordanlewis/gcassert v0.0.0-20230505190637-fed79d91cd5f h1:GoV80Ggs5ZeF6uDv/lYDGJAv2Wz3Mp/brqRbjvEm6mI= +github.com/jordanlewis/gcassert v0.0.0-20230505190637-fed79d91cd5f/go.mod h1:FsDIOYX6N9TFyHD602Yu/Zp04+KySTH00okDq+9IPqo= github.com/josharian/intern v1.0.0 h1:vlS4z54oSdjm0bgjRigI+G1HpF+tI+9rE5LLzOg8HmY= github.com/josharian/intern v1.0.0/go.mod h1:5DoeVV0s6jJacbCEi61lwdGj/aVlrQvzHFFd8Hwg//Y= github.com/jpillora/backoff v1.0.0 h1:uvFg412JmmHBHw7iwprIxkPMI+sGQ4kzOWsMeHnm2EA=