Skip to content

Make simple_json_parser a perfetto_component - #6909

Open
MichaelFQuigley wants to merge 5 commits into
mainfrom
dev/michaelquigley/dev/michaelquigley/simple-json-parser-header-only
Open

Make simple_json_parser a perfetto_component#6909
MichaelFQuigley wants to merge 5 commits into
mainfrom
dev/michaelquigley/dev/michaelquigley/simple-json-parser-header-only

Conversation

@MichaelFQuigley

@MichaelFQuigley MichaelFQuigley commented Jul 28, 2026

Copy link
Copy Markdown

Make simple_json_parser a perfetto_component.

This allows it to become a cc_library and be shared as a dependency in multiple
libraries while avoiding ODR violations.

This is in preparation for an upcoming change to parse JSON comments from
Proguard mapping (.map) files, such as:

# {"id":"com.android.tools.r8.mergedClasses", "class_id_field": "$cid", "merged_classes": [{ "name": "ClassA", "class_id": 0 }, { "name": "ClassB", "class_id": 1 }]}

Move implementation of array helper methods into simple_json_parser.h
to make simple_json_parser a header-only source set and avoid ODR
violations when included by multiple components.

This is in preparation for an upcoming change to parse JSON comments from
Proguard mapping (.map) files, such as:
# {"id":"com.android.tools.r8.mergedClasses", "class_id_field": "$cid", "merged_classes": [{ "name": "ClassA", "class_id": 0 }, { "name": "ClassB", "class_id": 1 }]}
@MichaelFQuigley
MichaelFQuigley requested a review from a team as a code owner July 28, 2026 18:51
@github-actions

github-actions Bot commented Jul 28, 2026

Copy link
Copy Markdown

@LalitMaganti LalitMaganti left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Sorry I'm not understanding the motivation for this patch. Is there a specific. Error etc you are working around?

There should not be any odr issue from have a C++ version of simplejsonpaser as a cc - if there was, we would have really big problems in a lot of places.

@MichaelFQuigley

Copy link
Copy Markdown
Author

I encountered this error when trying to upload #6911:

ODR violation in target //src/trace_processor:trace_processor_shell_lib, multiple paths include //src/trace_processor/util:simple_json_parser:
  //src/trace_processor:trace_processor_shell_lib (via //src/trace_processor:trace_processor_shell_lib)
  //src/trace_processor/util/deobfuscation:deobfuscator (via //src/trace_processor/util/deobfuscation:deobfuscator)
Traceback (most recent call last):
  File "/usr/local/google/home/michaelquigley/perfetto/tools/gen_bazel", line 1289, in <module>
    sys.exit(main())
             ~~~~^^
  File "/usr/local/google/home/michaelquigley/perfetto/tools/gen_bazel", line 1263, in main
    contents = generate_build(desc, desc_win, desc_android, args.targets or
                              default_targets, extras)
  File "/usr/local/google/home/michaelquigley/perfetto/tools/gen_bazel", line 1201, in generate_build
    checker = gn_utils.ODRChecker(gn, target_name)
  File "/usr/local/google/home/michaelquigley/perfetto/tools/gn_utils.py", line 327, in __init__
    raise Exception('%d ODR violations detected. Build generation aborted' %
                    num_violations)
Exception: 1 ODR violations detected. Build generation aborted

i.e. SimpleJsonParser's impl was linked into both the //src/trace_processor/util/deobfuscation:deobfuscator and //src/trace_processor:lib libraries in #6911.

I'm a bit new to Bazel-specific nuances, so apologies if there is a more idiomatic way to handle this.

@LalitMaganti

Copy link
Copy Markdown
Member

We should make this a perfetto_component as the correct way to solve that problem.

…r-header-only' of github.com:google/perfetto into dev/michaelquigley/simple-json-parser-header-only
@MichaelFQuigley

Copy link
Copy Markdown
Author

Thanks. Uploaded.

@MichaelFQuigley MichaelFQuigley changed the title Make simple_json_parser header only Make simple_json_parser a perfetto_component Jul 28, 2026
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.

2 participants