gn: Generalize process_version() and move it to build/util

There is nothing really chrome-specific in the process_version()
template, and if we stop always passing some files in chrome/ on every
invocation it is possible to generalize it and move it to build/util
instead (and fix some layering violation comments along the way).

Compared to its previous incarnation, process_version() no longer passes
LASTCHANGE, BRANDING and VERSION automatically to version.py. This makes
it easier for callers to specify their own |sources| with values that
may override those set in those 3 files.

A new wrapper, process_version_rc_template(), was introduced to cater
for callers (generally Windows ones) who need to process a .rc.version
file that requires the values from LASTCHANGE, BRANDING and VERSION. It
always passes those 3 files to process_version() before any optional
additional |sources|, and |template_file| defaults to
chrome_version.rc.version.
Since this template depends on files from //chrome, it lives in the
newly-added //chrome/process_version_rc_template.gni.

R=dpranke@chromium.org,brettw@chromium.org

Committed: https://crrev.com/cf276fc41653b2830a5ec796d4db09b199754677
Cr-Commit-Position: refs/heads/master@{#418514}

[Raphael Kubo da Costa (rakuco), 2016-09-05 10:02:46]

Description was changed from

==========
WIP: gn: Generalize process_version() and move it to build/util

There is nothing really chrome-specific in the process_version()
template, and if we stop always passing some files in chrome/ on every
invocation it is possible to generalize it and move it to build/util
instead (and fix some layering violation comments along the way).

Compared to its previous incarnation:
* process_version() no longer passes LASTCHANGE, BRANDING and VERSION
  automatically to version.py. This makes it easier for callers to
  specify their own |sources| with values that may override those set in
  those 3 files.
* A new wrapper, process_version_rc_template(), was introduced to cater
  for callers (generally Windows ones) who need to process a .rc.version
  file that requires the values from LASTCHANGE, BRANDING and VERSION.
  It always passes those 3 files to process_version() before any
  optional additional |sources|, and |template_file| defaults to
  chrome_version.rc.version.

R=dpranke@chromium.org,brettw@chromium.org
==========

to

==========
There is nothing really chrome-specific in the process_version()
template, and if we stop always passing some files in chrome/ on every
invocation it is possible to generalize it and move it to build/util
instead (and fix some layering violation comments along the way).

Compared to its previous incarnation:
* process_version() no longer passes LASTCHANGE, BRANDING and VERSION
  automatically to version.py. This makes it easier for callers to
  specify their own |sources| with values that may override those set in
  those 3 files.
* A new wrapper, process_version_rc_template(), was introduced to cater
  for callers (generally Windows ones) who need to process a .rc.version
  file that requires the values from LASTCHANGE, BRANDING and VERSION.
  It always passes those 3 files to process_version() before any
  optional additional |sources|, and |template_file| defaults to
  chrome_version.rc.version.

R=dpranke@chromium.org,brettw@chromium.org
==========

[Raphael Kubo da Costa (rakuco), 2016-09-05 10:03:43]

Brett/Dirk: please take a look at this one. That's the version I mentioned in
https://codereview.chromium.org/2308583002/

My plan is to CC the remaining OWNERS after you vet it.

[Dirk Pranke, 2016-09-06 01:35:25]

This looks pretty good to me, but brettw@ knows this stuff better and should
ultimate be the reviewer.

[brettw, 2016-09-06 22:52:31]

https://codereview.chromium.org/2308313003/diff/20001/build/util/process_vers...
File build/util/process_version.gni (right):

https://codereview.chromium.org/2308313003/diff/20001/build/util/process_vers...
build/util/process_version.gni:162: _template_file =
"//chrome/app/chrome_version.rc.version"
Since this depends on files from //chrome, it should really be in the //chrome
directory. This was why the script used to be in //chrome with TODOs about
layering violations.

Your change allows these layering violations to be fixed (yay), we just need to
split apart the definitions so stuff in //build never references anything
outside of //build.

[Raphael Kubo da Costa (rakuco), 2016-09-08 14:19:35]

Description was changed from

==========
There is nothing really chrome-specific in the process_version()
template, and if we stop always passing some files in chrome/ on every
invocation it is possible to generalize it and move it to build/util
instead (and fix some layering violation comments along the way).

Compared to its previous incarnation:
* process_version() no longer passes LASTCHANGE, BRANDING and VERSION
  automatically to version.py. This makes it easier for callers to
  specify their own |sources| with values that may override those set in
  those 3 files.
* A new wrapper, process_version_rc_template(), was introduced to cater
  for callers (generally Windows ones) who need to process a .rc.version
  file that requires the values from LASTCHANGE, BRANDING and VERSION.
  It always passes those 3 files to process_version() before any
  optional additional |sources|, and |template_file| defaults to
  chrome_version.rc.version.

R=dpranke@chromium.org,brettw@chromium.org
==========

to

==========
There is nothing really chrome-specific in the process_version()
template, and if we stop always passing some files in chrome/ on every
invocation it is possible to generalize it and move it to build/util
instead (and fix some layering violation comments along the way).

Compared to its previous incarnation, process_version() no longer passes
LASTCHANGE, BRANDING and VERSION automatically to version.py. This makes
it easier for callers to specify their own |sources| with values that
may override those set in those 3 files.

A new wrapper, process_version_rc_template(), was introduced to cater
for callers (generally Windows ones) who need to process a .rc.version
file that requires the values from LASTCHANGE, BRANDING and VERSION. It
always passes those 3 files to process_version() before any optional
additional |sources|, and |template_file| defaults to
chrome_version.rc.version.
Since this template depends on files from //chrome, it lives in the
newly-added //chrome/process_version_rc_template.gni.

R=dpranke@chromium.org,brettw@chromium.org
==========

[Raphael Kubo da Costa (rakuco), 2016-09-08 14:36:02]

Patch updated: I've moved process_version_rc_template() to
//chrome/process_version_rc_template.gni and I've rebased the commit on top of a
more recent master.

Having that new template in //chrome makes me wonder about
media/cdm/ppapi/BUILD.gn and third_party/widevine/cdm/BUILD.gn, as well as the
targets that need to pass //chrome/VERSION to process_version(): are there
layering violations in this case? If so, I don't think it's ever going to be
possible to get rid of them, since the targets do depend on files in chrome/.

[Raphael Kubo da Costa (rakuco), 2016-09-13 10:01:29]

ping

[brettw, 2016-09-13 17:59:15]

LGTM with one change (don't need to get another review). Thanks!

https://codereview.chromium.org/2308313003/diff/40001/build/config/chrome_bui...
File build/config/chrome_build.gni (right):

https://codereview.chromium.org/2308313003/diff/40001/build/config/chrome_bui...
build/config/chrome_build.gni:25: branding_file_path =
"//chrome/app/theme/$branding_path_component/BRANDING"
Can you move this to the chrome process_versoin.gni file? There's one place in
compoents that needs this also, but a layering violation from components to
chrome on branding seems less bad than one from build (since build is shared
with other projects).

[Raphael Kubo da Costa (rakuco), 2016-09-13 18:47:02]

Description was changed from

==========
There is nothing really chrome-specific in the process_version()
template, and if we stop always passing some files in chrome/ on every
invocation it is possible to generalize it and move it to build/util
instead (and fix some layering violation comments along the way).

Compared to its previous incarnation, process_version() no longer passes
LASTCHANGE, BRANDING and VERSION automatically to version.py. This makes
it easier for callers to specify their own |sources| with values that
may override those set in those 3 files.

A new wrapper, process_version_rc_template(), was introduced to cater
for callers (generally Windows ones) who need to process a .rc.version
file that requires the values from LASTCHANGE, BRANDING and VERSION. It
always passes those 3 files to process_version() before any optional
additional |sources|, and |template_file| defaults to
chrome_version.rc.version.
Since this template depends on files from //chrome, it lives in the
newly-added //chrome/process_version_rc_template.gni.

R=dpranke@chromium.org,brettw@chromium.org
==========

to

==========
There is nothing really chrome-specific in the process_version()
template, and if we stop always passing some files in chrome/ on every
invocation it is possible to generalize it and move it to build/util
instead (and fix some layering violation comments along the way).

Compared to its previous incarnation, process_version() no longer passes
LASTCHANGE, BRANDING and VERSION automatically to version.py. This makes
it easier for callers to specify their own |sources| with values that
may override those set in those 3 files.

A new wrapper, process_version_rc_template(), was introduced to cater
for callers (generally Windows ones) who need to process a .rc.version
file that requires the values from LASTCHANGE, BRANDING and VERSION. It
always passes those 3 files to process_version() before any optional
additional |sources|, and |template_file| defaults to
chrome_version.rc.version.
Since this template depends on files from //chrome, it lives in the
newly-added //chrome/process_version_rc_template.gni.

R=dpranke@chromium.org,brettw@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win10_chromium_x64_rel_ng
==========

[Raphael Kubo da Costa (rakuco), 2016-09-13 18:48:51]

raphael.kubo.da.costa@intel.com changed reviewers:
+ ddorwin@chromium.org, phajdan.jr@chromium.org, xhwang@chromium.org

[Raphael Kubo da Costa (rakuco), 2016-09-13 18:48:52]

+phajdan.jr for chrome/installer/linux
+ddorwin, xhwang for third_party/widevine/cdm

I've updated the patch and moved |branding_file_path| to
chrome/process_version_rc_template.gni as requested.

[Paweł Hajdan Jr., 2016-09-13 19:02:15]

chrome/installer/linux LGTM

[ddorwin, 2016-09-13 19:56:01]

third_party/widevine/cdm/BUILD.gn and media/cdm/ppapi/BUILD.gn LGTM

[Raphael Kubo da Costa (rakuco), 2016-09-13 21:28:11]

The CQ bit was checked by raphael.kubo.da.costa@intel.com

[Raphael Kubo da Costa (rakuco), 2016-09-13 21:28:12]

The patchset sent to the CQ was uploaded after l-g-t-m from brettw@chromium.org
Link to the patchset: https://codereview.chromium.org/2308313003/#ps60001
(title: "Move branding_file_path to chrome/process_version_rc_template.gni")

[commit-bot: I haz the power, 2016-09-13 21:28:44]

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, 2016-09-13 21:54:49]

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

[commit-bot: I haz the power, 2016-09-13 21:54:51]

Try jobs failed on following builders:
  win10_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win10_chromium_x6...)

