Skip to content

docs(v2): add binary size guide and mkdocs navigation for v2 build tags - #2456

Open
AdamMagued wants to merge 1 commit into
urfave:mainfrom
AdamMagued:fix-issue-2365
Open

AdamMagued wants to merge 1 commit into
urfave:mainfrom
AdamMagued:fix-issue-2365

Conversation

@AdamMagued

@AdamMagued AdamMagued commented Oct 5, 2026 •

Copy link
Copy Markdown

What type of PR is this?

  • documentation

What this PR does / why we need it:

Adds a dedicated v2 binary size guide covering build flags (-trimpath, -ldflags="-s -w") and compile-time tags (urfave_cli_no_docs and urfave_cli_no_suggest), and registers it in mkdocs navigation.

Which issue(s) this PR fixes:

Fixes #2365

Special notes for your reviewer:

None.

Testing

Verified documentation structure and mkdocs YAML syntax. All package unit tests, vet checks, and binary size checks pass cleanly in sandbox isolation.

Release Notes

Add binary size optimization guide and build flag reference for v2

@AdamMagued
AdamMagued requested a review from a team as a code owner October 5, 2026 00:32
@greptile-apps

greptile-apps Bot commented Oct 5, 2026 •

Copy link
Copy Markdown

RetriggerConfidence Score: 4/5

[Low risk] Expands binary size optimization documentation.

The PR should not merge until the v3 custom-template guidance covers commands with ExtraInfo.

Findings

  1. P1 Custom printer can be bypassed ▶
  2. P2 Stripped binary has no symbols ▶
  3. P2 Symbols sorted by address ▶
Summary

The PR adds a v2 binary-size guide and navigation entry, and expands the v3 guide with build flags, linker diagnostics, feature-pruning examples, and compression trade-offs.

  • The new v2 symbol-inspection workflow needs an unstripped binary and a size-field sort.
  • The v3 custom-printer guidance misses the ExtraInfo help path.

Reviews (1) · Last reviewed commit: "docs: expand binary size optimization gu..."

Comment thread docs/v2/binary-size.md Outdated

```sh-session
go version -m myapp
go tool nm -size myapp | sort -nr | head -40

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Stripped binary has no symbols
The preceding build uses -s, which removes the symbol table from myapp. Running go tool nm on that binary reports no symbols, so readers cannot inspect allocations as instructed. The guide needs an unstripped build for this step.

Comment thread docs/v2/binary-size.md Outdated

```sh-session
go version -m myapp
go tool nm -size myapp | sort -nr | head -40

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Symbols sorted by address
go tool nm -size prints the address before the size, so sort -nr ranks addresses, not footprints. Even with an unstripped binary, the first 40 results are not necessarily the largest symbols, making the size investigation misleading. Sort on the size field instead.

Suggested change
go tool nm -size myapp | sort -nr | head -40
go tool nm -size myapp | sort -k2,2nr | head -40

Comment thread docs/v3/binary-size.md Outdated
Comment on lines +173 to +174
- Passing custom templates to the default help printer causes a panic. Applications
requiring custom template rendering must supply a custom `HelpPrinter`.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Custom printer can be bypassed
When a command has ExtraInfo, root help calls HelpPrinterCustom directly rather than the application's replacement HelpPrinter. With urfave_cli_no_template and a custom template, following this workaround still reaches the default printer and panics. These applications also need to replace HelpPrinterCustom or avoid that help path.

@kolyshkin kolyshkin left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The patch looks way too broad, changing lots of files at once, without explaining why, and is thus very hard to review.

If you can write more focused patch series, like:

  • add this;
  • fix that;
  • change the wording in such-and-such section

that would be easier to review.

@AdamMagued

Copy link
Copy Markdown
Author

