CL 239754 eagerly initialized the environment. This turns out to be a
problem for gopls, which calls ApplyFixes with no ProcessEnv.
Reinitializing it every time seriously harmed the performance of
unimported completions. Move back to lazy initialization.
Working with invalid options has caused a lot of confusion; this is only
the most recent. We have to maintain backwards compatibility in the
externally visible API, but everywhere else we can require fully
populated options. That includes the source byte slice and the options.
LocalPrefix is really more of an Option than an attribute of the
ProcessEnv, and it is needed in ApplyFixes where we really don't want to
have to pass a ProcessEnv. Move it up to Options.
Change-Id: Ib9466c375a640a521721da4587091bf93bbdaa3c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/241159
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This CL got away from me a little.
For a number of reasons, the existing goimports API of passing in values
for various GO* values was not working. For one, the number of necessary
variables kept growing. For another, we tried to avoid calling `go env`
in GOPATH mode by using `build.Default`, but that turns out to be buggy;
see golang/go#39838. And finally, it created massive confusion about
whether the values were intended to be read from the OS environment, or
fully evaluated by the `go` command.
There are only two users of the internal imports API, so there really
shouldn't need to be more than two modes. For the command line tool, we
have to call `go env` to deal with the `go/build` bug. So we just do it.
Tests use that same path, but can augment the enviroment to set
themselves up. In contrast, `gopls` needs to fully specify the
environment. It can simply pass in the fully evaluated GO* values.
Finally, make the change I was actually here to make: propagate
GOMODCACHE and use it where appropriate.
Fixesgolang/go#39761.
Change-Id: I720c69839d91d66d98e94dfc5f065ba0279c5542
Reviewed-on: https://go-review.googlesource.com/c/tools/+/239754
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
In all cases, use a Logf field to configure debug logging. Non-nil means
that logging is enabled through the given function.
Fixes accidental debug spam from goimports, which had a separate Debug
flag that was supposed to guard logging, but wasn't used when creating
the gocommand.Invocation.
Change-Id: I448fa282111db556ac2e49801268d0affc19ae30
Reviewed-on: https://go-review.googlesource.com/c/tools/+/221557
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
goimports now wants various module flags, but I forgot to set them up in
the many places we create environments. Do so.
Change-Id: Ic3817caeb8fc4d564b49006ef6ca6842b2498eaf
Reviewed-on: https://go-review.googlesource.com/c/tools/+/211581
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
For various reasons we need an internal-facing imports API. Move imports
to internal/imports, leaving behind a small wrapper package. The wrapper
package captures the globals at time of call into the options struct.
Also converts the last goimports tests to use the test helpers, and
fixes go/packages in module mode to work with empty modules, which was
necessary to get those last tests converted.
Change-Id: Ib1212c67908741a1800b992ef1935d563c6ade32
Reviewed-on: https://go-review.googlesource.com/c/tools/+/175437
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
