language/go: Add unix as ignored tag.

[connyay]

Uncomment one line below and remove others.

Bug fix

What package or component does this PR mostly affect?

language/go

What does this PR do? Why is it needed?

This change reverts the change from #1512 and simplifies it to a simple pass through. The change in #1512 did not fix the underlying issue.

Which issues(s) does this PR fix?

Fixes #1465

Other notes for review

[connyay]

I raised concerns about the change in #1512 (comment) but I wasn't loud enough. I should have prevented that pr from landing - my apologies.

[sluongng]

@fmeum @linzhp I think we should review and merge this.

Latest v0.31.0 release broke our build with com_github_jackc_pgx_v5 package with errors

external/com_github_jackc_pgx_v5/internal/nbconn/nbconn.go:379:12: c.realNonblockingWrite undefined (type *NetConn has no field or method realNonblockingWrite)
external/com_github_jackc_pgx_v5/internal/nbconn/nbconn.go:412:12: c.realNonblockingRead undefined (type *NetConn has no field or method realNonblockingRead)

Checking the BUILD file shown that the unix source file was excluded

> ll /private/var/tmp/_bazel_sluongng/06e573a93bc2d6a9cad4ad41f00b4310/external/com_github_jackc_pgx_v5/internal/nbconn
Permissions Size User     Date Modified Name
.rw-r--r--@ 1.2k sluongng 30 May 16:58  bufferqueue.go
.rw-r--r--@  721 sluongng 30 May 16:58  BUILD.bazel
.rw-r--r--@  14k sluongng 30 May 16:58  nbconn.go
.rw-r--r--@  245 sluongng 30 May 16:58  nbconn_fake_non_block.go
.rw-r--r--@ 1.8k sluongng 30 May 16:58  nbconn_real_non_block.go
.rw-r--r--@  16k sluongng 30 May 16:58  nbconn_test.go

# BUILD.bazel does NOT have nbconn_real_non_block.go
go_library(
    name = "nbconn",
    srcs = [
        "bufferqueue.go",
        "nbconn.go",
        "nbconn_fake_non_block.go",
    ],
    importpath = "github.com/jackc/pgx/v5/internal/nbconn",
    importpath_aliases = ["github.com/jackc/pgx/internal/nbconn"],
    visibility = ["//:__subpackages__"],
    deps = ["//internal/iobufpool"],
)

[fmeum]

I didn't spot this when reviewing the PR - could you add a simple test case? Even if not we can merge this, thanks for the fix!

[sluongng]

I have this on top of existing master and it seems to pass

!master ~/work/bazelbuild/bazel-gazelle> git diff
diff --git a/language/go/fileinfo_test.go b/language/go/fileinfo_test.go
index 297962f..d3a3af3 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 file on darwin",
+                       filename: "foo_unix.go",
+                       os:       "darwin",
+                       want:     true,
+               }, {
+                       desc:     "unix file 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",

Which is surprising to me... as I was expecting some to fail...

[fmeum]

@sluongng @connyay Do you understand why current master is broken and in what way? I wonder why we are even handling the other OS tags instead of ignoring all of them.

[sluongng]

Let's not merge this, I found a fix

[fmeum]

@connyay Could you verify that #1554 fixes the issue you are seeing? Thanks for looking into this!

[connyay]

@connyay Could you verify that #1554 fixes the issue you are seeing? Thanks for looking into this!

Confirmed!