qbit/go
1
0
Fork 0
mirror of https://github.com/golang/go synced 2024-11-18 17:04:41 -07:00
Code Releases Activity

go/packages: set -mod=readonly when processing overlays in module mode

Browse Source 
To prevent us from adding additional module dependencies to modules, especially
if the file is in a different module. Sometimes adding additional dependencies
would be the right behavior, but sometimes we run go list to determine
information about files in other modules, and modifying modules outside the one
the user is operating in is bad  behavior.

Fixes golang/go#32499

Change-Id: I2a12e0a64dc6cd34fa98931cbacc30707e5ba494
Reviewed-on: https://go-review.googlesource.com/c/tools/+/190179
Run-TryBot: Michael Matloob <matloob@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This commit is contained in:
Michael Matloob 2019-08-13 17:33:18 -04:00
parent 597f577694
commit 5b18234b3a
3 changed files with 54 additions and 23 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

68
go/packages/golist.go
View File

@ -72,6 +72,28 @@ func (r *responseDeduper) addRoot(id string) {
r.dr.Roots = append(r.dr.Roots, id)
}
// goInfo contains global information from the go tool.
type goInfo struct {
rootDirs map[string]string
env goEnv
}
type goEnv struct {
modulesOn bool
}
func determineEnv(cfg *Config) goEnv {
buf, err := invokeGo(cfg, "env", "GOMOD")
if err != nil {
return goEnv{}
}
gomod := bytes.TrimSpace(buf.Bytes())
env := goEnv{}
env.modulesOn = len(gomod) > 0
return env
}
// goListDriver uses the go list command to interpret the patterns and produce
// the build system package structure.
// See driver for more details.
@ -88,20 +110,25 @@ func goListDriver(cfg *Config, patterns ...string) (*driverResponse, error) {
}
// start fetching rootDirs
var rootDirs map[string]string
var rootDirsReady = make(chan struct{})
var info goInfo
var rootDirsReady, envReady = make(chan struct{}), make(chan struct{})
go func() {
rootDirs = determineRootDirs(cfg)
info.rootDirs = determineRootDirs(cfg)
close(rootDirsReady)
}()
getRootDirs := func() map[string]string {
go func() {
info.env = determineEnv(cfg)
close(envReady)
}()
getGoInfo := func() *goInfo {
<-rootDirsReady
return rootDirs
<-envReady
return &info
}
// always pass getRootDirs to golistDriver
// always pass getGoInfo to golistDriver
golistDriver := func(cfg *Config, patterns ...string) (*driverResponse, error) {
return golistDriver(cfg, getRootDirs, patterns...)
return golistDriver(cfg, getGoInfo, patterns...)
}
// Determine files requested in contains patterns
@ -165,7 +192,7 @@ extractQueries:
var containsCandidates []string
if len(containFiles) != 0 {
if err := runContainsQueries(cfg, golistDriver, response, containFiles, getRootDirs); err != nil {
if err := runContainsQueries(cfg, golistDriver, response, containFiles, getGoInfo); err != nil {
return nil, err
}
}
@ -176,7 +203,7 @@ extractQueries:
}
}
modifiedPkgs, needPkgs, err := processGolistOverlay(cfg, response, getRootDirs)
modifiedPkgs, needPkgs, err := processGolistOverlay(cfg, response, getGoInfo)
if err != nil {
return nil, err
}
@ -184,7 +211,7 @@ extractQueries:
containsCandidates = append(containsCandidates, modifiedPkgs...)
containsCandidates = append(containsCandidates, needPkgs...)
}
if err := addNeededOverlayPackages(cfg, golistDriver, response, needPkgs, getRootDirs); err != nil {
if err := addNeededOverlayPackages(cfg, golistDriver, response, needPkgs, getGoInfo); err != nil {
return nil, err
}
// Check candidate packages for containFiles.
@ -216,28 +243,33 @@ extractQueries:
return response.dr, nil
}
func addNeededOverlayPackages(cfg *Config, driver driver, response *responseDeduper, pkgs []string, getRootDirs func() map[string]string) error {
func addNeededOverlayPackages(cfg *Config, driver driver, response *responseDeduper, pkgs []string, getGoInfo func() *goInfo) error {
if len(pkgs) == 0 {
return nil
}
dr, err := driver(cfg, pkgs...)
drivercfg := *cfg
if getGoInfo().env.modulesOn {
drivercfg.BuildFlags = append(drivercfg.BuildFlags, "-mod=readonly")
}
dr, err := driver(&drivercfg, pkgs...)
if err != nil {
return err
}
for _, pkg := range dr.Packages {
response.addPackage(pkg)
}
_, needPkgs, err := processGolistOverlay(cfg, response, getRootDirs)
_, needPkgs, err := processGolistOverlay(cfg, response, getGoInfo)
if err != nil {
return err
}
if err := addNeededOverlayPackages(cfg, driver, response, needPkgs, getRootDirs); err != nil {
if err := addNeededOverlayPackages(cfg, driver, response, needPkgs, getGoInfo); err != nil {
return err
}
return nil
}
func runContainsQueries(cfg *Config, driver driver, response *responseDeduper, queries []string, rootDirs func() map[string]string) error {
func runContainsQueries(cfg *Config, driver driver, response *responseDeduper, queries []string, goInfo func() *goInfo) error {
for _, query := range queries {
// TODO(matloob): Do only one query per directory.
fdir := filepath.Dir(query)
@ -600,7 +632,7 @@ func otherFiles(p *jsonPackage) [][]string {
// golistDriver uses the "go list" command to expand the pattern
// words and return metadata for the specified packages. dir may be
// "" and env may be nil, as per os/exec.Command.
func golistDriver(cfg *Config, rootsDirs func() map[string]string, words ...string) (*driverResponse, error) {
func golistDriver(cfg *Config, rootsDirs func() *goInfo, words ...string) (*driverResponse, error) {
// go list uses the following identifiers in ImportPath and Imports:
//
// "p" -- importable package or main (command)
@ -759,8 +791,8 @@ func golistDriver(cfg *Config, rootsDirs func() map[string]string, words ...stri
}
// getPkgPath finds the package path of a directory if it's relative to a root directory.
func getPkgPath(dir string, rootDirs func() map[string]string) (string, bool) {
for rdir, rpath := range rootDirs() {
func getPkgPath(dir string, goInfo func() *goInfo) (string, bool) {
for rdir, rpath := range goInfo().rootDirs {
// TODO(matloob): This doesn't properly handle symlinks.
r, err := filepath.Rel(rdir, dir)
if err != nil {

4
go/packages/golist_overlay.go
View File

@ -17,7 +17,7 @@ import (
// sometimes incorrect.
// TODO(matloob): Handle unsupported cases, including the following:
// - determining the correct package to add given a new import path
func processGolistOverlay(cfg *Config, response *responseDeduper, rootDirs func() map[string]string) (modifiedPkgs, needPkgs []string, err error) {
func processGolistOverlay(cfg *Config, response *responseDeduper, rootDirs func() *goInfo) (modifiedPkgs, needPkgs []string, err error) {
havePkgs := make(map[string]string) // importPath -> non-test package ID
needPkgsSet := make(map[string]bool)
modifiedPkgsSet := make(map[string]bool)
@ -75,7 +75,7 @@ func processGolistOverlay(cfg *Config, response *responseDeduper, rootDirs func(
// Try to find the module or gopath dir the file is contained in.
// Then for modules, add the module opath to the beginning.
var pkgPath string
for rdir, rpath := range rootDirs() {
for rdir, rpath := range rootDirs().rootDirs {
// TODO(matloob): This doesn't properly handle symlinks.
r, err := filepath.Rel(rdir, dir)
if err != nil {

5
go/packages/packages_test.go
View File

@ -1053,9 +1053,6 @@ const A = 1
// TestOverlayModFileChanges tests the behavior resulting from having files from
// multiple modules in overlays.
func TestOverlayModFileChanges(t *testing.T) {
// TODO: Enable this test when golang/go#32499 is resolved.
t.Skip()
// Create two unrelated modules in a temporary directory.
tmp, err := ioutil.TempDir("", "tmp")
if err != nil {
@ -1086,6 +1083,8 @@ func TestOverlayModFileChanges(t *testing.T) {
defer os.Remove(mod2)
want := `module mod2
go 1.11
`
if err := ioutil.WriteFile(filepath.Join(mod2, "go.mod"), []byte(want), 0775); err != nil {
t.Fatal(err)