This CL rolls forward CL 630276, fixing the issues with the longtest builders that required the revert in CL 630317.
The change this CL makes compared to CL 630276 is adding the
shortPathErrorList function to rewrite the paths in the modfile.Errors
in a modfile.ErrorList using base.ShortPath and calling it on the error
returned from modfile.Parse.
The following is the commit message from the original change:
This CL first removes the base.ShortPathConservative function. It had
two classes of uses. The first was in opening files where the paths end
up in error messages. In all those cases, the non-shortened paths are
used to open the files, and ShortPath is only used for the error
messages. The second is in base.RelPaths. RelPaths will now call
ShortPath for each of the paths passed in instead of calling
RelConservative and then doing the same check as ShortPath to see if the
path is shorter.
To avoid the possibility of incorrect relative paths ending up in error
messages (that might have command lines suggested for users to run), and
to avoid the possibility of incorrect relative paths appearing in the
output of base.RelPaths, base.ShortPaths always does an os.SameFile
check to make sure that the relative path its providing is actually
correct. Since this makes ShortPath slower than just manipulating paths
(because we need to stat the files), we need to be continue to enforce
that ShortPath is only called for error messages (with the exception of
base.RelPaths and its callers).
This is a simpler way of solving the problem that base.ShortPaths
intended to solve.
For #68383
Change-Id: Ibcefb343535bd82999916b2282e9b512bb683433
Cq-Include-Trybots: luci.golang.try:gotip-windows-amd64-longtest,gotip-linux-amd64-longtest
Reviewed-on: https://go-review.googlesource.com/c/go/+/630280
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Sam Thanawalla <samthanawalla@google.com>
import (
"errors"
+ "io/fs"
"os"
"path/filepath"
"runtime"
"strings"
"sync"
-
- "cmd/go/internal/str"
)
// UncachedCwd returns the current working directory.
}
// ShortPath returns an absolute or relative name for path, whatever is shorter.
-// There are rare cases where the path produced by ShortPath could be incorrect
-// so it should only be used when formatting paths for error messages, not to read
-// a file.
+// ShortPath should only be used when formatting paths for error messages.
func ShortPath(path string) string {
- if rel, err := filepath.Rel(Cwd(), path); err == nil && len(rel) < len(path) {
+ if rel, err := filepath.Rel(Cwd(), path); err == nil && len(rel) < len(path) && sameFile(rel, path) {
return rel
}
return path
}
-// ShortPathConservative is similar to ShortPath, but returns the input if the result of ShortPath
-// would meet conditions that could make it invalid. If the short path would reach into a
-// parent directory and the base path contains a symlink, a ".." component can
-// cross a symlink boundary. That could be a problem because the symlinks could be evaluated,
-// changing the relative location of the boundary, before the ".." terms are applied to
-// go to parents. The check here is a little more conservative: it checks
-// whether the path starts with a ../ or ..\ component, and if any of the parent directories
-// of the working directory are symlinks.
-// See #68383 for a case where this could happen.
-func ShortPathConservative(path string) string {
- if rel, err := relConservative(Cwd(), path); err == nil && len(rel) < len(path) {
- return rel
+func sameFile(path1, path2 string) bool {
+ fi1, err1 := os.Stat(path1)
+ fi2, err2 := os.Stat(path2)
+ if err1 != nil || err2 != nil {
+ // If there were errors statting the files return false,
+ // unless both of the files don't exist.
+ return os.IsNotExist(err1) && os.IsNotExist(err2)
}
- return path
+ return os.SameFile(fi1, fi2)
}
-func relConservative(basepath, targpath string) (string, error) {
- relpath, err := filepath.Rel(basepath, targpath)
- if err != nil {
- return "", err
+// ShortPathError rewrites the path in err using base.ShortPath, if err is a wrapped PathError.
+func ShortPathError(err error) error {
+ var pe *fs.PathError
+ if errors.As(err, &pe) {
+ pe.Path = ShortPath(pe.Path)
}
- if strings.HasPrefix(relpath, str.WithFilePathSeparator("..")) {
- expanded, err := filepath.EvalSymlinks(basepath)
- if err != nil || expanded != basepath { // The basepath contains a symlink. Be conservative and reject it.
- return "", errors.New("conservatively rejecting relative path that may be invalid")
- }
- }
- return relpath, nil
+ return err
}
// RelPaths returns a copy of paths with absolute paths
func RelPaths(paths []string) []string {
out := make([]string, 0, len(paths))
for _, p := range paths {
- rel, err := relConservative(Cwd(), p)
- if err == nil && len(rel) < len(p) {
- p = rel
- }
- out = append(out, p)
+ out = append(out, ShortPath(p))
}
return out
}
err = errWorkTooOld(gomod, workFile, tooNew.GoVersion)
} else {
err = fmt.Errorf("cannot load module %s listed in go.work file: %w",
- base.ShortPath(filepath.Dir(gomod)), err)
+ base.ShortPath(filepath.Dir(gomod)), base.ShortPathError(err))
}
}
errs = append(errs, err)
}
if inWorkspaceMode() {
if mr := findModuleRoot(absDir); mr != "" {
- return "", fmt.Errorf("%s is contained in a module that is not one of the workspace modules listed in go.work. You can add the module to the workspace using:\n\tgo work use %s", dirstr, base.ShortPathConservative(mr))
+ return "", fmt.Errorf("%s is contained in a module that is not one of the workspace modules listed in go.work. You can add the module to the workspace using:\n\tgo work use %s", dirstr, base.ShortPath(mr))
}
return "", fmt.Errorf("%s outside modules listed in go.work or their selected dependencies", dirstr)
}
// ReadModFile reads and parses the mod file at gomod. ReadModFile properly applies the
// overlay, locks the file while reading, and applies fix, if applicable.
func ReadModFile(gomod string, fix modfile.VersionFixer) (data []byte, f *modfile.File, err error) {
- // The path used to open the file shows up in errors. Use ShortPathConservative
- // so a more convenient path is displayed in the errors. ShortPath isn't used
- // because it's meant only to be used in errors, not to open files.
- gomod = base.ShortPathConservative(gomod)
if fsys.Replaced(gomod) {
// Don't lock go.mod if it's part of the overlay.
// On Plan 9, locking requires chmod, and we don't want to modify any file
f, err = modfile.Parse(gomod, data, fix)
if err != nil {
// Errors returned by modfile.Parse begin with file:line.
- return nil, nil, fmt.Errorf("errors parsing %s:\n%w", gomod, err)
+ return nil, nil, fmt.Errorf("errors parsing %s:\n%w", base.ShortPath(gomod), shortPathErrorList(err))
}
if f.Go != nil && gover.Compare(f.Go.Version, gover.Local()) > 0 {
toolchain := ""
if f.Toolchain != nil {
toolchain = f.Toolchain.Name
}
- return nil, nil, &gover.TooNewError{What: gomod, GoVersion: f.Go.Version, Toolchain: toolchain}
+ return nil, nil, &gover.TooNewError{What: base.ShortPath(gomod), GoVersion: f.Go.Version, Toolchain: toolchain}
}
if f.Module == nil {
// No module declaration. Must add module path.
- return nil, nil, fmt.Errorf("error reading %s: missing module declaration. To specify the module path:\n\tgo mod edit -module=example.com/mod", gomod)
+ return nil, nil, fmt.Errorf("error reading %s: missing module declaration. To specify the module path:\n\tgo mod edit -module=example.com/mod", base.ShortPath(gomod))
}
return data, f, err
}
+func shortPathErrorList(err error) error {
+ var el modfile.ErrorList
+ if errors.As(err, &el) {
+ for i := range el {
+ el[i].Filename = base.ShortPath(el[i].Filename)
+ }
+ }
+ return err
+}
+
// A modFileIndex is an index of data corresponding to a modFile
// at a specific point in time.
type modFileIndex struct {
lookDir := func(dir string) {
absDir, dir := pathRel(workDir, dir)
- file := base.ShortPathConservative(filepath.Join(absDir, "go.mod"))
+ file := filepath.Join(absDir, "go.mod")
fi, err := fsys.Stat(file)
if err != nil {
if os.IsNotExist(err) {
}
if !fi.Mode().IsRegular() {
- sw.Error(fmt.Errorf("%v is not a regular file", file))
+ sw.Error(fmt.Errorf("%v is not a regular file", base.ShortPath(file)))
return
}
for _, useDir := range args {
absArg, _ := pathRel(workDir, useDir)
- useDirShort := base.ShortPathConservative(absArg) // relative to the working directory rather than the workspace
- info, err := fsys.Stat(useDirShort)
+ info, err := fsys.Stat(absArg)
if err != nil {
// Errors raised from os.Stat are formatted to be more user-friendly.
if os.IsNotExist(err) {
- err = fmt.Errorf("directory %v does not exist", useDirShort)
+ err = fmt.Errorf("directory %v does not exist", base.ShortPath(absArg))
}
sw.Error(err)
continue
} else if !info.IsDir() {
- sw.Error(fmt.Errorf("%s is not a directory", useDirShort))
+ sw.Error(fmt.Errorf("%s is not a directory", base.ShortPath(absArg)))
continue
}
if !d.IsDir() {
if d.Type()&fs.ModeSymlink != 0 {
if target, err := fsys.Stat(path); err == nil && target.IsDir() {
- fmt.Fprintf(os.Stderr, "warning: ignoring symlink %s\n", base.ShortPathConservative(path))
+ fmt.Fprintf(os.Stderr, "warning: ignoring symlink %s\n", base.ShortPath(path))
}
}
return nil
} else {
abs = filepath.Join(workDir, use.Path)
}
- _, mf, err := modload.ReadModFile(base.ShortPathConservative(filepath.Join(abs, "go.mod")), nil)
+ _, mf, err := modload.ReadModFile(filepath.Join(abs, "go.mod"), nil)
if err != nil {
sw.Error(err)
continue