tp: translate classic flags to subcommand dispatch#5149
Open
LalitMaganti wants to merge 75 commits intomainfrom
Open
tp: translate classic flags to subcommand dispatch#5149LalitMaganti wants to merge 75 commits intomainfrom
LalitMaganti wants to merge 75 commits intomainfrom
Conversation
Adds a SubcommandContext struct to subcommand.h that provides subcommands with access to the PlatformInterface. Updates the Run() signature to take it, and fixes the unit test accordingly. This is prep work for wiring subcommands into the main dispatcher.
Add the common_flags module which extracts shared global flag parsing (--dev, --full-sort, etc.) and trace processor setup into reusable helpers. Add the query subcommand implementation and wire up the subcommand dispatcher in TraceProcessorShell::Run.
…mmand Address PR review feedback: - Declarative FlagSpec system: subcommands declare flags as data via GetFlags(), eliminating per-subcommand getopt boilerplate and auto-generating --help output. - base::Status return from Subcommand::Run() enabling RETURN_IF_ERROR throughout. - Classic-to-modern translation layer: classic -q/-Q flags are translated into query subcommand invocations rather than maintaining a separate execution path. RunClassic remains as fallback for untranslatable combinations (httpd, summary, metrics, etc). - DuckDB-style query interface per RFC discussion: * Inline: query trace.pb "SELECT ..." * Stdin: query trace.pb < file.sql * File: query -f file.sql trace.pb * Stdin: query -f - trace.pb - RETURN_IF_ERROR/ASSIGN_OR_RETURN in RunClassic. - Out/inout args by pointer per style guide. - Replaced GLOBAL_LONG_OPTIONS macro with GetGlobalLongOptions(). - Removed unnecessary protobuf_full dep from query_subcommand.
Add a subcommand-based CLI alongside the existing classic flag interface (RFC-0018). The classic path is completely untouched; a thin dispatcher at the top of Run() checks argv for a subcommand name and routes to it, otherwise falls through to classic. New infrastructure: - FlagSpec/StringFlag/BoolFlag: declarative flag definitions with handlers, used by ParseFlags() for centralized getopt + auto help. - GlobalOptions: shared config extracted into common_flags.h/cc (BuildConfig, SetupTraceProcessor, LoadTraceFile). - Subcommand base class: GetFlags() + Run() returning base::Status. Query subcommand supports DuckDB-style invocations: query trace.pb "SELECT ..." (positional inline SQL) query trace.pb < file.sql (stdin pipe) query -f file.sql trace.pb (file argument) query -f - trace.pb (explicit stdin)
Add two new subcommands to trace_processor_shell using the declarative flag system: - interactive: interactive SQL shell (renamed from repl per RFC-0018) - server: RPC server with http/stdio modes (renamed from serve) All subcommand sources are folded into the single :subcommand BUILD.gn target.
interactive_subcommand.cc includes interactive.h which includes metrics.h which needs protobuf headers. Add the direct dep.
Add two new subcommands to trace_processor_shell using the declarative flag system: - summarize: run trace summarization with specs, metrics-v2, etc. - export: export trace to sqlite database file All sources folded into the single :subcommand BUILD.gn target.
Add metrics subcommand to trace_processor_shell using the declarative flag system. Supports --run, --pre, --output, --extension (multi-valued), --post-query, and --interactive.
Add top-level help with subcommand descriptions, `help <command>` for per-subcommand usage, `--help-classic` for legacy flag reference, and file collision detection hint when a positional arg matches both a subcommand name and a file on disk. Also restore classic CLI integration tests that were removed in earlier PRs.
Merge all separate source_sets in shell/BUILD.gn (subcommand, interactive, metatrace, metrics, query, shell_utils, sql_packages, common_flags) into a single :shell target. Update parent BUILD.gn to reference "shell" instead of individual targets.
After running the query, if -i is set, drop into the interactive shell. This allows the classic -q/-Q -i combination to work through the subcommand path.
Instead of a separate code path for classic flags (-q, -Q, --httpd, --stdiod, --summary, --run-metrics, -e, etc.), translate them into the equivalent subcommand invocation and re-dispatch. This removes ~300 lines of duplicated logic (trace loading, config building, metric handling, etc.) which now lives exclusively in the subcommand implementations. Translation examples: tp -q file.sql trace.pb -> query -f file.sql trace.pb tp -Q "SQL" trace.pb -> query trace.pb "SQL" tp --httpd trace.pb -> server http trace.pb tp --stdiod -> server stdio tp --summary trace.pb -> summarize trace.pb tp --run-metrics x trace.pb -> metrics --run x trace.pb tp -e out.db trace.pb -> export sqlite -o out.db trace.pb tp trace.pb -> interactive trace.pb
🎨 Perfetto UI Builds
|
Cover translation paths with robust assertions: - bare trace -> interactive (computed result 123 from 100+23) - -q file -i -> query -f file -i (computed result 579 from 500+79) - -Q "SQL" -W -> query -W (computed result 357 from 300+57) - -Q "SQL" -> query (computed result 261 from 200+61) - -q file -> query -f file (computed result 476 from 400+76) - --stdiod -> server stdio (RPC protocol check) - --stdiod trace -> server stdio trace (clean exit) - --run-metrics -> metrics --run (textproto output check) - -e + -q -> error (checks "Cannot combine" message) - -p perf -Q "SQL" -> query -p (verifies perf file written) - --summary -q file -> summarize --post-query (computed 731) - --run-metrics -q file -> metrics --post-query (computed 819) All assertions use computed values (e.g. SELECT 100+23) where the result (123) cannot appear in the input SQL, preventing false positives from echoed input or stderr messages.
- Remove short flag -p from query subcommand's --perf-file (avoid collision with classic -p flag) - Add --perf-file support to metrics and summarize subcommands (needed by diff test infrastructure) - Forward --perf-file in classic-to-subcommand translation for metrics and summarize paths - Strengthen integration test assertions with computed values
The classic flag parser accepted --metatrace-categories but the translation layer did not forward it to the synthetic subcommand argv, causing the categories to be silently dropped. Store the raw categories string during classic parsing and forward it as --metatrace-categories in add_global_flags().
The old classic code path allowed -Q (query string) to run after summary/metrics as an independent step, but the subcommand system only supports --post-query (file). Rather than silently dropping the query string, emit an explicit error guiding users to -q.
The summarize subcommand only checked file extension (.pb) to determine if a spec file was binary protobuf. The old code had a GuessSummarySpecFormat function that also sniffed file content as a fallback, which is needed when spec files come from temp files with no extension (e.g. diff test harness). Restore the content-sniffing logic so binary spec files without a .pb extension are correctly detected.
…l-summarize-export
…l-summarize-export
…hell-build-consolidate
…tp-shell-classic-translation
…l-summarize-export
…hell-build-consolidate
…tp-shell-classic-translation
…hell-build-consolidate
…tp-shell-classic-translation
…hell-build-consolidate
…tp-shell-classic-translation
…hell-build-consolidate
…tp-shell-classic-translation
…hell-build-consolidate
…tp-shell-classic-translation
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Summary
-i(interactive after query) in QuerySubcommandTest plan
query -iproves interactive shell runs after query