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

x/tools/cmd/godoc: all std packages are treated as command #14447

Closed
minux opened this issue Feb 21, 2016 · 11 comments
Closed

x/tools/cmd/godoc: all std packages are treated as command #14447

minux opened this issue Feb 21, 2016 · 11 comments
Labels
FrozenDueToAge help wanted NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@minux
Copy link
Member

minux commented Feb 21, 2016

I thought there is something wrong with my environment, but I confirmed
this behavior with freshly installed Go tip and godoc tip on a new machine.

$ godoc syscall | head
use 'godoc cmd/syscall' for documentation on the syscall command 

PACKAGE DOCUMENTATION

package syscall
    import "syscall"

    Package syscall contains an interface to the low-level operating system
    primitives. The details vary depending on the underlying system, and by
    default, godoc will display the syscall documentation for the current
@minux minux added this to the Unreleased milestone Feb 21, 2016
@griesemer griesemer modified the milestones: Go1.9, Unreleased Nov 22, 2016
@griesemer griesemer assigned alandonovan and unassigned griesemer Nov 22, 2016
@bradfitz bradfitz added help wanted NeedsFix The path to resolution is known, but the work has not been done. labels Jul 12, 2017
@bradfitz
Copy link
Contributor

Not critical for Go 1.9. But also looks like a good HelpWanted project for somebody, self-contained in x/tools/godoc/cmdline.go probably.

@bradfitz bradfitz modified the milestones: Go1.10, Go1.9 Jul 14, 2017
@quasilyte
Copy link
Contributor

quasilyte commented Sep 16, 2017

Weird, but I can not get godoc to output cmd documentation at all.

$ which go
/usr/local/go/bin/go
$ go version
go version go1.8.1 linux/amd64
$ go env GOROOT
/usr/local/go
$ godoc cmd/asm
2017/09/17 02:12:37 cmd/asm: no such directory or package
$ godoc asm
2017/09/17 02:12:39 open /usr/local/go/src/asm: no such file or directory
$ godoc cmd/gofmt
2017/09/17 02:12:45 cmd/gofmt: no such directory or package

Looking at this comment (cmdline.go:72):

			// Then try $GOROOT/cmd.
			abspath = pathpkg.Join(pres.CmdFSRoot(), path)
			cinfo = pres.GetCmdPageInfo(abspath, relpath, mode)
  1. I think comment should say $GOROOT/src/cmd instead of $GOROOT/cmd.
  2. pres.CmdFSRoot() returns src, not src/cmd. This means that we will always think that path acutally is a command.

Let's check my guess:

