[go] cmd/go: fix relative-path normalization in go.work files

[Bryan Mills (Gerrit)]

Bryan Mills submitted this change.

View Change

Approvals: Michael Matloob: Looks good to me, approved Bryan Mills: Trusted; Run TryBots Gopher Robot: TryBots succeeded

cmd/go: fix relative-path normalization in go.work files

We had been statting 'dir' instead of 'absDir', which statted the
wrong directory if 'dir' was made relative to the location of the
go.work file and that location was different from the current working
directory.

While we're here, I also noticed some other dir-vs.-absDir
discrepancies. The haveDirs map had assumed relative, slash-separated
filesystem paths, but absolute paths on Windows are not
slash-separated and we do retain those. Moreover, if users hand-edit
the go.work file they may introduce non-canonical paths.
I have changed the haveDirs logic to retain absolute paths in their
platform-specific forms, and to call DropUse with the original path
from the go.work file instead of the newly-resolved path.

Fixes #50931
Updates #48257

Change-Id: Ib0a46179aa20c99f045aac5c7c02dbb55da455c8
Reviewed-on: https://go-review.googlesource.com/c/go/+/382240
Trust: Bryan Mills <bcm...@google.com>
Run-TryBot: Bryan Mills <bcm...@google.com>
Reviewed-by: Michael Matloob <mat...@golang.org>
TryBot-Result: Gopher Robot <go...@golang.org>
---
M src/cmd/go/internal/workcmd/use.go
A src/cmd/go/testdata/script/work_use_dot.txt
2 files changed, 90 insertions(+), 15 deletions(-)

diff --git a/src/cmd/go/internal/workcmd/use.go b/src/cmd/go/internal/workcmd/use.go
index 852e5b9..a5ba6c7 100644
--- a/src/cmd/go/internal/workcmd/use.go
+++ b/src/cmd/go/internal/workcmd/use.go
@@ -43,8 +43,6 @@
 }
 
 func runUse(ctx context.Context, cmd *base.Command, args []string) {
-	modload.InitWorkfile()
-
 	modload.ForceUseModules = true
 
 	var gowork string
@@ -56,29 +54,42 @@
 		base.Fatalf("go: %v", err)
 	}
 
-	haveDirs := make(map[string]bool)
-	for _, dir := range workFile.Use {
-		haveDirs[filepath.Join(filepath.Dir(gowork), filepath.FromSlash(dir.Path))] = true
+	haveDirs := make(map[string][]string) // absolute → original(s)
+	for _, use := range workFile.Use {
+		var absDir string
+		if filepath.IsAbs(use.Path) {
+			absDir = filepath.Clean(use.Path)
+		} else {
+			absDir = filepath.Join(filepath.Dir(gowork), use.Path)
+		}
+		haveDirs[absDir] = append(haveDirs[absDir], use.Path)
 	}
 
 	addDirs := make(map[string]bool)
 	removeDirs := make(map[string]bool)
 	lookDir := func(dir string) {
-		absDir := filepath.Join(base.Cwd(), dir)
-		// If the path is absolute, keep it absolute. If it's relative,
+		// If the path is absolute, try to keep it absolute. If it's relative,
 		// make it relative to the go.work file rather than the working directory.
+		absDir := dir
 		if !filepath.IsAbs(dir) {
+			absDir = filepath.Join(base.Cwd(), dir)
 			rel, err := filepath.Rel(filepath.Dir(gowork), absDir)
 			if err == nil {
-				dir = rel
+				// Normalize relative paths to use slashes, so that checked-in go.work
+				// files with relative paths within the repo are platform-independent.
+				dir = filepath.ToSlash(rel)
+			} else {
+				// The path can't be made relative to the go.work file,
+				// so it must be kept absolute instead.
+				dir = absDir
 			}
 		}
-		fi, err := os.Stat(filepath.Join(dir, "go.mod"))
+
+		fi, err := os.Stat(filepath.Join(absDir, "go.mod"))
 		if err != nil {
 			if os.IsNotExist(err) {
-
-				if haveDirs[absDir] {
-					removeDirs[dir] = true
+				for _, origDir := range haveDirs[absDir] {
+					removeDirs[origDir] = true
 				}
 				return
 			}
@@ -89,7 +100,7 @@
 			base.Errorf("go: %v is not regular", filepath.Join(dir, "go.mod"))
 		}
 
-		if !haveDirs[absDir] {
+		if len(haveDirs[absDir]) == 0 {
 			addDirs[dir] = true
 		}
 	}
@@ -109,10 +120,10 @@
 	}
 
 	for dir := range removeDirs {
-		workFile.DropUse(filepath.ToSlash(dir))
+		workFile.DropUse(dir)
 	}
 	for dir := range addDirs {
-		workFile.AddUse(filepath.ToSlash(dir), "")
+		workFile.AddUse(dir, "")
 	}
 	modload.UpdateWorkFile(workFile)
 	modload.WriteWorkFile(gowork, workFile)
diff --git a/src/cmd/go/testdata/script/work_use_dot.txt b/src/cmd/go/testdata/script/work_use_dot.txt
new file mode 100644
index 0000000..c24aae3
--- /dev/null
+++ b/src/cmd/go/testdata/script/work_use_dot.txt
@@ -0,0 +1,33 @@
+cp go.work go.work.orig
+
+# 'go work use .' should add an entry for the current directory.
+cd bar/baz
+go work use .
+cmp ../../go.work ../../go.work.rel
+
+# If the current directory lacks a go.mod file, 'go work use .'
+# should remove its entry.
+mv go.mod go.mod.bak
+go work use .
+cmp ../../go.work ../../go.work.orig
+
+mv go.mod.bak go.mod
+go work use $PWD
+cmpenv ../../go.work ../../go.work.abs
+
+-- go.mod --
+module example
+go 1.18
+-- go.work --
+go 1.18
+-- go.work.rel --
+go 1.18
+
+use bar/baz
+-- go.work.abs --
+go 1.18
+
+use $PWD
+-- bar/baz/go.mod --
+module example/bar/baz
+go 1.18

To view, visit change 382240. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: go

Gerrit-Branch: master

Gerrit-Change-Id: Ib0a46179aa20c99f045aac5c7c02dbb55da455c8

Gerrit-Change-Number: 382240

Gerrit-PatchSet: 5

Gerrit-Owner: Bryan Mills <bcm...@google.com>

Gerrit-Reviewer: Bryan Mills <bcm...@google.com>

Gerrit-Reviewer: Gopher Robot <go...@golang.org>

Gerrit-Reviewer: Michael Matloob <mat...@golang.org>

Gerrit-MessageType: merged