From 70b8a7072c96117cb673b58434ea5801158a6afe Mon Sep 17 00:00:00 2001 From: Guilhem Fanton <8671905+gfanton@users.noreply.github.com> Date: Thu, 13 Feb 2025 17:21:36 +0100 Subject: [PATCH] fix(gnodev): fix the use of filepath instead of path on windows (#3737) fix #3722 Using `filepath` to work with the `path` creates some obvious issues on Windows, primarily because the `Clean` function replaces `/` with `\\`. This PR updates the usage of `filepath` to `path` where necessary. At some point, adapting path-specific tests on Windows would be very helpful. --- @Milosevic02, can you confirm that this fix resolves most of your issues with gnodev? **EDIT:** It seems that the fix is effective: https://github.com/gnolang/gno/issues/3722#issuecomment-2653988097 --------- Signed-off-by: gfanton <8671905+gfanton@users.noreply.github.com> --- contribs/gnodev/cmd/gnobro/main.go | 5 +- contribs/gnodev/cmd/gnodev/command_staging.go | 3 +- contribs/gnodev/cmd/gnodev/setup_loader.go | 3 +- contribs/gnodev/pkg/browser/model.go | 4 +- contribs/gnodev/pkg/browser/utils.go | 4 +- contribs/gnodev/pkg/dev/query_path.go | 4 +- contribs/gnodev/pkg/events/events.go | 4 +- contribs/gnodev/pkg/packages/loader_glob.go | 17 +++--- .../gnodev/pkg/packages/resolver_local.go | 3 +- .../gnodev/pkg/packages/resolver_remote.go | 4 +- contribs/gnodev/pkg/packages/utils_other.go | 9 ++++ contribs/gnodev/pkg/packages/utils_windows.go | 11 ++++ contribs/gnodev/pkg/proxy/path_interceptor.go | 25 +++++---- .../gnodev/pkg/proxy/path_interceptor_test.go | 3 +- contribs/gnodev/pkg/watcher/watch.go | 52 +++++++++---------- gno.land/pkg/gnoweb/handler.go | 6 +-- gno.land/pkg/gnoweb/webclient_html.go | 6 +-- gno.land/pkg/gnoweb/weburl/url.go | 6 +-- 18 files changed, 101 insertions(+), 68 deletions(-) create mode 100644 contribs/gnodev/pkg/packages/utils_other.go create mode 100644 contribs/gnodev/pkg/packages/utils_windows.go diff --git a/contribs/gnodev/cmd/gnobro/main.go b/contribs/gnodev/cmd/gnobro/main.go index 91713d6c6d8..7e47fb96780 100644 --- a/contribs/gnodev/cmd/gnobro/main.go +++ b/contribs/gnodev/cmd/gnobro/main.go @@ -11,7 +11,7 @@ import ( "net/url" "os" "os/signal" - "path/filepath" + gopath "path" "strings" "time" @@ -239,7 +239,6 @@ func runLocal(ctx context.Context, gnocl *gnoclient.Client, cfg *broCfg, bcfg br ) var errgs errgroup.Group - if cfg.dev { devpoint, err := getDevEndpoint(cfg) if err != nil { @@ -453,7 +452,7 @@ func ValidatePathCommandMiddleware(pathPrefix string) wish.Middleware { return case 1: // check for valid path path := cmd[0] - if strings.HasPrefix(path, pathPrefix) && filepath.Clean(path) == path { + if strings.HasPrefix(path, pathPrefix) && gopath.Clean(path) == path { s.Context().SetValue("path", path) next(s) return diff --git a/contribs/gnodev/cmd/gnodev/command_staging.go b/contribs/gnodev/cmd/gnodev/command_staging.go index 7b1a0ab3f5a..d02d7c289fe 100644 --- a/contribs/gnodev/cmd/gnodev/command_staging.go +++ b/contribs/gnodev/cmd/gnodev/command_staging.go @@ -3,6 +3,7 @@ package main import ( "context" "flag" + "path" "path/filepath" "github.com/gnolang/gno/contribs/gnodev/pkg/packages" @@ -28,7 +29,7 @@ var defaultStagingOptions = AppConfig{ interactive: false, unsafeAPI: false, lazyLoader: false, - paths: filepath.Join(DefaultDomain, "/**"), // Load every package under the main domain}, + paths: path.Join(DefaultDomain, "/**"), // Load every package under the main domain}, // As we have no reason to configure this yet, set this to random port // to avoid potential conflict with other app diff --git a/contribs/gnodev/cmd/gnodev/setup_loader.go b/contribs/gnodev/cmd/gnodev/setup_loader.go index 8f10a6a5a76..69342657352 100644 --- a/contribs/gnodev/cmd/gnodev/setup_loader.go +++ b/contribs/gnodev/cmd/gnodev/setup_loader.go @@ -3,6 +3,7 @@ package main import ( "fmt" "log/slog" + gopath "path" "path/filepath" "regexp" "strings" @@ -103,5 +104,5 @@ func guessPath(cfg *AppConfig, dir string) (path string) { } rname := reInvalidChar.ReplaceAllString(filepath.Base(dir), "-") - return filepath.Join(cfg.chainDomain, "/r/dev/", rname) + return gopath.Join(cfg.chainDomain, "/r/dev/", rname) } diff --git a/contribs/gnodev/pkg/browser/model.go b/contribs/gnodev/pkg/browser/model.go index bdd7eac9c82..148497e77db 100644 --- a/contribs/gnodev/pkg/browser/model.go +++ b/contribs/gnodev/pkg/browser/model.go @@ -6,7 +6,7 @@ import ( "errors" "fmt" "log/slog" - "path/filepath" + gopath "path" "regexp" "strings" @@ -576,5 +576,5 @@ func (m *model) getCurrentPath() string { return m.urlPrefix } - return filepath.Join(m.urlPrefix, path) + return gopath.Join(m.urlPrefix, path) } diff --git a/contribs/gnodev/pkg/browser/utils.go b/contribs/gnodev/pkg/browser/utils.go index b322bea552a..94f0bd6344c 100644 --- a/contribs/gnodev/pkg/browser/utils.go +++ b/contribs/gnodev/pkg/browser/utils.go @@ -1,7 +1,7 @@ package browser import ( - "path/filepath" + gopath "path" "strings" "github.com/gnolang/gno/gno.land/pkg/gnoweb" @@ -27,7 +27,7 @@ func cleanupRealmPath(prefix, realm string) string { // trim any slash path = strings.TrimPrefix(path, "/") // clean up path - path = filepath.Clean(path) + path = gopath.Clean(path) return path } diff --git a/contribs/gnodev/pkg/dev/query_path.go b/contribs/gnodev/pkg/dev/query_path.go index e899d8212e4..b33576ccc71 100644 --- a/contribs/gnodev/pkg/dev/query_path.go +++ b/contribs/gnodev/pkg/dev/query_path.go @@ -3,7 +3,7 @@ package dev import ( "fmt" "net/url" - "path/filepath" + "path" "github.com/gnolang/gno/contribs/gnodev/pkg/address" "github.com/gnolang/gno/tm2/pkg/crypto" @@ -24,7 +24,7 @@ func ResolveQueryPath(bk *address.Book, query string) (QueryPath, error) { return qpath, fmt.Errorf("malformed path/query: %w", err) } - qpath.Path = filepath.Clean(upath.Path) + qpath.Path = path.Clean(upath.Path) // Check for creator option creator := upath.Query().Get("creator") diff --git a/contribs/gnodev/pkg/events/events.go b/contribs/gnodev/pkg/events/events.go index c387c331eed..3137b5d50bb 100644 --- a/contribs/gnodev/pkg/events/events.go +++ b/contribs/gnodev/pkg/events/events.go @@ -41,8 +41,8 @@ type PackagesUpdate struct { } type PackageUpdate struct { - Package string `json:"package"` - Files []string `json:"files"` + PackageDir string `json:"package"` + Files []string `json:"files"` } func (PackagesUpdate) Type() Type { return EvtPackagesUpdate } diff --git a/contribs/gnodev/pkg/packages/loader_glob.go b/contribs/gnodev/pkg/packages/loader_glob.go index dabfe613574..6e3d1158cb5 100644 --- a/contribs/gnodev/pkg/packages/loader_glob.go +++ b/contribs/gnodev/pkg/packages/loader_glob.go @@ -5,6 +5,7 @@ import ( "go/token" "io/fs" "os" + "path" "path/filepath" "strings" ) @@ -33,7 +34,7 @@ func (l GlobLoader) MatchPaths(globs ...string) ([]string, error) { mpaths := []string{} for _, input := range globs { - cleanInput := filepath.Clean(input) + cleanInput := path.Clean(input) gpath, err := Parse(cleanInput) if err != nil { return nil, fmt.Errorf("invalid glob path %q: %w", input, err) @@ -45,18 +46,20 @@ func (l GlobLoader) MatchPaths(globs ...string) ([]string, error) { continue } - // root := filepath.Join(l.Root, base) root := l.Root err = filepath.WalkDir(root, func(dirpath string, d fs.DirEntry, err error) error { if err != nil { return err } - relPath, relErr := filepath.Rel(root, dirpath) - if relErr != nil { - return relErr + path, err := filepath.Rel(root, dirpath) + if err != nil { + return err } + // normalize filepath to path + path = NormalizeFilepathToPath(path) + if !d.IsDir() { return nil } @@ -65,8 +68,8 @@ func (l GlobLoader) MatchPaths(globs ...string) ([]string, error) { return fs.SkipDir } - if gpath.Match(relPath) { - mpaths = append(mpaths, relPath) + if gpath.Match(path) { + mpaths = append(mpaths, path) } return nil diff --git a/contribs/gnodev/pkg/packages/resolver_local.go b/contribs/gnodev/pkg/packages/resolver_local.go index 13448aca52d..0312b262403 100644 --- a/contribs/gnodev/pkg/packages/resolver_local.go +++ b/contribs/gnodev/pkg/packages/resolver_local.go @@ -3,6 +3,7 @@ package packages import ( "fmt" "go/token" + "path" "path/filepath" "strings" ) @@ -20,7 +21,7 @@ func NewLocalResolver(path, dir string) *LocalResolver { } func (r *LocalResolver) Name() string { - return fmt.Sprintf("local<%s>", filepath.Base(r.Dir)) + return fmt.Sprintf("local<%s>", path.Base(r.Dir)) } func (r LocalResolver) IsValid() bool { diff --git a/contribs/gnodev/pkg/packages/resolver_remote.go b/contribs/gnodev/pkg/packages/resolver_remote.go index 94396f70c83..ff7b8fa723a 100644 --- a/contribs/gnodev/pkg/packages/resolver_remote.go +++ b/contribs/gnodev/pkg/packages/resolver_remote.go @@ -6,7 +6,7 @@ import ( "fmt" "go/parser" "go/token" - "path/filepath" + gopath "path" "strings" "github.com/gnolang/gno/gno.land/pkg/sdk/vm" @@ -56,7 +56,7 @@ func (res *remoteResolver) Resolve(fset *token.FileSet, path string) (*Package, files := bytes.Split(qres.Response.Data, []byte{'\n'}) for _, filename := range files { fname := string(filename) - fpath := filepath.Join(path, fname) + fpath := gopath.Join(path, fname) qres, err := res.RPCClient.ABCIQuery(qpath, []byte(fpath)) if err != nil { return nil, fmt.Errorf("unable to query path") diff --git a/contribs/gnodev/pkg/packages/utils_other.go b/contribs/gnodev/pkg/packages/utils_other.go new file mode 100644 index 00000000000..0455aa57480 --- /dev/null +++ b/contribs/gnodev/pkg/packages/utils_other.go @@ -0,0 +1,9 @@ +//go:build !windows +// +build !windows + +package packages + +// NormalizeFilepathToPath normalize path an unix like path +func NormalizeFilepathToPath(path string) string { + return path +} diff --git a/contribs/gnodev/pkg/packages/utils_windows.go b/contribs/gnodev/pkg/packages/utils_windows.go new file mode 100644 index 00000000000..23c7fa54379 --- /dev/null +++ b/contribs/gnodev/pkg/packages/utils_windows.go @@ -0,0 +1,11 @@ +//go:build windows +// +build windows + +package packages + +import "strings" + +// NormalizeFilepathToPath normalize path an unix like path +func NormalizeFilepathToPath(path string) string { + return strings.ReplaceAll(path, "\\", "/") +} diff --git a/contribs/gnodev/pkg/proxy/path_interceptor.go b/contribs/gnodev/pkg/proxy/path_interceptor.go index 84d2f92b22f..88240d904a5 100644 --- a/contribs/gnodev/pkg/proxy/path_interceptor.go +++ b/contribs/gnodev/pkg/proxy/path_interceptor.go @@ -10,7 +10,7 @@ import ( "log/slog" "net" "net/http" - "path/filepath" + gopath "path" "strings" "sync" @@ -220,7 +220,11 @@ func (upaths uniqPaths) list() []string { return paths } -func (upaths uniqPaths) add(path string) { upaths[path] = struct{}{} } +// Add a path to +func (upaths uniqPaths) add(path string) { + path = cleanupPath(path) + upaths[path] = struct{}{} +} // handleRequest parses and processes the RPC request body. func (proxy *PathInterceptor) handleRequest(body []byte) error { @@ -312,13 +316,6 @@ func handleQuery(path string, data []byte, upaths uniqPaths) error { case "vm/qrender", "vm/qfile", "vm/qfuncs", "vm/qeval": path, _, _ := strings.Cut(string(data), ":") // Cut arguments out - path = filepath.Clean(path) - - // If path is a file, grab the directory instead - if ext := filepath.Ext(path); ext != "" { - path = filepath.Dir(path) - } - upaths.add(path) return nil @@ -328,3 +325,13 @@ func handleQuery(path string, data []byte, upaths uniqPaths) error { // XXX: handle more cases } + +func cleanupPath(path string) string { + path = gopath.Clean(path) + // If path is a file, grab the directory instead + if ext := gopath.Ext(path); ext != "" { + path = gopath.Dir(path) + } + + return path +} diff --git a/contribs/gnodev/pkg/proxy/path_interceptor_test.go b/contribs/gnodev/pkg/proxy/path_interceptor_test.go index c7082adfa30..0e1e95e82a4 100644 --- a/contribs/gnodev/pkg/proxy/path_interceptor_test.go +++ b/contribs/gnodev/pkg/proxy/path_interceptor_test.go @@ -3,6 +3,7 @@ package proxy_test import ( "net" "net/http" + "path" "path/filepath" "testing" @@ -86,7 +87,7 @@ func TestProxy(t *testing.T) { cli, err := client.NewHTTPClient(interceptor.TargetAddress()) require.NoError(t, err) - res, err := cli.ABCIQuery("vm/qfile", []byte(filepath.Join(targetPath, "foo.gno"))) + res, err := cli.ABCIQuery("vm/qfile", []byte(path.Join(targetPath, "foo.gno"))) require.NoError(t, err) assert.Nil(t, res.Response.Error) diff --git a/contribs/gnodev/pkg/watcher/watch.go b/contribs/gnodev/pkg/watcher/watch.go index 5f277fd6646..0a7d284bd37 100644 --- a/contribs/gnodev/pkg/watcher/watch.go +++ b/contribs/gnodev/pkg/watcher/watch.go @@ -58,7 +58,7 @@ func (p *PackageWatcher) startWatching() { defer close(pkgsUpdateChan) var debounceTimer <-chan time.Time - pathList := []string{} + filesList := []string{} var err error for err == nil { @@ -69,10 +69,10 @@ func (p *PackageWatcher) startWatching() { err = fmt.Errorf("watch error: %w", watchErr) case <-debounceTimer: // Process and emit package updates after the debounce interval - updates := p.generatePackagesUpdateList(pathList) + updates := p.generatePackagesUpdateList(filesList) for _, update := range updates { p.logger.Info("packages update", - "pkg", update.Package, + "pkg", update.PackageDir, "files", update.Files, ) } @@ -84,7 +84,7 @@ func (p *PackageWatcher) startWatching() { }) // Reset the path list and debounce timer - pathList = []string{} + filesList = []string{} debounceTimer = nil case evt := <-p.watcher.Events: // Only handle write operations @@ -92,7 +92,7 @@ func (p *PackageWatcher) startWatching() { continue } - pathList = append(pathList, evt.Name) + filesList = append(filesList, evt.Name) // Set up the debounce timer debounceTimer = time.After(timeout) @@ -125,26 +125,26 @@ func (p *PackageWatcher) UpdatePackagesWatch(pkgs ...packages.Package) { continue } - path, err := filepath.Abs(pkg.Location) + dir, err := filepath.Abs(pkg.Location) if err != nil { p.logger.Error("Unable to get absolute path", "path", pkg.Location, "error", err) continue } - newPkgs[path] = struct{}{} + newPkgs[dir] = struct{}{} } - for path := range oldPkgs { - if _, exists := newPkgs[path]; !exists { - p.watcher.Remove(path) - p.logger.Debug("Watcher list: removed", "path", path) + for dir := range oldPkgs { + if _, exists := newPkgs[dir]; !exists { + p.watcher.Remove(dir) + p.logger.Debug("Watcher list: removed", "path", dir) } } - for path := range newPkgs { - if _, exists := oldPkgs[path]; !exists { - p.watcher.Add(path) - p.logger.Debug("Watcher list: added", "path", path) + for dir := range newPkgs { + if _, exists := oldPkgs[dir]; !exists { + p.watcher.Add(dir) + p.logger.Debug("Watcher list: added", "path", dir) } } } @@ -154,29 +154,29 @@ func (p *PackageWatcher) generatePackagesUpdateList(paths []string) PackageUpdat mpkgs := map[string]*events.PackageUpdate{} // Pkg -> Update watchList := p.watcher.WatchList() - for _, path := range paths { - for _, pkg := range watchList { - if len(pkg) == len(path) { - continue // Skip if pkg == path + for _, file := range paths { + for _, watchDir := range watchList { + if len(watchDir) == len(file) { + continue // Skip if watchDir == file } // Check if a package directory contain our path directory - dirPath := filepath.Dir(path) - if !strings.HasPrefix(pkg, dirPath) { + dir := filepath.Dir(file) + if !strings.HasPrefix(watchDir, dir) { continue } // Accumulate file updates for each package - pkgu, ok := mpkgs[pkg] + pkgu, ok := mpkgs[watchDir] if !ok { pkgsUpdate = append(pkgsUpdate, events.PackageUpdate{ - Package: pkg, - Files: []string{}, + PackageDir: watchDir, + Files: []string{}, }) pkgu = &pkgsUpdate[len(pkgsUpdate)-1] } - pkgu.Files = append(pkgu.Files, path) + pkgu.Files = append(pkgu.Files, file) } } @@ -188,7 +188,7 @@ type PackageUpdateList []events.PackageUpdate func (pkgsu PackageUpdateList) PackagesPath() []string { pkgs := make([]string, len(pkgsu)) for i, pkg := range pkgsu { - pkgs[i] = pkg.Package + pkgs[i] = pkg.PackageDir } return pkgs } diff --git a/gno.land/pkg/gnoweb/handler.go b/gno.land/pkg/gnoweb/handler.go index cfad9919506..41a61c4cead 100644 --- a/gno.land/pkg/gnoweb/handler.go +++ b/gno.land/pkg/gnoweb/handler.go @@ -6,7 +6,7 @@ import ( "fmt" "log/slog" "net/http" - "path/filepath" + "path" "strings" "time" @@ -207,14 +207,14 @@ func (h *WebHandler) GetHelpView(gnourl *weburl.GnoURL) (int, *components.View) } } - realmName := filepath.Base(gnourl.Path) + realmName := path.Base(gnourl.Path) return http.StatusOK, components.HelpView(components.HelpData{ SelectedFunc: selFn, SelectedArgs: selArgs, RealmName: realmName, // TODO: get chain domain and use that. ChainId: h.Static.ChainId, - PkgPath: filepath.Join(h.Static.Domain, gnourl.Path), + PkgPath: path.Join(h.Static.Domain, gnourl.Path), Remote: h.Static.RemoteHelp, Functions: fsigs, }) diff --git a/gno.land/pkg/gnoweb/webclient_html.go b/gno.land/pkg/gnoweb/webclient_html.go index 72b1b3f8b06..efbc08870de 100644 --- a/gno.land/pkg/gnoweb/webclient_html.go +++ b/gno.land/pkg/gnoweb/webclient_html.go @@ -5,7 +5,7 @@ import ( "fmt" "io" "log/slog" - "path/filepath" + gopath "path" "strings" "github.com/alecthomas/chroma/v2" @@ -121,7 +121,7 @@ func (s *HTMLWebClient) SourceFile(w io.Writer, path, fileName string) (*FileMet } // XXX: Consider moving this into gnoclient - fullPath := filepath.Join(s.domain, strings.Trim(path, "/"), fileName) + fullPath := gopath.Join(s.domain, strings.Trim(path, "/"), fileName) source, err := s.query(qpath, []byte(fullPath)) if err != nil { @@ -230,7 +230,7 @@ func (s *HTMLWebClient) FormatSource(w io.Writer, fileName string, src []byte) e var lexer chroma.Lexer // Determine the lexer to be used based on the file extension. - switch strings.ToLower(filepath.Ext(fileName)) { + switch strings.ToLower(gopath.Ext(fileName)) { case ".gno": lexer = lexers.Get("go") case ".md": diff --git a/gno.land/pkg/gnoweb/weburl/url.go b/gno.land/pkg/gnoweb/weburl/url.go index cbe861e9e42..8b62ada198f 100644 --- a/gno.land/pkg/gnoweb/weburl/url.go +++ b/gno.land/pkg/gnoweb/weburl/url.go @@ -4,7 +4,7 @@ import ( "errors" "fmt" "net/url" - "path/filepath" + gopath "path" "regexp" "slices" "strings" @@ -179,8 +179,8 @@ func ParseGnoURL(u *url.URL) (*GnoURL, error) { // A file is considered as one that either ends with an extension or // contains an uppercase rune - ext := filepath.Ext(upath) - base := filepath.Base(upath) + ext := gopath.Ext(upath) + base := gopath.Base(upath) if ext != "" || strings.ToLower(base) != base { file = base upath = strings.TrimSuffix(upath, base)