Don't use templating to allow for dead-code elimination #228
+76
−11
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
marckhouzam
force-pushed
the
marck/allowDeadCodeElimination
branch
2 times, most recently
from
d241e6b to
1a6dfb6
Compare
marckhouzam
force-pushed
the
marck/allowDeadCodeElimination
branch
from
1a6dfb6 to
72c4312
Compare
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: spf13/cobra#1956 Signed-off-by: Marc Khouzam <[email protected]>
marckhouzam
force-pushed
the
marck/allowDeadCodeElimination
branch
from
72c4312 to
7e3ff24
Compare
prkalle
approved these changes
Feb 19, 2025
ajitgunturi
pushed a commit
to ajitgunturi/tanzu-plugin-runtime
that referenced
this pull request
Feb 25, 2025
…#228) 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]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What this PR does / why we need it
This change has no visible effect to the behaviour of plugins, except for allowing them to possibly reduce their binary size.
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.
This PR therefore removes the use of templates in tanzu-plugin-runtime.
Note that we had technically already moved to using go code instead of templates but we had just kept the template engine calling the go code.
Note that the new
TestDeadcodeEliminationtest is meant to notify us if templates are added. However, if any new dependency used by tanzu-plugin-runtime uses templates or reflection, it would also turn off dead-code elimination and fail the test. If there is a justification for that to happen, we will just need to remove the test.Ref: spf13/cobra#1956
Which issue(s) this PR fixes
Fixes # N/A
Describe testing done for PR
I built a plugin using this new PR and verified that both its usage and help printouts were not changed:
Release note
Additional information
Special notes for your reviewer