New issue
Advanced search Search tips

Issue 864576 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Aug 22
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome , Mac
Pri: 2
Type: Bug



Sign in to add a comment

Split up json_schema_api.gni

Project Member Reported by rdevlin....@chromium.org, Jul 17

Issue description

tools/json_schema_compiler/json_schema_api.gni is used in the generation of extensions API code, and can generate three different types of output:
- Generated C++ types from a schema file (like [1])
- Generated extension function registration (like [2])
- Generated JSON strings (like [3])

These are all vaguely related, but reasonably separate actions.  Having them all combined into a single gni template results in it being much more difficult to understand both the template (which has a bunch of `if (this type of action) { }`) and the invocations of the template.

Instead, we should simply have three different gni templates, one for generating each type of output.

[1] https://chromium.googlesource.com/chromium/src/out/+/108618872a04398bd0574dd0923aa1128be0414c/Debug/gen/chrome/common/extensions/api/tabs.h
[2] https://chromium.googlesource.com/chromium/src/out/+/108618872a04398bd0574dd0923aa1128be0414c/Debug/gen/extensions/browser/api/generated_api_registration.cc
[3] https://chromium.googlesource.com/chromium/src/out/+/108618872a04398bd0574dd0923aa1128be0414c/Debug/gen/extensions/common/api/generated_schemas.cc
 
Project Member

Comment 1 by bugdroid1@chromium.org, Jul 25

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6ecc07f6868ada18e8e23abeb54ad18b4cdeb439

commit 6ecc07f6868ada18e8e23abeb54ad18b4cdeb439
Author: Devlin Cronin <rdevlin.cronin@chromium.org>
Date: Wed Jul 25 22:23:21 2018

[Extensions Schema Compiler] Clean up json_schema_api.gni

Do some quick clean up of json_schema_compiler.gni before splitting
it out into multiple targets.
- Remove ineffective visibility assignments and update docs
- Remove stale references (generate_static_library) and unused
  options (output_name).

Bug:  864576 
Change-Id: I395bfcc0f222cfdae269f222a7b4d28e2fcdeed9
Reviewed-on: https://chromium-review.googlesource.com/1140504
Commit-Queue: Devlin <rdevlin.cronin@chromium.org>
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Reviewed-by: Istiaque Ahmed <lazyboy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#578098}
[modify] https://crrev.com/6ecc07f6868ada18e8e23abeb54ad18b4cdeb439/tools/json_schema_compiler/json_schema_api.gni

Project Member

Comment 2 by bugdroid1@chromium.org, Aug 6

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7ecf29239647c9dee5948fc34737e1a51dafc388

commit 7ecf29239647c9dee5948fc34737e1a51dafc388
Author: Devlin Cronin <rdevlin.cronin@chromium.org>
Date: Mon Aug 06 03:49:56 2018

[Extensions Schema Compiler] Extract C++ Type Generation

Extract C++ type generation from the json_schema_api gni
template into a separate generate_types gni template. This has the
advantage of cleaning up which arguments are necessary and used for
the template, as well as being able to isolate the different
generation steps (which may have different dependencies or
dependents).

As part of this, introduce separate "bulk" `api` targets in each
of the API directories, which each dependent can depend on. This
obviates the need to depend on e.g. `api` and `api_registration`.

A follow up will break up the JSON string generation bundle and the
extension function registration bundle.

Bug:  864576 

Change-Id: I50a1fbe059ec5b7d73c405b230a221c1c6ce3af1
Reviewed-on: https://chromium-review.googlesource.com/1150930
Commit-Queue: Devlin <rdevlin.cronin@chromium.org>
Reviewed-by: Luke Halliwell <halliwell@chromium.org>
Reviewed-by: Albert Chaulk <achaulk@chromium.org>
Reviewed-by: Istiaque Ahmed <lazyboy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#580799}
[modify] https://crrev.com/7ecf29239647c9dee5948fc34737e1a51dafc388/chrome/common/extensions/api/BUILD.gn
[modify] https://crrev.com/7ecf29239647c9dee5948fc34737e1a51dafc388/chromecast/browser/BUILD.gn
[modify] https://crrev.com/7ecf29239647c9dee5948fc34737e1a51dafc388/chromecast/common/BUILD.gn
[modify] https://crrev.com/7ecf29239647c9dee5948fc34737e1a51dafc388/chromecast/common/extensions_api/BUILD.gn
[modify] https://crrev.com/7ecf29239647c9dee5948fc34737e1a51dafc388/extensions/common/api/BUILD.gn
[modify] https://crrev.com/7ecf29239647c9dee5948fc34737e1a51dafc388/extensions/shell/BUILD.gn
[modify] https://crrev.com/7ecf29239647c9dee5948fc34737e1a51dafc388/extensions/shell/common/api/BUILD.gn
[modify] https://crrev.com/7ecf29239647c9dee5948fc34737e1a51dafc388/tools/json_schema_compiler/json_schema_api.gni
[modify] https://crrev.com/7ecf29239647c9dee5948fc34737e1a51dafc388/tools/json_schema_compiler/test/BUILD.gn

Project Member

Comment 3 by bugdroid1@chromium.org, Aug 22

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0a4a371a768a65931d796451e21d2f59a0cbfd71

commit 0a4a371a768a65931d796451e21d2f59a0cbfd71
Author: Devlin Cronin <rdevlin.cronin@chromium.org>
Date: Wed Aug 22 01:48:31 2018

[Extensions Schema Compiler] Break up bundle generation

Break up extension API bundle generation into two targets separate
gn templates: generated_json_strings and function_registration. This
simplifies the flow of control within the template, as well as the
number of bespoke arguments that the template needs to be invoked
with.

With this, each of the json_schema_api generating actions are now
separate.

Bug:  864576 

Change-Id: I126e2f746137b89ac879e35252a16667f2f4367d
Reviewed-on: https://chromium-review.googlesource.com/1170427
Reviewed-by: Istiaque Ahmed <lazyboy@chromium.org>
Reviewed-by: Luke Halliwell <halliwell@chromium.org>
Commit-Queue: Devlin <rdevlin.cronin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#584947}
[modify] https://crrev.com/0a4a371a768a65931d796451e21d2f59a0cbfd71/chrome/browser/extensions/api/BUILD.gn
[modify] https://crrev.com/0a4a371a768a65931d796451e21d2f59a0cbfd71/chrome/common/extensions/api/BUILD.gn
[modify] https://crrev.com/0a4a371a768a65931d796451e21d2f59a0cbfd71/chromecast/common/extensions_api/BUILD.gn
[modify] https://crrev.com/0a4a371a768a65931d796451e21d2f59a0cbfd71/extensions/browser/api/BUILD.gn
[modify] https://crrev.com/0a4a371a768a65931d796451e21d2f59a0cbfd71/extensions/common/api/BUILD.gn
[modify] https://crrev.com/0a4a371a768a65931d796451e21d2f59a0cbfd71/extensions/shell/common/api/BUILD.gn
[modify] https://crrev.com/0a4a371a768a65931d796451e21d2f59a0cbfd71/tools/json_schema_compiler/json_schema_api.gni

Status: Fixed (was: Started)

Sign in to add a comment