From 83d4e7279be24a368cc693201b6f411b93d968f9 Mon Sep 17 00:00:00 2001 From: healthy-pod Date: Tue, 6 Jun 2023 15:44:27 -0700 Subject: [PATCH] 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 + dev | 2 +- pkg/BUILD.bazel | 3 + pkg/cmd/dev/generate.go | 83 ++----------- pkg/cmd/generate-cgo/BUILD.bazel | 21 ++++ pkg/cmd/generate-cgo/main.go | 196 +++++++++++++++++++++++++++++++ 6 files changed, 229 insertions(+), 77 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 fd60c4d7d770..b1bd30c09011 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/dev b/dev index 015f9d5ba688..6b374446a690 100755 --- a/dev +++ b/dev @@ -8,7 +8,7 @@ fi set -euo pipefail # Bump this counter to force rebuilding `dev` on all machines. -DEV_VERSION=73 +DEV_VERSION=74 THIS_DIR=$(cd "$(dirname "$0")" && pwd) BINARY_DIR=$THIS_DIR/bin/dev-versions diff --git a/pkg/BUILD.bazel b/pkg/BUILD.bazel index 40a82717c5b2..2c9ddabca00b 100644 --- a/pkg/BUILD.bazel +++ b/pkg/BUILD.bazel @@ -1004,6 +1004,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", @@ -2621,6 +2623,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..dd661ce2ab45 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,86 +270,19 @@ 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 } - bazelBin, err := d.getBazelBin(ctx) - 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", + fmt.Sprintf("--run_under=cd %s && ", workspace), } - - 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..35a40109cc4b --- /dev/null +++ b/pkg/cmd/generate-cgo/main.go @@ -0,0 +1,196 @@ +// 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 ( + "fmt" + "log" + "os" + "os/exec" + "path/filepath" + "runtime" + "strings" + "text/template" + + "github.com/alessio/shellescape" + "github.com/cockroachdb/errors" +) + +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 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 getBazelInfo(info string) (string, error) { + args := []string{"info", info, "--color=no"} + logCommand("bazel", args...) + cmd := exec.Command("bazel", args...) + var outBuf, errBuf strings.Builder + cmd.Stdout = &outBuf + cmd.Stderr = &errBuf + if err := cmd.Run(); err != nil { + return "", errors.Wrapf(err, "Stderr: %s", errBuf.String()) + } else { + return strings.TrimSpace(outBuf.String()), 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...) + var outBuf, errBuf strings.Builder + cmd.Stdout = &outBuf + cmd.Stderr = &errBuf + if err := cmd.Run(); err != nil { + return errors.Wrapf(err, "Stderr: %s", errBuf.String()) + } + + 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 := getBazelInfo("execution_root") + 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() { + var err error + workspace, err = getBazelInfo("workspace") + if err != nil { + log.Fatal(err) + } + bazelBin, err = getBazelInfo("bazel-bin") + if err != nil { + log.Fatal(err) + } + if err := generateCgo(); err != nil { + log.Fatal(err) + } +}