Skip missing translations when generating JSON resources.

Skip missing translations when generating JSON resources.

It doesn't make sense to replace missing translations with english
strings because Chrome can do this automatically when getting
resources.

BUG=369572
Review-Url: https://codereview.chromium.org/1676793002
Cr-Original-Original-Commit-Position: refs/heads/master@{#480994}
Committed: https://chromium.googlesource.com/chromium/src/+/d2f53be161b01a4cf04dfa387b66bda710f601c0
Review-Url: https://codereview.chromium.org/1676793002
Cr-Original-Commit-Position: refs/heads/master@{#482826}
Committed: https://chromium.googlesource.com/chromium/src/+/1deaebd297b7212932be4afe34ae0b60c45bd9fd
Review-Url: https://codereview.chromium.org/1676793002
Cr-Commit-Position: refs/heads/master@{#483512}
Committed: https://chromium.googlesource.com/chromium/src/+/af0dc05d501cafad6ce25a775ada2559cabd12f0

[Sergey Ulanov, 2016-02-06 01:03:45]

sergeyu@chromium.org changed reviewers:
+ thakis@chromium.org

[Sergey Ulanov, 2016-02-23 18:17:39]

thakis: ping

[Nico, 2016-02-23 21:44:50]

I don't understand this change. Chrome could do this at runtime, but we want to
be able to fail the build on missing strings for official builds, else we won't
detect missing strings. Doesn't this change make that impossible?

[Sergey Ulanov, 2016-02-24 18:08:03]

On 2016/02/23 21:44:50, Nico wrote:
> I don't understand this change. Chrome could do this at runtime, but we want
to
> be able to fail the build on missing strings for official builds, else we
won't
> detect missing strings. Doesn't this change make that impossible?

We have a script that's ran as part of the release process for the CRD app. It
verifies the JSON files generated by grit to make sure all strings are
translated for all languages. It's not part of the build process. I don't think
we ever want to fail grit during build time, even for official builds. E.g. it's
acceptable to have beta builds with some translations missing.

[Sergey Ulanov, 2017-05-24 00:06:05]

thakis: ping

[Nico, 2017-05-24 13:48:54]

On 2016/02/24 18:08:03, Sergey Ulanov wrote:
> On 2016/02/23 21:44:50, Nico wrote:
> > I don't understand this change. Chrome could do this at runtime, but we want
> to
> > be able to fail the build on missing strings for official builds, else we
> won't
> > detect missing strings. Doesn't this change make that impossible?
> 
> We have a script that's ran as part of the release process for the CRD app. It
> verifies the JSON files generated by grit to make sure all strings are
> translated for all languages. It's not part of the build process.

Do all other things that use json output have scripts like this too?

Maybe you could put this behavior behind an opt-in flag?

> I don't think
> we ever want to fail grit during build time, even for official builds. E.g.
it's
> acceptable to have beta builds with some translations missing.

[Sergey Ulanov, 2017-05-24 18:41:49]

Patchset #2 (id:20001) has been deleted

[Sergey Ulanov, 2017-05-24 18:42:27]

Patchset #2 (id:40001) has been deleted

[Sergey Ulanov, 2017-05-24 18:47:14]

On 2017/05/24 13:48:54, Nico wrote:
> Do all other things that use json output have scripts like this too?

I don't think so, but all places where json generator is used set
fallback_to_english="true", see
https://codesearch.chromium.org/search/?q=type%3D%22chrome_messages_json%22+f...
. When fallback_to_english flag is set grit doesn't generate any errors for
missing strings. 

> 
> Maybe you could put this behavior behind an opt-in flag?

I've updated this change to keep the old behavior when
fallback_to_english=false. It also simplified this change a bit.

[Sergey Ulanov, 2017-06-13 23:44:24]

thakis: ping!

[Nico, 2017-06-15 21:18:13]

I have this paged out again, but iirc the state of the discussion was:

- there's more than 1 client of this script
- this diagnostic is in the way of one client
- it's useful for the other clients

Is that correct? If so, just removing it seems bad for all but the 1 client.

[Sergey Ulanov, 2017-06-15 21:36:56]

On 2017/06/15 21:18:13, Nico wrote:
> I have this paged out again, but iirc the state of the discussion was:
> 
> - there's more than 1 client of this script
> - this diagnostic is in the way of one client
> - it's useful for the other clients

The last item is incorrect. It makes no difference for other clients because
they all set fallback_to_english="true".

> 
> Is that correct? If so, just removing it seems bad for all but the 1 client.

[Sergey Ulanov, 2017-06-15 21:39:20]

On 2017/06/15 21:36:56, Sergey Ulanov wrote:
> On 2017/06/15 21:18:13, Nico wrote:
> > I have this paged out again, but iirc the state of the discussion was:
> > 
> > - there's more than 1 client of this script
> > - this diagnostic is in the way of one client
> > - it's useful for the other clients
> 
> The last item is incorrect. It makes no difference for other clients because
> they all set fallback_to_english="true".

And also if there were clients that didn't set fallback_to_english="true" then
this change would not affect them either, as I'm not changing the current
behavior for fallback_to_english="false" (in the last version of this change).

> 
> > 
> > Is that correct? If so, just removing it seems bad for all but the 1 client.

[Sergey Ulanov, 2017-06-15 21:47:09]

To clarify it further:
1. Nothing changes when for clients that set fallback_to_english = false.
2. When fallback_to_english=true, currently the fallback is handled by grd. It
doesn't report any errors for missing translations in that case.
3. After this change the fallback to english will be handled by chrome instead
of by grit when generating .json resources.

Currently all grd files that use JSON generator set fallback_to_english="true",
i.e. grit will never report missing translations for these files. This change
makes it possible to implement this check outside of grit.

[Sergey Ulanov, 2017-06-19 17:13:15]

thakis: ping

[Sergey Ulanov, 2017-06-20 21:42:22]

pingly ping

[Nico, 2017-06-20 21:43:59]

lgtm, thanks for explaining. Sorry about the long holdup.

[Sergey Ulanov, 2017-06-20 21:51:54]

The CQ bit was checked by sergeyu@chromium.org

[commit-bot: I haz the power, 2017-06-20 21:52:18]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-06-20 22:30:32]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1497995514663000,
"parent_rev": "d98c08f75e5c26f49a9e05e9d0946a2f6d29d3f0", "commit_rev":
"d2f53be161b01a4cf04dfa387b66bda710f601c0"}

[commit-bot: I haz the power, 2017-06-20 22:30:45]

Description was changed from

==========
Skip missing translations when generating JSON resources.

It doesn't make sense to replace missing translations with english
strings because Chrome can do this automatically when getting
resources.

BUG=369572
==========

to

==========
Skip missing translations when generating JSON resources.

It doesn't make sense to replace missing translations with english
strings because Chrome can do this automatically when getting
resources.

BUG=369572

Review-Url: https://codereview.chromium.org/1676793002
Cr-Commit-Position: refs/heads/master@{#480994}
Committed:
https://chromium.googlesource.com/chromium/src/+/d2f53be161b01a4cf04dfa387b66...
==========

[commit-bot: I haz the power, 2017-06-20 22:30:46]

Committed patchset #2 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/d2f53be161b01a4cf04dfa387b66...

[pdr., 2017-06-20 22:56:46]

On 2017/06/20 at 22:30:46, commit-bot wrote:
> Committed patchset #2 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/d2f53be161b01a4cf04dfa387b66...

Could this be causing this build failure?

https://luci-logdog.appspot.com/v/?s=chromium%2Fbb%2Fchromium.win%2FWin_Build...

FAILED: gen/remoting/host/win/remoting_host_messages.mc 
C:/b/depot_tools/python276_bin/python.exe
../../remoting/tools/build/remoting_localize.py --locale_dir
gen/remoting/webapp/_locales --template
../../remoting/host/win/host_messages.mc.jinja2 --output
gen/remoting/host/win/remoting_host_messages.mc --encoding utf-16 am ar bg bn ca
cs da de el en en-GB es et fa fake-bidi fi fil fr gu he hi hr hu id it ja kn ko
lt lv ml mr ms nb nl pl pt-PT ro ru sk sl sr sv sw ta te th tr uk vi zh-CN zh-TW
pt-BR es-419
Traceback (most recent call last):
  File "../../remoting/tools/build/remoting_localize.py", line 794, in <module>
    sys.exit(DoMain(sys.argv[1:]))
  File "../../remoting/tools/build/remoting_localize.py", line 791, in DoMain
    return Localize(options.template, locales, options)
  File "../../remoting/tools/build/remoting_localize.py", line 731, in Localize
    WriteIfChanged(options.output, template.render(context), options.encoding)
  File "C:\b\c\b\win\src\third_party\jinja2\environment.py", line 989, in render
    return self.environment.handle_exception(exc_info, True)
  File "C:\b\c\b\win\src\third_party\jinja2\environment.py", line 754, in
handle_exception
    reraise(exc_type, exc_value, tb)
  File "../../remoting/host/win\host_messages.mc.jinja2", line 26, in top-level
template code
    {% trans %}HOST_CATEGORY{% endtrans %}
  File "../../remoting/tools/build/remoting_localize.py", line 642, in <lambda>
    return lambda message: self.GetText(message)
  File "../../remoting/tools/build/remoting_localize.py", line 630, in GetText
    return self.message_map[self.language][message]
KeyError: u'HOST_CATEGORY'

[findit-for-me, 2017-06-20 23:05:07]

A revert of this CL (patchset #2 id:60001) has been created in
https://codereview.chromium.org/2948893002/ by
findit-for-me@appspot.gserviceaccount.com.

The reason for reverting is: 
Findit (https://goo.gl/kROfz5) identified CL at revision 480994 as the
culprit for failures in the build cycles as shown on:
https://findit-for-me.appspot.com/waterfall/culprit?key=ag9zfmZpbmRpdC1mb3Itb....

[Sergey Ulanov, 2017-06-21 00:24:11]

Description was changed from

==========
Skip missing translations when generating JSON resources.

It doesn't make sense to replace missing translations with english
strings because Chrome can do this automatically when getting
resources.

BUG=369572

Review-Url: https://codereview.chromium.org/1676793002
Cr-Commit-Position: refs/heads/master@{#480994}
Committed:
https://chromium.googlesource.com/chromium/src/+/d2f53be161b01a4cf04dfa387b66...
==========

to

==========
Skip missing translations when generating JSON resources.

It doesn't make sense to replace missing translations with english
strings because Chrome can do this automatically when getting
resources.

BUG=369572

Review-Url: https://codereview.chromium.org/1676793002
Cr-Commit-Position: refs/heads/master@{#480994}
Committed:
https://chromium.googlesource.com/chromium/src/+/d2f53be161b01a4cf04dfa387b66...
==========

[Sergey Ulanov, 2017-06-28 00:37:31]

Fixed the problem that broke build on windows in
https://chromium-review.googlesource.com/c/549835/ . Relanding

[Sergey Ulanov, 2017-06-28 00:37:41]

The CQ bit was checked by sergeyu@chromium.org

[commit-bot: I haz the power, 2017-06-28 00:38:07]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-06-28 00:52:31]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1498610261039710,
"parent_rev": "d113d16354bdf66f1471a131cac316e4ddb68e4d", "commit_rev":
"095dd0be63a91436a1e4ea8b9da502d03d208349"}

[commit-bot: I haz the power, 2017-06-28 00:52:47]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1498610261039710,
"parent_rev": "e2856777652263b14611173aea65024db6a27919", "commit_rev":
"1deaebd297b7212932be4afe34ae0b60c45bd9fd"}

[commit-bot: I haz the power, 2017-06-28 00:53:04]

Description was changed from

==========
Skip missing translations when generating JSON resources.

It doesn't make sense to replace missing translations with english
strings because Chrome can do this automatically when getting
resources.

BUG=369572

Review-Url: https://codereview.chromium.org/1676793002
Cr-Commit-Position: refs/heads/master@{#480994}
Committed:
https://chromium.googlesource.com/chromium/src/+/d2f53be161b01a4cf04dfa387b66...
==========

to

==========
Skip missing translations when generating JSON resources.

It doesn't make sense to replace missing translations with english
strings because Chrome can do this automatically when getting
resources.

BUG=369572

Review-Url: https://codereview.chromium.org/1676793002
Cr-Original-Commit-Position: refs/heads/master@{#480994}
Committed:
https://chromium.googlesource.com/chromium/src/+/d2f53be161b01a4cf04dfa387b66...
Review-Url: https://codereview.chromium.org/1676793002
Cr-Commit-Position: refs/heads/master@{#482826}
Committed:
https://chromium.googlesource.com/chromium/src/+/1deaebd297b7212932be4afe34ae...
==========

[commit-bot: I haz the power, 2017-06-28 00:53:05]

Committed patchset #2 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/1deaebd297b7212932be4afe34ae...

[pdr., 2017-06-28 01:00:18]

On 2017/06/28 at 00:53:05, commit-bot wrote:
> Committed patchset #2 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/1deaebd297b7212932be4afe34ae...

I think this may be causing the compile to fail again:
https://luci-logdog.appspot.com/v/?s=chromium%2Fbb%2Fchromium.linux%2FLinux_B...

[Sergey Ulanov, 2017-06-28 01:00:54]

A revert of this CL (patchset #2 id:60001) has been created in
https://codereview.chromium.org/2960853003/ by sergeyu@chromium.org.

The reason for reverting is: Broke windows build:
https://luci-logdog.appspot.com/v/?s=chromium%2Fbb%2Fchromium.linux%2FLinux_B....

[Sergey Ulanov, 2017-06-28 01:07:58]

Description was changed from

==========
Skip missing translations when generating JSON resources.

It doesn't make sense to replace missing translations with english
strings because Chrome can do this automatically when getting
resources.

BUG=369572

Review-Url: https://codereview.chromium.org/1676793002
Cr-Original-Commit-Position: refs/heads/master@{#480994}
Committed:
https://chromium.googlesource.com/chromium/src/+/d2f53be161b01a4cf04dfa387b66...
Review-Url: https://codereview.chromium.org/1676793002
Cr-Commit-Position: refs/heads/master@{#482826}
Committed:
https://chromium.googlesource.com/chromium/src/+/1deaebd297b7212932be4afe34ae...
==========

to

==========
Skip missing translations when generating JSON resources.

It doesn't make sense to replace missing translations with english
strings because Chrome can do this automatically when getting
resources.

BUG=369572

Review-Url: https://codereview.chromium.org/1676793002
Cr-Original-Commit-Position: refs/heads/master@{#480994}
Committed:
https://chromium.googlesource.com/chromium/src/+/d2f53be161b01a4cf04dfa387b66...
Review-Url: https://codereview.chromium.org/1676793002
Cr-Commit-Position: refs/heads/master@{#482826}
Committed:
https://chromium.googlesource.com/chromium/src/+/1deaebd297b7212932be4afe34ae...
==========

[Sergey Ulanov, 2017-06-28 18:38:37]

The CQ bit was checked by sergeyu@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-06-28 18:38:58]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-06-28 19:25:18]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-06-28 19:25:19]

Dry run: Try jobs failed on following builders:
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[Sergey Ulanov, 2017-06-29 20:47:12]

The CQ bit was checked by sergeyu@chromium.org

[Sergey Ulanov, 2017-06-29 20:47:12]

The patchset sent to the CQ was uploaded after l-g-t-m from thakis@chromium.org
Link to the patchset: https://codereview.chromium.org/1676793002/#ps80001
(title: "fix fake-bidi support")

[commit-bot: I haz the power, 2017-06-29 20:47:57]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-06-29 22:11:23]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1498769232051220,
"parent_rev": "b394d18f263bb63178bcfee2e248f96d0c593f42", "commit_rev":
"af0dc05d501cafad6ce25a775ada2559cabd12f0"}

[commit-bot: I haz the power, 2017-06-29 22:11:39]

Description was changed from

==========
Skip missing translations when generating JSON resources.

It doesn't make sense to replace missing translations with english
strings because Chrome can do this automatically when getting
resources.

BUG=369572

Review-Url: https://codereview.chromium.org/1676793002
Cr-Original-Commit-Position: refs/heads/master@{#480994}
Committed:
https://chromium.googlesource.com/chromium/src/+/d2f53be161b01a4cf04dfa387b66...
Review-Url: https://codereview.chromium.org/1676793002
Cr-Commit-Position: refs/heads/master@{#482826}
Committed:
https://chromium.googlesource.com/chromium/src/+/1deaebd297b7212932be4afe34ae...
==========

to

==========
Skip missing translations when generating JSON resources.

It doesn't make sense to replace missing translations with english
strings because Chrome can do this automatically when getting
resources.

BUG=369572

Review-Url: https://codereview.chromium.org/1676793002
Cr-Original-Original-Commit-Position: refs/heads/master@{#480994}
Committed:
https://chromium.googlesource.com/chromium/src/+/d2f53be161b01a4cf04dfa387b66...
Review-Url: https://codereview.chromium.org/1676793002
Cr-Original-Commit-Position: refs/heads/master@{#482826}
Committed:
https://chromium.googlesource.com/chromium/src/+/1deaebd297b7212932be4afe34ae...
Review-Url: https://codereview.chromium.org/1676793002
Cr-Commit-Position: refs/heads/master@{#483512}
Committed:
https://chromium.googlesource.com/chromium/src/+/af0dc05d501cafad6ce25a775ada...
==========

[commit-bot: I haz the power, 2017-06-29 22:11:41]

Committed patchset #3 (id:80001) as
https://chromium.googlesource.com/chromium/src/+/af0dc05d501cafad6ce25a775ada...