From 2fc948bb5654accaa7023d0fd2e38031a291a74c Mon Sep 17 00:00:00 2001 From: waymobetta Date: Thu, 1 Feb 2024 17:10:36 -0800 Subject: [PATCH 01/15] feat: init addPkg file sorting --- gno.land/pkg/sdk/vm/keeper.go | 141 +++++++++++++--------------------- 1 file changed, 53 insertions(+), 88 deletions(-) diff --git a/gno.land/pkg/sdk/vm/keeper.go b/gno.land/pkg/sdk/vm/keeper.go index 16162e1004c..bd49c1d58b0 100644 --- a/gno.land/pkg/sdk/vm/keeper.go +++ b/gno.land/pkg/sdk/vm/keeper.go @@ -3,10 +3,9 @@ package vm // TODO: move most of the logic in ROOT/gno.land/... import ( - "bytes" "fmt" "os" - "regexp" + "slices" "strings" gno "github.com/gnolang/gno/gnovm/pkg/gnolang" @@ -29,7 +28,6 @@ const ( type VMKeeperI interface { AddPackage(ctx sdk.Context, msg MsgAddPackage) error Call(ctx sdk.Context, msg MsgCall) (res string, err error) - Run(ctx sdk.Context, msg MsgRun) (res string, err error) } var _ VMKeeperI = &VMKeeper{} @@ -131,8 +129,6 @@ func (vm *VMKeeper) getGnoStore(ctx sdk.Context) gno.Store { } } -var reReservedPath = regexp.MustCompile(`gno\.land/r/g[a-z0-9]+/run`) - // AddPackage adds a package with given fileset. func (vm *VMKeeper) AddPackage(ctx sdk.Context, msg MsgAddPackage) error { creator := msg.Creator @@ -155,11 +151,6 @@ func (vm *VMKeeper) AddPackage(ctx sdk.Context, msg MsgAddPackage) error { if pv := store.GetPackage(pkgPath, false); pv != nil { return ErrInvalidPkgPath("package already exists: " + pkgPath) } - - if reReservedPath.MatchString(pkgPath) { - return ErrInvalidPkgPath("reserved package name: " + pkgPath) - } - // Pay deposit from creator. pkgAddr := gno.DerivePkgAddr(pkgPath) @@ -174,12 +165,62 @@ func (vm *VMKeeper) AddPackage(ctx sdk.Context, msg MsgAddPackage) error { if err != nil { return err } + + // TODO + // enforce sorting msg.Files based on Go conventions for predictability + pkgNames := []string{} + + // add std.MemFile.Name to slice -> sort + // create slice of file names + for _, pkgFile := range msg.Package.Files { + pkgNames = append( + pkgNames, + pkgFile.Name, // string + ) + } + + // introduce enforcement of lexical/non-lexical + // sort filenames based on Go conventions + // slices.Sort sorts any type that supports the operators < <= >= > + slices.Sort(pkgNames) + + var memPkgSorted std.MemPackage + pFiles := make([]*std.MemFile, 0, len(msg.Package.Files)) + + // add to new []*std.MemFile to enforce order + for _, pkgName := range pkgNames { + for _, nsFile := range msg.Package.Files { + // bail if not the most immediate pkgName + if nsFile.Name != pkgName { + continue + } + + pFiles = append( + pFiles, + nsFile, + ) + } + + memPkgSorted = std.MemPackage{ + Name: pkgName, + Path: msg.Package.Path, + Files: pFiles, + } + } + + // new instance of MsgAddPackage + msgSorted := MsgAddPackage{ + Creator: msg.Creator, + Package: &memPkgSorted, + Deposit: msg.Deposit, + } + // Parse and run the files, construct *PV. msgCtx := stdlibs.ExecContext{ ChainID: ctx.ChainID(), Height: ctx.BlockHeight(), Timestamp: ctx.BlockTime().Unix(), - Msg: msg, + Msg: msgSorted, OrigCaller: creator.Bech32(), OrigSend: deposit, OrigSendSpent: new(std.Coins), @@ -281,7 +322,7 @@ func (vm *VMKeeper) Call(ctx sdk.Context, msg MsgCall) (res string, err error) { m.Release() }() rtvs := m.Eval(xn) - ctx.Logger().Info("CPUCYCLES call", "num-cycles", m.Cycles) + ctx.Logger().Info("CPUCYCLES call: ", m.Cycles) for i, rtv := range rtvs { res = res + rtv.String() if i < len(rtvs)-1 { @@ -292,82 +333,6 @@ func (vm *VMKeeper) Call(ctx sdk.Context, msg MsgCall) (res string, err error) { // TODO pay for gas? TODO see context? } -// Run executes arbitrary Gno code in the context of the caller's realm. -func (vm *VMKeeper) Run(ctx sdk.Context, msg MsgRun) (res string, err error) { - caller := msg.Caller - pkgAddr := caller - store := vm.getGnoStore(ctx) - send := msg.Send - memPkg := msg.Package - - // Validate arguments. - callerAcc := vm.acck.GetAccount(ctx, caller) - if callerAcc == nil { - return "", std.ErrUnknownAddress(fmt.Sprintf("account %s does not exist", caller)) - } - if err := msg.Package.Validate(); err != nil { - return "", ErrInvalidPkgPath(err.Error()) - } - - // Send send-coins to pkg from caller. - err = vm.bank.SendCoins(ctx, caller, pkgAddr, send) - if err != nil { - return "", err - } - - // Parse and run the files, construct *PV. - msgCtx := stdlibs.ExecContext{ - ChainID: ctx.ChainID(), - Height: ctx.BlockHeight(), - Timestamp: ctx.BlockTime().Unix(), - Msg: msg, - OrigCaller: caller.Bech32(), - OrigSend: send, - OrigSendSpent: new(std.Coins), - OrigPkgAddr: pkgAddr.Bech32(), - Banker: NewSDKBanker(vm, ctx), - } - // Parse and run the files, construct *PV. - buf := new(bytes.Buffer) - m := gno.NewMachineWithOptions( - gno.MachineOptions{ - PkgPath: "", - Output: buf, - Store: store, - Alloc: store.GetAllocator(), - Context: msgCtx, - MaxCycles: vm.maxCycles, - }) - defer m.Release() - _, pv := m.RunMemPackage(memPkg, false) - ctx.Logger().Info("CPUCYCLES", "addpkg", m.Cycles) - - m2 := gno.NewMachineWithOptions( - gno.MachineOptions{ - PkgPath: "", - Output: buf, - Store: store, - Alloc: store.GetAllocator(), - Context: msgCtx, - MaxCycles: vm.maxCycles, - }) - m2.SetActivePackage(pv) - defer func() { - if r := recover(); r != nil { - err = errors.Wrap(fmt.Errorf("%v", r), "VM call panic: %v\n%s\n", - r, m2.String()) - return - } - m2.Release() - }() - m2.RunMain() - ctx.Logger().Info("CPUCYCLES call", - "cycles", m2.Cycles, - ) - res = buf.String() - return res, nil -} - // QueryFuncs returns public facing function signatures. func (vm *VMKeeper) QueryFuncs(ctx sdk.Context, pkgPath string) (fsigs FunctionSignatures, err error) { store := vm.getGnoStore(ctx) From 849962b7e23c7caeb91d579f6f8d0521e8e8b68d Mon Sep 17 00:00:00 2001 From: waymobetta Date: Thu, 1 Feb 2024 17:17:20 -0800 Subject: [PATCH 02/15] fix: undo remove Run --- gno.land/pkg/sdk/vm/keeper.go | 88 ++++++++++++++++++++++++++++++++++- 1 file changed, 87 insertions(+), 1 deletion(-) diff --git a/gno.land/pkg/sdk/vm/keeper.go b/gno.land/pkg/sdk/vm/keeper.go index bd49c1d58b0..07b57799c25 100644 --- a/gno.land/pkg/sdk/vm/keeper.go +++ b/gno.land/pkg/sdk/vm/keeper.go @@ -3,8 +3,10 @@ package vm // TODO: move most of the logic in ROOT/gno.land/... import ( + "bytes" "fmt" "os" + "regexp" "slices" "strings" @@ -28,6 +30,7 @@ const ( type VMKeeperI interface { AddPackage(ctx sdk.Context, msg MsgAddPackage) error Call(ctx sdk.Context, msg MsgCall) (res string, err error) + Run(ctx sdk.Context, msg MsgRun) (res string, err error) } var _ VMKeeperI = &VMKeeper{} @@ -129,6 +132,8 @@ func (vm *VMKeeper) getGnoStore(ctx sdk.Context) gno.Store { } } +var reReservedPath = regexp.MustCompile(`gno\.land/r/g[a-z0-9]+/run`) + // AddPackage adds a package with given fileset. func (vm *VMKeeper) AddPackage(ctx sdk.Context, msg MsgAddPackage) error { creator := msg.Creator @@ -151,6 +156,11 @@ func (vm *VMKeeper) AddPackage(ctx sdk.Context, msg MsgAddPackage) error { if pv := store.GetPackage(pkgPath, false); pv != nil { return ErrInvalidPkgPath("package already exists: " + pkgPath) } + + if reReservedPath.MatchString(pkgPath) { + return ErrInvalidPkgPath("reserved package name: " + pkgPath) + } + // Pay deposit from creator. pkgAddr := gno.DerivePkgAddr(pkgPath) @@ -322,7 +332,7 @@ func (vm *VMKeeper) Call(ctx sdk.Context, msg MsgCall) (res string, err error) { m.Release() }() rtvs := m.Eval(xn) - ctx.Logger().Info("CPUCYCLES call: ", m.Cycles) + ctx.Logger().Info("CPUCYCLES call", "num-cycles", m.Cycles) for i, rtv := range rtvs { res = res + rtv.String() if i < len(rtvs)-1 { @@ -333,6 +343,82 @@ func (vm *VMKeeper) Call(ctx sdk.Context, msg MsgCall) (res string, err error) { // TODO pay for gas? TODO see context? } +// Run executes arbitrary Gno code in the context of the caller's realm. +func (vm *VMKeeper) Run(ctx sdk.Context, msg MsgRun) (res string, err error) { + caller := msg.Caller + pkgAddr := caller + store := vm.getGnoStore(ctx) + send := msg.Send + memPkg := msg.Package + + // Validate arguments. + callerAcc := vm.acck.GetAccount(ctx, caller) + if callerAcc == nil { + return "", std.ErrUnknownAddress(fmt.Sprintf("account %s does not exist", caller)) + } + if err := msg.Package.Validate(); err != nil { + return "", ErrInvalidPkgPath(err.Error()) + } + + // Send send-coins to pkg from caller. + err = vm.bank.SendCoins(ctx, caller, pkgAddr, send) + if err != nil { + return "", err + } + + // Parse and run the files, construct *PV. + msgCtx := stdlibs.ExecContext{ + ChainID: ctx.ChainID(), + Height: ctx.BlockHeight(), + Timestamp: ctx.BlockTime().Unix(), + Msg: msg, + OrigCaller: caller.Bech32(), + OrigSend: send, + OrigSendSpent: new(std.Coins), + OrigPkgAddr: pkgAddr.Bech32(), + Banker: NewSDKBanker(vm, ctx), + } + // Parse and run the files, construct *PV. + buf := new(bytes.Buffer) + m := gno.NewMachineWithOptions( + gno.MachineOptions{ + PkgPath: "", + Output: buf, + Store: store, + Alloc: store.GetAllocator(), + Context: msgCtx, + MaxCycles: vm.maxCycles, + }) + defer m.Release() + _, pv := m.RunMemPackage(memPkg, false) + ctx.Logger().Info("CPUCYCLES", "addpkg", m.Cycles) + + m2 := gno.NewMachineWithOptions( + gno.MachineOptions{ + PkgPath: "", + Output: buf, + Store: store, + Alloc: store.GetAllocator(), + Context: msgCtx, + MaxCycles: vm.maxCycles, + }) + m2.SetActivePackage(pv) + defer func() { + if r := recover(); r != nil { + err = errors.Wrap(fmt.Errorf("%v", r), "VM call panic: %v\n%s\n", + r, m2.String()) + return + } + m2.Release() + }() + m2.RunMain() + ctx.Logger().Info("CPUCYCLES call", + "cycles", m2.Cycles, + ) + res = buf.String() + return res, nil +} + // QueryFuncs returns public facing function signatures. func (vm *VMKeeper) QueryFuncs(ctx sdk.Context, pkgPath string) (fsigs FunctionSignatures, err error) { store := vm.getGnoStore(ctx) From 0221f35235d0af05796a3878a1c522a3a3aa0f07 Mon Sep 17 00:00:00 2001 From: waymobetta Date: Thu, 1 Feb 2024 17:28:58 -0800 Subject: [PATCH 03/15] fix: replace slices with sort --- gno.land/pkg/sdk/vm/keeper.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/gno.land/pkg/sdk/vm/keeper.go b/gno.land/pkg/sdk/vm/keeper.go index 07b57799c25..e38edc15076 100644 --- a/gno.land/pkg/sdk/vm/keeper.go +++ b/gno.land/pkg/sdk/vm/keeper.go @@ -7,7 +7,7 @@ import ( "fmt" "os" "regexp" - "slices" + "sort" "strings" gno "github.com/gnolang/gno/gnovm/pkg/gnolang" @@ -192,7 +192,7 @@ func (vm *VMKeeper) AddPackage(ctx sdk.Context, msg MsgAddPackage) error { // introduce enforcement of lexical/non-lexical // sort filenames based on Go conventions // slices.Sort sorts any type that supports the operators < <= >= > - slices.Sort(pkgNames) + sort.Strings(pkgNames) var memPkgSorted std.MemPackage pFiles := make([]*std.MemFile, 0, len(msg.Package.Files)) From 20be382e4bede21be0790bb37de6e3c5cc4c248e Mon Sep 17 00:00:00 2001 From: waymobetta Date: Thu, 1 Feb 2024 17:33:10 -0800 Subject: [PATCH 04/15] fix: comments --- gno.land/pkg/sdk/vm/keeper.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/gno.land/pkg/sdk/vm/keeper.go b/gno.land/pkg/sdk/vm/keeper.go index e38edc15076..5d173ff7d50 100644 --- a/gno.land/pkg/sdk/vm/keeper.go +++ b/gno.land/pkg/sdk/vm/keeper.go @@ -189,9 +189,8 @@ func (vm *VMKeeper) AddPackage(ctx sdk.Context, msg MsgAddPackage) error { ) } - // introduce enforcement of lexical/non-lexical // sort filenames based on Go conventions - // slices.Sort sorts any type that supports the operators < <= >= > + // https://cs.opensource.google/go/go/+/refs/tags/go1.21.6:src/sort/sort.go;l=39 sort.Strings(pkgNames) var memPkgSorted std.MemPackage From a191c935aedac1d2bcaf1b5ce05e6b911bf846b5 Mon Sep 17 00:00:00 2001 From: waymobetta Date: Mon, 5 Feb 2024 11:55:14 -0800 Subject: [PATCH 05/15] fix: refactor/optimize --- gno.land/pkg/sdk/vm/keeper.go | 55 +++++------------------------------ 1 file changed, 8 insertions(+), 47 deletions(-) diff --git a/gno.land/pkg/sdk/vm/keeper.go b/gno.land/pkg/sdk/vm/keeper.go index 5d173ff7d50..2a383f2f4e4 100644 --- a/gno.land/pkg/sdk/vm/keeper.go +++ b/gno.land/pkg/sdk/vm/keeper.go @@ -176,60 +176,21 @@ func (vm *VMKeeper) AddPackage(ctx sdk.Context, msg MsgAddPackage) error { return err } - // TODO // enforce sorting msg.Files based on Go conventions for predictability - pkgNames := []string{} - - // add std.MemFile.Name to slice -> sort - // create slice of file names - for _, pkgFile := range msg.Package.Files { - pkgNames = append( - pkgNames, - pkgFile.Name, // string - ) - } - - // sort filenames based on Go conventions - // https://cs.opensource.google/go/go/+/refs/tags/go1.21.6:src/sort/sort.go;l=39 - sort.Strings(pkgNames) - - var memPkgSorted std.MemPackage - pFiles := make([]*std.MemFile, 0, len(msg.Package.Files)) - - // add to new []*std.MemFile to enforce order - for _, pkgName := range pkgNames { - for _, nsFile := range msg.Package.Files { - // bail if not the most immediate pkgName - if nsFile.Name != pkgName { - continue - } - - pFiles = append( - pFiles, - nsFile, - ) - } - - memPkgSorted = std.MemPackage{ - Name: pkgName, - Path: msg.Package.Path, - Files: pFiles, - } - } - - // new instance of MsgAddPackage - msgSorted := MsgAddPackage{ - Creator: msg.Creator, - Package: &memPkgSorted, - Deposit: msg.Deposit, - } + // https://cs.opensource.google/go/go/+/refs/tags/go1.21.6:src/sort/sort.go;l=33 + sort.Slice( + msg.Package.Files, + func(i, j int) bool { + return msg.Package.Files[i].Name < msg.Package.Files[j].Name + }, + ) // Parse and run the files, construct *PV. msgCtx := stdlibs.ExecContext{ ChainID: ctx.ChainID(), Height: ctx.BlockHeight(), Timestamp: ctx.BlockTime().Unix(), - Msg: msgSorted, + Msg: msg, OrigCaller: creator.Bech32(), OrigSend: deposit, OrigSendSpent: new(std.Coins), From 7dc76fbf3283e4faa0561cdaaeaca6ac3a8ee7b6 Mon Sep 17 00:00:00 2001 From: waymobetta Date: Mon, 5 Feb 2024 16:57:30 -0800 Subject: [PATCH 06/15] fix: remove link --- gno.land/pkg/sdk/vm/keeper.go | 1 - 1 file changed, 1 deletion(-) diff --git a/gno.land/pkg/sdk/vm/keeper.go b/gno.land/pkg/sdk/vm/keeper.go index 2a383f2f4e4..b455e103d60 100644 --- a/gno.land/pkg/sdk/vm/keeper.go +++ b/gno.land/pkg/sdk/vm/keeper.go @@ -177,7 +177,6 @@ func (vm *VMKeeper) AddPackage(ctx sdk.Context, msg MsgAddPackage) error { } // enforce sorting msg.Files based on Go conventions for predictability - // https://cs.opensource.google/go/go/+/refs/tags/go1.21.6:src/sort/sort.go;l=33 sort.Slice( msg.Package.Files, func(i, j int) bool { From 9ee870e764bd2fa5ce6590490d95d79bb952f658 Mon Sep 17 00:00:00 2001 From: waymobetta Date: Fri, 1 Mar 2024 08:25:54 -0800 Subject: [PATCH 07/15] fix: move sort + refactor validate --- gno.land/pkg/sdk/vm/keeper.go | 9 --------- 1 file changed, 9 deletions(-) diff --git a/gno.land/pkg/sdk/vm/keeper.go b/gno.land/pkg/sdk/vm/keeper.go index b455e103d60..b7249f55665 100644 --- a/gno.land/pkg/sdk/vm/keeper.go +++ b/gno.land/pkg/sdk/vm/keeper.go @@ -7,7 +7,6 @@ import ( "fmt" "os" "regexp" - "sort" "strings" gno "github.com/gnolang/gno/gnovm/pkg/gnolang" @@ -176,14 +175,6 @@ func (vm *VMKeeper) AddPackage(ctx sdk.Context, msg MsgAddPackage) error { return err } - // enforce sorting msg.Files based on Go conventions for predictability - sort.Slice( - msg.Package.Files, - func(i, j int) bool { - return msg.Package.Files[i].Name < msg.Package.Files[j].Name - }, - ) - // Parse and run the files, construct *PV. msgCtx := stdlibs.ExecContext{ ChainID: ctx.ChainID(), From 3c645c1d480faae8fcadbd43375f1aa125e444a1 Mon Sep 17 00:00:00 2001 From: waymobetta Date: Fri, 1 Mar 2024 08:26:09 -0800 Subject: [PATCH 08/15] fix: move sort + refactor validate --- tm2/pkg/std/memfile.go | 28 ++++++++++++++++++++-------- 1 file changed, 20 insertions(+), 8 deletions(-) diff --git a/tm2/pkg/std/memfile.go b/tm2/pkg/std/memfile.go index 66ae854b65f..e62b5ec154b 100644 --- a/tm2/pkg/std/memfile.go +++ b/tm2/pkg/std/memfile.go @@ -3,6 +3,7 @@ package std import ( "fmt" "regexp" + "sort" "strings" "github.com/gnolang/gno/tm2/pkg/errors" @@ -50,22 +51,33 @@ var ( // file names must contain dots. // NOTE: this is to prevent conflicts with nested paths. func (mempkg *MemPackage) Validate() error { + // add assertion that MemPkg contains at least 1 file + if len(mempkg.Files) <= 0 { + return errors.New(fmt.Sprintf("no files found within package %q", mempkg.Name)) + } + if !rePkgName.MatchString(mempkg.Name) { return errors.New(fmt.Sprintf("invalid package name %q", mempkg.Name)) } if !rePkgOrRlmPath.MatchString(mempkg.Path) { return errors.New(fmt.Sprintf("invalid package/realm path %q", mempkg.Path)) } - fnames := map[string]struct{}{} - for _, memfile := range mempkg.Files { - if !reFileName.MatchString(memfile.Name) { - return errors.New(fmt.Sprintf("invalid file name %q", memfile.Name)) - } - if _, exists := fnames[memfile.Name]; exists { - return errors.New(fmt.Sprintf("duplicate file name %q", memfile.Name)) + // enforce sorting msg.Files based on Go conventions for predictability + sort.Slice( + mempkg.Files, + func(i, j int) bool { + return mempkg.Files[i].Name < mempkg.Files[j].Name + }, + ) + + prev := mempkg.Files[0].Name + for _, file := range mempkg.Files[1:] { + if prev == file.Name { + return errors.New(fmt.Sprintf("duplicate file name %q", file.Name)) } - fnames[memfile.Name] = struct{}{} + prev = file.Name } + return nil } From 1decefc9c4f0c8f212a7d1b3376616afce89208f Mon Sep 17 00:00:00 2001 From: jon roethke Date: Fri, 1 Mar 2024 08:30:26 -0800 Subject: [PATCH 09/15] fix: comment --- tm2/pkg/std/memfile.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tm2/pkg/std/memfile.go b/tm2/pkg/std/memfile.go index 3f14fcd1362..c19ad430f39 100644 --- a/tm2/pkg/std/memfile.go +++ b/tm2/pkg/std/memfile.go @@ -62,7 +62,7 @@ func (mempkg *MemPackage) Validate() error { if !rePkgOrRlmPath.MatchString(mempkg.Path) { return errors.New(fmt.Sprintf("invalid package/realm path %q, failed to match %q", mempkg.Path, rePkgOrRlmPath)) } - // enforce sorting msg.Files based on Go conventions for predictability + // enforce sorting files based on Go conventions for predictability sort.Slice( mempkg.Files, func(i, j int) bool { From 0cc62bd837fbd9de11e9f7ccb6c705c27e4b185b Mon Sep 17 00:00:00 2001 From: Morgan Bazalgette Date: Wed, 20 Mar 2024 19:00:49 +0100 Subject: [PATCH 10/15] change to checking if is sorted --- tm2/pkg/std/memfile.go | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/tm2/pkg/std/memfile.go b/tm2/pkg/std/memfile.go index c19ad430f39..3d83aad12b6 100644 --- a/tm2/pkg/std/memfile.go +++ b/tm2/pkg/std/memfile.go @@ -47,6 +47,14 @@ var ( reFileName = regexp.MustCompile(`^([a-zA-Z0-9_]*\.[a-z0-9_\.]*|LICENSE|README)$`) ) +func (mempkg *MemPackage) SortFiles() { + sort.Slice(mempkg.Files, mempkg.sortFunc) +} + +func (mempkg *MemPackage) sortFunc(i, j int) bool { + return mempkg.Files[i].Name < mempkg.Files[j].Name +} + // path must not contain any dots after the first domain component. // file names must contain dots. // NOTE: this is to prevent conflicts with nested paths. @@ -63,12 +71,13 @@ func (mempkg *MemPackage) Validate() error { return errors.New(fmt.Sprintf("invalid package/realm path %q, failed to match %q", mempkg.Path, rePkgOrRlmPath)) } // enforce sorting files based on Go conventions for predictability - sort.Slice( + sorted := sort.SliceIsSorted( mempkg.Files, - func(i, j int) bool { - return mempkg.Files[i].Name < mempkg.Files[j].Name - }, + mempkg.sortFunc, ) + if !sorted { + return fmt.Errorf("mempackage %q contains unsorted filenames", mempkg.Path) + } prev := mempkg.Files[0].Name for _, file := range mempkg.Files[1:] { From 731a2f9d33a065eddd0ed50490c7f5b1400c44e1 Mon Sep 17 00:00:00 2001 From: Morgan Bazalgette Date: Wed, 20 Mar 2024 19:07:16 +0100 Subject: [PATCH 11/15] remove unused func --- tm2/pkg/std/memfile.go | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/tm2/pkg/std/memfile.go b/tm2/pkg/std/memfile.go index 3d83aad12b6..a1b66ead7b2 100644 --- a/tm2/pkg/std/memfile.go +++ b/tm2/pkg/std/memfile.go @@ -47,14 +47,6 @@ var ( reFileName = regexp.MustCompile(`^([a-zA-Z0-9_]*\.[a-z0-9_\.]*|LICENSE|README)$`) ) -func (mempkg *MemPackage) SortFiles() { - sort.Slice(mempkg.Files, mempkg.sortFunc) -} - -func (mempkg *MemPackage) sortFunc(i, j int) bool { - return mempkg.Files[i].Name < mempkg.Files[j].Name -} - // path must not contain any dots after the first domain component. // file names must contain dots. // NOTE: this is to prevent conflicts with nested paths. @@ -73,7 +65,9 @@ func (mempkg *MemPackage) Validate() error { // enforce sorting files based on Go conventions for predictability sorted := sort.SliceIsSorted( mempkg.Files, - mempkg.sortFunc, + func(i, j int) bool { + return mempkg.Files[i].Name < mempkg.Files[j].Name + }, ) if !sorted { return fmt.Errorf("mempackage %q contains unsorted filenames", mempkg.Path) @@ -81,6 +75,9 @@ func (mempkg *MemPackage) Validate() error { prev := mempkg.Files[0].Name for _, file := range mempkg.Files[1:] { + if !reFileName.MatchString(file.Name) { + return fmt.Errorf("invalid file name %q, failed to match %q", memfile.Name, reFileName) + } if prev == file.Name { return errors.New(fmt.Sprintf("duplicate file name %q", file.Name)) } From e10e0c39967002f6354fc7035b31a39ecca6f31f Mon Sep 17 00:00:00 2001 From: Morgan Bazalgette Date: Wed, 20 Mar 2024 19:08:39 +0100 Subject: [PATCH 12/15] change all to errorf --- tm2/pkg/std/memfile.go | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/tm2/pkg/std/memfile.go b/tm2/pkg/std/memfile.go index a1b66ead7b2..2980961ec53 100644 --- a/tm2/pkg/std/memfile.go +++ b/tm2/pkg/std/memfile.go @@ -5,8 +5,6 @@ import ( "regexp" "sort" "strings" - - "github.com/gnolang/gno/tm2/pkg/errors" ) type MemFile struct { @@ -53,14 +51,14 @@ var ( func (mempkg *MemPackage) Validate() error { // add assertion that MemPkg contains at least 1 file if len(mempkg.Files) <= 0 { - return errors.New(fmt.Sprintf("no files found within package %q", mempkg.Name)) + return fmt.Errorf("no files found within package %q", mempkg.Name) } if !rePkgName.MatchString(mempkg.Name) { - return errors.New(fmt.Sprintf("invalid package name %q, failed to match %q", mempkg.Name, rePkgName)) + return fmt.Errorf("invalid package name %q, failed to match %q", mempkg.Name, rePkgName) } if !rePkgOrRlmPath.MatchString(mempkg.Path) { - return errors.New(fmt.Sprintf("invalid package/realm path %q, failed to match %q", mempkg.Path, rePkgOrRlmPath)) + return fmt.Errorf("invalid package/realm path %q, failed to match %q", mempkg.Path, rePkgOrRlmPath) } // enforce sorting files based on Go conventions for predictability sorted := sort.SliceIsSorted( @@ -76,10 +74,10 @@ func (mempkg *MemPackage) Validate() error { prev := mempkg.Files[0].Name for _, file := range mempkg.Files[1:] { if !reFileName.MatchString(file.Name) { - return fmt.Errorf("invalid file name %q, failed to match %q", memfile.Name, reFileName) + return fmt.Errorf("invalid file name %q, failed to match %q", file.Name, reFileName) } if prev == file.Name { - return errors.New(fmt.Sprintf("duplicate file name %q", file.Name)) + return fmt.Errorf("duplicate file name %q", file.Name) } prev = file.Name } From 483c662234efdf68598fe05a6b7b6b6e65fdebd2 Mon Sep 17 00:00:00 2001 From: Morgan Bazalgette Date: Wed, 20 Mar 2024 19:10:16 +0100 Subject: [PATCH 13/15] perform check on first file, too --- tm2/pkg/std/memfile.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tm2/pkg/std/memfile.go b/tm2/pkg/std/memfile.go index 2980961ec53..2887718cc06 100644 --- a/tm2/pkg/std/memfile.go +++ b/tm2/pkg/std/memfile.go @@ -71,12 +71,12 @@ func (mempkg *MemPackage) Validate() error { return fmt.Errorf("mempackage %q contains unsorted filenames", mempkg.Path) } - prev := mempkg.Files[0].Name - for _, file := range mempkg.Files[1:] { + var prev string + for i, file := range mempkg.Files { if !reFileName.MatchString(file.Name) { return fmt.Errorf("invalid file name %q, failed to match %q", file.Name, reFileName) } - if prev == file.Name { + if i > 0 && prev == file.Name { return fmt.Errorf("duplicate file name %q", file.Name) } prev = file.Name From db7aca28b76dd284069199f38684a745ac7cd296 Mon Sep 17 00:00:00 2001 From: Morgan Bazalgette Date: Wed, 20 Mar 2024 19:15:58 +0100 Subject: [PATCH 14/15] add test to check for failure --- tm2/pkg/std/memfile.go | 2 +- tm2/pkg/std/memfile_test.go | 44 +++++++++++++++++++++++++++++++++++++ 2 files changed, 45 insertions(+), 1 deletion(-) create mode 100644 tm2/pkg/std/memfile_test.go diff --git a/tm2/pkg/std/memfile.go b/tm2/pkg/std/memfile.go index 2887718cc06..5935428ac28 100644 --- a/tm2/pkg/std/memfile.go +++ b/tm2/pkg/std/memfile.go @@ -68,7 +68,7 @@ func (mempkg *MemPackage) Validate() error { }, ) if !sorted { - return fmt.Errorf("mempackage %q contains unsorted filenames", mempkg.Path) + return fmt.Errorf("mempackage %q has unsorted files", mempkg.Path) } var prev string diff --git a/tm2/pkg/std/memfile_test.go b/tm2/pkg/std/memfile_test.go new file mode 100644 index 00000000000..02f3fef3d49 --- /dev/null +++ b/tm2/pkg/std/memfile_test.go @@ -0,0 +1,44 @@ +package std + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestMemPackage_Validate(t *testing.T) { + tt := []struct { + name string + mpkg *MemPackage + errContains string + }{ + { + "Correct", + &MemPackage{ + Name: "hey", + Path: "gno.land/r/demo/hey", + Files: []*MemFile{{Name: "a.gno"}}, + }, + "", + }, + { + "Unsorted", + &MemPackage{ + Name: "hey", + Path: "gno.land/r/demo/hey", + Files: []*MemFile{{Name: "b.gno"}, {Name: "a.gno"}}, + }, + `mempackage "gno.land/r/demo/hey" has unsorted files`, + }, + } + for _, tc := range tt { + t.Run(tc.name, func(t *testing.T) { + err := tc.mpkg.Validate() + if tc.errContains == "" { + assert.NoError(t, err) + } else { + assert.ErrorContains(t, err, tc.errContains) + } + }) + } +} From 9e8b415bc009b8a4a64bed772f83aff3e2100f97 Mon Sep 17 00:00:00 2001 From: Morgan Bazalgette Date: Wed, 20 Mar 2024 19:19:43 +0100 Subject: [PATCH 15/15] new test for duplicate files --- tm2/pkg/std/memfile_test.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/tm2/pkg/std/memfile_test.go b/tm2/pkg/std/memfile_test.go index 02f3fef3d49..44031cd1b8b 100644 --- a/tm2/pkg/std/memfile_test.go +++ b/tm2/pkg/std/memfile_test.go @@ -30,6 +30,15 @@ func TestMemPackage_Validate(t *testing.T) { }, `mempackage "gno.land/r/demo/hey" has unsorted files`, }, + { + "Duplicate", + &MemPackage{ + Name: "hey", + Path: "gno.land/r/demo/hey", + Files: []*MemFile{{Name: "a.gno"}, {Name: "a.gno"}}, + }, + `duplicate file name "a.gno"`, + }, } for _, tc := range tt { t.Run(tc.name, func(t *testing.T) {