vulcanize_gn.py: Stop writing twice to the declared GN output file.

vulcanize_gn.py: Stop writing twice to the declared GN output file.

This confuses the build system when an error happens after the 1st write
and before the script completely finishes.

BUG=729191
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2915393002
Cr-Commit-Position: refs/heads/master@{#477390}
Committed: https://chromium.googlesource.com/chromium/src/+/4309919f5a61e0701e073f21ed312e4d14cfc322

[dpapad, 2017-06-02 23:55:25]

Description was changed from

==========
vulcanize_gn.py: Stop writing twice to the declared GN output file.

This confuses the build system when an error happens after the 1st write and
before the 2nd.

BUG=729191
==========

to

==========
vulcanize_gn.py: Stop writing twice to the declared GN output file.

This confuses the build system when an error happens after the 1st write and
before the 2nd.

BUG=729191
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[dpapad, 2017-06-02 23:55:29]

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

[commit-bot: I haz the power, 2017-06-02 23:56:09]

Dry run: CQ is trying da patch.

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

[dpapad, 2017-06-02 23:58:16]

Description was changed from

==========
vulcanize_gn.py: Stop writing twice to the declared GN output file.

This confuses the build system when an error happens after the 1st write and
before the 2nd.

BUG=729191
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
vulcanize_gn.py: Stop writing twice to the declared GN output file.

This confuses the build system when an error happens after the 1st write
and before the script completely finishes.

BUG=729191
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[commit-bot: I haz the power, 2017-06-03 00:25:18]

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

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

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

[dpapad, 2017-06-03 00:40:08]

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

[commit-bot: I haz the power, 2017-06-03 00:40:42]

Dry run: CQ is trying da patch.

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

[dpapad, 2017-06-03 01:00:34]

dpapad@chromium.org changed reviewers:
+ dbeam@chromium.org

[commit-bot: I haz the power, 2017-06-03 01:14:09]

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

[commit-bot: I haz the power, 2017-06-03 01:14:10]

Dry run: Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Dan Beam, 2017-06-03 01:19:06]

lgtm w/nit

https://codereview.chromium.org/2915393002/diff/1/chrome/browser/resources/vu...
File chrome/browser/resources/vulcanize_gn.py (right):

https://codereview.chromium.org/2915393002/diff/1/chrome/browser/resources/vu...
chrome/browser/resources/vulcanize_gn.py:163: tmp_crisper_input.write(output)
nit:

crisper_input = tempfile.NamedTemporaryFile(mode='wt+', delete=False)
crisper_input.write(output)
crisper_input.close()

might be more readable, IMO

[dpapad, 2017-06-03 01:26:59]

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

[dpapad, 2017-06-03 01:27:10]

https://codereview.chromium.org/2915393002/diff/1/chrome/browser/resources/vu...
File chrome/browser/resources/vulcanize_gn.py (right):

https://codereview.chromium.org/2915393002/diff/1/chrome/browser/resources/vu...
chrome/browser/resources/vulcanize_gn.py:163: tmp_crisper_input.write(output)
On 2017/06/03 at 01:19:05, Dan Beam wrote:
> nit:
> 
> crisper_input = tempfile.NamedTemporaryFile(mode='wt+', delete=False)
> crisper_input.write(output)
> crisper_input.close()
> 
> might be more readable, IMO

Done.

[commit-bot: I haz the power, 2017-06-03 01:27:40]

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-03 01:59:40]

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

[commit-bot: I haz the power, 2017-06-03 01:59:41]

Dry run: Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[dpapad, 2017-06-05 19:46:07]

https://codereview.chromium.org/2915393002/diff/40001/chrome/browser/resource...
File chrome/browser/resources/vulcanize_gn.py (right):

https://codereview.chromium.org/2915393002/diff/40001/chrome/browser/resource...
chrome/browser/resources/vulcanize_gn.py:173: '--js', crisper_output.name])
So this is causing a problem. Crisper substitutes the temp file's name in the
<script> tag that adds at the end of the HTML file. I can think of two ways to
fix.

1) Let crisper do its thing, and then search and replace the <script> tag with
the correct file path.
2) Use crisper's --only-split flag, which prevents it from adding any <script>
tag, and add the correct tag ourselves later.

Both approaches require additional processing the vulcanized HTML file after
crisper does. Any thoughts?

[dpapad, 2017-06-05 22:10:04]

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

[commit-bot: I haz the power, 2017-06-05 22:10:41]

Dry run: CQ is trying da patch.

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

[dpapad, 2017-06-05 22:12:11]

https://codereview.chromium.org/2915393002/diff/40001/chrome/browser/resource...
File chrome/browser/resources/vulcanize_gn.py (right):

https://codereview.chromium.org/2915393002/diff/40001/chrome/browser/resource...
chrome/browser/resources/vulcanize_gn.py:173: '--js', crisper_output.name])
On 2017/06/05 at 19:46:07, dpapad wrote:
> So this is causing a problem. Crisper substitutes the temp file's name in the
<script> tag that adds at the end of the HTML file. I can think of two ways to
fix.
> 
> 1) Let crisper do its thing, and then search and replace the <script> tag with
the correct file path.
> 2) Use crisper's --only-split flag, which prevents it from adding any <script>
tag, and add the correct tag ourselves later.
> 
> Both approaches require additional processing the vulcanized HTML file after
crisper does. Any thoughts?

PTAL. I fixed this in patch#4 using approach #2 above.

[commit-bot: I haz the power, 2017-06-05 22:39:38]

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

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

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[dpapad, 2017-06-05 22:46:20]

