diff --git a/v23/verror/pkgpath.go b/v23/verror/pkgpath.go index 59e6c6968..f67b1031c 100644 --- a/v23/verror/pkgpath.go +++ b/v23/verror/pkgpath.go @@ -5,16 +5,12 @@ package verror import ( - "fmt" - "io/ioutil" - "os" "path" "path/filepath" + "reflect" "runtime" "strings" "sync" - - "golang.org/x/mod/modfile" ) type pathCache struct { @@ -22,20 +18,6 @@ type pathCache struct { paths map[string]string } -func enclosingGoMod(dir string) (string, error) { - for { - gomodfile := filepath.Join(dir, "go.mod") - if fi, err := os.Stat(gomodfile); err == nil && !fi.IsDir() { - return dir, nil - } - d := filepath.Dir(dir) - if d == dir { - return "", fmt.Errorf("failed to find enclosing go.mod for dir %v", dir) - } - dir = d - } -} - var pkgPathCache = pathCache{ paths: make(map[string]string), } @@ -53,40 +35,94 @@ func (pc *pathCache) set(dir, pkg string) { pc.paths[dir] = pkg } -func (pc *pathCache) pkgPath(file string) (string, error) { - dir := filepath.Clean(filepath.Dir(file)) - if p, ok := pc.has(dir); ok { - return p, nil - } - root, err := enclosingGoMod(dir) - if err != nil { - return "", err - } - gomodfile := filepath.Join(root, "go.mod") - gomod, err := ioutil.ReadFile(gomodfile) - if err != nil { - return "", err - } - module := modfile.ModulePath(gomod) - if len(module) == 0 { - return "", fmt.Errorf("failed to read module path from %v", gomodfile) +// IDPath returns a string of the form . +// where is derived from the type of the supplied +// value. Typical usage would be except that dummy can be replaced +// by an existing type defined in the package. +// +// type dummy int +// verror.ID(verror.IDPath(dummy(0), "MyError")) +// +func IDPath(val interface{}, id string) ID { + return ID(reflect.TypeOf(val).PkgPath() + "." + id) +} + +// longestCommonSuffix for a package path and filename. +func longestCommonSuffix(pkgPath, filename string) (string, string) { + longestPkg, longestFilePath := "", "" + for { + fl := filepath.Base(filename) + pl := path.Base(pkgPath) + if fl == pl { + longestPkg = path.Join(fl, longestPkg) + longestFilePath = filepath.Join(fl, longestFilePath) + filename = filepath.Dir(filename) + pkgPath = path.Dir(pkgPath) + if fl == "/" { + break + } + continue + } + break } + return longestPkg, longestFilePath +} + +type pathState struct { + pkg string // pkg path for the value passed to init + dir string // the directory component for the file passed to init + // The portion of the local file path that is outside of the go module, + // e.g. for /a/b/c/core/v23/verror it would be /a/b/c/core. + filePrefix string + // the portion of the package path that does not appear in the file name, + // e.g. for /a/b/c/core/v23/verror and v.io/v23/verror it would be v.io. + pkgPrefix string +} + +func (ps *pathState) init(pkgPath string, file string) { + ps.pkg = pkgPath + ps.dir = filepath.Dir(file) + pkgLCS, fileLCS := longestCommonSuffix(ps.pkg, ps.dir) + ps.filePrefix = filepath.Clean(strings.TrimSuffix(ps.dir, fileLCS)) + ps.pkgPrefix = path.Clean(strings.TrimSuffix(ps.pkg, pkgLCS)) +} + +var ( + ps = &pathState{} + initOnce sync.Once +) - pkgPath := strings.TrimPrefix(dir, root) - if !strings.HasPrefix(pkgPath, module) { - pkgPath = path.Join(module, pkgPath) +func convertFileToPkgName(filename string) string { + return path.Clean(strings.ReplaceAll(filename, string(filepath.Separator), "/")) +} + +func (pc *pathCache) pkgPath(file string) string { + initOnce.Do(func() { + type dummy int + _, file, _, _ := runtime.Caller(0) + ps.init(reflect.TypeOf(dummy(0)).PkgPath(), file) + }) + pdir := filepath.Dir(file) + rel := strings.TrimPrefix(pdir, ps.filePrefix) + if rel == pdir { + return "" } - pc.set(dir, pkgPath) - return pkgPath, nil + relPkg := convertFileToPkgName(rel) + pkgPath := path.Join(ps.pkgPrefix, relPkg) + pc.set(filepath.Dir(file), pkgPath) + return pkgPath } func ensurePackagePath(id ID) ID { + sid := string(id) + if strings.Contains(sid, ".") && sid[0] != '.' { + return id + } _, file, _, _ := runtime.Caller(2) - pkg, err := pkgPathCache.pkgPath(file) - if err != nil { - panic(fmt.Sprintf("failed to determine package name for %v: %v", file, err)) + pkg := pkgPathCache.pkgPath(file) + if len(pkg) == 0 { + return id } - sid := string(id) if strings.HasPrefix(sid, pkg) { return id } diff --git a/v23/verror/pkgpath_test.go b/v23/verror/pkgpath_test.go new file mode 100644 index 000000000..3c571685a --- /dev/null +++ b/v23/verror/pkgpath_test.go @@ -0,0 +1,51 @@ +package verror + +import ( + "path/filepath" + "reflect" + "runtime" + "strings" + "testing" +) + +func TestLCS(t *testing.T) { + for i, tc := range []struct { + pkg, path string + lcsPkg, lcsPath string + }{ + {"a", "b", "", ""}, + {"/a/b/c", "/a/b/c", "/a/b/c", "/a/b/c"}, + {"/a/b/c/d", "/x/y/c/d", "c/d", "c/d"}, + } { + lcsPkg, lcsPath := longestCommonSuffix(tc.pkg, tc.path) + if got, want := lcsPkg, tc.lcsPkg; got != want { + t.Errorf("%v: got %v, want %v", i, got, want) + } + if got, want := lcsPath, tc.lcsPath; got != want { + t.Errorf("%v: got %v, want %v", i, got, want) + } + } +} + +func TestPkgPath(t *testing.T) { + type dummy int + ps := &pathState{} + _, file, _, _ := runtime.Caller(0) + ps.init(reflect.TypeOf(dummy(0)).PkgPath(), file) + if got, want := ps.pkg, "v.io/v23/verror"; got != want { + t.Errorf("got %v, want %v", got, want) + } + if got, want := ps.dir, filepath.Dir(file); got != want { + t.Errorf("got %v, want %v", got, want) + } + if got, want := strings.HasPrefix(file, ps.filePrefix), true; got != want { + t.Errorf("got %v, want %v", got, want) + } + if got, want := ps.pkgPrefix, "v.io"; got != want { + t.Errorf("got %v, want %v", got, want) + } + ps.init("github.com/grailbio/base", "/a/b/src/base/file") + if got, want := ps.pkgPrefix, "github.com/grailbio"; got != want { + t.Errorf("got %v, want %v", got, want) + } +} diff --git a/v23/verror/verror.go b/v23/verror/verror.go index b70a43aea..0c4e16d76 100644 --- a/v23/verror/verror.go +++ b/v23/verror/verror.go @@ -178,9 +178,8 @@ type IDAction struct { // Register returns a IDAction with the given ID and Action fields, and // inserts a message into the default i18n Catalogue in US English. -// Other languages can be added by adding to the Catalogue. Register -// will internally prepend the caller's package path to id if it's not -// already present. +// Other languages can be added by adding to the Catalogue. +// IDPath can be used to generate an appropriate ID. func Register(id ID, action ActionCode, englishText string) IDAction { id = ensurePackagePath(id) i18n.Cat().SetWithBase(defaultLangID(i18n.NoLangID), i18n.MsgID(id), englishText) @@ -189,8 +188,7 @@ func Register(id ID, action ActionCode, englishText string) IDAction { // NewIDAction creates a new instance of IDAction with the given ID and Action // field. It should be used when localization support is not required instead -// of Register. NewIDAction will internally prepend the caller's package path -// to id if it's not already present. +// of Register. IDPath can be used to func NewIDAction(id ID, action ActionCode) IDAction { return IDAction{ensurePackagePath(id), action} } diff --git a/v23/verror/verror_id_test.go b/v23/verror/verror_id_test.go new file mode 100644 index 000000000..3e0f94f31 --- /dev/null +++ b/v23/verror/verror_id_test.go @@ -0,0 +1,24 @@ +package verror_test + +import ( + "testing" + + "v.io/v23/flow" + "v.io/v23/verror" + "v.io/x/ref/lib/security" +) + +func TestPackageErrorIDs(t *testing.T) { + for i, tc := range []struct { + err error + id string + }{ + {verror.ErrInternal.Errorf(nil, "x"), "v.io/v23/verror.Internal"}, + {flow.ErrAuth.Errorf(nil, "x"), "v.io/v23/flow.Auth"}, + {security.ErrBadPassphrase.Errorf(nil, "x"), "v.io/x/ref/lib/security.errBadPassphrase"}, + } { + if got, want := verror.ErrorID(tc.err), verror.ID(tc.id); got != want { + t.Errorf("%v: got %v, want %v", i, got, want) + } + } +} diff --git a/x/ref/runtime/internal/flow/util_test.go b/x/ref/runtime/internal/flow/util_test.go index 10b1217ed..75244d92f 100644 --- a/x/ref/runtime/internal/flow/util_test.go +++ b/x/ref/runtime/internal/flow/util_test.go @@ -24,7 +24,7 @@ func TestMaybeWrapError(t *testing.T) { {verror.ErrUnknown.Errorf(ctx, "some random thing"), false}, {flow.ErrAuth.Errorf(ctx, ""), false}, } - for _, test := range tests { + for i, test := range tests { werr := MaybeWrapError(flow.ErrAuth, ctx, test.err) // If the returned error is not equal to the original error it was wrapped. msg := "" @@ -33,9 +33,9 @@ func TestMaybeWrapError(t *testing.T) { } if wasWrapped := werr.Error() != msg; wasWrapped != test.wrap { if test.wrap { - t.Errorf("wanted %v to be wrapped", test.err) + t.Errorf("%v: wanted %v to be wrapped", i, test.err) } else { - t.Errorf("did not want %v to be wrapped", test.err) + t.Errorf("%v: did not want %v to be wrapped", i, test.err) } } }