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: inconsistent reporting of synopsis vs command documentation #7119

Open
griesemer opened this Issue Jan 13, 2014 · 5 comments

Comments

Projects
None yet
5 participants
@griesemer
Contributor

griesemer commented Jan 13, 2014

1) In a $GOROOT/cmd/foo directory, create two files doc.go, main.go:

$ cat doc.go
// +build ignore

// Package foo is a foo bar.
package main

$ cat main.go
package main

2) godoc cmd/foo won't show any documentation, only the title:
COMMAND DOCUMENTATION

3) run godoc -http=:9000

4) Navigate to: http://localhost:9000/cmd/
The synopsis for command foo is there:
foo         Package foo is a foo bar.

5) Click on command foo: The documentation only shows the title (not even the synopsis
anymore):
Command foo

Problem (suspicion, haven't analyzed in detail): The reason is that the synopsis is
collected in a separate pass, by traversing all directories and extracting documentation
from any .go file available. But when the documentation for the package is shown, the
go/build package is used to determine which files to include, and in this case the
documentation file is ignored from the build.

The solution is to remove the build tag and it will work consistently.  However, it is
confusing that a) the synopsis shows up anyway, and b) that for C commands documentation
is shown even though the build tag is (must be!) present. If one simply copies a C
command doc.go file and adjusts it for a command written in Go one is somewhat surprised
that it doesn't work.

Not sure what the proper solution is - perhaps godoc should say that files were excluded
via build tags? Then the (user) error would be at least not silent.

@griesemer griesemer added new labels Jan 13, 2014

@bradfitz bradfitz removed the new label Dec 18, 2014

@dmitshur

This comment has been minimized.

Member

dmitshur commented Dec 30, 2014

I've also found an issue with cmd/godoc, and I think it's very similar/related to this issue, so I'll add details here rather than creating a separate issue (is that the right thing to do?).

The problem is that cmd/godoc will display (erroneously) docs/summary for things that are not a valid Go package, which can be misleading.

From the help/description of cmd/godoc:

Godoc extracts and generates documentation for Go programs.

godoc fmt                # documentation for package fmt

Where "package fmt" implies to me the parameter should be a valid Go package.

Consider these steps:

~ $ go get -d -u github.com/shurcooL/play/93
package github.com/shurcooL/play/93
    imports github.com/shurcooL/play/93
    imports github.com/shurcooL/play/93: no buildable Go source files in /tmp/tempgopath/src/github.com/shurcooL/play/93
~ $ go list github.com/shurcooL/play/93
can't load package: package github.com/shurcooL/play/93: no buildable Go source files in /tmp/tempgopath/src/github.com/shurcooL/play/93
~ $ go build github.com/shurcooL/play/93
can't load package: package github.com/shurcooL/play/93: no buildable Go source files in /tmp/tempgopath/src/github.com/shurcooL/play/93
~ $ godoc github.com/shurcooL/play/93
COMMAND DOCUMENTATION

    Package 93 is a test of "package documentation" functionality. It
    contains a single .go file with package name documentation.


~ $ 

Clearly, github.com/shurcooL/play/93 is not a valid Go package because it does not contain any .go files that are not ignored (it contains one, but it is ignored because the package name is documentation). cmd/godoc suggests it's a valid command, but it's not.

See golang/gddo#225 (comment) for more details on why it happens.

@minux

This comment has been minimized.

Member

minux commented Dec 30, 2014

I don't get it. package documentation is reserved for documenting commands,
so if there is valid Go file with package documentation, godoc will treat
it as
the documentation for a command contained in that directory.

To what extend should godoc try to verify the package/command? Should it
try compiling it to be sure that it's a valid package/command?

@dmitshur

This comment has been minimized.

Member

dmitshur commented Dec 30, 2014

Hi @minux,

package documentation is reserved for documenting commands,

That's not true as far as I can tell, it can be used to document any Go package (command or library). Can you show where it says otherwise, because I don't see that documented and in practice, it works on non-main packages too (Edit: I am wrong about this, sorry. It didn't look like it was for commands only, but the docs it contains seem to be ignored for non-main packages.).

To what extend should godoc try to verify the package/command? Should it
try compiling it to be sure that it's a valid package/command?

cmd/godoc currently respects builds tags, right? It looks at the given folder, finds all .go files but only considers ones that are not ignored due to (business logic taken from http://godoc.org/go/build#Context.Import):

  • .go files in package documentation
  • files starting with _ or . (likely editor temporary files)
  • files with build constraints not satisfied by the context

If it finds that there are 0 .go files (either because there are really no .go files, or because the ones that were there were all ignored due to above rules), it should not try to display any docs (what docs can be displayed if there are 0 .go files?).

@minux

This comment has been minimized.

Member

minux commented Dec 30, 2014

On Tue, Dec 30, 2014 at 12:18 AM, Dmitri Shuralyov <notifications@github.com

wrote:

That's not true as far as I can tell, it can be used to document any Go
package (command or library). Can you show where it says otherwise, because
I don't see that documented and in practice

go/src/go/build/build.go

Lines 675 to 678 in 4b96409

if pkg == "documentation" {
p.IgnoredGoFiles = append(p.IgnoredGoFiles, name)
continue
}
,
it works on non-main packages too.

Does it work for non-commands packages? I highly doubt it.
(I mean godoc, not go/build, go/build ignores the file for a related but
different reason)
https://go.googlesource.com/tools/+/master/godoc/server.go#97

You quoted go/build source code, but that doesn't prove the point, you
should look into
the source code of the godoc package itself.

To what extend should godoc try to verify the package/command? Should it
try compiling it to be sure that it's a valid package/command?

cmd/godoc currently respects builds tags, right? It looks at the given
folder, finds all .go files but only considers ones that are not ignored
due to (business logic taken from http://godoc.org/go/build#Context.Import
):

  • .go files in package documentation
  • files starting with _ or . (likely editor temporary files)
  • files with build constraints not satisfied by the context

If it finds that there are 0 .go files (either because there are really no
.go files, or because the ones that were there were all ignored due to
above rules), it should not try to display any docs (what docs can be
displayed if there are 0 .go files?).

Read the comment I quoted and see why package documentation is a special
case.
This behavior won't go away as long as we still have commands written in C.

@dmitshur

This comment has been minimized.

Member

dmitshur commented Dec 30, 2014

Hmm, I realize now that godoc differs from build.Import in that it does not ignore .go files in package documentation. On the contrary, it must use that file. So the view of a "Go package" is different for cmd/godoc vs. cmd/go/build.Import.

https://go.googlesource.com/tools/+/master/godoc/server.go#97

Read the comment I quoted and see why package documentation is a special
case.
This behavior won't go away as long as we still have commands written in C.

I see, that makes sense, thank you.

Anyway, the reason I considered this an (minor) issue is because IMO the behavior of godoc.org and cmd/godoc should match when displaying Go package docs, and in this case they do not. But I need to think more about this and perhaps the issue is in godoc.org after all.

Also, this concept of package name documentation does not seem to be documented well, but your link hints as to why: it's a historic feature, and +build ignore is recommended. But still, given that it is a part of the Go project, it should be better (more explicitly) documented. (I was trying hard to understand what it does because I need to be able to tell when a given folder contains a "Go package" or not.)

@rsc rsc added this to the Unplanned milestone Apr 10, 2015

@rsc rsc removed the release-none label Apr 10, 2015

@rsc rsc changed the title from cmd/godoc: inconsistent reporting of synopsis vs command documentation to x/tools/cmd/godoc: inconsistent reporting of synopsis vs command documentation Apr 14, 2015

@rsc rsc removed the repo-tools label Apr 14, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment