script: Do not lose flags when retrying command

Since the parsing of the flags overwrite 'cmd.args' this caused
the retry of the command to lose the flags. Keep the original
args to the side and use those when parsing the arguments.

Signed-off-by: Jussi Maki <[email protected]>

script/engine.go

@@ -320,7 +320,8 @@ func (e *Engine) Execute(s *State, file string, script *bufio.Reader, log io.Wri
 				regexpArgs = usage.RegexpArgs(rawArgs...)
 			}
 		}
-		cmd.args = expandArgs(s, cmd.rawArgs, regexpArgs)
+		cmd.origArgs = expandArgs(s, cmd.rawArgs, regexpArgs)
+		cmd.args = cmd.origArgs
 
 		// Run the command.
 		err = e.runCommand(s, cmd, impl)
@@ -436,6 +437,7 @@ type command struct {
 	conds      []condition // all must be satisfied
 	name       string      // the name of the command; must be non-empty
 	rawArgs    [][]argFragment
+	origArgs   []string // original arguments before pflag parsing
 	args       []string // shell-expanded arguments following name
 	background bool     // command should run in background (ends with a trailing &)
 }
@@ -695,7 +697,7 @@ func (e *Engine) runCommand(s *State, cmd *command, impl Cmd) error {
 	if usage.Flags != nil {
 		usage.Flags(fs)
 	}
-	if err := fs.Parse(cmd.args); err != nil {
+	if err := fs.Parse(cmd.origArgs); err != nil {
 		if errors.Is(err, pflag.ErrHelp) {
 			out := new(strings.Builder)
 			err = e.ListCmds(out, true, "^"+cmd.name+"$")

script/scripttest/scripttest_test.go

@@ -6,13 +6,15 @@ package scripttest_test
 
 import (
 	"context"
+	"errors"
 	"os"
 	"strings"
 	"testing"
 	"time"
 
 	"github.com/cilium/hive/script"
 	"github.com/cilium/hive/script/scripttest"
+	"github.com/spf13/pflag"
 )
 
 func TestAll(t *testing.T) {
@@ -33,6 +35,37 @@ func TestAll(t *testing.T) {
 					return
 				}, nil
 			})
+
+		engine.Cmds["retrytest"] = script.Command(
+			// This is a simple test command to verify that the flags are not
+			// misplaced when retrying a command.
+			script.CmdUsage{
+				Flags: func(fs *pflag.FlagSet) {
+					fs.String("not-empty", "", "this should not be empty")
+				},
+			},
+			func(s *script.State, args ...string) (script.WaitFunc, error) {
+				notEmpty, err := s.Flags.GetString("not-empty")
+				if err != nil {
+					return nil, err
+				}
+				if len(notEmpty) == 0 {
+					return nil, errors.New("not-empty is empty")
+				}
+				if len(args) != 1 || args[0] != "abc" {
+					return nil, errors.New("expected one arg 'abc'")
+				}
+
+				// Check if the file was already created (this command ran already)
+				// otherwise create it so we succeed second time.
+				_, err = os.Stat(s.Path("retrytest"))
+				if err == nil {
+					return nil, nil
+				}
+				os.WriteFile(s.Path("retrytest"), []byte("xxx"), 0644)
+				return nil, errors.New("retrytest")
+			},
+		)
 		return engine
 	}, env, "testdata/*.txt")
 }

script/scripttest/testdata/retry.txt

@@ -0,0 +1,4 @@
+# Test command retrying and the flag parsing when retrying.
+# The 'retrytest' command should run two times and the flags
+# should not be lost.
+* retrytest --not-empty=xyz abc