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.

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

go/build: erroneous contents for Doc field in *Package for image package #23594

Closed
jimmyfrasche opened this issue Jan 28, 2018 · 6 comments
Closed

go/build: erroneous contents for Doc field in *Package for image package #23594

jimmyfrasche opened this issue Jan 28, 2018 · 6 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Go1.11

Comments

@jimmyfrasche
Copy link
Member 

go list -f '{{.Doc}}' image

and

go1.10rc1 list -f '{{.Doc}}' image

and

package main

import (
	"fmt"
	"go/build"
	"log"
)

func main() {
	pkg, err := build.Import("image", "", 0)
	if err != nil {
		log.Fatal(err)
	}
	fmt.Println(pkg.Doc)
}

all print

This example demonstrates decoding a JPEG image and examining its pixels.

rather than

Package image implements a basic 2-D image library.

which is the correct package synopsis.

The erroneous synopsis comes from the package documentation of the image_test package which provides the package-level Example.

It's possible that other packages are effected but this is the only one I found.

From a cursory look at *go/build.Context.Import it appears that this is caused by the loop grabbing the first synopsis found in any file (lines 816-818 below) when it seems as if it should ignore test and xtest files using the variables isTest and isXTest

go/src/go/build/build.go

Lines 798 to 818 in 00587e8

isTest := strings.HasSuffix(name, "_test.go")
isXTest := false
if isTest && strings.HasSuffix(pkg, "_test") {
isXTest = true
pkg = pkg[:len(pkg)-len("_test")]
}
if p.Name == "" {
p.Name = pkg
firstFile = name
} else if pkg != p.Name {
badFile(&MultiplePackageError{
Dir: p.Dir,
Packages: []string{p.Name, pkg},
Files: []string{firstFile, name},
})
p.InvalidGoFiles = append(p.InvalidGoFiles, name)
}
if pf.Doc != nil && p.Doc == "" {
p.Doc = doc.Synopsis(pf.Doc.Text())
}

System details

go version go1.9.3 linux/amd64
GOARCH="amd64"
GOBIN="/home/jimmy/bin"
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/jimmy/code/go"
GORACE=""
GOROOT="/home/jimmy/ext/go"
GOTOOLDIR="/home/jimmy/ext/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build433721585=/tmp/go-build -gno-record-gcc-switches"
CXX="g++"
CGO_ENABLED="1"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOROOT/bin/go version: go version go1.9.3 linux/amd64
GOROOT/bin/go tool compile -V: compile version go1.9.3
uname -sr: Linux 4.13.0-1-amd64
Distributor ID:	Debian
Description:	Debian GNU/Linux testing (buster)
Release:	testing
Codename:	buster
/lib/x86_64-linux-gnu/libc.so.6: GNU C Library (Debian GLIBC 2.26-4) stable release version 2.26, by Roland McGrath et al.
gdb --version: GNU gdb (Debian 7.12-6+b1) 7.12.0.20161007-git
@jimmyfrasche
Copy link
Member Author

I would have put the bit of code on the playground but using go/build doesn't work there. Is that another bug or just a limitation of the playground?

@ianlancetaylor ianlancetaylor added the NeedsFix The path to resolution is known, but the work has not been done. label Jan 29, 2018
@ianlancetaylor ianlancetaylor added this to the Go1.11 milestone Jan 29, 2018
@josharian
Copy link
Contributor

Nice find.

I would have put the bit of code on the playground but using go/build doesn't work there. Is that another bug or just a limitation of the playground?

Depends on how you view it. The playground's filesystem is fake -- see "Faking the file system" in https://blog.golang.org/playground. It can be prepopulated, so we could prepopulate it with the standard library. But I don't really think that'd be worth the effort.

FWIW, I actually prefer pasted code to playground links, at least for small bits of code. Making skimming issues easier, particularly on my phone.

@jimmyfrasche
Copy link
Member Author

@josharian fair enough, filed #23603 nonetheless, since it would be useful albeit niche.

(anchored link to section Faking the file system)

@mvdan
Copy link
Member

mvdan commented Feb 19, 2018

@jimmyfrasche it seems like you described a patch above - why not send it as a CL?

@gopherbot
Copy link

Change https://golang.org/cl/96976 mentions this issue: go/build: correct value of .Doc field

@jimmyfrasche
Copy link
Member Author

@mvdan a fatal combination of laziness and forgetfulness—thanks for the reminder!

In testing I found this issue also affected the flag and container/heap packages.

@jimmyfrasche jimmyfrasche mentioned this issue Mar 11, 2018
Open
@golang golang locked and limited conversation to collaborators Mar 7, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Milestone
Go1.11
Development

No branches or pull requests
5 participants
@josharian @jimmyfrasche @ianlancetaylor @mvdan @gopherbot