x/tools/go/packages: clarify naming convention of overlay files

[fxrlv]

Go version

go version go1.22.3 darwin/arm64

Output of go env in your module/workspace:

GO111MODULE=''
GOARCH='arm64'
GOBIN=''
GOCACHE='/Users/fxrlv/Library/Caches/go-build'
GOENV='/Users/fxrlv/Library/Application Support/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='arm64'
GOHOSTOS='darwin'
GOINSECURE=''
GOMODCACHE='/Users/fxrlv/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='darwin'
GOPATH='/Users/fxrlv/go'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/opt/homebrew/Cellar/go/1.22.3/libexec'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/opt/homebrew/Cellar/go/1.22.3/libexec/pkg/tool/darwin_arm64'
GOVCS=''
GOVERSION='go1.22.3'
GCCGO='gccgo'
AR='ar'
CC='cc'
CXX='c++'
CGO_ENABLED='1'
GOMOD='/dev/null'
GOWORK=''
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
PKG_CONFIG='pkg-config'
GOGCCFLAGS='-fPIC -arch arm64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -ffile-prefix-map=/var/folders/mg/1l0261p97kzflrmx68mm4z7c0000gn/T/go-build1456754968=/tmp/go-build -gno-record-gcc-switches -fno-common'

What did you do?

https://pkg.go.dev/golang.org/x/tools/go/packages#DriverRequest

// Overlay maps file paths (relative to the driver's working directory) to the byte contents
// of overlay files.
Overlay map[string][]byte `json:"overlay"`

https://pkg.go.dev/golang.org/x/tools/go/packages#Config

// Overlay provides a mapping of absolute file paths to file contents.
// If the file with the given path already exists, the parser will use the
// alternative file contents provided by the map.
//
// Overlays provide incomplete support for when a given file doesn't
// already exist on disk. See the package doc above for more details.
Overlay map[string][]byte

The DriverRequest's documentation states that paths are relative meanwhile Config's documentation states the opposite. There is no problem here but little inconsistency.

What did you see happen?

The next driver invocation that happens inside packages.Load(...) totally violates the path convention the documentation set.

https://github.com/golang/tools/blob/c3aae998cf1d05bd3465e576730c67a9df71b4fa/go/packages/external.go#L112

req, err := json.Marshal(DriverRequest{
    // ...
    Overlay:    cfg.Overlay,
})

cfg.Overlay is not changed anywhere before. So on the left side there should be relative paths, on the other in fact absolute paths.

That is what a gopackagesdriver actually gets. Minimal repro looks like the following.

package main

import (
    "encoding/json"
    "fmt"
    "os"

    "golang.org/x/tools/go/packages"
)

func main() {
    var req packages.DriverRequest

    err := json.NewDecoder(os.Stdin).Decode(&req)
    if err != nil {
        fmt.Fprintln(os.Stderr, err)
        os.Exit(1)
    }

    for path := range req.Overlay {
        fmt.Fprintln(os.Stderr, path)
        os.Exit(1)
    }
}

This driver is throwing an error that contains an absolute path.

What did you expect to see?

I expect the docs should not be misleading.

I think there is no reason for a driver to operate with relative paths. So I would propose to fix the docs instead of changing the packages.Load(...) behavior.

[cagedmantis]

cc @matloob

[matloob]

I might be misunderstanding the bug report but I'm not sure I see the inconsistency.

The intention of the driverRequest doc is not to say that the path has to be absolute, but that relative paths are interpreted relative to the driver's working directory. Maybe we can be more clear and explicitly state that the path can be absolute?

[fxrlv]

Oh, I see. Very first looking at the doc I thought that paths are supposed to be relative and that is the reason why that mark about driver’s workdir is there.

Having that paths can be both relative and absolute it’s unclear what’s expected behavior when one path is listed twice in both forms. Maybe it’s also worth mentioning. What do you think?

[matloob]

I wouldn't mind if we added a clarifying comment. Using the default cmd/go driver it's an error to provide two paths that canonicalize to the same path in an overlay (https://cs.opensource.google/go/go/+/647870becc230b022b431a4ef8b7c9b31382db6c:src/cmd/go/internal/fsys/fsys.go;l=204)

Feel free to send a CL