cmd/go: remove deleted subdirectories in 'go work use'

Also remove absolute names (relative to PWD) when updating relative
directories, and relative names when updating absolute directories.

Fixes #50959

Change-Id: If129019cad7146e82face7f23427b28240d29cfc
Reviewed-on: https://go-review.googlesource.com/c/go/+/383837
Trust: Bryan Mills <bcmills@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
Reviewed-by: Michael Matloob <matloob@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>

src/cmd/go/internal/modload/init.go

@@ -301,7 +301,7 @@ func InitWorkfile() {
 	}
 }
 
-// WorkFilePath returns the path of the go.work file, or "" if not in
+// WorkFilePath returns the absolute path of the go.work file, or "" if not in
 // workspace mode. WorkFilePath must be called after InitWorkfile.
 func WorkFilePath() string {
 	return workFilePath

src/cmd/go/internal/workcmd/use.go

@@ -10,7 +10,10 @@ import (
 	"cmd/go/internal/base"
 	"cmd/go/internal/fsys"
 	"cmd/go/internal/modload"
+	"cmd/go/internal/str"
 	"context"
+	"errors"
+	"fmt"
 	"io/fs"
 	"os"
 	"path/filepath"
@@ -56,44 +59,34 @@ func runUse(ctx context.Context, cmd *base.Command, args []string) {
 	if err != nil {
 		base.Fatalf("go: %v", err)
 	}
+	workDir := filepath.Dir(gowork) // Absolute, since gowork itself is absolute.
 
 	haveDirs := make(map[string][]string) // absolute → original(s)
 	for _, use := range workFile.Use {
-		var absDir string
+		var abs string
 		if filepath.IsAbs(use.Path) {
-			absDir = filepath.Clean(use.Path)
+			abs = filepath.Clean(use.Path)
 		} else {
-			absDir = filepath.Join(filepath.Dir(gowork), use.Path)
+			abs = filepath.Join(workDir, use.Path)
 		}
-		haveDirs[absDir] = append(haveDirs[absDir], use.Path)
+		haveDirs[abs] = append(haveDirs[abs], use.Path)
 	}
 
-	addDirs := make(map[string]bool)
+	// keepDirs maps each absolute path to keep to the literal string to use for
-	removeDirs := make(map[string]bool)
+	// that path (either an absolute or a relative path), or the empty string if
+	// all entries for the absolute path should be removed.
+	keepDirs := make(map[string]string)
+
+	// lookDir updates the entry in keepDirs for the directory dir,
+	// which is either absolute or relative to the current working directory
+	// (not necessarily the directory containing the workfile).
 	lookDir := func(dir string) {
-		// If the path is absolute, try to keep it absolute. If it's relative,
+		absDir, dir := pathRel(workDir, dir)
-		// 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 {
-				// 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(absDir, "go.mod"))
 		if err != nil {
 			if os.IsNotExist(err) {
-				for _, origDir := range haveDirs[absDir] {
+				keepDirs[absDir] = ""
-					removeDirs[origDir] = true
-				}
 				return
 			}
 			base.Errorf("go: %v", err)
@@ -103,31 +96,96 @@ func runUse(ctx context.Context, cmd *base.Command, args []string) {
 			base.Errorf("go: %v is not regular", filepath.Join(dir, "go.mod"))
 		}
 
-		if len(haveDirs[absDir]) == 0 {
+		if dup := keepDirs[absDir]; dup != "" && dup != dir {
-			addDirs[dir] = true
+			base.Errorf(`go: already added "%s" as "%s"`, dir, dup)
 		}
+		keepDirs[absDir] = dir
 	}
 
 	for _, useDir := range args {
-		if *useR {
+		if !*useR {
-			fsys.Walk(useDir, func(path string, info fs.FileInfo, err error) error {
+			lookDir(useDir)
-				if !info.IsDir() {
-					return nil
-				}
-				lookDir(path)
-				return nil
-			})
 			continue
 		}
-		lookDir(useDir)
+
+		// Add or remove entries for any subdirectories that still exist.
+		err := fsys.Walk(useDir, func(path string, info fs.FileInfo, err error) error {
+			if !info.IsDir() {
+				if info.Mode()&fs.ModeSymlink != 0 {
+					if target, err := fsys.Stat(path); err == nil && target.IsDir() {
+						fmt.Fprintf(os.Stderr, "warning: ignoring symlink %s\n", path)
+					}
+				}
+				return nil
+			}
+			lookDir(path)
+			return nil
+		})
+		if err != nil && !errors.Is(err, os.ErrNotExist) {
+			base.Errorf("go: %v", err)
+		}
+
+		// Remove entries for subdirectories that no longer exist.
+		// Because they don't exist, they will be skipped by Walk.
+		absArg, _ := pathRel(workDir, useDir)
+		for absDir, _ := range haveDirs {
+			if str.HasFilePathPrefix(absDir, absArg) {
+				if _, ok := keepDirs[absDir]; !ok {
+					keepDirs[absDir] = "" // Mark for deletion.
+				}
+			}
+		}
 	}
 
-	for dir := range removeDirs {
+	base.ExitIfErrors()
-		workFile.DropUse(dir)
+
-	}
+	for absDir, keepDir := range keepDirs {
-	for dir := range addDirs {
+		nKept := 0
-		workFile.AddUse(dir, "")
+		for _, dir := range haveDirs[absDir] {
+			if dir == keepDir { // (note that dir is always non-empty)
+				nKept++
+			} else {
+				workFile.DropUse(dir)
+			}
+		}
+		if keepDir != "" && nKept != 1 {
+			// If we kept more than one copy, delete them all.
+			// We'll recreate a unique copy with AddUse.
+			if nKept > 1 {
+				workFile.DropUse(keepDir)
+			}
+			workFile.AddUse(keepDir, "")
+		}
 	}
 	modload.UpdateWorkFile(workFile)
 	modload.WriteWorkFile(gowork, workFile)
 }
+
+// pathRel returns the absolute and canonical forms of dir for use in a
+// go.work file located in directory workDir.
+//
+// If dir is relative, it is intepreted relative to base.Cwd()
+// and its canonical form is relative to workDir if possible.
+// If dir is absolute or cannot be made relative to workDir,
+// its canonical form is absolute.
+//
+// Canonical absolute paths are clean.
+// Canonical relative paths are clean and slash-separated.
+func pathRel(workDir, dir string) (abs, canonical string) {
+	if filepath.IsAbs(dir) {
+		abs = filepath.Clean(dir)
+		return abs, abs
+	}
+
+	abs = filepath.Join(base.Cwd(), dir)
+	rel, err := filepath.Rel(workDir, abs)
+	if err != nil {
+		// The path can't be made relative to the go.work file,
+		// so it must be kept absolute instead.
+		return abs, abs
+	}
+
+	// Normalize relative paths to use slashes, so that checked-in go.work
+	// files with relative paths within the repo are platform-independent.
+	return abs, filepath.ToSlash(rel)
+}

src/cmd/go/testdata/script/work_use_deleted.txt

@@ -0,0 +1,22 @@
+go work use -r .
+cmp go.work go.work.want
+
+-- go.work --
+go 1.18
+
+use (
+	.
+	sub
+	sub/dir/deleted
+)
+-- go.work.want --
+go 1.18
+
+use sub/dir
+-- sub/README.txt --
+A go.mod file has been deleted from this directory.
+In addition, the entire subdirectory sub/dir/deleted
+has been deleted, along with sub/dir/deleted/go.mod.
+-- sub/dir/go.mod --
+module example/sub/dir
+go 1.18

src/cmd/go/testdata/script/work_use_dot.txt

@@ -1,6 +1,7 @@
 cp go.work go.work.orig
 
-# 'go work use .' should add an entry for the current directory.
+# If the current directory contains a go.mod file,
+# 'go work use .' should add an entry for it.
 cd bar/baz
 go work use .
 cmp ../../go.work ../../go.work.rel
@@ -11,9 +12,28 @@ mv go.mod go.mod.bak
 go work use .
 cmp ../../go.work ../../go.work.orig
 
+# If the path is absolute, it should remain absolute.
 mv go.mod.bak go.mod
 go work use $PWD
-cmpenv ../../go.work ../../go.work.abs
+grep -count=1 '^use ' ../../go.work
+grep '^use ["]?'$PWD'["]?$' ../../go.work
+
+# An absolute path should replace an entry for the corresponding relative path
+# and vice-versa.
+go work use .
+cmp ../../go.work ../../go.work.rel
+go work use $PWD
+grep -count=1 '^use ' ../../go.work
+grep '^use ["]?'$PWD'["]?$' ../../go.work
+
+# If both the absolute and relative paths are named, 'go work use' should error
+# out: we don't know which one to use, and shouldn't add both because the
+# resulting workspace would contain a duplicate module.
+cp ../../go.work.orig ../../go.work
+! go work use $PWD .
+stderr '^go: already added "bar/baz" as "'$PWD'"$'
+cmp ../../go.work ../../go.work.orig
+
 
 -- go.mod --
 module example
@@ -24,10 +44,6 @@ go 1.18
 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