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/go: track tools/tooling updates to support modules #24661

Closed
42 of 63 tasks
flibustenet opened this issue Apr 3, 2018 · 96 comments
Closed
42 of 63 tasks

cmd/go: track tools/tooling updates to support modules #24661

flibustenet opened this issue Apr 3, 2018 · 96 comments

Comments

@flibustenet
Copy link

@flibustenet flibustenet commented Apr 3, 2018

Many Go tools that previously worked with GOPATH need to be upgraded in order to work with modules.

Note: Many of these tools have been deprecated with modules, and their features have been incorporated into gopls. This is indicated below.

This is a tracking issue for the conversion of tools to work with modules.

@gopherbot gopherbot added this to the vgo milestone Apr 3, 2018
@rsc rsc changed the title x/vgo how to works with tools like guru ? x/vgo: integrate with guru, goimports, etc Apr 3, 2018
@rsc
Copy link
Contributor

@rsc rsc commented Apr 3, 2018

Yes, this is an issue we're very aware of. Will leave this open to track it.

@myitcv
Copy link
Member

@myitcv myitcv commented Apr 7, 2018

@rsc - just to check this issue effectively encapsulates what we discussed in golang-dev?

@myitcv myitcv changed the title x/vgo: integrate with guru, goimports, etc x/vgo: integrate with guru, goimports, and other tools Apr 9, 2018
@myitcv myitcv changed the title x/vgo: integrate with guru, goimports, and other tools x/vgo: integrate with guru, goimports, and other tools/tooling Apr 9, 2018
@myitcv
Copy link
Member

@myitcv myitcv commented Apr 9, 2018

Changed the title so that this issue matches on use of the words "tool", "tools" or "tooling"

Will keep a list of tools here too (please edit):

  • Requiring build output:
    • guru
    • gocode
    • stringer
  • Requiring source code

@gopherbot
Copy link

@gopherbot gopherbot commented Apr 9, 2018

Change https://golang.org/cl/105855 mentions this issue: x/vgo: add deplist command to get build information about (test) deps

myitcv added a commit to myitcv/vgo that referenced this issue Apr 30, 2018
This CL is a work in progress for golang/go#24661. Following the pattern laid
down by the vet command, we want to pass the build details of the
transitive (test) deps of a list of packages onto vetters/linters etc
that need to load imports of said packages for go/types (and similar)
analysis.

Fixes golang/go#24661

Change-Id: If1496dd6a3ed501ad6f124226a05f5d57284c57d
myitcv added a commit to myitcv/vgo that referenced this issue May 20, 2018
This CL is a work in progress for golang/go#24661. Following the pattern laid
down by the vet command, we want to pass the build details of the
transitive (test) deps of a list of packages onto vetters/linters etc
that need to load imports of said packages for go/types (and similar)
analysis.

Fixes golang/go#24661

Change-Id: If1496dd6a3ed501ad6f124226a05f5d57284c57d
@rsc
Copy link
Contributor

@rsc rsc commented Jun 6, 2018

@rsc rsc removed this from the vgo milestone Jul 12, 2018
@rsc rsc added this to the Go1.11 milestone Jul 12, 2018
@rsc rsc added the modules label Jul 12, 2018
@rsc rsc changed the title x/vgo: integrate with guru, goimports, and other tools/tooling cmd/go: integrate with guru, goimports, and other tools/tooling Jul 12, 2018
@gopherbot
Copy link

@gopherbot gopherbot commented Jul 20, 2018

Change https://golang.org/cl/125296 mentions this issue: go/build: invoke go command to find modules during Import, Context.Import

gopherbot pushed a commit that referenced this issue Jul 28, 2018
…port

The introduction of modules has broken (intentionally) the rule
that the source code for a package x/y/z is in GOPATH/src/x/y/z
(or GOROOT/src/x/y/z). This breaks the code in go/build.Import,
which uses that rule to find the directory for a package.

In the long term, the fix is to move programs that load packages
off of go/build and onto golang.org/x/tools/go/packages, which
we hope will eventually become go/packages. That code invokes
the go command to learn what it needs to know about where
packages are.

In the short term, though, there are lots of programs that use go/build
and will not be able to find code in module dependencies.
To help those programs, go/build now runs the go command to
ask where a package's source code can be found, if it sees that
modules are in use. (If modules are not in use, it falls back to the
usual lookup code and does not invoke the go command, so that
existing uses are unaffected and not slowed down.)

Helps #24661.
Fixes #26504.

Change-Id: I0dac68854cf5011005c3b2272810245d81b7cc5a
Reviewed-on: https://go-review.googlesource.com/125296
Reviewed-by: Michael Matloob <matloob@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
@myitcv
Copy link
Member

@myitcv myitcv commented Aug 1, 2018

@alandonovan @ianthehat - is it worth creating a number of issues, specific to each tool that's being worked on?

I keep directing people to this umbrella issue... but maybe there's merit in getting a bit more granularity on things? Equally, maybe not.