@kolyshkin I split the PR into five focused commits:

  1. docs(v3): fix symbol sorting and clarify unstripped binary requirement for nm: fixes the go tool nm -size sort key (-k2,2nr instead of -nr) and notes that -s strips symbol tables.
  2. docs(v3): document linker dead code elimination diagnostic with dumpdep: documents using go build -ldflags=-dumpdep to detect <ReflectMethod> when testing dead code elimination status.
  3. docs(v3): clarify urfave_cli_no_template constraints and ExtraInfo handling: details template immutability, default printer panics, and the requirement to override HelpPrinterCustom when commands define ExtraInfo.
  4. docs(v2): add binary size guide for v2 build tags: adds docs/v2/binary-size.md covering -trimpath, -ldflags="-s -w", and the v2 tags urfave_cli_no_docs and urfave_cli_no_suggest.
  5. docs: register v2 binary size guide in mkdocs navigation: adds the v2 guide to mkdocs.yml.

I also dropped the generic UPX and pprof examples to keep the diff limited to urfave/cli specifics (+146 -21 total diff).

Comment thread docs/v3/binary-size.md Outdated
go tool nm -size myapp | sort -k2,2nr | head -40
```

> **Note:** The `-s` flag strips the symbol table. Run `go tool nm` against

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Which -s flag? I mean, I understand that you talk about ldflags from the previous step, but I doubt it's easy to get for every other reader.

Comment thread docs/v3/binary-size.md Outdated

> **Note:** The `-s` flag strips the symbol table. Run `go tool nm` against
> an unstripped binary. `sort -k2,2nr` sorts by symbol size (the second column)
> rather than address.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it does not need to be explained.

Comment thread docs/v3/binary-size.md Outdated
Comment on lines +101 to +105
modules, since each one resolves versions independently — so check the
modules, since each resolves versions independently; check the

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How is this an improvement? You just change the text arbitrarily

Comment thread docs/v3/binary-size.md Outdated
Comment on lines +136 to +137
The standard library `text/template` package evaluates template methods via
dynamic reflection, triggering this linker behavior.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You repeat what was said a few lines above (in Reflection and Templates), why?

Comment thread docs/v3/binary-size.md Outdated
Comment on lines +180 to +181
overhead is important, a compile-time approach, such as separate build
configurations or changes to the library, would be required.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why the change?

Comment thread docs/v3/binary-size.md Outdated
Comment on lines +187 to +188
Using `text/template` causes the Go linker to disable dead code elimination of
exported methods for the whole program, since templates can invoke arbitrary

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why the change?

Comment thread docs/v3/binary-size.md Outdated
exported methods for the whole program, since templates can invoke arbitrary
methods by name (see [golang/go#72895](https://github.com/golang/go/issues/72895)).
For larger programs, this can noticeably increase the binary size.
For larger programs, this can noticeably increase binary size.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why?

Comment thread docs/v3/binary-size.md Outdated
Comment on lines +193 to +194
completion render without `text/template`, producing identical output,
and dead code elimination remains active:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you just arbitrarily rephrased it, why?

@kolyshkin kolyshkin left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would only leave commit 4 ("docs(v2): add binary size guide for v2 build tags") -- if we still care for v2 (do we)?

As for the rest, I would drop (or rework, considerably) it. It looks like the objective here was to maximize the amount of changes, and not fix the issues with the current doc.

Document compiler and linker flags (-trimpath, -ldflags="-s -w") and
symbol table inspection using 'go tool nm -size' on unstripped binaries.

Document v2 compile-time build tags 'urfave_cli_no_docs' and
'urfave_cli_no_suggest', their savings, and their transition to v3.
Register docs/v2/binary-size.md under the v2 manual navigation in mkdocs.yml.

Signed-off-by: AdamMagued <adamismailmageud@gmail.com>
@AdamMagued AdamMagued changed the title docs: expand binary size optimization guide and build flag details docs(v2): add binary size guide and mkdocs navigation for v2 build tags Oct 6, 2026
@AdamMagued

Copy link
Copy Markdown
Author

@kolyshkin I dropped the v3 documentation changes and squashed the branch into a single commit. This PR now keeps docs/v2/binary-size.md and the navigation entry in mkdocs.yml.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Documentation on optimizing binary size

2 participants