cmd/go, x/tools: many tests are broken with go1.19.10

[findleyr]

Many tests, for example x/tools/go/packages, are broken with Go 1.19.10.

Here's an example trybot failure:
https://storage.googleapis.com/go-build-log/3ba9c890/linux-amd64_b61d03c6.log

And here's the failure for go/packages, which may be illustrative:

> go1.19.10 test ./go/packages
--- FAIL: TestMultiplePackageVersionsIssue36188 (0.00s)
    --- FAIL: TestMultiplePackageVersionsIssue36188/GOPATH (0.13s)
        packages_test.go:2199: err: exit status 1: stderr: go build golang.org/fake/b: golang.org/fake/a/a.go:1:19: import "golang.org/fake/b" is a program, not an importable package
            
    --- FAIL: TestMultiplePackageVersionsIssue36188/Modules (0.12s)
        packages_test.go:2199: err: exit status 1: stderr: go build golang.org/fake/b: a/a.go:1:19: import "golang.org/fake/b" is a program, not an importable package
            
--- FAIL: TestReturnErrorForUnexpectedDirectoryLayout (0.00s)
    --- FAIL: TestReturnErrorForUnexpectedDirectoryLayout/Modules (0.10s)
        packages_test.go:1716: want error message: unexpected directory layout, got: err: exit status 1: stderr: go build b: a.go:1:19: package b is not in GOROOT (/usr/local/google/home/rfindley/sdk/go1.19.10/src/b)
--- FAIL: TestInvalidPackageName (0.00s)
    --- FAIL: TestInvalidPackageName/GOPATH (0.06s)
        packages_test.go:2735: err: exit status 1: stderr: go build golang.org/fake: golang.org/fake/main.go:1:9: expected 'IDENT', found 'default'
            
    --- FAIL: TestInvalidPackageName/Modules (0.14s)
        packages_test.go:2735: err: exit status 1: stderr: go build golang.org/fake: main.go:1:9: expected 'IDENT', found 'default'
            
--- FAIL: TestMissingDependency (0.00s)
    --- FAIL: TestMissingDependency/GOPATH (0.10s)
        packages_test.go:1732: err: exit status 1: stderr: go build this/package/doesnt/exist: golang.org/fake/a/a.go:1:19: cannot find package "this/package/doesnt/exist" in any of:
            	/usr/local/google/home/rfindley/sdk/go1.19.10/src/this/package/doesnt/exist (from $GOROOT)
            	/tmp/TestMissingDependency_GOPATH2093183701/fake/src/this/package/doesnt/exist (from $GOPATH)
            
    --- FAIL: TestMissingDependency/Modules (0.19s)
        packages_test.go:1732: err: exit status 1: stderr: go build this/package/doesnt/exist: a/a.go:1:19: package this/package/doesnt/exist is not in GOROOT (/usr/local/google/home/rfindley/sdk/go1.19.10/src/this/package/doesnt/exist)
            
--- FAIL: TestInvalidFilesBeforeOverlay (0.00s)
    --- FAIL: TestInvalidFilesBeforeOverlay/GOPATH (0.06s)
        --- FAIL: TestInvalidFilesBeforeOverlay/GOPATH/no_overlay (0.06s)
            overlay_test.go:739: err: exit status 1: stderr: go build golang.org/fake/d: golang.org/fake/d/d.go:1:1: expected 'package', found 'EOF'
                go build golang.org/fake: golang.org/fake/main.go:1:1: expected 'package', found 'EOF'
                
    --- FAIL: TestInvalidFilesBeforeOverlay/Modules (0.08s)
        --- FAIL: TestInvalidFilesBeforeOverlay/Modules/no_overlay (0.04s)
            overlay_test.go:739: err: exit status 1: stderr: go build golang.org/fake: main.go:1:1: expected 'package', found 'EOF'
                go build golang.org/fake/d: d/d.go:1:1: expected 'package', found 'EOF'
                
--- FAIL: TestInvalidFilesBeforeOverlayContains (0.00s)
    --- FAIL: TestInvalidFilesBeforeOverlayContains/Modules (3.40s)
        --- FAIL: TestInvalidFilesBeforeOverlayContains/Modules/test_variant (1.11s)
            overlay_test.go:825: expected package ID "golang.org/fake/d [golang.org/fake/d.test]", got "command-line-arguments [command-line-arguments.test]"
        --- FAIL: TestInvalidFilesBeforeOverlayContains/Modules/second_file (0.20s)
            overlay_test.go:825: expected package ID "golang.org/fake/d", got "command-line-arguments"
        --- FAIL: TestInvalidFilesBeforeOverlayContains/Modules/main (1.16s)
            overlay_test.go:825: expected package ID "golang.org/fake", got "command-line-arguments"
        --- FAIL: TestInvalidFilesBeforeOverlayContains/Modules/xtest (0.91s)
            overlay_test.go:825: expected package ID "golang.org/fake/d_test [golang.org/fake/d.test]", got "command-line-arguments_test [command-line-arguments.test]"
    --- FAIL: TestInvalidFilesBeforeOverlayContains/GOPATH (3.79s)
        --- FAIL: TestInvalidFilesBeforeOverlayContains/GOPATH/test_variant (1.29s)
            overlay_test.go:825: expected package ID "golang.org/fake/d [golang.org/fake/d.test]", got "command-line-arguments [command-line-arguments.test]"
        --- FAIL: TestInvalidFilesBeforeOverlayContains/GOPATH/second_file (0.24s)
            overlay_test.go:825: expected package ID "golang.org/fake/d", got "command-line-arguments"
        --- FAIL: TestInvalidFilesBeforeOverlayContains/GOPATH/main (1.26s)
            overlay_test.go:825: expected package ID "golang.org/fake", got "command-line-arguments"
        --- FAIL: TestInvalidFilesBeforeOverlayContains/GOPATH/xtest (1.00s)
            overlay_test.go:825: expected package ID "golang.org/fake/d_test [golang.org/fake/d.test]", got "command-line-arguments_test [command-line-arguments.test]"

