Skip to content

Commit

Permalink
Don't use templating to allow for dead-code elimination (#228)
Browse files Browse the repository at this point in the history
When templating is used, the linker cannot know which functions are
called and which ones are not; this is because templating uses 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]>
  • Loading branch information
marckhouzam authored Feb 19, 2025
1 parent d7a1929 commit 2075d4a
Show file tree
Hide file tree
Showing 3 changed files with 76 additions and 11 deletions.
6 changes: 5 additions & 1 deletion plugin/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
65 changes: 65 additions & 0 deletions plugin/root_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,11 @@
package plugin

import (
"os"
"os/exec"
"path/filepath"
"runtime"
"strings"
"testing"

"github.com/stretchr/testify/assert"
Expand All @@ -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")
}
}
16 changes: 6 additions & 10 deletions plugin/usage.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ package plugin

import (
"fmt"
"os"
"strings"
"text/template"

Expand All @@ -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.
Expand Down Expand Up @@ -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:"
Expand Down Expand Up @@ -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,
Expand Down

0 comments on commit 2075d4a

Please sign in to comment.