Bugfix: run all persistent pre-runs and post-runs of all parents

Currently, only one of the persistent pre-runs and post-runs is executed.
It is always the first one found in the parents chain, starting at this command.
Expected behavior is to execute all parents' persistent pre-runs and post-runs.

Dependent projects implemented various workarounds for this:
- manually building persistent hook chains (in every hook).
- applying some kind of monkey-patching on top of Cobra.

This change eliminates the necessity for such workarounds.

Note: when merged, commands which built hook chains manually
may call parents' persistent pre-runs and post-runs a more than once.
This is not a big deal when persistent hooks are used properly.
Otherwise, such projects need to be modified.

Based on the ticket history there is a limited number of projects which need to update:
- spf13#216
- spf13#252

Signed-off-by: Volodymyr Khoroz <volodymyr.khoroz@foundries.io>

command.go

@@ -934,15 +934,18 @@ func (c *Command) execute(a []string) (err error) {
 		return err
 	}
 
+	// Execute all persistent pre-runs from root parent till this command.
+	parents := make([]*Command, 0, 5)
 	for p := c; p != nil; p = p.Parent() {
+		parents = append([]*Command{p}, parents...)
+	}
+	for _, p := range parents {
 		if p.PersistentPreRunE != nil {
 			if err := p.PersistentPreRunE(c, argWoFlags); err != nil {
 				return err
 			}
-			break
 		} else if p.PersistentPreRun != nil {
 			p.PersistentPreRun(c, argWoFlags)
-			break
 		}
 	}
 	if c.PreRunE != nil {
@@ -974,15 +977,14 @@ func (c *Command) execute(a []string) (err error) {
 	} else if c.PostRun != nil {
 		c.PostRun(c, argWoFlags)
 	}
+	// Execute all persistent post-runs from this command till the root parent.
 	for p := c; p != nil; p = p.Parent() {
 		if p.PersistentPostRunE != nil {
 			if err := p.PersistentPostRunE(c, argWoFlags); err != nil {
 				return err
 			}
-			break
 		} else if p.PersistentPostRun != nil {
 			p.PersistentPostRun(c, argWoFlags)
-			break
 		}
 	}
 

command_test.go

@@ -1546,41 +1546,53 @@ func TestPersistentHooks(t *testing.T) {
 		childPersPostArgs string
 	)
 
+	var hookRunOrder []string
+
 	parentCmd := &Command{
 		Use: "parent",
 		PersistentPreRun: func(_ *Command, args []string) {
 			parentPersPreArgs = strings.Join(args, " ")
+			hookRunOrder = append(hookRunOrder, "parent PersistentPreRun")
 		},
 		PreRun: func(_ *Command, args []string) {
 			parentPreArgs = strings.Join(args, " ")
+			hookRunOrder = append(hookRunOrder, "parent PreRun")
 		},
 		Run: func(_ *Command, args []string) {
 			parentRunArgs = strings.Join(args, " ")
+			hookRunOrder = append(hookRunOrder, "parent Run")
 		},
 		PostRun: func(_ *Command, args []string) {
 			parentPostArgs = strings.Join(args, " ")
+			hookRunOrder = append(hookRunOrder, "parent PostRun")
 		},
 		PersistentPostRun: func(_ *Command, args []string) {
 			parentPersPostArgs = strings.Join(args, " ")
+			hookRunOrder = append(hookRunOrder, "parent PersistentPostRun")
 		},
 	}
 
 	childCmd := &Command{
 		Use: "child",
 		PersistentPreRun: func(_ *Command, args []string) {
 			childPersPreArgs = strings.Join(args, " ")
+			hookRunOrder = append(hookRunOrder, "child PersistentPreRun")
 		},
 		PreRun: func(_ *Command, args []string) {
 			childPreArgs = strings.Join(args, " ")
+			hookRunOrder = append(hookRunOrder, "child PreRun")
 		},
 		Run: func(_ *Command, args []string) {
 			childRunArgs = strings.Join(args, " ")
+			hookRunOrder = append(hookRunOrder, "child Run")
 		},
 		PostRun: func(_ *Command, args []string) {
 			childPostArgs = strings.Join(args, " ")
+			hookRunOrder = append(hookRunOrder, "child PostRun")
 		},
 		PersistentPostRun: func(_ *Command, args []string) {
 			childPersPostArgs = strings.Join(args, " ")
+			hookRunOrder = append(hookRunOrder, "child PersistentPostRun")
 		},
 	}
 	parentCmd.AddCommand(childCmd)
@@ -1597,19 +1609,9 @@ func TestPersistentHooks(t *testing.T) {
 		name string
 		got  string
 	}{
-		// TODO: currently PersistentPreRun* defined in parent does not
-		// run if the matching child subcommand has PersistentPreRun.
-		// If the behavior changes (https://github.com/spf13/cobra/issues/252)
-		// this test must be fixed.
-		{"parentPersPreArgs", parentPersPreArgs},
 		{"parentPreArgs", parentPreArgs},
 		{"parentRunArgs", parentRunArgs},
 		{"parentPostArgs", parentPostArgs},
-		// TODO: currently PersistentPostRun* defined in parent does not
-		// run if the matching child subcommand has PersistentPostRun.
-		// If the behavior changes (https://github.com/spf13/cobra/issues/252)
-		// this test must be fixed.
-		{"parentPersPostArgs", parentPersPostArgs},
 	} {
 		if v.got != "" {
 			t.Errorf("Expected blank %s, got %q", v.name, v.got)
@@ -1620,6 +1622,8 @@ func TestPersistentHooks(t *testing.T) {
 		name string
 		got  string
 	}{
+		{"parentPersPreArgs", parentPersPreArgs},
+		{"parentPersPostArgs", parentPersPostArgs},
 		{"childPersPreArgs", childPersPreArgs},
 		{"childPreArgs", childPreArgs},
 		{"childRunArgs", childRunArgs},
@@ -1630,6 +1634,24 @@ func TestPersistentHooks(t *testing.T) {
 			t.Errorf("Expected %s %q, got %q", v.name, onetwo, v.got)
 		}
 	}
+
+	for idx, exp := range []string{
+		"parent PersistentPreRun",
+		"child PersistentPreRun",
+		"child PreRun",
+		"child Run",
+		"child PostRun",
+		"child PersistentPostRun",
+		"parent PersistentPostRun",
+	} {
+		if len(hookRunOrder) > idx {
+			if act := hookRunOrder[idx]; act != exp {
+				t.Errorf("Expected %q at %d, got %q", exp, idx, act)
+			}
+		} else {
+			t.Errorf("Expected %q at %d, got nothing", exp, idx)
+		}
+	}
 }
 
 // Related to https://github.com/spf13/cobra/issues/521.