So I'm definitely wouldn't push back if you said "let's keep things here" 👍

@sam3d
Copy link

@sam3d sam3d commented Aug 2, 2018

How about the case of goimports importing packages that are both:

  • Within the module's local directory
  • Outside of the system scoped $GOPATH

Assuming the following directory structure:

./
├── go.mod
├── main.go
└── test
    └── test.go

With ./go.mod containing:

module example.com

goimports will happily leave alone and even format ./main.go:

package main

import (
	"log"

	"example.com/test"
)

func main() {
	log.Print("Hello, world!")
	test.Start()
}

With ./test/test.go:

package test

import "fmt"

func Start() {
	fmt.Println("Started properly")
}

However, if the "example.com/test" import is removed from main.go, it wont get added or found again. If its import path is changed in any way to render it incorrect (say to "example.com/tes"), goimports simply deletes the line.

It's odd - goimports can clearly tell that package test exists when it's been provided already, but it can't actively seek and add it if it hasn't been.

Is this something that requires looking into the module cache for, or is this a different issue?

@ianthehat
Copy link

@ianthehat ianthehat commented Aug 2, 2018

goimports will assume that the import line "example.com/test" imports a package called test if it cannot find any sources for that package, and it knows the test symbol is used, so it won't delete the line. It does not mean that it knew the package existed...

goimports scans your GOPATH and GOROOT, anything not there it will not find right now.