[Raphael Kubo da Costa (rakuco), 2016-09-14 08:28:10]

The CQ bit was checked by raphael.kubo.da.costa@intel.com

[commit-bot: I haz the power, 2016-09-14 08:28:29]

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, 2016-09-14 08:48:46]

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

[commit-bot: I haz the power, 2016-09-14 08:48:48]

Try jobs failed on following builders:
  win10_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win10_chromium_x6...)

[Raphael Kubo da Costa (rakuco), 2016-09-14 08:49:43]

Description was changed from

==========
There is nothing really chrome-specific in the process_version()
template, and if we stop always passing some files in chrome/ on every
invocation it is possible to generalize it and move it to build/util
instead (and fix some layering violation comments along the way).

Compared to its previous incarnation, process_version() no longer passes
LASTCHANGE, BRANDING and VERSION automatically to version.py. This makes
it easier for callers to specify their own |sources| with values that
may override those set in those 3 files.

A new wrapper, process_version_rc_template(), was introduced to cater
for callers (generally Windows ones) who need to process a .rc.version
file that requires the values from LASTCHANGE, BRANDING and VERSION. It
always passes those 3 files to process_version() before any optional
additional |sources|, and |template_file| defaults to
chrome_version.rc.version.
Since this template depends on files from //chrome, it lives in the
newly-added //chrome/process_version_rc_template.gni.

