-
Notifications
You must be signed in to change notification settings - Fork 92
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Now support unix build tag thanks to https://github.com/bazelbuild/bazel-gazelle/pull/1512/files Full changelog: https://github.com/bazelbuild/bazel-gazelle/releases/tag/v0.31.0
- Loading branch information
Showing
3 changed files
with
238 additions
and
9 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,230 @@ | ||
| From e770d465ac089e1a5a25da2034180cd1bb05b32b Mon Sep 17 00:00:00 2001 | ||
| From: Son Luong Ngoc <sluongng@gmail.com> | ||
| Date: Wed, 31 May 2023 16:28:49 +0200 | ||
| Subject: [PATCH] fileinfo: fix not detecting 'unix' files to be OS specific | ||
| (#1554) | ||
|
|
||
| The build file generation logic works like this today: | ||
|
|
||
| ``` | ||
| GenerateRules() | ||
| package.go->*goTarget.addFiles | ||
| package.go->getPlatformStringsAddFunction() | ||
| fileinfo.go->isOSArchSpecific() | ||
| ``` | ||
|
|
||
| Depending on whether the file has special Go build directives, Gazelle | ||
| would use a different logic to include/exclude the file into the 'srcs' | ||
| attribute of the Go targets. | ||
|
|
||
| In #1512, we added support for 'unix' build directive but did not tell | ||
| Gazelle to treat files with 'unix' file as OS specific file. This caused | ||
| Gazelle to mismatch the OS in a later stage and exclude the needed file | ||
| instead. | ||
|
|
||
| Fix that logic and provide some additional tests to reinforce the | ||
| fileinfo logic. | ||
|
|
||
| Also added a small knob to print stdout of Gazelle when it's run under | ||
| go_repository with `debug_mode` attribute set to True. Most Gazelle's | ||
| logs are printed to stdout and not stderr. | ||
| --- | ||
| internal/go_repository.bzl | 3 +- | ||
| language/go/fileinfo.go | 3 +- | ||
| language/go/fileinfo_test.go | 137 ++++++++++++++++++++++++++++++++++- | ||
| 3 files changed, 139 insertions(+), 4 deletions(-) | ||
|
|
||
| diff --git a/internal/go_repository.bzl b/internal/go_repository.bzl | ||
| index 48fc1811d..a0f7943cb 100644 | ||
| --- a/internal/go_repository.bzl | ||
| +++ b/internal/go_repository.bzl | ||
| @@ -314,7 +314,8 @@ def _go_repository_impl(ctx): | ||
| result.stderr, | ||
| )) | ||
| if ctx.attr.debug_mode and result.stderr: | ||
| - print("%s: %s" % (ctx.name, result.stderr)) | ||
| + print("%s gazelle.stdout: %s" % (ctx.name, result.stdout)) | ||
| + print("%s gazelle.stderr: %s" % (ctx.name, result.stderr)) | ||
|
|
||
| # Apply patches if necessary. | ||
| patch(ctx) | ||
| diff --git a/language/go/fileinfo.go b/language/go/fileinfo.go | ||
| index 8e6fe266b..d9ba96ae4 100644 | ||
| --- a/language/go/fileinfo.go | ||
| +++ b/language/go/fileinfo.go | ||
| @@ -501,7 +501,7 @@ func isOSArchSpecific(info fileInfo, cgoTags *cgoTagsAndOpts) (osSpecific, archS | ||
| checkTags := func(tags []string) { | ||
| for _, tag := range tags { | ||
| _, osOk := rule.KnownOSSet[tag] | ||
| - if osOk { | ||
| + if osOk || tag == "unix" { | ||
| osSpecific = true | ||
| } | ||
| _, archOk := rule.KnownArchSet[tag] | ||
| @@ -564,7 +564,6 @@ func checkConstraints(c *config.Config, os, arch, osSuffix, archSuffix string, t | ||
| return false | ||
| } | ||
| return matchesOS(os, tag) | ||
| - | ||
| } | ||
|
|
||
| if _, ok := rule.KnownArchSet[tag]; ok { | ||
| diff --git a/language/go/fileinfo_test.go b/language/go/fileinfo_test.go | ||
| index 297962fa1..3021a6294 100644 | ||
| --- a/language/go/fileinfo_test.go | ||
| +++ b/language/go/fileinfo_test.go | ||
| @@ -62,7 +62,6 @@ func TestOtherFileInfo(t *testing.T) { | ||
| if diff := cmp.Diff(tc.wantTags, got.tags, fileInfoCmpOption); diff != "" { | ||
| t.Errorf("(-want, +got): %s", diff) | ||
| } | ||
| - | ||
| }) | ||
| } | ||
| } | ||
| @@ -427,6 +426,40 @@ func TestCheckConstraints(t *testing.T) { | ||
| os: "darwin", | ||
| arch: "amd64", | ||
| want: false, | ||
| + }, { | ||
| + desc: "unix filename on darwin", | ||
| + filename: "foo_unix.go", | ||
| + os: "darwin", | ||
| + want: true, | ||
| + }, { | ||
| + desc: "unix filename on windows", | ||
| + filename: "foo_unix.go", | ||
| + os: "windows", | ||
| + want: true, | ||
| + }, { | ||
| + desc: "non-unix tag on linux", | ||
| + filename: "foo_bar.go", | ||
| + os: "darwin", | ||
| + content: "//go:build !unix\n\npackage foo", | ||
| + want: false, | ||
| + }, { | ||
| + desc: "non-unix tag on windows", | ||
| + filename: "foo_bar.go", | ||
| + os: "windows", | ||
| + content: "//go:build !unix\n\npackage foo", | ||
| + want: true, | ||
| + }, { | ||
| + desc: "unix tag on windows", | ||
| + filename: "foo_bar.go", | ||
| + os: "windows", | ||
| + content: "//go:build unix\n\npackage foo", | ||
| + want: false, | ||
| + }, { | ||
| + desc: "unix tag on linux", | ||
| + filename: "foo_bar.go", | ||
| + os: "linux", | ||
| + content: "//go:build unix\n\npackage foo", | ||
| + want: true, | ||
| }, { | ||
| desc: "goos unsatisfied tags satisfied", | ||
| filename: "foo_linux.go", | ||
| @@ -580,3 +613,105 @@ import "C" | ||
| }) | ||
| } | ||
| } | ||
| + | ||
| +func TestIsOSArchSpecific(t *testing.T) { | ||
| + for _, tc := range []struct { | ||
| + desc string | ||
| + filename, content string | ||
| + | ||
| + expectOSSpecific bool | ||
| + expectArchSpecific bool | ||
| + }{ | ||
| + { | ||
| + desc: "normal", | ||
| + filename: "foo.go", | ||
| + content: "package foo", | ||
| + expectOSSpecific: false, | ||
| + expectArchSpecific: false, | ||
| + }, | ||
| + { | ||
| + desc: "unix directive", | ||
| + filename: "foo.go", | ||
| + content: "//go:build unix\n\npackage foo", | ||
| + expectOSSpecific: true, | ||
| + expectArchSpecific: false, | ||
| + }, | ||
| + { | ||
| + desc: "exclude-unix directive", | ||
| + filename: "foo.go", | ||
| + content: "//go:build !unix\n\npackage foo", | ||
| + expectOSSpecific: true, | ||
| + expectArchSpecific: false, | ||
| + }, | ||
| + { | ||
| + desc: "arch directive", | ||
| + filename: "foo.go", | ||
| + content: "//go:build arm64\n\npackage foo", | ||
| + expectOSSpecific: false, | ||
| + expectArchSpecific: true, | ||
| + }, | ||
| + { | ||
| + desc: "exclude-arch directive", | ||
| + filename: "foo.go", | ||
| + content: "//go:build !arm64\n\npackage foo", | ||
| + expectOSSpecific: false, | ||
| + expectArchSpecific: true, | ||
| + }, | ||
| + { | ||
| + desc: "os directive", | ||
| + filename: "foo.go", | ||
| + content: "//go:build linux\n\npackage foo", | ||
| + expectOSSpecific: true, | ||
| + expectArchSpecific: false, | ||
| + }, | ||
| + { | ||
| + desc: "exclude-os directive", | ||
| + filename: "foo.go", | ||
| + content: "//go:build !linux\n\npackage foo", | ||
| + expectOSSpecific: true, | ||
| + expectArchSpecific: false, | ||
| + }, | ||
| + { | ||
| + desc: "os and arch directive", | ||
| + filename: "foo.go", | ||
| + content: "//go:build linux && amd64\n\npackage foo", | ||
| + expectOSSpecific: true, | ||
| + expectArchSpecific: true, | ||
| + }, | ||
| + { | ||
| + desc: "unix and arch directive", | ||
| + filename: "foo.go", | ||
| + content: "//go:build unix && amd64\n\npackage foo", | ||
| + expectOSSpecific: true, | ||
| + expectArchSpecific: true, | ||
| + }, | ||
| + } { | ||
| + t.Run(tc.desc, func(t *testing.T) { | ||
| + tmpDir, err := os.MkdirTemp(os.Getenv("TEST_TEMPDIR"), "TestIsOSSpecific_*") | ||
| + if err != nil { | ||
| + t.Fatal(err) | ||
| + } | ||
| + t.Cleanup(func() { | ||
| + os.RemoveAll(tmpDir) | ||
| + }) | ||
| + | ||
| + path := filepath.Join(tmpDir, tc.filename) | ||
| + if err := ioutil.WriteFile(path, []byte(tc.content), 0o666); err != nil { | ||
| + t.Fatal(err) | ||
| + } | ||
| + fi := goFileInfo(path, "") | ||
| + var cgoTags *cgoTagsAndOpts | ||
| + if len(fi.copts) > 0 { | ||
| + cgoTags = fi.copts[0] | ||
| + } | ||
| + | ||
| + gotOSSpecific, gotArchSpecific := isOSArchSpecific(fi, cgoTags) | ||
| + if diff := cmp.Diff(tc.expectOSSpecific, gotOSSpecific); diff != "" { | ||
| + t.Errorf("(-want, +got): %s", diff) | ||
| + } | ||
| + if diff := cmp.Diff(tc.expectArchSpecific, gotArchSpecific); diff != "" { | ||
| + t.Errorf("(-want, +got): %s", diff) | ||
| + } | ||
| + }) | ||
| + } | ||
| +} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters