cockroachdb / cockroach

CockroachDB — the cloud native, distributed SQL database designed for high availability, effortless scale, and control over data placement.
https://www.cockroachlabs.com
Other
30.21k stars 3.82k forks source link

dev: support --gcflags for `dev build` #80791

Open irfansharif opened 2 years ago

irfansharif commented 2 years ago

Is your feature request related to a problem? Please describe.

It's often useful to compile packages with additional flags passed in to go build/go tool compile. To figure out what's sitting on the heap, what's inlined, to get disassembly, etc.

$ go build -gcflags '-m' ./pkg/spanconfig/spanconfiglimiter

The Makefile had a helpful GCFLAGS env var you could set. To do it with dev you'd have to first dev gen and then invoke go build ..., which isn't re-using cached artifacts.

Describe the solution you'd like

Ricky reports the following is possible using buildozer.

$ buildozer 'add gc_goopts "S"' //pkg/sql:sql 
fixed /Users/ricky/go/src/github.com/cockroachdb/cockroach/pkg/sql/BUILD.bazel
cockroach$ git diff pkg/sql/BUILD.bazel 
diff --git a/pkg/sql/BUILD.bazel b/pkg/sql/BUILD.bazel
index 09dbe3d514..854f264d43 100644
--- a/pkg/sql/BUILD.bazel
+++ b/pkg/sql/BUILD.bazel
@@ -246,6 +246,7 @@ go_library(
         ":gen-txneventtype-stringer",  # keep
         ":gen-txntype-stringer",  # keep
     ],
+    gc_goopts = ["S"],
     importpath = "github.com/cockroachdb/cockroach/pkg/sql",
     visibility = ["//visibility:public"],
     deps = [

Describe alternatives you've considered

We could also just not do anything. I don't know if passing in additional gcflags is possible without mucking around with BUILD.bazel files directly, which comes with UX issues since this thing is checked in/commands can be cancelled without undoing the changes/etc. It makes dev do more than "just translate to equivalent bazel flags".

Jira issue: CRDB-15511

Epic CRDB-17171

irfansharif commented 2 years ago

Another possible outcome here is to spruce up the help text with everything we've accumulated thus far in https://cockroachlabs.atlassian.net/wiki/spaces/CRDB/pages/2221703221/Developing+with+Bazel#dev-vs.-make, including example uses, ways to muck with gcflags, build tags ("deadlock" is a common one).

srosenberg commented 1 year ago

Bazel now supports propagating gcflags via gc_goopts [1]. E.g., build cockroach and spit out assembly,

 ./dev build cockroach  --@io_bazel_rules_go//go/config:gc_goopts=-S

Perhaps, we should fold this option into dev since its job is to simplify the gazillion of obscure bazel options. However, note that it doesn't work for all gcflags, specifically the ones which require a comma, e.g., -json=version,dest [2].

[1] https://github.com/bazelbuild/rules_go/pull/3340 [2] https://github.com/golang/go/commit/cd53fddabb0f39288785cb2777f5250b9a3652b0

bdarnell commented 7 months ago

The command in the previous comment isn't working for me:

bdarnell@Bens-MacBook-Pro-2 cockroach % ./dev build cockroach  --@io_bazel_rules_go//go/config:gc_goopts=-S 
ERROR: unknown flag: --@io_bazel_rules_go//go/config:gc_goopts
rickystewart commented 7 months ago

@bdarnell That comment is missing a --. Actually it must be:

./dev build cockroach --  --@io_bazel_rules_go//go/config:gc_goopts=-S

This is because the argument is a Bazel argument, not a dev one. The -- tells the extra args to go to Bazel.