R=dpranke@chromium.org,brettw@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win10_chromium_x64_rel_ng
==========

to

==========
There is nothing really chrome-specific in the process_version()
template, and if we stop always passing some files in chrome/ on every
invocation it is possible to generalize it and move it to build/util
instead (and fix some layering violation comments along the way).

Compared to its previous incarnation, process_version() no longer passes
LASTCHANGE, BRANDING and VERSION automatically to version.py. This makes
it easier for callers to specify their own |sources| with values that
may override those set in those 3 files.

A new wrapper, process_version_rc_template(), was introduced to cater
for callers (generally Windows ones) who need to process a .rc.version
file that requires the values from LASTCHANGE, BRANDING and VERSION. It
always passes those 3 files to process_version() before any optional
additional |sources|, and |template_file| defaults to
chrome_version.rc.version.
Since this template depends on files from //chrome, it lives in the
newly-added //chrome/process_version_rc_template.gni.

R=dpranke@chromium.org,brettw@chromium.org
==========

[Raphael Kubo da Costa (rakuco), 2016-09-14 08:49:51]

The CQ bit was checked by raphael.kubo.da.costa@intel.com

[commit-bot: I haz the power, 2016-09-14 08:50:08]

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, 2016-09-14 08:54:41]

Description was changed from

==========
There is nothing really chrome-specific in the process_version()
template, and if we stop always passing some files in chrome/ on every
invocation it is possible to generalize it and move it to build/util
instead (and fix some layering violation comments along the way).

Compared to its previous incarnation, process_version() no longer passes
LASTCHANGE, BRANDING and VERSION automatically to version.py. This makes
it easier for callers to specify their own |sources| with values that
may override those set in those 3 files.

