Don't use templating to allow for dead-code elimination

When templating is used, the linker cannot know which functions are
called and which one are not; this is because tamplating use calls to
"MethodByName()" which can be used to call any function (similar to
reflection).

With the release of Cobra 1.9.1, templates are no longer used by default
in Cobra. By also not using templates in the tanzu-plugin-runtime it now
gives an opportunity for plugins to try to avoid templates to allow
dead-code elimination to work, if they so choose.

Ref: spf13/cobra#1956

Signed-off-by: Marc Khouzam <[email protected]>

Author: marckhouzam

plugin/root.go

@@ -45,8 +45,12 @@ func newRootCmd(descriptor *PluginDescriptor) *cobra.Command {
 			cobra.CommandDisplayNameAnnotation: cmdName,
 		},
 	}
+	// Instead of using templates, use go functions.
+	// This allows for dead-code-elimination.
+	// The below call will set the format for both usage and help printouts.
+	cmd.SetUsageFunc(UsageFunc)
+	// Keep this call for backwards-compatibility, in case a plugin uses the templating
 	cobra.AddTemplateFuncs(TemplateFuncs)
-	cmd.SetUsageTemplate(cmdTemplate)
 
 	cmd.AddCommand(
 		newDescribeCmd(descriptor.Description),

plugin/root_test.go

@@ -4,6 +4,11 @@
 package plugin
 
 import (
+	"os"
+	"os/exec"
+	"path/filepath"
+	"runtime"
+	"strings"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -27,3 +32,63 @@ func Test_newRootCmd(t *testing.T) {
 	assert.Equal("Test Plugin", cmd.Use)
 	assert.Equal(("Description of the plugin"), cmd.Short)
 }
+
+// TestDeadcodeElimination checks that a simple program using the tanzu-plugin-runtime
+// is linked taking full advantage of the linker's deadcode elimination step.
+//
+// If reflect.Value.MethodByName/reflect.Value.Method are reachable the
+// linker will not always be able to prove that exported methods are
+// unreachable, making deadcode elimination less effective. Using
+// text/template and html/template makes reflect.Value.MethodByName
+// reachable.
+//
+// This test checks that those function can be proven to be unreachable by
+// the linker.
+//
+// Taken from https://github.com/spf13/cobra/blob/f98cf4216d3cb5235e6e0cd00ee00959deb1dc65/cobra_test.go#L245
+// See also: https://github.com/spf13/cobra/pull/1956
+func TestDeadcodeElimination(t *testing.T) {
+	if runtime.GOOS == "windows" {
+		t.Skip("go tool nm fails on windows")
+	}
+
+	// check that a simple program using tanzu-plugin-runtime is
+	// linked with deadcode elimination enabled.
+	const (
+		dirname  = "test_deadcode"
+		progname = "test_deadcode_elimination"
+	)
+	_ = os.Mkdir(dirname, 0770)
+	defer os.RemoveAll(dirname)
+	filename := filepath.Join(dirname, progname+".go")
+	err := os.WriteFile(filename, []byte(`package main
+
+import "github.com/vmware-tanzu/tanzu-plugin-runtime/plugin"
+
+func main() {
+	p, _ := plugin.NewPlugin(&plugin.PluginDescriptor{
+		Name:        "deadcode-check",
+		Description: "some desc",
+		Group:       "Manage",
+		Target:      "global",
+		Version:     "v0.0.0",
+	})
+	_ = p.Execute()
+}
+`), 0600)
+	if err != nil {
+		t.Fatalf("could not write test program: %v", err)
+	}
+	buf, err := exec.Command("go", "build", filename).CombinedOutput()
+	if err != nil {
+		t.Fatalf("could not compile test program: %s", string(buf))
+	}
+	defer os.Remove(progname)
+	buf, err = exec.Command("go", "tool", "nm", progname).CombinedOutput()
+	if err != nil {
+		t.Fatalf("could not run go tool nm: %v", err)
+	}
+	if strings.Contains(string(buf), "MethodByName") {
+		t.Error("compiled programs contains MethodByName symbol")
+	}
+}

plugin/usage.go

@@ -5,7 +5,6 @@ package plugin
 
 import (
 	"fmt"
-	"os"
 	"strings"
 	"text/template"
 
@@ -17,11 +16,11 @@ import (
 
 // UsageFunc is the usage func for a plugin.
 var UsageFunc = func(c *cobra.Command) error {
-	t, err := template.New("usage").Funcs(TemplateFuncs).Parse(cmdTemplate)
-	if err != nil {
-		return err
-	}
-	return t.Execute(os.Stdout, c)
+	// Instead of using templates, use a go function to generate the usage string.
+	// This allows for dead-code-elimination.
+	helpMsg := printHelp(c)
+	_, err := fmt.Fprintf(c.OutOrStdout(), "%s", helpMsg)
+	return err
 }
 
 // CmdTemplate is the template for plugin commands.
@@ -50,9 +49,6 @@ const CmdTemplate = `{{ bold "Usage:" }}
 {{ $target := index .Annotations "target" }}{{ if or (eq $target "kubernetes") (eq $target "k8s") }}Use "{{if beginsWith .CommandPath "tanzu "}}{{.CommandPath}}{{else}}tanzu {{.CommandPath}}{{end}} [command] --help" for more information about a command.{{end}}Use "{{if beginsWith .CommandPath "tanzu "}}{{.CommandPath}}{{else}}tanzu{{ $target := index .Annotations "target" }}{{ if and (ne $target "global") (ne $target "") }} {{ $target }} {{ else }} {{ end }}{{.CommandPath}}{{end}} [command] --help" for more information about a command.{{end}}
 `
 
-// cmdTemplate is the template for plugin commands.
-const cmdTemplate = `{{ printHelp . }}`
-
 // Constants for help text labels
 const (
 	usageStr                = "Usage:"
@@ -367,9 +363,9 @@ func printHelp(cmd *cobra.Command) string {
 
 // TemplateFuncs are the template usage funcs.
 var TemplateFuncs = template.FuncMap{
-	"printHelp": printHelp,
 	// The below are not needed but are kept for backwards-compatibility
 	// in case it is being used through the API
+	"printHelp":               printHelp,
 	"rpad":                    stringutils.Rpad,
 	"bold":                    stringutils.Sboldf,
 	"underline":               stringutils.Sunderlinef,