golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
123.98k stars 17.67k forks source link

go/packages: Package.OtherFiles contains globs #69555

Closed josharian closed 1 month ago

josharian commented 1 month ago

Go version

go version go1.23.1

Output of go env in your module/workspace:

-

What did you do?

Used go/packages on a package containing this code:

//go:embed *
var FS embed.FS

Then did:

    packages.Visit(pkgs, nil, func(p *packages.Package) {
    for _, f := range p.OtherFiles {
        fmt.Println(f)

What did you see happen?

Got output like:

/path/to/package/*

That * comes from the go:embed directive, I believe.

What did you expect to see?

The glob being fully resolved. The docs for OtherFiles say:

    // OtherFiles lists the absolute file paths of the package's non-Go source files,
    // including assembly, C, C++, Fortran, Objective-C, SWIG, and so on.
    OtherFiles []string

* is not an absolute file path in this case. :)

josharian commented 1 month ago

Worse than that, if the embed directive is something like:

//go:embed *.txt *.md
var FS embed.FS

then OtherFiles includes the "path":

/path/to/package/*.txt *.md

(exactly like that, including the space)

adonovan commented 1 month ago

Hi @josharian, I can't reproduce it at tip using the code below (a patch to gopackages).

xtools$ git diff
diff --git a/go/packages/gopackages/main.go b/go/packages/gopackages/main.go
index 9a0e7ad92..9d7994362 100644
--- a/go/packages/gopackages/main.go
+++ b/go/packages/gopackages/main.go
@@ -114,7 +114,7 @@ func (app *application) Run(ctx context.Context, args ...string) error {
        default:
                return tool.CommandLineErrorf("invalid mode: %s", app.Mode)
        }
-       cfg.Mode |= packages.NeedModule
+       cfg.Mode |= packages.NeedModule | packages.NeedEmbedPatterns

        lpkgs, err := packages.Load(cfg, args...)
        if err != nil {
@@ -178,6 +178,8 @@ func (app *application) print(lpkg *packages.Package) {
        }
        fmt.Printf("\tpackage %s\n", lpkg.Name)

+       log.Println(lpkg, lpkg.OtherFiles, lpkg.EmbedPatterns)
+
        // characterize type info
        if lpkg.Types == nil {
                fmt.Printf("\thas no exported type info\n")

xtools$ go run ./go/packages/gopackages ./gopls/internal/server   # the server package contains an embed glob
Go package "golang.org/x/tools/gopls/internal/server":
    module golang.org/x/tools/gopls@
    package server
2024/09/20 14:04:58 golang.org/x/tools/gopls/internal/server [] [/Users/adonovan/w/xtools/gopls/internal/server/assets/*]
    has no exported type info
...

Observe that the log statement shows no rogue OtherFiles, but EmbedPatterns contains globs as it should.

Could you provide a repro in a similar form? Thanks.

josharian commented 1 month ago

Hmm. Trying to bisect between my code and yours...

In the meantime, FYI, LoadMode.String is missing a few of the newer Need* constants.

gopherbot commented 1 month ago

Change https://go.dev/cl/614875 mentions this issue: go/packages: fix LoadMode.String

josharian commented 1 month ago

OK, found the problem. This is in go-fuzz, in code so old NeedEmbed didn't exist...so it was manually working around it. :( So this was entirely user error. Sorry about the noise.