A new wrapper, process_version_rc_template(), was introduced to cater
for callers (generally Windows ones) who need to process a .rc.version
file that requires the values from LASTCHANGE, BRANDING and VERSION. It
always passes those 3 files to process_version() before any optional
additional |sources|, and |template_file| defaults to
chrome_version.rc.version.
Since this template depends on files from //chrome, it lives in the
newly-added //chrome/process_version_rc_template.gni.

R=dpranke@chromium.org,brettw@chromium.org
==========

to

==========
There is nothing really chrome-specific in the process_version()
template, and if we stop always passing some files in chrome/ on every
invocation it is possible to generalize it and move it to build/util
instead (and fix some layering violation comments along the way).

Compared to its previous incarnation, process_version() no longer passes
LASTCHANGE, BRANDING and VERSION automatically to version.py. This makes
it easier for callers to specify their own |sources| with values that
may override those set in those 3 files.

A new wrapper, process_version_rc_template(), was introduced to cater
for callers (generally Windows ones) who need to process a .rc.version
file that requires the values from LASTCHANGE, BRANDING and VERSION. It
always passes those 3 files to process_version() before any optional
additional |sources|, and |template_file| defaults to
chrome_version.rc.version.
Since this template depends on files from //chrome, it lives in the
newly-added //chrome/process_version_rc_template.gni.

R=dpranke@chromium.org,brettw@chromium.org
==========

[commit-bot: I haz the power, 2016-09-14 08:54:43]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-09-14 08:56:58]

Description was changed from

==========
There is nothing really chrome-specific in the process_version()
template, and if we stop always passing some files in chrome/ on every
invocation it is possible to generalize it and move it to build/util
instead (and fix some layering violation comments along the way).

Compared to its previous incarnation, process_version() no longer passes
LASTCHANGE, BRANDING and VERSION automatically to version.py. This makes
it easier for callers to specify their own |sources| with values that
may override those set in those 3 files.

A new wrapper, process_version_rc_template(), was introduced to cater
for callers (generally Windows ones) who need to process a .rc.version
file that requires the values from LASTCHANGE, BRANDING and VERSION. It
always passes those 3 files to process_version() before any optional
additional |sources|, and |template_file| defaults to
chrome_version.rc.version.
Since this template depends on files from //chrome, it lives in the
newly-added //chrome/process_version_rc_template.gni.

R=dpranke@chromium.org,brettw@chromium.org
==========

to

==========
There is nothing really chrome-specific in the process_version()
template, and if we stop always passing some files in chrome/ on every
invocation it is possible to generalize it and move it to build/util
instead (and fix some layering violation comments along the way).

Compared to its previous incarnation, process_version() no longer passes
LASTCHANGE, BRANDING and VERSION automatically to version.py. This makes
it easier for callers to specify their own |sources| with values that
may override those set in those 3 files.

A new wrapper, process_version_rc_template(), was introduced to cater
for callers (generally Windows ones) who need to process a .rc.version
file that requires the values from LASTCHANGE, BRANDING and VERSION. It
always passes those 3 files to process_version() before any optional
additional |sources|, and |template_file| defaults to
chrome_version.rc.version.
Since this template depends on files from //chrome, it lives in the
newly-added //chrome/process_version_rc_template.gni.

R=dpranke@chromium.org,brettw@chromium.org

Committed: https://crrev.com/cf276fc41653b2830a5ec796d4db09b199754677
Cr-Commit-Position: refs/heads/master@{#418514}
==========

[commit-bot: I haz the power, 2016-09-14 08:57:00]

Patchset 4 (id:??) landed as
https://crrev.com/cf276fc41653b2830a5ec796d4db09b199754677
Cr-Commit-Position: refs/heads/master@{#418514}

[Yuta Kitamura, 2016-09-14 09:46:39]

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

The reason for reverting is: This appears to cause compile errors on
"Google Chrome Linux x64", closing the main waterfall.

https://build.chromium.org/p/chromium.chrome/builders/Google%20Chrome%20Linux...

FAILED: installer/version.txt 
python ../../build/util/version.py --official -f ../../chrome/VERSION -f
../../chrome/app/theme/google_chrome/BRANDING -o installer/version.txt
Traceback (most recent call last):
  File "../../build/util/version.py", line 170, in <module>
    sys.exit(main())
  File "../../build/util/version.py", line 159, in main
    """ % values
KeyError: 'LASTCHANGE'.

[brettw, 2016-09-14 17:03:09]

Please reach out if you're having trouble understanding this error, since it
seems to be on the branded Chrome build only.