cmd/go: promote -fullpath from test flag to build flag

[rogpeppe]

What version of Go are you using (go version)?

f90b4cd6554f4f20280aa5229cf42650ed47221d

Does this issue reproduce with the latest release?

N/A

What did you do?

I ran this testscript:

! exec go test -C ./x -fullpath .
! stderr '\./x.go'

-- x/x.go --
package foo

var x = mistake

-- go.mod --
module test

What did you expect to see?

A passing test.

What did you see instead?

> ! exec go test -C ./x -fullpath .
[stdout]
FAIL
[stderr]
# test/x
./x.go:3:9: undefined: mistake
[exit status 1]
> ! stderr '\./x.go'
FAIL: /tmp/testscript142857279/x.txtar/script.txtar:2: unexpected match for `\./x.go` found in stderr: ./x.go

It seems to me that the intent of -fullpath is to cause all filenames to print in a way that's absolute, even when those errors come from the compiler rather than the test binary. I'd expect the compiler error to print an absolute path there too.

[ianlancetaylor]

This means moving -fullpath from being a test option to being a build option, and then using it to pass something to cmd/compile. We would presumably still have the -test.fullpath option, which would be different, but -fullpath would imply -test.fullpath.

CC @bcmills @matloob

[rogpeppe]

Yup, that sounds about right. I think that would be worth doing.

[bcmills]

Sounds reasonable. This is essentially a revision on proposal #37708, so it should probably be a proposal itself.

[rsc]

This proposal has been added to the active column of the proposals project
and will now be reviewed at the weekly proposal review meetings.
— rsc for the proposal review group

[rsc]

I think this would mean making -fullpath a build flag (as opposed to a test flag) and then have base.ShortPath not shorten when the flag is set. Does anyone object to this?

[rsc]

Based on the discussion above, this proposal seems like a likely accept.
— rsc for the proposal review group

[rsc]

No change in consensus, so accepted. 🎉
This issue now tracks the work of implementing the proposal.
— rsc for the proposal review group

[quantonganh]

If we add -fullpath as build option, it may lead to confusion with the -trimpath flag:

        -trimpath
                remove all file system paths from the resulting executable.
                Instead of absolute file system paths, the recorded file names
                will begin either a module path@version (when using modules),
                or a plain import path (when using the standard library, or GOPATH).

What do you think if we just pass -test.fullpath to cmd/compile?

[gopherbot]

Change https://go.dev/cl/535075 mentions this issue: cmd/go/internal/work: go test -fullpath should include full paths for build errors

[bcmills]

@quantonganh, if -fullpath and -trimpath are both specified, the “full path” logged by the test should correspond to the short filename joined with the package import path.

It isn't clear to me what -fullpath as a build option should do in that case, though. 🤔

[bcmills]

Also: if -fullpath is (also) a build flag, should it apply uniformly to all build commands? It seems inconsistent to apply it to only the build portion of go test.

[karnigen]

I'm glad it's implemented now, but I can't figure out how to use -fullpath during builds, not just tests? What should the command look like specifically?
I'm trying, for example: go -C go build -o golib.a -fullpath -buildmode=c-archive golib.go, but I can't get it to work. How is it supposed to be used?

[karnigen]

To streamline editor error navigation, I've created a simple filter as a workaround. This tool reads stdin, replaces relative paths, and writes the modified output to stdout.
Usage: go -C <project_path> build . 2>&1 | gobuildfilter [project_path]

package main

func usage() {
	fmt.Println("Usage: go -C <project_path> build . 2>&1 | gobuildfilter [project_path]")
	fmt.Println("       filter replaces './' with the specified path, tu support go -C and editor integration")
	fmt.Println("       In bash script 'set -eo pipefail' or 'bash -eo pipefail' to stop on error")
	fmt.Println()
	flag.PrintDefaults()
}

func main() {
	flag.Usage = usage
	flag.Parse()

	// Reads stdin, replaces "./", writes to stdout.
	path := "."
	if flag.NArg() == 1 {
		path = flag.Arg(0)
	}

	if path[len(path)-1] != '/' {
		path += "/"
	}

	scanner := bufio.NewScanner(os.Stdin)
	for scanner.Scan() {
		line := scanner.Text()
		fmt.Println(strings.Replace(line, "./", path, -1))
	}
	if err := scanner.Err(); err != nil {
		fmt.Fprintln(os.Stderr, "Error:", err)
	}
}

[matloob]

@karnigen This isn't implemented in the go command yet. There's a CL out for review but it hasn't been submitted or released.

[firelizzard18]

As a tool builder this would be very useful. Now that go test -json outputs build errors as a test event it is trivial to capture them. Updating those events so they respect -fullpath would make it much easier to attach the error to the correct location when reporting it to the development environment. It would also make it much easier to detect whether a build error occurred within a specific test and to correctly associate it with that test when reporting to the DE.