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

cmd/doc: support -mod=readonly and other build flags #35200

Open
gertcuykens opened this issue Oct 28, 2019 · 10 comments
Open

cmd/doc: support -mod=readonly and other build flags #35200

gertcuykens opened this issue Oct 28, 2019 · 10 comments
Labels
NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@gertcuykens
Copy link
Contributor

gertcuykens commented Oct 28, 2019

Currently the other godoc html tool need significant changes before module support is ready. Also gopls has a long way to go to be a reliable tool for inspecting modules documentation which leave us with go doc to be the only tool we have for inspect modules.

GO111MODULE="on" (force to always enable modules)
% go doc github.com/gertcuykens/module/v5
doc: cannot find module providing package github.com/gertcuykens/module/v5: working directory is not part of a module
% mkdir test
% cd test
% go mod init test
go: creating new go.mod: module test
% go doc github.com/gertcuykens/module/v5
package module // import "github.com/gertcuykens/module/v5"
type CakeBaker struct{}
% cat go.mod 
module test
go 1.14
require github.com/gertcuykens/module/v5 v5.0.0 // indirect

What needs to be done for go doc specifically so that we have at least one reliable tool is:

A) go doc should never update go.mod files which it does and also does if cd is in GOPATH (related to #35070)

B) go doc should not require any go.mod file to be present in current working directory to inspect any existing module espacialy when GO111MODULE is always set to on

@stamblerre
Copy link
Contributor

Coincidentally, @rentziass just sent out CL 203609 today, which adds support for a gopls symbols command. It would be fairly trivial to add a gopls doc which works per-package and also shows documentation in the style of go doc.

@gertcuykens
Copy link
Contributor Author

gertcuykens commented Oct 28, 2019

Ok thx going to test this as soon as possible to figure out if gopls can be used as a replacement of go doc in a short amount of time :) #32875

@gertcuykens
Copy link
Contributor Author

gertcuykens commented Oct 28, 2019

Please correct me if am wrong but https://go-review.googlesource.com/c/tools/+/203609/1/internal/lsp/cmd/symbols.go doesn't support modules, only input files located on your disk, so to be able to do what go doc does by first downloading the module and then look up a specific symbol or give a overview of all symbols of that specific module requires a lot more work I guess.

@jayconrod
Copy link
Contributor

A) go doc should never update go.mod files which it does and also does if cd is in GOPATH (related to #35070)

This part is working as intended. go doc is part of the go command. Every subcommand related to modules will modify go.mod if it needs to look up a module (or fail if -mod=readonly) is set. go doc should not have significant differences from other commands.

B) go doc should not require any go.mod file to be present in current working directory to inspect any existing module espacialy when GO111MODULE is always set to on

This part is covered by #33710.

Also related: #32027.

cc @bcmills

@gertcuykens
Copy link
Contributor Author

gertcuykens commented Oct 29, 2019

This part is working as intended. go doc is part of the go command. Every subcommand related to modules will modify go.mod if it needs to look up a module (or fail if -mod=readonly) is set. go doc should not have significant differences from other commands.

Ok kind of agree, but you don't have the mod readonly flag in go doc?

% go doc -mod=readonly github.com/gertcuykens/module/v5
flag provided but not defined: -mod

Anyway you are right this is more like a duplicate of #33710

@jayconrod
Copy link
Contributor

Ok kind of agree, but you don't have the mod readonly flag in go doc?

That's true, we should probably add that and most of the other relevant build flags. I'll reopen this issue for that.

@jayconrod jayconrod reopened this Oct 30, 2019
@jayconrod jayconrod changed the title cmd/go: doc should never modify go.mod and should work everywhere cmd/doc: support -mod=readonly and other build flags Oct 30, 2019
@jayconrod jayconrod added the NeedsFix The path to resolution is known, but the work has not been done. label Oct 30, 2019
@jayconrod jayconrod added this to the Backlog milestone Oct 30, 2019
@agnivade
Copy link
Contributor

Now that GOFLAGS is there, it is a simple way to pass flags across binaries. And go doc already supports the -mod flag through GOFLAGS and others too, like -x. Are we sure we want to add explicit command line flags to all go subcommands such as these ?

@gertcuykens
Copy link
Contributor Author

GOFLAGS will do but I am pretty sure 99% of the community has no idea what the general GOFLAGS variables are. And if we go for GOFLAGS only, which is fine by me and maybe even preferable because it prevents flag clutter, then it should be made consistent with other commands by removing for example -mod from all other go commands.

@esprehn
Copy link

esprehn commented Aug 4, 2022

We were bit by this recently. godoc will mutate the go.sum unless told otherwise, which means when run locally things appear to work fine, but if run remotely on a readonly filesystem they fail. It looks like the other go commands now run in readonly mode by default, should godoc do that as well? It's surprising it'll fix up things automatically.

@bcmills
Copy link
Contributor

bcmills commented Aug 24, 2022

@esprehn, now that -mod=readonly is the default I think it's worth filling a separate issue for the mutations you observed (along with clear steps to reproduce them).

I'd like to keep this issue focused on flag support (for example, using -mod=readonly vs. -mod=vendor when a vendor subdirectory is present).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

6 participants