On 2017/06/05 at 22:12:11, dpapad wrote:
>
https://codereview.chromium.org/2915393002/diff/40001/chrome/browser/resource...
> File chrome/browser/resources/vulcanize_gn.py (right):
> 
>
https://codereview.chromium.org/2915393002/diff/40001/chrome/browser/resource...
> chrome/browser/resources/vulcanize_gn.py:173: '--js', crisper_output.name])
> On 2017/06/05 at 19:46:07, dpapad wrote:
> > So this is causing a problem. Crisper substitutes the temp file's name in
the <script> tag that adds at the end of the HTML file. I can think of two ways
to fix.
> > 
> > 1) Let crisper do its thing, and then search and replace the <script> tag
with the correct file path.
> > 2) Use crisper's --only-split flag, which prevents it from adding any
<script> tag, and add the correct tag ourselves later.
> > 
> > Both approaches require additional processing the vulcanized HTML file after
crisper does. Any thoughts?
> 
> PTAL. I fixed this in patch#4 using approach #2 above.

And... MD History tests are failing. Investigating.

[dpapad, 2017-06-05 23:12:57]

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

[commit-bot: I haz the power, 2017-06-05 23:13:38]

Dry run: CQ is trying da patch.

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

[dpapad, 2017-06-05 23:27:41]

On 2017/06/05 at 22:46:20, dpapad wrote:
> On 2017/06/05 at 22:12:11, dpapad wrote:
> >
https://codereview.chromium.org/2915393002/diff/40001/chrome/browser/resource...
> > File chrome/browser/resources/vulcanize_gn.py (right):
> > 
> >
https://codereview.chromium.org/2915393002/diff/40001/chrome/browser/resource...
> > chrome/browser/resources/vulcanize_gn.py:173: '--js', crisper_output.name])
> > On 2017/06/05 at 19:46:07, dpapad wrote:
> > > So this is causing a problem. Crisper substitutes the temp file's name in
the <script> tag that adds at the end of the HTML file. I can think of two ways
to fix.
> > > 
> > > 1) Let crisper do its thing, and then search and replace the <script> tag
with the correct file path.
> > > 2) Use crisper's --only-split flag, which prevents it from adding any
<script> tag, and add the correct tag ourselves later.
> > > 
> > > Both approaches require additional processing the vulcanized HTML file
after crisper does. Any thoughts?
> > 
> > PTAL. I fixed this in patch#4 using approach #2 above.
> 
> And... MD History tests are failing. Investigating.

Fixed. I was looking for "</div></body></html>" which was overly specific and
did not occur in the History's case.

[commit-bot: I haz the power, 2017-06-06 03:08:01]

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

[commit-bot: I haz the power, 2017-06-06 03:08:02]

Dry run: This issue passed the CQ dry run.

[Dan Beam, 2017-06-06 19:09:27]

lgtm

https://codereview.chromium.org/2915393002/diff/80001/chrome/browser/resource...
File chrome/browser/resources/vulcanize_gn.py (right):

https://codereview.chromium.org/2915393002/diff/80001/chrome/browser/resource...
chrome/browser/resources/vulcanize_gn.py:181: with open(html_out_path, "r+") as
f:
nit: rest of this file uses ' instead of "

https://codereview.chromium.org/2915393002/diff/80001/chrome/browser/resource...
chrome/browser/resources/vulcanize_gn.py:183: newData = data.replace(
python_vars_like_this

[dpapad, 2017-06-06 19:30:48]

https://codereview.chromium.org/2915393002/diff/80001/chrome/browser/resource...
File chrome/browser/resources/vulcanize_gn.py (right):

https://codereview.chromium.org/2915393002/diff/80001/chrome/browser/resource...
chrome/browser/resources/vulcanize_gn.py:181: with open(html_out_path, "r+") as
f:
On 2017/06/06 at 19:09:27, Dan Beam wrote:
> nit: rest of this file uses ' instead of "

Done.

https://codereview.chromium.org/2915393002/diff/80001/chrome/browser/resource...
chrome/browser/resources/vulcanize_gn.py:183: newData = data.replace(
On 2017/06/06 at 19:09:27, Dan Beam wrote:
> python_vars_like_this

Done.

[dpapad, 2017-06-06 19:31:00]

The CQ bit was checked by dpapad@chromium.org

[dpapad, 2017-06-06 19:31:00]

The patchset sent to the CQ was uploaded after l-g-t-m from dbeam@chromium.org
Link to the patchset: https://codereview.chromium.org/2915393002/#ps100001
(title: "Nits.")

[commit-bot: I haz the power, 2017-06-06 19:31: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, 2017-06-06 20:33:52]

CQ is committing da patch.

Bot data: {"patchset_id": 100001, "attempt_start_ts": 1496777460614180,
"parent_rev": "6f511f5c040c7d7443786ca4e34ae40aca9f5377", "commit_rev":
"4309919f5a61e0701e073f21ed312e4d14cfc322"}

[commit-bot: I haz the power, 2017-06-06 20:34:04]

Description was changed from

==========
vulcanize_gn.py: Stop writing twice to the declared GN output file.

This confuses the build system when an error happens after the 1st write
and before the script completely finishes.

BUG=729191
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
vulcanize_gn.py: Stop writing twice to the declared GN output file.

This confuses the build system when an error happens after the 1st write
and before the script completely finishes.

BUG=729191
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2915393002
Cr-Commit-Position: refs/heads/master@{#477390}
Committed:
https://chromium.googlesource.com/chromium/src/+/4309919f5a61e0701e073f21ed31...
==========

[commit-bot: I haz the power, 2017-06-06 20:34:05]

Committed patchset #6 (id:100001) as
https://chromium.googlesource.com/chromium/src/+/4309919f5a61e0701e073f21ed31...