From 7e3ff249f97335dd993a01294fffe5218307d154 Mon Sep 17 00:00:00 2001 From: Marc Khouzam Date: Mon, 17 Feb 2025 21:42:32 -0500 Subject: [PATCH] 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 templating 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: https://github.com/spf13/cobra/pull/1956 Signed-off-by: Marc Khouzam --- plugin/root.go | 6 ++++- plugin/root_test.go | 65 +++++++++++++++++++++++++++++++++++++++++++++ plugin/usage.go | 16 +++++------ 3 files changed, 76 insertions(+), 11 deletions(-) diff --git a/plugin/root.go b/plugin/root.go index 13aa1ce3b..cfbca2d98 100644 --- a/plugin/root.go +++ b/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), diff --git a/plugin/root_test.go b/plugin/root_test.go index 372a3110f..30375a355 100644 --- a/plugin/root_test.go +++ b/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") + } +} diff --git a/plugin/usage.go b/plugin/usage.go index d51a8658b..365966c5d 100644 --- a/plugin/usage.go +++ b/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,