It will be made to use x/tools/go/packages plus some extra stuff that is beyond the scope of that package. When it does, this case will work (does not involve the module cache, just the list of packages in the current module, which you can already get with go mod -packages

@sam3d
Copy link

@sam3d sam3d commented Aug 2, 2018

Thank you for explaining @ianthehat! I've misunderstood how goimports handles symbols.

@metakeule
Copy link

@metakeule metakeule commented Aug 11, 2018

I have two questions:

  1. will x/tools/go/packages move to the standard library with Go 1.12?
  2. I can't see any information about the version of a plugin, so how is a tool that assists in importing (like GoSublime) supposed to get a list of possible imports (with versions)?

@metakeule
Copy link

@metakeule metakeule commented Aug 11, 2018

Also: How to prevent that import completion will get really slow with larger projects? (see mdempsky/gocode#32)

@myitcv
Copy link
Member

@myitcv myitcv commented Aug 11, 2018

@metakeule

will x/tools/go/packages move to the standard library with Go 1.12?

The plan is for it to move to the standard library at some point, yes, once it has stabilised (ref).

I can't see any information about the version of a plugin

This is similar to the goimports "problem"; see #26717

How to prevent that import completion will get really slow with larger projects?

Many of the problems describe in mdempsky/gocode#32 relate to speed problems brought about by the use of the -source flag, which drives the use of the source importer. go/packages will work using the underlying build cache and so should not suffer these problems.

But in case lag does become an issue for gocode with its current architecture, go/packages will be offering support for long-running programs. So it's conceivable that gocode's cache will switch to be module-based, where it effectively "watches" for relevant changes and refreshes its cache accordingly. Its cache will then be primed for fast responses to ad-hoc queries.

cc @dominikh (in case there's any overlap of interest here with your work)

@gopherbot
Copy link

@gopherbot gopherbot commented Jan 9, 2020

Change https://golang.org/cl/189818 mentions this issue: cmd/bundle: use caller's module version via go/packages

@thepudds
Copy link

@thepudds thepudds commented Feb 13, 2020

dvyukov/go-fuzz was listed above as not supporting modules, but updated list to indicate it does now support modules (dvyukov/go-fuzz#274).

The current approach for go-fuzz works reasonably well for fuzzing modules, but it is not the "forever" solution for go-fuzz. In particular, the current approach will stop working if or when GOPATH mode goes away completely (because a portion of the implementation still relies on being able to disable module mode temporarily, though that is more-or-less transparent to the user).

@gopherbot
Copy link

@gopherbot gopherbot commented Sep 15, 2020

Change https://golang.org/cl/255053 mentions this issue: cmd/bundle: expand test coverage to all modes

@powerman
Copy link

@powerman powerman commented Sep 16, 2020

Maybe it's time to update this list? I'm pretty sure most of mentioned linters works just fine today with modules (at least within golangci-lint). Probably other things has changed too since Feb.

@stamblerre
Copy link
Contributor

@stamblerre stamblerre commented Sep 16, 2020

Just went through and updated a bit. A lot of the tools in this list have been replaced by gopls as well.

gopherbot pushed a commit to golang/tools that referenced this issue Sep 29, 2020
Also, remove obsolete -underscore option.

Fixes golang/go#32031
Updates golang/go#24661

Change-Id: I47940a7ccfaf82a042eadc76f47304fc1dd8cbdd
Reviewed-on: https://go-review.googlesource.com/c/tools/+/189818
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
gopherbot pushed a commit to golang/tools that referenced this issue Sep 29, 2020
As of CL 189818, bundle has been updated to use the go/packages API to
load packages. That API supports module mode and legacy GOPATH mode.
Update the test to provide coverage for all modes.

Updates golang/go#24661.

Change-Id: Ied6196f7317b9b0289faf80dbfe9815e9b98b55d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/255053
Trust: Dmitri Shuralyov <dmitshur@golang.org>
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
@fzipp
Copy link
Contributor

@fzipp fzipp commented Oct 23, 2020

I am the author of gocyclo. It does not need an upgrade to support modules, it works on a file-by-file basis.

@mdempsky
Copy link
Member

@mdempsky mdempsky commented Oct 31, 2020

maligned (github.com/mdempsky/maligned) is updated to support modules. (It's now implemented as an x/tools/go/analysis.Analyzer.)

@matttproud
Copy link
Contributor

@matttproud matttproud commented Feb 22, 2021

Not sure if this is useful for anyone, but I briefly looked at porting eg, gomvpkg, gorename from package loader to package packages as a spare time activity to take advantage of user-providable GOPACKAGESDRIVER drivers for custom workspaces (due to gopls having (temporarily) disabled support for overrides).

Basic findings (naive), which I am writing for myself should I ever wonder this again:

Porting the tools does not look terribly difficult, but it appears that loader's support for file-orientation (*loader.Conf).CreateFromFiles as opposed to package-orientation makes this a little bit trickier the old behavior. I presume one would have better luck forking and pivoting the tool interface behavior versus updating them in situ.

@josharian
Copy link
Contributor

@josharian josharian commented Feb 22, 2021

I ported eg already. But I see to my horror that I never hit submit. I’ll plan to do that when I’m next at a computer. https://go-review.googlesource.com/c/tools/+/258304 might be useful.

@mvdan
Copy link
Member

@mvdan mvdan commented Feb 22, 2021

At the risk of being too off-topic, if you found eg useful, you might want to take a look at https://github.com/rsc/rf.

@thepudds
Copy link

@thepudds thepudds commented Feb 22, 2021

@matttproud

I briefly looked at porting eg, gomvpkg, gorename

And just to briefly expand on @mvdan's comment, https://github.com/rsc/rf is also useful if you wanted a replacement for gomvpkg and gorename.

Here is a sample use, where you can see it doing eg-like example-based refactoring, as well as moving packages and renaming identifiers:

527a189

rf is effectively a superset of multiple tools on the main list at the start of this issue. (It is still WIP as far as I understand, but still usable).

@matttproud
Copy link
Contributor

@matttproud matttproud commented Feb 22, 2021

@mdempsky
Copy link
Member

@mdempsky mdempsky commented Feb 22, 2021

Apropos to this conversation, rsc/rf originally used x/tools/go/packages, but for performance switched to a custom loader (which does not support GOPACKAGESDRIVER, at least currently): rsc/rf@8880f26

@mvdan
Copy link
Member

@mvdan mvdan commented May 8, 2021

I wonder if there's value in keeping this issue open indefinitely. It's been three years, and practically every maintained Go tool now supports modules. For those that haven't done it yet, it probably makes sense to open an issue on their trackers and continue there. And if the tool isn't maintained anymore, it's always an option to fork and continue development.

Perhaps we keep this issue open for a little longer (a year?), but I personally wouldn't go beyond that. The point of this issue was to coordinate work, keep track of progress, and potentially identify issues with supporting modules. I think we've succeeded at that :)

@mvdan
Copy link
Member

@mvdan mvdan commented May 12, 2021

It seems like we all agree this issue has served it's purpose, and should be closed soon.

If you're the author or maintainer of a tool that still needs to add support for Go modules, please file an issue in your issue tracker about it, and link to it in a comment here for others to follow. Discussion and progress should continue there.

I'll leave this issue open for another four weeks, to leave time for those comments, and any remaining input. Thanks!

@ainar-g
Copy link
Contributor

@ainar-g ainar-g commented May 13, 2021

Apologies if this is not the right place to ask, but as far as I know, there still isn't a tool like gomvpkg that would allow one to move a package within a module and also update all references to it within the module?

@mvdan
Copy link
Member

@mvdan mvdan commented May 13, 2021

@ainar-g I think rf would satisfy your need, as per this test, though note that it's not ready for general use and lacks documentation.

@ainar-g
Copy link
Contributor

@ainar-g ainar-g commented May 13, 2021

@mvdan, thanks, but I've already tried rf and have seen that example. The example seems to be about moving an identifier between packages and not packages themselves. There is also an example of moving files between packages, but renaming a package file-by-file is rather… suboptimal. I've tried several other scripts, and all of them fail with various errors.

I guess it's mv + ed + vim time again.

@mvdan
Copy link
Member

@mvdan mvdan commented Jun 9, 2021

I'll leave this issue open for another four weeks, to leave time for those comments, and any remaining input. Thanks!

Four weeks elapsed, so closing. Thanks all for the work that happened here!

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

No branches or pull requests