diff --git a/godoc/pres.go b/godoc/pres.go
index 8551177..5e62b08 100644
--- a/godoc/pres.go
+++ b/godoc/pres.go
@@ -119,7 +119,7 @@ func NewPresentation(c *Corpus) *Presentation {
                p:       p,
                c:       c,
                pattern: "/cmd/",
-               fsRoot:  "/src",
+               fsRoot:  "/src/cmd",
        }
        p.pkgHandler = handlerServer{
                p:           p,

Tests are passing.

$ GOPATH=`pwd` go install golang.org/x/tools/cmd/godoc
$ ./bin/godoc cmd/asm
COMMAND DOCUMENTATION

    Asm, typically invoked as ``go tool asm'', assembles the source file
    into an object file named for the basename of the argument source file
    with a .o suffix. The object file can then be combined with other
    objects into a package archive.


    Command Line
<rest output>

$ ./bin/godoc asm 
<same output as above>

Now check the case that is described by @minux:

$ ./bin/godoc syscall | head
PACKAGE DOCUMENTATION

package syscall
    import "syscall"

    Package syscall contains an interface to the low-level operating system
    primitives. The details vary depending on the underlying system, and by
    default, godoc will display the syscall documentation for the current
    system. If you want godoc to display syscall documentation for another
    system, set $GOOS and $GOARCH to the desired system. For example, if you

Seems like it works as intended.

I do believe the real solution should look different, otherwise pattern would not make much sense.
Not ready to dive into that any deeper though.

@quasilyte
Copy link
Contributor

quasilyte commented Sep 16, 2017

It seems like it matches cmd/foo as foo even if foo is not a command. Is this intended?

$ ls -R
.:
    src
./src:
    foo
./src/foo:
    foo.go

$ godoc -goroot=`pwd` foo | head -n 4
use 'godoc cmd/foo' for documentation on the foo command 

PACKAGE DOCUMENTATION

$ godoc -goroot=`pwd` cmd/foo | head -n 4
PACKAGE DOCUMENTATION

package foo
    import "."

Note: patch provided above does make cmd/foo for foo fail.

../xtools/bin/godoc -goroot=`pwd` cmd/foo | head -n 4
2017/09/17 02:36:05 cmd/foo: no such directory or package

@agnivade
Copy link
Contributor

agnivade commented Oct 4, 2017

Hi Iskander,

It seems like it matches cmd/foo as foo even if foo is not a command. Is this intended?

Yes, I see this too. It should definitely be a bug. More so, if foo is indeed a command, it always shows the output for foo as a package.

$godoc fmt | head
use 'godoc cmd/fmt' for documentation on the fmt command 

PACKAGE DOCUMENTATION

package fmt
    import "fmt"

    Package fmt implements formatted I/O with functions analogous to C's
    printf and scanf. The format 'verbs' are derived from C's but are
    simpler.
$godoc cmd/fmt | head
PACKAGE DOCUMENTATION

package fmt
    import "."

    Package fmt implements formatted I/O with functions analogous to C's
    printf and scanf. The format 'verbs' are derived from C's but are
    simpler.

I can take this up if you are not actively working on this.

@quasilyte
Copy link
Contributor

Hello @agnivade.

I can take this up if you are not actively working on this.

I am not. Good luck.

@agnivade
Copy link
Contributor

Looked into this. There is a complex path of lots of if-elses which decides on what is a cmd and what is a src package. I feel this will be best solved by someone with prior knowledge of godoc.

@gopherbot
Copy link

Change https://golang.org/cl/94841 mentions this issue: godoc: Corrects fsRoot to prevent misleading message about cmd/

@bradfitz
Copy link
Contributor

Fix was reverted for breaking the build. Reopening.

alindeman added a commit to alindeman/tools that referenced this issue Feb 23, 2018
godoc is erroneously detecting paths like syscall and unsafe as possible
commands.

A previous attempt at a fix adjusted the parameters of pkgHandler in
pres.go, but that change had unexpected ripple effects that broke links
and other features of godoc. This change is scoped only to the godoc
command line tool.

cmdline_test.go is updated to match the parameters used in the real
pkgHandler in pres.go.

Fixes golang/go#14447
@gopherbot
Copy link

Change https://golang.org/cl/96515 mentions this issue: godoc: correct abspath when looking for cmds

@ysmolski
Copy link
Member

@agnivade, I have tested your last CL and it works as expected:

...x/tools/godoc ((4b4231b9...) $) % cat ~/go/src/p1/p1.go
package p1
import "p2"

const V2 = p2.V
...x/tools/godoc ((4b4231b9...) $) % godoc p1
PACKAGE DOCUMENTATION

package p1
    import "p1"


CONSTANTS

const V2 = p2.V


...x/tools/godoc ((4b4231b9...) $) % godoc cmd/p1
2018/03/23 12:11:11 cmd/p1: no such directory or package
...x/tools/godoc ((4b4231b9...) $) % godoc syscall|head
PACKAGE DOCUMENTATION

package syscall
    import "syscall"

    Package syscall contains an interface to the low-level operating system
    primitives. The details vary depending on the underlying system, and by
    default, godoc will display the syscall documentation for the current
    system. If you want godoc to display syscall documentation for another
    system, set $GOOS and $GOARCH to the desired system. For example, if you
...x/tools/godoc ((4b4231b9...) $) % godoc cmd/syscall
2018/03/23 12:11:28 cmd/syscall: no such directory or package
...x/tools/godoc ((4b4231b9...) $) % godoc cmd/fmt|head
2018/03/23 12:11:34 cmd/fmt: no such directory or package
...x/tools/godoc ((4b4231b9...) $) % godoc cmd/gofmt|head
COMMAND DOCUMENTATION

    Gofmt formats Go programs. It uses tabs for indentation and blanks for
    alignment. Alignment assumes that an editor is using a fixed-width font.

    Without an explicit path, it processes the standard input. Given a file,
    it operates on that file; given a directory, it operates on all .go
    files in that directory, recursively. (Files starting with a period are
    ignored.) By default, gofmt prints the reformatted sources to standard
    output.

@agnivade
Copy link
Contributor

It's not my CL 😝

@golang golang locked and limited conversation to collaborators Jun 13, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge help wanted NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants