Skip to content

Commit

Permalink
gopls/internal/cache: use a named bool type for allowNetwork
Browse files Browse the repository at this point in the history
As suggested on CL 626715, using a named bool clarifies call sites.

Change-Id: Ie36f406dcca382c7edc277895826265ae9040ee2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/628235
LUCI-TryBot-Result: Go LUCI <[email protected]>
Reviewed-by: Alan Donovan <[email protected]>
  • Loading branch information
findleyr committed Nov 14, 2024
1 parent c043599 commit b1c39aa
Show file tree
Hide file tree
Showing 8 changed files with 27 additions and 20 deletions.
4 changes: 2 additions & 2 deletions gopls/internal/cache/load.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ var errNoPackages = errors.New("no packages returned")
// errors associated with specific modules.
//
// If scopes contains a file scope there must be exactly one scope.
func (s *Snapshot) load(ctx context.Context, allowNetwork bool, scopes ...loadScope) (err error) {
func (s *Snapshot) load(ctx context.Context, allowNetwork AllowNetwork, scopes ...loadScope) (err error) {
if ctx.Err() != nil {
// Check context cancellation before incrementing id below: a load on a
// cancelled context should be a no-op.
Expand Down Expand Up @@ -363,7 +363,7 @@ func (m *moduleErrorMap) Error() string {
// multiple modules in one config, so buildOverlay needs to filter overlays by
// module.
// TODO(rfindley): ^^ is this still true?
func (s *Snapshot) config(ctx context.Context, allowNetwork bool) *packages.Config {
func (s *Snapshot) config(ctx context.Context, allowNetwork AllowNetwork) *packages.Config {
cfg := &packages.Config{
Context: ctx,
Dir: s.view.root.Path(),
Expand Down
2 changes: 1 addition & 1 deletion gopls/internal/cache/mod.go
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,7 @@ func modWhyImpl(ctx context.Context, snapshot *Snapshot, fh file.Handle) (map[st
for _, req := range pm.File.Require {
args = append(args, req.Mod.Path)
}
inv, cleanupInvocation, err := snapshot.GoCommandInvocation(false, fh.URI().DirPath(), "mod", args)
inv, cleanupInvocation, err := snapshot.GoCommandInvocation(NoNetwork, fh.URI().DirPath(), "mod", args)
if err != nil {
return nil, err
}
Expand Down
2 changes: 1 addition & 1 deletion gopls/internal/cache/mod_tidy.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ func modTidyImpl(ctx context.Context, snapshot *Snapshot, pm *ParsedModule) (*Ti
defer cleanup()

args := []string{"tidy", "-modfile=" + filepath.Join(tempDir, "go.mod")}
inv, cleanupInvocation, err := snapshot.GoCommandInvocation(false, pm.URI.DirPath(), "mod", args, "GOWORK=off")
inv, cleanupInvocation, err := snapshot.GoCommandInvocation(NoNetwork, pm.URI.DirPath(), "mod", args, "GOWORK=off")
if err != nil {
return nil, err
}
Expand Down
21 changes: 14 additions & 7 deletions gopls/internal/cache/snapshot.go
Original file line number Diff line number Diff line change
Expand Up @@ -375,7 +375,7 @@ func (s *Snapshot) RunGoModUpdateCommands(ctx context.Context, modURI protocol.D
// TODO(rfindley): we must use ModFlag and ModFile here (rather than simply
// setting Args), because without knowing the verb, we can't know whether
// ModFlag is appropriate. Refactor so that args can be set by the caller.
inv, cleanupInvocation, err := s.GoCommandInvocation(true, modURI.DirPath(), "", nil, "GOWORK=off")
inv, cleanupInvocation, err := s.GoCommandInvocation(NetworkOK, modURI.DirPath(), "", nil, "GOWORK=off")
if err != nil {
return nil, nil, err
}
Expand Down Expand Up @@ -449,6 +449,15 @@ func TempModDir(ctx context.Context, fs file.Source, modURI protocol.DocumentURI
return dir, cleanup, nil
}

// AllowNetwork determines whether Go commands are permitted to use the
// network. (Controlled via GOPROXY=off.)
type AllowNetwork bool

const (
NoNetwork AllowNetwork = false
NetworkOK AllowNetwork = true
)

// GoCommandInvocation populates inv with configuration for running go commands
// on the snapshot.
//
Expand All @@ -459,10 +468,8 @@ func TempModDir(ctx context.Context, fs file.Source, modURI protocol.DocumentURI
// additional refactoring is still required: the responsibility for Env and
// BuildFlags should be more clearly expressed in the API.
//
// If allowNetwork is set, do not set GOPROXY=off.
//
// TODO(rfindley): use a named boolean for allowNetwork.
func (s *Snapshot) GoCommandInvocation(allowNetwork bool, dir, verb string, args []string, env ...string) (_ *gocommand.Invocation, cleanup func(), _ error) {
// If allowNetwork is NoNetwork, set GOPROXY=off.
func (s *Snapshot) GoCommandInvocation(allowNetwork AllowNetwork, dir, verb string, args []string, env ...string) (_ *gocommand.Invocation, cleanup func(), _ error) {
inv := &gocommand.Invocation{
Verb: verb,
Args: args,
Expand Down Expand Up @@ -687,7 +694,7 @@ func (s *Snapshot) MetadataForFile(ctx context.Context, uri protocol.DocumentURI
// - ...but uri is not unloadable
if (shouldLoad || len(ids) == 0) && !unloadable {
scope := fileLoadScope(uri)
err := s.load(ctx, false, scope)
err := s.load(ctx, NoNetwork, scope)

//
// Return the context error here as the current operation is no longer
Expand Down Expand Up @@ -1254,7 +1261,7 @@ func (s *Snapshot) reloadWorkspace(ctx context.Context) {
scopes = []loadScope{viewLoadScope{}}
}

err := s.load(ctx, false, scopes...)
err := s.load(ctx, NoNetwork, scopes...)

// Unless the context was canceled, set "shouldLoad" to false for all
// of the metadata we attempted to load.
Expand Down
2 changes: 1 addition & 1 deletion gopls/internal/cache/view.go
Original file line number Diff line number Diff line change
Expand Up @@ -688,7 +688,7 @@ func (s *Snapshot) initialize(ctx context.Context, firstAttempt bool) {
if len(scopes) > 0 {
scopes = append(scopes, packageLoadScope("builtin"))
}
loadErr := s.load(ctx, true, scopes...)
loadErr := s.load(ctx, NetworkOK, scopes...)

// A failure is retryable if it may have been due to context cancellation,
// and this is not the initial workspace load (firstAttempt==true).
Expand Down
2 changes: 1 addition & 1 deletion gopls/internal/golang/assembly.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ import (
// - cross-link jumps and block labels, like github.com/aclements/objbrowse.
func AssemblyHTML(ctx context.Context, snapshot *cache.Snapshot, pkg *cache.Package, symbol string, web Web) ([]byte, error) {
// Compile the package with -S, and capture its stderr stream.
inv, cleanupInvocation, err := snapshot.GoCommandInvocation(false, pkg.Metadata().CompiledGoFiles[0].DirPath(), "build", []string{"-gcflags=-S", "."})
inv, cleanupInvocation, err := snapshot.GoCommandInvocation(cache.NoNetwork, pkg.Metadata().CompiledGoFiles[0].DirPath(), "build", []string{"-gcflags=-S", "."})
if err != nil {
return nil, err // e.g. failed to write overlays (rare)
}
Expand Down
2 changes: 1 addition & 1 deletion gopls/internal/golang/gc_annotations.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ func GCOptimizationDetails(ctx context.Context, snapshot *cache.Snapshot, mp *me
if !strings.HasPrefix(outDir, "/") {
outDirURI = protocol.DocumentURI(strings.Replace(string(outDirURI), "file:///", "file://", 1))
}
inv, cleanupInvocation, err := snapshot.GoCommandInvocation(false, pkgDir, "build", []string{
inv, cleanupInvocation, err := snapshot.GoCommandInvocation(cache.NoNetwork, pkgDir, "build", []string{
fmt.Sprintf("-gcflags=-json=0,%s", outDirURI),
fmt.Sprintf("-o=%s", tmpFile.Name()),
".",
Expand Down
12 changes: 6 additions & 6 deletions gopls/internal/server/command.go
Original file line number Diff line number Diff line change
Expand Up @@ -589,7 +589,7 @@ func (c *commandHandler) Vendor(ctx context.Context, args command.URIArg) error
// modules.txt in-place. In that case we could theoretically allow this
// command to run concurrently.
stderr := new(bytes.Buffer)
inv, cleanupInvocation, err := deps.snapshot.GoCommandInvocation(true, args.URI.DirPath(), "mod", []string{"vendor"})
inv, cleanupInvocation, err := deps.snapshot.GoCommandInvocation(cache.NetworkOK, args.URI.DirPath(), "mod", []string{"vendor"})
if err != nil {
return err
}
Expand Down Expand Up @@ -751,7 +751,7 @@ func (c *commandHandler) runTests(ctx context.Context, snapshot *cache.Snapshot,
var failedTests int
for _, funcName := range tests {
args := []string{pkgPath, "-v", "-count=1", fmt.Sprintf("-run=^%s$", regexp.QuoteMeta(funcName))}
inv, cleanupInvocation, err := snapshot.GoCommandInvocation(false, uri.DirPath(), "test", args)
inv, cleanupInvocation, err := snapshot.GoCommandInvocation(cache.NoNetwork, uri.DirPath(), "test", args)
if err != nil {
return err
}
Expand All @@ -767,7 +767,7 @@ func (c *commandHandler) runTests(ctx context.Context, snapshot *cache.Snapshot,
// Run `go test -run=^$ -bench Func` on each test.
var failedBenchmarks int
for _, funcName := range benchmarks {
inv, cleanupInvocation, err := snapshot.GoCommandInvocation(false, uri.DirPath(), "test", []string{
inv, cleanupInvocation, err := snapshot.GoCommandInvocation(cache.NoNetwork, uri.DirPath(), "test", []string{
pkgPath, "-v", "-run=^$", fmt.Sprintf("-bench=^%s$", regexp.QuoteMeta(funcName)),
})
if err != nil {
Expand Down Expand Up @@ -828,7 +828,7 @@ func (c *commandHandler) Generate(ctx context.Context, args command.GenerateArgs
if args.Recursive {
pattern = "./..."
}
inv, cleanupInvocation, err := deps.snapshot.GoCommandInvocation(true, args.Dir.Path(), "generate", []string{"-x", pattern})
inv, cleanupInvocation, err := deps.snapshot.GoCommandInvocation(cache.NetworkOK, args.Dir.Path(), "generate", []string{"-x", pattern})
if err != nil {
return err
}
Expand Down Expand Up @@ -857,7 +857,7 @@ func (c *commandHandler) GoGetPackage(ctx context.Context, args command.GoGetPac
}
defer cleanupModDir()

inv, cleanupInvocation, err := snapshot.GoCommandInvocation(true, modURI.DirPath(), "list",
inv, cleanupInvocation, err := snapshot.GoCommandInvocation(cache.NetworkOK, modURI.DirPath(), "list",
[]string{"-f", "{{.Module.Path}}@{{.Module.Version}}", "-mod=mod", "-modfile=" + filepath.Join(tempDir, "go.mod"), args.Pkg},
"GOWORK=off",
)
Expand Down Expand Up @@ -991,7 +991,7 @@ func addModuleRequire(invoke func(...string) (*bytes.Buffer, error), args []stri
// TODO(rfindley): inline.
func (s *server) getUpgrades(ctx context.Context, snapshot *cache.Snapshot, uri protocol.DocumentURI, modules []string) (map[string]string, error) {
args := append([]string{"-mod=readonly", "-m", "-u", "-json"}, modules...)
inv, cleanup, err := snapshot.GoCommandInvocation(true, uri.DirPath(), "list", args)
inv, cleanup, err := snapshot.GoCommandInvocation(cache.NetworkOK, uri.DirPath(), "list", args)
if err != nil {
return nil, err
}
Expand Down

0 comments on commit b1c39aa

Please sign in to comment.