Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Evaluate build tags as both true and false #1938

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions Design.rst
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
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
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]struct{}

func (ts tagSet) clone() tagSet {
c := make(tagSet, len(ts))
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": struct{}{}},
},
}
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] = struct{}{}
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
43 changes: 19 additions & 24 deletions language/go/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,21 @@ func testConfig(t *testing.T, args ...string) (*config.Config, []language.Langua
return c, langs, cexts
}

func newTagSet(tags ...string) tagSet {
ts := make(tagSet)
for _, t := range tags {
ts[t] = struct{}{}
}
return ts
}

var expectedBuildTags = []tagSet{
newTagSet("gc"),
newTagSet("gc", "foo"),
newTagSet("gc", "bar"),
newTagSet("gc", "foo", "bar"),
}

func TestCommandLine(t *testing.T) {
c, _, _ := testConfig(
t,
Expand All @@ -68,10 +83,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 +114,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 +279,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
14 changes: 10 additions & 4 deletions language/go/fileinfo.go
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,19 @@ func checkConstraints(c *config.Config, os, arch, osSuffix, archSuffix string, t
return false
}
return arch == tag

}

return goConf.genericTags[tag]
_, ok := ts[tag]
return ok
}

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
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