Skip to content

Commit

Permalink
Evaluate build tags as both true and false
Browse files Browse the repository at this point in the history 
Gazelle already iterates over all supported os/arch combos to include go
files. This is similar but expanded to custom tags. When adding a new
build tag, we create a copy with the tag set to true and append to the
original. This means that build tag evaluation will grow exponentially.

Fixes bazel-contrib#1262
  • Loading branch information
Patrick Scott committed Sep 27, 2024
1 parent d16fc42 commit 8162813
Show file tree
Hide file tree
Showing 6 changed files with 67 additions and 71 deletions.
4 changes: 2 additions & 2 deletions Design.rst
View file
Original file line number Diff line number Diff line change
Expand Up @@ -107,8 +107,8 @@ Here are a few examples. See the `full list of directives`_.

* ``# gazelle:prefix`` - sets the Go import path prefix for the current
directory.
* ``# gazelle:build_tags`` - sets the list of build tags which Gazelle considers
to be true on all platforms.
* ``# gazelle:build_tags`` - sets the list of build tags which Gazelle will
defer to Bazel for evaluation.

There are a few directives which are not applied to the ``Config`` object but
are interpreted directly in packages where they are relevant.
Expand Down
10 changes: 6 additions & 4 deletions README.rst
View file
Original file line number Diff line number Diff line change
Expand Up @@ -485,11 +485,12 @@ The following flags are accepted:
+-------------------------------------------------------------------+----------------------------------------+
| :flag:`-build_tags tag1,tag2` | |
+-------------------------------------------------------------------+----------------------------------------+
| List of Go build tags Gazelle will consider to be true. Gazelle applies |
| List of Go build tags Gazelle will defer to Bazel for evaluation. Gazelle applies |
| constraints when generating Go rules. It assumes certain tags are true on |
| certain platforms (for example, ``amd64,linux``). It assumes all Go release |
| tags are true (for example, ``go1.8``). It considers other tags to be false |
| (for example, ``ignore``). This flag overrides that behavior. |
| (for example, ``ignore``). This flag allows custom tags to be evaluated by |
| Bazel at build time. |
| |
| Bazel may still filter sources with these tags. Use |
| ``bazel build --define gotags=foo,bar`` to set tags at build time. |
Expand Down Expand Up @@ -751,11 +752,12 @@ The following directives are recognized:
+---------------------------------------------------+----------------------------------------+
| :direc:`# gazelle:build_tags foo,bar` | none |
+---------------------------------------------------+----------------------------------------+
| List of Go build tags Gazelle will consider to be true. Gazelle applies |
| List of Go build tags Gazelle will defer to Bazel for evaluation. Gazelle applies |
| constraints when generating Go rules. It assumes certain tags are true on |
| certain platforms (for example, ``amd64,linux``). It assumes all Go release |
| tags are true (for example, ``go1.8``). It considers other tags to be false |
| (for example, ``ignore``). This flag overrides that behavior. |
| (for example, ``ignore``). This flag allows custom tags to be evaluated by |
| Bazel at build time. |
| |
| Bazel may still filter sources with these tags. Use |
| ``bazel build --define gotags=foo,bar`` to set tags at build time. |
Expand Down
45 changes: 24 additions & 21 deletions language/go/config.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,16 @@ import (

var minimumRulesGoVersion = version.Version{0, 29, 0}

type tagSet map[string]bool

func (ts tagSet) clone() tagSet {
c := make(tagSet)
for k, v := range ts {
c[k] = v
}
return c
}

// goConfig contains configuration values related to Go rules.
type goConfig struct {
// The name under which the rules_go repository can be referenced from the
Expand All @@ -53,7 +63,7 @@ type goConfig struct {

// genericTags is a set of tags that Gazelle considers to be true. Set with
// -build_tags or # gazelle:build_tags. Some tags, like gc, are always on.
genericTags map[string]bool
genericTags []tagSet

// prefix is a prefix of an import path, used to generate importpath
// attributes. Set with -go_prefix or # gazelle:prefix.
Expand Down Expand Up @@ -178,8 +188,10 @@ func newGoConfig() *goConfig {
goProtoCompilers: defaultGoProtoCompilers,
goGrpcCompilers: defaultGoGrpcCompilers,
goGenerateProto: true,
genericTags: []tagSet{
{"gc": true},
},
}
gc.preprocessTags()
return gc
}

Expand All @@ -189,28 +201,18 @@ func getGoConfig(c *config.Config) *goConfig {

func (gc *goConfig) clone() *goConfig {
gcCopy := *gc
gcCopy.genericTags = make(map[string]bool)
for k, v := range gc.genericTags {
gcCopy.genericTags[k] = v
gcCopy.genericTags = make([]tagSet, 0, len(gc.genericTags))
for _, ts := range gc.genericTags {
gcCopy.genericTags = append(gcCopy.genericTags, ts.clone())
}
gcCopy.goProtoCompilers = gc.goProtoCompilers[:len(gc.goProtoCompilers):len(gc.goProtoCompilers)]
gcCopy.goGrpcCompilers = gc.goGrpcCompilers[:len(gc.goGrpcCompilers):len(gc.goGrpcCompilers)]
gcCopy.submodules = gc.submodules[:len(gc.submodules):len(gc.submodules)]
return &gcCopy
}

// preprocessTags adds some tags which are on by default before they are
// used to match files.
func (gc *goConfig) preprocessTags() {
if gc.genericTags == nil {
gc.genericTags = make(map[string]bool)
}
gc.genericTags["gc"] = true
}

// setBuildTags sets genericTags by parsing as a comma separated list. An
// error will be returned for tags that wouldn't be recognized by "go build".
// preprocessTags should be called before this.
func (gc *goConfig) setBuildTags(tags string) error {
if tags == "" {
return nil
Expand All @@ -219,7 +221,13 @@ func (gc *goConfig) setBuildTags(tags string) error {
if strings.HasPrefix(t, "!") {
return fmt.Errorf("build tags can't be negated: %s", t)
}
gc.genericTags[t] = true
var newSets []tagSet
for _, ts := range gc.genericTags {
c := ts.clone()
c[t] = true
newSets = append(newSets, c)
}
gc.genericTags = append(gc.genericTags, newSets...)
}
return nil
}
Expand Down Expand Up @@ -578,11 +586,6 @@ Update io_bazel_rules_go to a newer version in your WORKSPACE file.`
for _, d := range f.Directives {
switch d.Key {
case "build_tags":
if err := gc.setBuildTags(d.Value); err != nil {
log.Print(err)
continue
}
gc.preprocessTags()
if err := gc.setBuildTags(d.Value); err != nil {
log.Print(err)
}
Expand Down
35 changes: 11 additions & 24 deletions language/go/config_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,13 @@ func testConfig(t *testing.T, args ...string) (*config.Config, []language.Langua
return c, langs, cexts
}

var expectedBuildTags = []tagSet{
{"gc": true},
{"gc": true, "foo": true},
{"gc": true, "bar": true},
{"gc": true, "foo": true, "bar": true},
}

func TestCommandLine(t *testing.T) {
c, _, _ := testConfig(
t,
Expand All @@ -68,10 +75,8 @@ func TestCommandLine(t *testing.T) {
"-external=vendored",
"-repo_root=.")
gc := getGoConfig(c)
for _, tag := range []string{"foo", "bar", "gc"} {
if !gc.genericTags[tag] {
t.Errorf("expected tag %q to be set", tag)
}
if diff := cmp.Diff(expectedBuildTags, gc.genericTags); diff != "" {
t.Errorf("(-want, +got): %s", diff)
}
if gc.prefix != "example.com/repo" {
t.Errorf(`got prefix %q; want "example.com/repo"`, gc.prefix)
Expand Down Expand Up @@ -101,10 +106,8 @@ func TestDirectives(t *testing.T) {
cext.Configure(c, "test", f)
}
gc := getGoConfig(c)
for _, tag := range []string{"foo", "bar", "gc"} {
if !gc.genericTags[tag] {
t.Errorf("expected tag %q to be set", tag)
}
if diff := cmp.Diff(expectedBuildTags, gc.genericTags); diff != "" {
t.Errorf("(-want, +got): %s", diff)
}
if gc.prefix != "y" {
t.Errorf(`got prefix %q; want "y"`, gc.prefix)
Expand Down Expand Up @@ -268,22 +271,6 @@ load("@io_bazel_rules_go//proto:go_proto_library.bzl", "go_proto_library")
}
}

func TestPreprocessTags(t *testing.T) {
gc := newGoConfig()
expectedTags := []string{"gc"}
for _, tag := range expectedTags {
if !gc.genericTags[tag] {
t.Errorf("tag %q not set", tag)
}
}
unexpectedTags := []string{"x", "cgo", "go1.8", "go1.7"}
for _, tag := range unexpectedTags {
if gc.genericTags[tag] {
t.Errorf("tag %q unexpectedly set", tag)
}
}
}

func TestPrefixFallback(t *testing.T) {
c, _, cexts := testConfig(t)
for _, tc := range []struct {
Expand Down
13 changes: 9 additions & 4 deletions language/go/fileinfo.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -569,7 +569,7 @@ func checkConstraints(c *config.Config, os, arch, osSuffix, archSuffix string, t
}

goConf := getGoConfig(c)
checker := func(tag string) bool {
checker := func(tag string, ts tagSet) bool {
if isIgnoredTag(tag) {
return true
}
Expand All @@ -585,13 +585,18 @@ func checkConstraints(c *config.Config, os, arch, osSuffix, archSuffix string, t
return false
}
return arch == tag

}

return goConf.genericTags[tag]
return ts[tag]
}

return tags.eval(checker) && cgoTags.eval(checker)
for _, ts := range goConf.genericTags {
c := func(tag string) bool { return checker(tag, ts) }
if tags.eval(c) && cgoTags.eval(c) {
return true
}
}
return false
}

// rulesGoSupportsOS returns whether the os tag is recognized by the version of
Expand Down
31 changes: 15 additions & 16 deletions language/go/fileinfo_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -386,7 +386,7 @@ func TestCheckConstraints(t *testing.T) {
defer os.RemoveAll(dir)
for _, tc := range []struct {
desc string
genericTags map[string]bool
genericTags string
os, arch, filename, content string
want bool
}{
Expand Down Expand Up @@ -466,12 +466,12 @@ func TestCheckConstraints(t *testing.T) {
want: false,
}, {
desc: "tags all satisfied",
genericTags: map[string]bool{"a": true, "b": true},
genericTags: "a,b",
content: "// +build a,b\n\npackage foo",
want: true,
}, {
desc: "tags some satisfied",
genericTags: map[string]bool{"a": true},
genericTags: "a",
content: "// +build a,b\n\npackage foo",
want: false,
}, {
Expand All @@ -480,36 +480,36 @@ func TestCheckConstraints(t *testing.T) {
want: true,
}, {
desc: "tag satisfied negated",
genericTags: map[string]bool{"a": true},
genericTags: "a",
content: "// +build !a\n\npackage foo",
want: false,
want: true,
}, {
desc: "tag double negative",
content: "// +build !!a\n\npackage foo",
want: false,
}, {
desc: "tag group and satisfied",
genericTags: map[string]bool{"foo": true, "bar": true},
genericTags: "foo,bar",
content: "// +build foo,bar\n\npackage foo",
want: true,
}, {
desc: "tag group and unsatisfied",
genericTags: map[string]bool{"foo": true},
genericTags: "foo",
content: "// +build foo,bar\n\npackage foo",
want: false,
}, {
desc: "tag line or satisfied",
genericTags: map[string]bool{"foo": true},
genericTags: "foo",
content: "// +build foo bar\n\npackage foo",
want: true,
}, {
desc: "tag line or unsatisfied",
genericTags: map[string]bool{"foo": true},
genericTags: "foo",
content: "// +build !foo bar\n\npackage foo",
want: false,
want: true,
}, {
desc: "tag lines and satisfied",
genericTags: map[string]bool{"foo": true, "bar": true},
genericTags: "foo,bar",
content: `
// +build foo
// +build bar
Expand All @@ -518,7 +518,7 @@ package foo`,
want: true,
}, {
desc: "tag lines and unsatisfied",
genericTags: map[string]bool{"foo": true},
genericTags: "foo",
content: `
// +build foo
// +build bar
Expand All @@ -528,7 +528,7 @@ package foo`,
}, {
desc: "cgo tags satisfied",
os: "linux",
genericTags: map[string]bool{"foo": true},
genericTags: "foo",
content: `
// +build foo
Expand Down Expand Up @@ -581,9 +581,8 @@ import "C"
t.Run(tc.desc, func(t *testing.T) {
c, _, _ := testConfig(t)
gc := getGoConfig(c)
gc.genericTags = tc.genericTags
if gc.genericTags == nil {
gc.genericTags = map[string]bool{"gc": true}
if err := gc.setBuildTags(tc.genericTags); err != nil {
t.Errorf("error setting build tags %q", tc.genericTags)
}
filename := tc.filename
if filename == "" {
Expand Down

0 comments on commit 8162813

Please sign in to comment.