New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 691081 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
please use my google.com address
Closed: Feb 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 1
Type: Bug



Sign in to add a comment

analyze didn't think typemaps affect a build

Project Member Reported by jam@chromium.org, Feb 10 2017

Issue description

See this cl: https://codereview.chromium.org/2685343003/
I only changed mojo typemaps. However analyze didn't think a build was necessary, which is mistaken.
 

Comment 1 by roc...@chromium.org, Feb 10 2017

We use GN's read_file to extract data from typemap files - perhaps analyze
overlooks this form of dependency?

Comment 2 by jam@chromium.org, Feb 10 2017

Cc: brettw@chromium.org
What target(s) would you have expected to be affected? Do you see the typemaps as either build or data deps of them?

Comment 4 by jam@chromium.org, Feb 10 2017

content should have been rebuilt.

Re data deps, I presumed that gn should figure this out automatically without having to list them in the gn files?

Comment 5 by jam@chromium.org, Feb 10 2017

could this be similar to  bug 672244  or  bug 639328  ?

Comment 6 by roc...@chromium.org, Feb 10 2017

So yeah I'm guessing it's because the only dependency on these files is via
read_file calls. Typemapping is hard. mojom.gni reads (in the global scope)
every typemap file listed in the global bindings configurations (
https://cs.chromium.org/chromium/src/mojo/public/tools/bindings/mojom.gni?rcl=0ddb4374a9221dba28e9ea8a1edaacb32dcc0861&l=40
)

Then when processing a mojom target, we scan its list of source mojom files
to look for matches in the set of typemap entries (each typemap file
specifies the name of the mojom file it regards), and this is used to
inject additional metadata in the generated target(s) based on the active
typemap configuration for the target (chromium or blink).

If read_file dependencies don't translate to build dependencies given that
the contents of the read file can influence build behavior, that seems like
a bug?

One option could be to add any typemap files we use for a target, to the
set of inputs on its generation action, even though the typemap files
aren't actually used by the generator script.
It looks like those files are being read outside of any particular target or template, so GN wouldn't know what targets are actually affected by them.

(And no, this is unrelated to the grit issues).
Status: Available (was: Unconfirmed)

Comment 9 by roc...@chromium.org, Feb 10 2017

Owner: roc...@chromium.org
Status: Assigned (was: Available)
I will fix this using magic.
Project Member

Comment 10 by bugdroid1@chromium.org, Feb 13 2017

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

commit ba3fc0dad59f9be26ce99f2aae4626cfc0330d2d
Author: rockot <rockot@chromium.org>
Date: Mon Feb 13 17:58:25 2017

Mojo Bindings: Put typemap files in the build dependency tree

This ensures that any typemap configuration files used to configure an
internal type-mappings-generator target are included in that target's
set of inputs. Otherwise this dependency only comes in the form of
read_file() calls made outside of any target scope, and GN fails to
deduce the effect this has on mojom targets.

BUG= 691081 

Review-Url: https://codereview.chromium.org/2691483005
Cr-Commit-Position: refs/heads/master@{#450002}

[modify] https://crrev.com/ba3fc0dad59f9be26ce99f2aae4626cfc0330d2d/mojo/public/tools/bindings/blink_bindings_configuration.gni
[modify] https://crrev.com/ba3fc0dad59f9be26ce99f2aae4626cfc0330d2d/mojo/public/tools/bindings/chromium_bindings_configuration.gni
[modify] https://crrev.com/ba3fc0dad59f9be26ce99f2aae4626cfc0330d2d/mojo/public/tools/bindings/mojom.gni

Status: Fixed (was: Assigned)

Sign in to add a comment