fix: use path instead of filepath

secDre4mer · web-flow · commit 938db1c6c209 · 2025-05-21T21:22:05.000+02:00
Currently, yaegi uses the `filepath` package to join paths.

However, this has issues: YAEGI uses an `fs.FS` internally for source code access, and `fs` specifies that `/` separated paths must be used. On Unix systems, this obviously makes no difference; but on Windows, with a non-default FS, this causes errors (since YAEGI creates `\` separated paths, but the FS expects `/` separated paths).

Furthermore, generally speaking, Golang import paths and the like are always `/` separated anyway, regardless of OS.

To clear this up, this PR changes path handling to `/` separated paths. This has the advantage of being accepted by all OS (even Windows will accept `/` separated paths). Paths passed by the user to `CompilePath` or similar are changed to `/` separators, if necessary.
diff --git a/_test/issue-1280.go b/_test/issue-1280.go
@@ -16,7 +16,7 @@ type DB struct {
 }
 
 func main() {
-	f, err := os.Open("/dev/null")
+	f, err := os.Open(os.DevNull)
 	if err != nil {
 		log.Fatal(err)
 	}
diff --git a/cmd/yaegi/yaegi_test.go b/cmd/yaegi/yaegi_test.go
@@ -37,6 +37,9 @@ func applyCIMultiplier(timeout time.Duration) time.Duration {
 }
 
 func TestYaegiCmdCancel(t *testing.T) {
+	if runtime.GOOS == "windows" {
+		t.Skip("skipping cancel test since windows has no os.Interrupt signal")
+	}
 	tmp := t.TempDir()
 	yaegi := filepath.Join(tmp, "yaegi")
 
diff --git a/example/pkg/pkg_test.go b/example/pkg/pkg_test.go
@@ -164,7 +164,7 @@ func TestPackagesError(t *testing.T) {
 		{
 			desc:     "different packages in the same directory",
 			goPath:   "./_pkg9/",
-			expected: `1:21: import "github.com/foo/pkg" error: found packages pkg and pkgfalse in ` + filepath.FromSlash("_pkg9/src/github.com/foo/pkg"),
+			expected: `1:21: import "github.com/foo/pkg" error: found packages pkg and pkgfalse in _pkg9/src/github.com/foo/pkg`,
 		},
 	}
 
diff --git a/interp/build.go b/interp/build.go
@@ -5,7 +5,6 @@ import (
 	"go/build"
 	"go/parser"
 	"path"
-	"path/filepath"
 	"strconv"
 	"strings"
 )
@@ -135,7 +134,7 @@ func skipFile(ctx *build.Context, p string, skipTest bool) bool {
 		return true
 	}
 	p = strings.TrimSuffix(path.Base(p), ".go")
-	if pp := filepath.Base(p); strings.HasPrefix(pp, "_") || strings.HasPrefix(pp, ".") {
+	if pp := path.Base(p); strings.HasPrefix(pp, "_") || strings.HasPrefix(pp, ".") {
 		return true
 	}
 	if skipTest && strings.HasSuffix(p, "_test") {
diff --git a/interp/cfg.go b/interp/cfg.go
@@ -5,7 +5,7 @@ import (
 	"go/constant"
 	"log"
 	"math"
-	"path/filepath"
+	"path"
 	"reflect"
 	"strings"
 	"unicode"
@@ -64,7 +64,7 @@ func (interp *Interpreter) cfg(root *node, sc *scope, importPath, pkgName string
 	var initNodes []*node
 	var err error
 
-	baseName := filepath.Base(interp.fset.Position(root.pos).Filename)
+	baseName := path.Base(interp.fset.Position(root.pos).Filename)
 
 	root.Walk(func(n *node) bool {
 		// Pre-order processing
@@ -1643,7 +1643,7 @@ func (interp *Interpreter) cfg(root *node, sc *scope, importPath, pkgName string
 					break
 				}
 				// retry with the filename, in case ident is a package name.
-				sym, level, found = sc.lookup(filepath.Join(n.ident, baseName))
+				sym, level, found = sc.lookup(path.Join(n.ident, baseName))
 				if !found {
 					err = n.cfgErrorf("undefined: %s", n.ident)
 					break
@@ -2612,8 +2612,8 @@ func (n *node) isType(sc *scope) bool {
 		}
 	case selectorExpr:
 		pkg, name := n.child[0].ident, n.child[1].ident
-		baseName := filepath.Base(n.interp.fset.Position(n.pos).Filename)
-		suffixedPkg := filepath.Join(pkg, baseName)
+		baseName := path.Base(n.interp.fset.Position(n.pos).Filename)
+		suffixedPkg := path.Join(pkg, baseName)
 		sym, _, ok := sc.lookup(suffixedPkg)
 		if !ok {
 			sym, _, ok = sc.lookup(pkg)
diff --git a/interp/gta.go b/interp/gta.go
@@ -2,7 +2,6 @@ package interp
 
 import (
 	"path"
-	"path/filepath"
 )
 
 // gta performs a global types analysis on the AST, registering types,
@@ -15,7 +14,7 @@ func (interp *Interpreter) gta(root *node, rpath, importPath, pkgName string) ([
 	var err error
 	var revisit []*node
 
-	baseName := filepath.Base(interp.fset.Position(root.pos).Filename)
+	baseName := path.Base(interp.fset.Position(root.pos).Filename)
 
 	root.Walk(func(n *node) bool {
 		if err != nil {
@@ -125,7 +124,7 @@ func (interp *Interpreter) gta(root *node, rpath, importPath, pkgName string) ([
 				}
 			}
 			for _, c := range n.child[:l] {
-				asImportName := filepath.Join(c.ident, baseName)
+				asImportName := path.Join(c.ident, baseName)
 				sym, exists := sc.sym[asImportName]
 				if !exists {
 					sc.sym[c.ident] = &symbol{index: sc.add(n.typ), kind: varSym, global: true, typ: n.typ, node: n}
@@ -203,7 +202,7 @@ func (interp *Interpreter) gta(root *node, rpath, importPath, pkgName string) ([
 			case ident == "init":
 				// init functions do not get declared as per the Go spec.
 			default:
-				asImportName := filepath.Join(ident, baseName)
+				asImportName := path.Join(ident, baseName)
 				if _, exists := sc.sym[asImportName]; exists {
 					// redeclaration error
 					err = n.cfgErrorf("%s redeclared in this block", ident)
@@ -257,7 +256,7 @@ func (interp *Interpreter) gta(root *node, rpath, importPath, pkgName string) ([
 					// map them by their names, otherwise we could have collisions from same-name
 					// imports in different source files of the same package. Therefore, we suffix
 					// the key with the basename of the source file.
-					name = filepath.Join(name, baseName)
+					name = path.Join(name, baseName)
 					if sym, exists := sc.sym[name]; !exists {
 						sc.sym[name] = &symbol{kind: pkgSym, typ: &itype{cat: binPkgT, path: ipath, scope: sc}}
 						break
@@ -284,7 +283,7 @@ func (interp *Interpreter) gta(root *node, rpath, importPath, pkgName string) ([
 					if name == "" {
 						name = pkgName
 					}
-					name = filepath.Join(name, baseName)
+					name = path.Join(name, baseName)
 					if sym, exists := sc.sym[name]; !exists {
 						sc.sym[name] = &symbol{kind: pkgSym, typ: &itype{cat: srcPkgT, path: ipath, scope: sc}}
 						break
@@ -345,7 +344,7 @@ func (interp *Interpreter) gta(root *node, rpath, importPath, pkgName string) ([
 			}
 			n.typ.str = n.typ.path + "." + n.typ.name
 
-			asImportName := filepath.Join(typeName, baseName)
+			asImportName := path.Join(typeName, baseName)
 			if _, exists := sc.sym[asImportName]; exists {
 				// redeclaration error
 				err = n.cfgErrorf("%s redeclared in this block", typeName)
diff --git a/interp/interp.go b/interp/interp.go
@@ -500,6 +500,7 @@ func (interp *Interpreter) Eval(src string) (res reflect.Value, err error) {
 // by the interpreter, and a non nil error in case of failure.
 // The main function of the main package is executed if present.
 func (interp *Interpreter) EvalPath(path string) (res reflect.Value, err error) {
+	path = filepath.ToSlash(path) // Ensure path is in Unix format. Since we work with fs.FS, we need to use Unix path.
 	if !isFile(interp.opt.filesystem, path) {
 		_, err := interp.importSrc(mainID, path, NoTest)
 		return res, err
@@ -647,7 +648,7 @@ func (interp *Interpreter) ImportUsed() {
 }
 
 func key2name(name string) string {
-	return filepath.Join(name, DefaultSourceName)
+	return path.Join(name, DefaultSourceName)
 }
 
 func fixKey(k string) string {
diff --git a/interp/interp_consistent_test.go b/interp/interp_consistent_test.go
@@ -177,6 +177,9 @@ func TestInterpConsistencyBuild(t *testing.T) {
 			os.Stdout = backupStdout
 
 			bin := filepath.Join(dir, strings.TrimSuffix(file.Name(), ".go"))
+			if runtime.GOOS == "windows" {
+				bin += ".exe"
+			}
 
 			cmdBuild := exec.Command("go", "build", "-tags=dummy", "-o", bin, filePath)
 			outBuild, err := cmdBuild.CombinedOutput()
diff --git a/interp/interp_file_test.go b/interp/interp_file_test.go
@@ -7,7 +7,6 @@ import (
 	"go/token"
 	"os"
 	"path/filepath"
-	"regexp"
 	"runtime"
 	"strings"
 	"testing"
@@ -92,9 +91,10 @@ func runCheck(t *testing.T, p string) {
 		t.Fatal(err)
 	}
 
+	res := strings.TrimSpace(stdout.String())
 	// Remove path in output, to have results independent of location.
-	re := regexp.MustCompile(p + ":")
-	if res := re.ReplaceAllString(strings.TrimSpace(stdout.String()), ""); res != wanted {
+	res = strings.ReplaceAll(res, filepath.ToSlash(p)+":", "")
+	if res != wanted {
 		t.Errorf("\ngot:  %q,\nwant: %q", res, wanted)
 	}
 }
diff --git a/interp/program.go b/interp/program.go
@@ -4,7 +4,8 @@ import (
 	"context"
 	"go/ast"
 	"go/token"
-	"os"
+	"io/fs"
+	"path/filepath"
 	"reflect"
 	"runtime"
 	"runtime/debug"
@@ -35,12 +36,13 @@ func (interp *Interpreter) Compile(src string) (*Program, error) {
 
 // CompilePath parses and compiles a Go code located at the given path.
 func (interp *Interpreter) CompilePath(path string) (*Program, error) {
+	path = filepath.ToSlash(path) // Ensure path is in Unix format. Since we work with fs.FS, we need to use Unix path.
 	if !isFile(interp.filesystem, path) {
 		_, err := interp.importSrc(mainID, path, NoTest)
 		return nil, err
 	}
 
-	b, err := os.ReadFile(path)
+	b, err := fs.ReadFile(interp.filesystem, path)
 	if err != nil {
 		return nil, err
 	}
diff --git a/interp/src.go b/interp/src.go
@@ -4,7 +4,7 @@ import (
 	"errors"
 	"fmt"
 	"io/fs"
-	"os"
+	"path"
 	"path/filepath"
 	"strings"
 )
@@ -33,13 +33,13 @@ func (interp *Interpreter) importSrc(rPath, importPath string, skipTest bool) (s
 		if rPath == mainID {
 			rPath = "."
 		}
-		dir = filepath.Join(filepath.Dir(interp.name), rPath, importPath)
-	} else if dir, rPath, err = interp.pkgDir(interp.context.GOPATH, rPath, importPath); err != nil {
+		dir = path.Join(path.Dir(interp.name), rPath, importPath)
+	} else if dir, rPath, err = interp.pkgDir(filepath.ToSlash(interp.context.GOPATH), rPath, importPath); err != nil {
 		// Try again, assuming a root dir at the source location.
 		if rPath, err = interp.rootFromSourceLocation(); err != nil {
 			return "", err
 		}
-		if dir, rPath, err = interp.pkgDir(interp.context.GOPATH, rPath, importPath); err != nil {
+		if dir, rPath, err = interp.pkgDir(filepath.ToSlash(interp.context.GOPATH), rPath, importPath); err != nil {
 			return "", err
 		}
 	}
@@ -68,7 +68,7 @@ func (interp *Interpreter) importSrc(rPath, importPath string, skipTest bool) (s
 			continue
 		}
 
-		name = filepath.Join(dir, name)
+		name = path.Join(dir, name)
 		var buf []byte
 		if buf, err = fs.ReadFile(interp.opt.filesystem, name); err != nil {
 			return "", err
@@ -181,29 +181,36 @@ func (interp *Interpreter) rootFromSourceLocation() (string, error) {
 	if sourceFile == DefaultSourceName {
 		return "", nil
 	}
-	wd, err := os.Getwd()
-	if err != nil {
-		return "", err
+
+	_, isRealFS := interp.opt.filesystem.(*realFS)
+	if isRealFS {
+		// In the "real" FS, GOPATH will be an absolute path, so we need to convert
+		// the source file to an absolute path to compare them.
+		absPath, err := filepath.Abs(filepath.FromSlash(sourceFile))
+		if err != nil {
+			return "", err
+		}
+		sourceFile = filepath.ToSlash(absPath)
 	}
-	pkgDir := filepath.Join(wd, filepath.Dir(sourceFile))
-	root := strings.TrimPrefix(pkgDir, filepath.Join(interp.context.GOPATH, "src")+"/")
-	if root == wd {
+	pkgDir := path.Dir(sourceFile)
+	goPath := path.Join(filepath.ToSlash(interp.context.GOPATH), "src") + "/"
+	if !strings.HasPrefix(pkgDir, goPath) {
 		return "", fmt.Errorf("package location %s not in GOPATH", pkgDir)
 	}
-	return root, nil
+	return strings.TrimPrefix(pkgDir, goPath), nil
 }
 
 // pkgDir returns the absolute path in filesystem for a package given its import path
 // and the root of the subtree dependencies.
 func (interp *Interpreter) pkgDir(goPath string, root, importPath string) (string, string, error) {
-	rPath := filepath.Join(root, "vendor")
-	dir := filepath.Join(goPath, "src", rPath, importPath)
+	rPath := path.Join(root, "vendor")
+	dir := path.Join(goPath, "src", rPath, importPath)
 
 	if _, err := fs.Stat(interp.opt.filesystem, dir); err == nil {
 		return dir, rPath, nil // found!
 	}
 
-	dir = filepath.Join(goPath, "src", effectivePkg(root, importPath))
+	dir = path.Join(goPath, "src", effectivePkg(root, importPath))
 
 	if _, err := fs.Stat(interp.opt.filesystem, dir); err == nil {
 		return dir, root, nil // found!
@@ -216,7 +223,7 @@ func (interp *Interpreter) pkgDir(goPath string, root, importPath string) (strin
 		return "", "", fmt.Errorf("unable to find source related to: %q", importPath)
 	}
 
-	rootPath := filepath.Join(goPath, "src", root)
+	rootPath := path.Join(goPath, "src", root)
 	prevRoot, err := previousRoot(interp.opt.filesystem, rootPath, root)
 	if err != nil {
 		return "", "", err
@@ -229,21 +236,21 @@ const vendor = "vendor"
 
 // Find the previous source root (vendor > vendor > ... > GOPATH).
 func previousRoot(filesystem fs.FS, rootPath, root string) (string, error) {
-	rootPath = filepath.Clean(rootPath)
-	parent, final := filepath.Split(rootPath)
-	parent = filepath.Clean(parent)
+	rootPath = path.Clean(rootPath)
+	parent, final := path.Split(rootPath)
+	parent = path.Clean(parent)
 
 	// TODO(mpl): maybe it works for the special case main, but can't be bothered for now.
 	if root != mainID && final != vendor {
-		root = strings.TrimSuffix(root, string(filepath.Separator))
-		prefix := strings.TrimSuffix(strings.TrimSuffix(rootPath, root), string(filepath.Separator))
+		root = strings.TrimSuffix(root, "/")
+		prefix := strings.TrimSuffix(strings.TrimSuffix(rootPath, root), "/")
 
 		// look for the closest vendor in one of our direct ancestors, as it takes priority.
 		var vendored string
 		for {
-			fi, err := fs.Stat(filesystem, filepath.Join(parent, vendor))
+			fi, err := fs.Stat(filesystem, path.Join(parent, vendor))
 			if err == nil && fi.IsDir() {
-				vendored = strings.TrimPrefix(strings.TrimPrefix(parent, prefix), string(filepath.Separator))
+				vendored = strings.TrimPrefix(strings.TrimPrefix(parent, prefix), "/")
 				break
 			}
 			if !errors.Is(err, fs.ErrNotExist) {
@@ -255,7 +262,7 @@ func previousRoot(filesystem fs.FS, rootPath, root string) (string, error) {
 			}
 
 			// stop when we reach GOPATH/src/blah
-			parent = filepath.Dir(parent)
+			parent = path.Dir(parent)
 			if parent == prefix {
 				break
 			}
@@ -265,7 +272,7 @@ func previousRoot(filesystem fs.FS, rootPath, root string) (string, error) {
 			// TODO(mpl): It should probably be a critical error actually,
 			// as we shouldn't have gone that high up in the tree.
 			// TODO(dennwc): This partially fails on Windows, since it cannot recognize drive letters as "root".
-			if parent == string(filepath.Separator) || parent == "." || parent == "" {
+			if parent == "/" || parent == "." || parent == "" {
 				break
 			}
 		}
@@ -277,7 +284,7 @@ func previousRoot(filesystem fs.FS, rootPath, root string) (string, error) {
 
 	// TODO(mpl): the algorithm below might be redundant with the one above,
 	// but keeping it for now. Investigate/simplify/remove later.
-	splitRoot := strings.Split(root, string(filepath.Separator))
+	splitRoot := strings.Split(root, "/")
 	var index int
 	for i := len(splitRoot) - 1; i >= 0; i-- {
 		if splitRoot[i] == "vendor" {
@@ -290,12 +297,12 @@ func previousRoot(filesystem fs.FS, rootPath, root string) (string, error) {
 		return "", nil
 	}
 
-	return filepath.Join(splitRoot[:index]...), nil
+	return path.Join(splitRoot[:index]...), nil
 }
 
-func effectivePkg(root, path string) string {
-	splitRoot := strings.Split(root, string(filepath.Separator))
-	splitPath := strings.Split(path, string(filepath.Separator))
+func effectivePkg(root, p string) string {
+	splitRoot := strings.Split(root, "/")
+	splitPath := strings.Split(p, "/")
 
 	var result []string
 
@@ -315,10 +322,10 @@ func effectivePkg(root, path string) string {
 
 	var frag string
 	for i := len(result) - 1; i >= 0; i-- {
-		frag = filepath.Join(frag, result[i])
+		frag = path.Join(frag, result[i])
 	}
 
-	return filepath.Join(root, frag)
+	return path.Join(root, frag)
 }
 
 // isPathRelative returns true if path starts with "./" or "../".
diff --git a/interp/src_test.go b/interp/src_test.go
diff --git a/interp/type.go b/interp/type.go

Original file line number	Diff line number	Diff line change
`@@ -16,7 +16,7 @@ type DB struct {`
`16`	`16`	`}`
`17`	`17`
`18`	`18`	`func main() {`
`19`		`- f, err := os.Open("/dev/null")`
	`19`	`+ f, err := os.Open(os.DevNull)`
`20`	`20`	`if err != nil {`
`21`	`21`	`log.Fatal(err)`
`22`	`22`	`}`
Original file line number	Diff line number	Diff line change
`@@ -37,6 +37,9 @@ func applyCIMultiplier(timeout time.Duration) time.Duration {`
`37`	`37`	`}`
`38`	`38`
`39`	`39`	`func TestYaegiCmdCancel(t *testing.T) {`
	`40`	`+ if runtime.GOOS == "windows" {`
	`41`	`+ t.Skip("skipping cancel test since windows has no os.Interrupt signal")`
	`42`	`+ }`
`40`	`43`	`tmp := t.TempDir()`
`41`	`44`	`yaegi := filepath.Join(tmp, "yaegi")`
`42`	`45`
Original file line number	Diff line number	Diff line change
`@@ -164,7 +164,7 @@ func TestPackagesError(t *testing.T) {`
`164`	`164`	`{`
`165`	`165`	`desc: "different packages in the same directory",`
`166`	`166`	`goPath: "./_pkg9/",`
`167`		- expected: `1:21: import "github.com/foo/pkg" error: found packages pkg and pkgfalse in ` + filepath.FromSlash("_pkg9/src/github.com/foo/pkg"),
	`167`	+ expected: `1:21: import "github.com/foo/pkg" error: found packages pkg and pkgfalse in _pkg9/src/github.com/foo/pkg`,
`168`	`168`	`},`
`169`	`169`	`}`
`170`	`170`
Original file line number	Diff line number	Diff line change
`@@ -5,7 +5,6 @@ import (`
`5`	`5`	`"go/build"`
`6`	`6`	`"go/parser"`
`7`	`7`	`"path"`
`8`		`- "path/filepath"`
`9`	`8`	`"strconv"`
`10`	`9`	`"strings"`
`11`	`10`	`)`
`@@ -135,7 +134,7 @@ func skipFile(ctx *build.Context, p string, skipTest bool) bool {`
`135`	`134`	`return true`
`136`	`135`	`}`
`137`	`136`	`p = strings.TrimSuffix(path.Base(p), ".go")`
`138`		`- if pp := filepath.Base(p); strings.HasPrefix(pp, "_") \|\| strings.HasPrefix(pp, ".") {`
	`137`	`+ if pp := path.Base(p); strings.HasPrefix(pp, "_") \|\| strings.HasPrefix(pp, ".") {`
`139`	`138`	`return true`
`140`	`139`	`}`
`141`	`140`	`if skipTest && strings.HasSuffix(p, "_test") {`
Original file line number	Diff line number	Diff line change
`@@ -500,6 +500,7 @@ func (interp *Interpreter) Eval(src string) (res reflect.Value, err error) {`
`500`	`500`	`// by the interpreter, and a non nil error in case of failure.`
`501`	`501`	`// The main function of the main package is executed if present.`
`502`	`502`	`func (interp *Interpreter) EvalPath(path string) (res reflect.Value, err error) {`
	`503`	`+ path = filepath.ToSlash(path) // Ensure path is in Unix format. Since we work with fs.FS, we need to use Unix path.`
`503`	`504`	`if !isFile(interp.opt.filesystem, path) {`
`504`	`505`	`_, err := interp.importSrc(mainID, path, NoTest)`
`505`	`506`	`return res, err`
`@@ -647,7 +648,7 @@ func (interp *Interpreter) ImportUsed() {`
`647`	`648`	`}`
`648`	`649`
`649`	`650`	`func key2name(name string) string {`
`650`		`- return filepath.Join(name, DefaultSourceName)`
	`651`	`+ return path.Join(name, DefaultSourceName)`
`651`	`652`	`}`
`652`	`653`
`653`	`654`	`func fixKey(k string) string {`
Original file line number	Diff line number	Diff line change
`@@ -7,7 +7,6 @@ import (`
`7`	`7`	`"go/token"`
`8`	`8`	`"os"`
`9`	`9`	`"path/filepath"`
`10`		`- "regexp"`
`11`	`10`	`"runtime"`
`12`	`11`	`"strings"`
`13`	`12`	`"testing"`
`@@ -92,9 +91,10 @@ func runCheck(t *testing.T, p string) {`
`92`	`91`	`t.Fatal(err)`
`93`	`92`	`}`
`94`	`93`
	`94`	`+ res := strings.TrimSpace(stdout.String())`
`95`	`95`	`// Remove path in output, to have results independent of location.`
`96`		`- re := regexp.MustCompile(p + ":")`
`97`		`- if res := re.ReplaceAllString(strings.TrimSpace(stdout.String()), ""); res != wanted {`
	`96`	`+ res = strings.ReplaceAll(res, filepath.ToSlash(p)+":", "")`
	`97`	`+ if res != wanted {`
`98`	`98`	`t.Errorf("\ngot: %q,\nwant: %q", res, wanted)`
`99`	`99`	`}`
`100`	`100`	`}`