Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

x/tools/go/packages: relative "files=" query path are not relative to Config.Dir #65965

Open
fho opened this issue Feb 27, 2024 · 2 comments
Open

x/tools/go/packages: relative "files=" query path are not relative to Config.Dir #65965

fho opened this issue Feb 27, 2024 · 2 comments
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Tools This label describes issues relating to any tools in the x/tools repository.
Milestone
Unreleased

Comments

@fho
Copy link

fho commented Feb 27, 2024

Go version

go version go1.22.0 linux/amd64

Output of go env in your module/workspace:

GO111MODULE=''
GOARCH='amd64'
GOBIN='/home/fho/go/bin/'
GOCACHE='/home/fho/.cache/go-build'
GOENV='/home/fho/.config/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='amd64'
GOHOSTOS='linux'
GOINSECURE=''
GOMODCACHE='/home/fho/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='linux'
GOPATH='/home/fho/go'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/usr/lib/go'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/usr/lib/go/pkg/tool/linux_amd64'
GOVCS=''
GOVERSION='go1.22.0'
GCCGO='gccgo'
GOAMD64='v1'
AR='ar'
CC='gcc'
CXX='g++'
CGO_ENABLED='1'
GOMOD='/tmp/repro/go.mod'
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 -m64 -pthread -Wl,--no-gc-sections -fmessage-length=0 -ffile-prefix-map=/tmp/go-build3159040424=/tmp/go-build -gno-record-gcc-switches'

What did you do?

packages.Load accepts file paths via the file= pattern.
The *packages.Config parameter has a Dir field that is documented as:

Dir is the directory in which to run the build system's query tool
that provides information about the packages.
If Dir is empty, the tool is run in the current directory.

Therefore I expect that relative file query paths are relative to cfg.Dir.

I wrote a small tool to reproduce the issue.
The first command-line argument is used as cfg.Dir parameter, the second command line argument is passed as file= query pattern.

Tool + package to test the query can be found in: https://github.com/fho/golist_rel_file_query

What did you see happen?

When a relative path is specified as file= pattern, the path is made absolute to the current working directory instead of to cfg.Dir.

The go list query results in 0 packages:

$ pwd
/tmp/repro

$ tree
.
├── golist.go
├── go.mod
├── go.sum
├── oneprinter
│   └── main.go
└── util
    └── util.go
$ go run golist.go /tmp/repro/oneprinter/ main.go
packages.Load returned 0 packages

What did you expect to see?

When a relative path is specified as file= pattern, the path is made absolute to the cfg.Dir field.

Same results for running in the directory /tmp/repro:

go run golist.go /tmp/repro/oneprinter/ main.go

and running in the directory /tmp/repro/oneprinter


$ go run ../golist.go /tmp/repro/oneprinter/ main.go
packages.Load returned 1 packages
{"ID":"repro/oneprinter","Name":"main","PkgPath":"repro/oneprinter","GoFiles":["/tmp/repro/oneprinter/main.go"],"Imports":{"fmt":"fmt","repro/util":"repro/util"}}

This might be the cause for: #58726
Thanks to @barash-simplesurance for discovering the issue.
@gopherbot gopherbot added the Tools This label describes issues relating to any tools in the x/tools repository. label Feb 27, 2024
@gopherbot gopherbot added this to the Unreleased milestone Feb 27, 2024
@fho fho mentioned this issue Feb 27, 2024
Closed
@adonovan
Copy link
Member

adonovan commented Feb 27, 2024 •
edited

Most filenames in the go/packages API are specified as absolute, and relative paths are (implicitly) relative to the application's working directory, as usual. The documentation for Config.Dir says only that it is the working directory of the query tool, not the base for all relative paths in the API.

So I think this is really a problem of documentation, and that we should clarify that file= patterns follow the usual convention.

@fho
Copy link
Author

fho commented Feb 28, 2024 •
edited

I think what makes it confusing and non-intuitive is that the base of the
relative paths changes depending on the query pattern type and if an external or
the goListDriver is used:

  • When the golistDriver is used, relative file query paths are relative to the
    application's working directory (cwd).
  • When an externalDriver is used, relative file query paths are relative to
    cfg.Dir.
    (The externalDriver is executed in cfg.Dir and the file query path is passed
    as specified in the call to packages.Load.)
  • When a package path is passed as query, e.g. ./..., ./ specifies the
    current directory. The relative path is interpreted as relative to cfg.Dir
    not the applications's cwd.

I would expect a consistent behavior in all these cases. The current directory
should always either be cfg.Dir or the application's cwd.
What makes it inconsistent is that the golistDriver uses the application's cwd
and not cfg.Dir to make file= query paths absolute.
In all other cases, that I'm aware of, relative paths are relative to cfg.Dir.

Documenting this different behavior would help a lot. I think making it
consistent would be even better.

@cagedmantis cagedmantis added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Feb 29, 2024
fho added a commit to simplesurance/baur that referenced this issue Mar 4, 2024
@fho
resolve/gosource: add rel file query with multiple main packages test
d68378d 
Add a testcase for the gosource resolver that specifies as file query pattern
cmd/**/main.go and runs the goresolver from a different directory then the test
(aka app dir) directory.

This testcase currently fails because the relative file query patterns are not
relative to the specified working dir (app dir) but to the cwd
(golang/go#65965).

The existing testcase query_main_file is refactored for this purpose
The test_config.json files got a new field WorkingDir, the $WORKDIR
test_config.json variable is renamed to $TESTDIR.
fho added a commit to simplesurance/baur that referenced this issue Mar 4, 2024
@fho
resolve/gosource: add rel file query with multiple main packages test
71df7d4 
Add a testcase for the gosource resolver that specifies as file query pattern
cmd/**/main.go and runs the goresolver from a different directory then the test
(aka app dir) directory.

This testcase currently fails because the relative file query patterns are not
relative to the specified working dir (app dir) but to the cwd
(golang/go#65965).

The existing testcase query_main_file is refactored for this purpose
The test_config.json files got a new field WorkingDir, the $WORKDIR
test_config.json variable is renamed to $TESTDIR.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Milestone
Unreleased
Development

No branches or pull requests
4 participants
@cagedmantis @fho @adonovan @gopherbot