Given the amount of tests that fail, it seems likely that this failure mode is user-facing.

CC @bcmills @matloob @rsc

[bcmills]

At least this should be pretty east to bisect. 😅

[bcmills]

(Bisecting now.)

[bcmills]

c160b49b6d328c86bd76ca2fff9009a71347333f is the first bad commit
commit c160b49b6d328c86bd76ca2fff9009a71347333f
Author: Bryan C. Mills <bcmills@google.com>
Date:   Fri May 12 14:15:16 2023 -0400

    [release-branch.go1.19] cmd/go: disallow package directories containing newlines

    Directory or file paths containing newlines may cause tools (such as
    cmd/cgo) that emit "//line" or "#line" -directives to write part of
    the path into non-comment lines in generated source code. If those
    lines contain valid Go code, it may be injected into the resulting
    binary.

    (Note that Go import paths and file paths within module zip files
    already could not contain newlines.)

    Thanks to Juho Nurminen of Mattermost for reporting this issue.

    Updates #60167.
    Fixes #60515.
    Fixes CVE-2023-29402.

    Change-Id: If55d0400c02beb7a5da5eceac60f1abeac99f064
    Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/1882606
    Reviewed-by: Roland Shoemaker <bracewell@google.com>
    Run-TryBot: Roland Shoemaker <bracewell@google.com>
    Reviewed-by: Russ Cox <rsc@google.com>
    Reviewed-by: Damien Neil <dneil@google.com>
    (cherry picked from commit 41f9046495564fc728d6f98384ab7276450ac7e2)
    Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/1902229
    Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/1904343
    Reviewed-by: Michael Knyszek <mknyszek@google.com>
    Reviewed-by: Bryan Mills <bcmills@google.com>
    Reviewed-on: https://go-review.googlesource.com/c/go/+/501218
    Run-TryBot: David Chase <drchase@google.com>
    Auto-Submit: Michael Knyszek <mknyszek@google.com>
    TryBot-Result: Gopher Robot <gobot@golang.org>

 src/cmd/go/internal/load/pkg.go                  |   4 +
 src/cmd/go/internal/work/exec.go                 |   6 ++
 src/cmd/go/script_test.go                        |   1 +
 src/cmd/go/testdata/script/build_cwd_newline.txt | 100 +++++++++++++++++++++++
 4 files changed, 111 insertions(+)
 create mode 100644 src/cmd/go/testdata/script/build_cwd_newline.txt

Well then. Time to figure out what old bug we're tickling on the release branch. 😩

[bcmills]

I suspect that the go/packages failures are due to stricter error-reporting in cmd/go/internal/work/exec.go:

	if p.Error != nil {
		// Don't try to build anything for packages with errors. There may be a
		// problem with the inputs that makes the package unsafe to build.
		return p.Error
	}

But I don't know why that would be specific to 1.19. (My intuition is that it may be related to #29666, but we started using more precise -json flags at 1.19!)

[bcmills]

This may also be related to the bug fixed in https://go.dev/cl/437298. I'll see if that backports cleanly, and if so whether it fixes the problem.

[bcmills]

I got it backported (with a couple of minor merge conflicts), but it doesn't seem to fix x/tools. 😞

[gopherbot]

Change https://go.dev/cl/502175 mentions this issue: go/packages/packagestest: set Config.Logf if the test is run verbosely

[gopherbot]

Change https://go.dev/cl/502195 mentions this issue: [release-branch.go1.19] cmd/go: do not exit with non-zero code from go list -e -export

[gopherbot]

Change https://go.dev/cl/502196 mentions this issue: [release-branch.go1.19] cmd/go/internal/work: make formatOutput return an error that includes the import path

[bcmills]

I have identified two backport CLs from #25842 that together appear to fix all of the broken x/tools tests.

[findleyr]

Great, thank you, Bryan!

[bcmills]

At this point this is just waiting on the backport CLs to be merged by the release team. (I think @prattmic is on interrupts this week, but I'm not sure when in the process the release team usually does those merges.)

[gopherbot]

Closed by merging c045822 to release-branch.go1.19.