[build] Prepare moving v8.gyp to src/

[build] Prepare moving v8.gyp to src/

This will allow to pull in gyp as a deps to the same location
as chromium (tools/gyp not build/gyp), needed for gn switch.

This is the first step of a 3-way move.
1) Copy v8.gyp in v8
2) Update references in embedders (follow up)
3) Remove old v8.gyp (follow up)

BUG=chromium:474921
LOG=n
NOTRY=true

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

[Michael Achenbach, 2016-04-25 09:39:01]

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

[commit-bot: I haz the power, 2016-04-25 09:39:11]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1920793002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1920793002/40001

[commit-bot: I haz the power, 2016-04-25 09:40:42]

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

[commit-bot: I haz the power, 2016-04-25 09:40:42]

Dry run: Try jobs failed on following builders:
  v8_android_arm_compile_rel on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_android_arm_compile_rel/...)
  v8_linux64_avx2_rel_ng on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_avx2_rel_ng/buil...)

[Michael Achenbach, 2016-04-25 09:44:44]

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

[commit-bot: I haz the power, 2016-04-25 09:44:52]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1920793002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1920793002/60001

[Michael Achenbach, 2016-04-25 09:45:49]

Description was changed from

==========
[build] Prepare moving v8.gyp

BUG=
==========

to

==========
[build] Prepare moving v8.gyp

BUG=chromium:474921
LOG=n
==========

[commit-bot: I haz the power, 2016-04-25 09:46:42]

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

[commit-bot: I haz the power, 2016-04-25 09:46:43]

Dry run: Try jobs failed on following builders:
  v8_android_arm_compile_rel on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_android_arm_compile_rel/...)
  v8_linux64_asan_rel_ng on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_asan_rel_ng/buil...)
  v8_linux64_avx2_rel_ng on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_avx2_rel_ng/buil...)
  v8_linux64_rel_ng on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_rel_ng/builds/4779)
  v8_linux_arm64_rel_ng on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_arm64_rel_ng/build...)
  v8_linux_arm_rel_ng on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_arm_rel_ng/builds/656)
  v8_linux_dbg_ng on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_dbg_ng/builds/4801)
  v8_linux_gcc_compile_rel on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_gcc_compile_rel/bu...)
  v8_linux_mips64el_compile_rel on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_mips64el_compile_r...)
  v8_linux_nodcheck_rel_ng on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_nodcheck_rel_ng/bu...)
  v8_linux_rel_ng on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_rel_ng/builds/4783)

[Michael Achenbach, 2016-04-25 09:52:34]

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

[commit-bot: I haz the power, 2016-04-25 09:52:42]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1920793002/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1920793002/80001

[Michael Achenbach, 2016-04-25 10:19:51]

Description was changed from

==========
[build] Prepare moving v8.gyp

BUG=chromium:474921
LOG=n
==========

to

==========
[build] Prepare moving v8.gyp

This will allow to pull in gyp as a deps to the same location
as chromium (tools/gyp not build/gyp), needed for gn switch.

This is the first step of a 3-way move.
1) Copy v8.gyp in v8
2) Update references in embedders (follow up)
3) Remove old v8.gyp (follow up)

BUG=chromium:474921
LOG=n
==========

[Michael Achenbach, 2016-04-25 10:19:51]

machenbach@chromium.org changed reviewers:
+ jkummerow@chromium.org, jochen@chromium.org

[Michael Achenbach, 2016-04-25 10:19:59]

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

[commit-bot: I haz the power, 2016-04-25 10:20:03]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1920793002/100001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1920793002/100001

[Michael Achenbach, 2016-04-25 10:21:16]

PTAL. Patch 5 shows that everything passes with the old v8.gyp removed. Patch 6
keeps it to not break embedders just now.

[Michael Achenbach, 2016-04-25 10:24:22]

FYI: I'll also send a PSA around when this lands.

[commit-bot: I haz the power, 2016-04-25 10:37:51]

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

[commit-bot: I haz the power, 2016-04-25 10:37:52]

Dry run: This issue passed the CQ dry run.

[Jakob Kummerow, 2016-04-25 11:21:11]

LGTM ().

The CL description should probably be updated to "Move v8.gyp to src/".

[Michael Achenbach, 2016-04-25 11:22:15]

Description was changed from

==========
[build] Prepare moving v8.gyp

This will allow to pull in gyp as a deps to the same location
as chromium (tools/gyp not build/gyp), needed for gn switch.

This is the first step of a 3-way move.
1) Copy v8.gyp in v8
2) Update references in embedders (follow up)
3) Remove old v8.gyp (follow up)

BUG=chromium:474921
LOG=n
==========

to

==========
[build] Prepare moving v8.gyp to src/

This will allow to pull in gyp as a deps to the same location
as chromium (tools/gyp not build/gyp), needed for gn switch.

This is the first step of a 3-way move.
1) Copy v8.gyp in v8
2) Update references in embedders (follow up)
3) Remove old v8.gyp (follow up)

BUG=chromium:474921
LOG=n
==========

[Michael Achenbach, 2016-04-25 11:23:34]

On 2016/04/25 11:21:11, Jakob wrote:
> LGTM ().
> 
> The CL description should probably be updated to "Move v8.gyp to src/".

I added the "src/" info, but I'm not sure if I should use "Move" as this CL just
copies it. The move is comprised of all three CLs of this dance.

[jochen (gone - plz use gerrit), 2016-04-25 11:30:20]

can you make sure that it's actually a file move for git (the v8.gyp) one, so it
doesn't look like an initial commit?

[jochen (gone - plz use gerrit), 2016-04-25 11:30:47]

because now it looks like "A src/v8.gyp" and not "A+ src/v8.gyp"

[Michael Achenbach, 2016-04-25 11:46:41]

How about patch 7? It's still not perfect, especially in the beginning of the
file, but much better.

[jochen (gone - plz use gerrit), 2016-04-25 11:51:55]

lgtm

[Michael Achenbach, 2016-04-25 12:10:15]

The CQ bit was checked by machenbach@chromium.org

[Michael Achenbach, 2016-04-25 12:10:16]

The patchset sent to the CQ was uploaded after l-g-t-m from
jkummerow@chromium.org
Link to the patchset: https://codereview.chromium.org/1920793002/#ps120001
(title: "Better base files.")

[commit-bot: I haz the power, 2016-04-25 12:10:28]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1920793002/120001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1920793002/120001

[Michael Achenbach, 2016-04-25 12:17:11]

Description was changed from

==========
[build] Prepare moving v8.gyp to src/

This will allow to pull in gyp as a deps to the same location
as chromium (tools/gyp not build/gyp), needed for gn switch.

This is the first step of a 3-way move.
1) Copy v8.gyp in v8
2) Update references in embedders (follow up)
3) Remove old v8.gyp (follow up)

BUG=chromium:474921
LOG=n
==========

to

==========
[build] Prepare moving v8.gyp to src/

This will allow to pull in gyp as a deps to the same location
as chromium (tools/gyp not build/gyp), needed for gn switch.

This is the first step of a 3-way move.
1) Copy v8.gyp in v8
2) Update references in embedders (follow up)
3) Remove old v8.gyp (follow up)

BUG=chromium:474921
LOG=n
NOTRY=true
==========

[Michael Achenbach, 2016-04-25 12:17:19]

The CQ bit was unchecked by machenbach@chromium.org

[Michael Achenbach, 2016-04-25 12:17:26]

The CQ bit was checked by machenbach@chromium.org

[commit-bot: I haz the power, 2016-04-25 12:17:36]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1920793002/120001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1920793002/120001

[commit-bot: I haz the power, 2016-04-25 12:19:37]

Description was changed from

==========
[build] Prepare moving v8.gyp to src/

This will allow to pull in gyp as a deps to the same location
as chromium (tools/gyp not build/gyp), needed for gn switch.

This is the first step of a 3-way move.
1) Copy v8.gyp in v8
2) Update references in embedders (follow up)
3) Remove old v8.gyp (follow up)

BUG=chromium:474921
LOG=n
NOTRY=true
==========

to

==========
[build] Prepare moving v8.gyp to src/

This will allow to pull in gyp as a deps to the same location
as chromium (tools/gyp not build/gyp), needed for gn switch.

This is the first step of a 3-way move.
1) Copy v8.gyp in v8
2) Update references in embedders (follow up)
3) Remove old v8.gyp (follow up)

BUG=chromium:474921
LOG=n
NOTRY=true
==========

[commit-bot: I haz the power, 2016-04-25 12:19:37]

Committed patchset #7 (id:120001)

[commit-bot: I haz the power, 2016-04-25 12:20:34]

Description was changed from

==========
[build] Prepare moving v8.gyp to src/

This will allow to pull in gyp as a deps to the same location
as chromium (tools/gyp not build/gyp), needed for gn switch.

This is the first step of a 3-way move.
1) Copy v8.gyp in v8
2) Update references in embedders (follow up)
3) Remove old v8.gyp (follow up)

BUG=chromium:474921
LOG=n
NOTRY=true
==========

to

==========
[build] Prepare moving v8.gyp to src/

This will allow to pull in gyp as a deps to the same location
as chromium (tools/gyp not build/gyp), needed for gn switch.

This is the first step of a 3-way move.
1) Copy v8.gyp in v8
2) Update references in embedders (follow up)
3) Remove old v8.gyp (follow up)

BUG=chromium:474921
LOG=n
NOTRY=true

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

[commit-bot: I haz the power, 2016-04-25 12:20:35]

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

[Nico, 2016-04-25 21:05:26]

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

[Nico, 2016-04-25 21:05:26]

I think this broke the world (silently).
https://build.chromium.org/p/chromium.memory/builders/Linux%20Chromium%20OS%2...
e.g. has lots of 

ninja: warning: multiple rules generate gen/debug-support.cc. builds involving
this target will not be correct; continuing anyway [-w dupbuild=warn]

(which is only not yet an error due to an accident). It started in that build,
and that build has https://codereview.chromium.org/1915963002 which includes
this change.

This seems potentially very bad, so I'd recommend reverting this if the fix
isn't obvious.

[adamk, 2016-04-25 23:28:43]

On 2016/04/25 21:05:26, Nico wrote:
> I think this broke the world (silently).
>
https://build.chromium.org/p/chromium.memory/builders/Linux%20Chromium%20OS%2...
> e.g. has lots of 
> 
> ninja: warning: multiple rules generate gen/debug-support.cc. builds involving
> this target will not be correct; continuing anyway [-w dupbuild=warn]
> 
> (which is only not yet an error due to an accident). It started in that build,
> and that build has https://codereview.chromium.org/1915963002 which includes
> this change.
> 
> This seems potentially very bad, so I'd recommend reverting this if the fix
> isn't obvious.

I tried simply updating Chromium to use the new location of v8.gyp, but that
didn't get rid of the warnings.

[Michael Achenbach, 2016-04-26 07:25:06]

On 2016/04/25 23:28:43, adamk wrote:
> On 2016/04/25 21:05:26, Nico wrote:
> > I think this broke the world (silently).
> >
>
https://build.chromium.org/p/chromium.memory/builders/Linux%20Chromium%20OS%2...
> > e.g. has lots of 
> > 
> > ninja: warning: multiple rules generate gen/debug-support.cc. builds
involving
> > this target will not be correct; continuing anyway [-w dupbuild=warn]
> > 
> > (which is only not yet an error due to an accident). It started in that
build,
> > and that build has https://codereview.chromium.org/1915963002 which includes
> > this change.
> > 
> > This seems potentially very bad, so I'd recommend reverting this if the fix
> > isn't obvious.
> 
> I tried simply updating Chromium to use the new location of v8.gyp, but that
> didn't get rid of the warnings.

I commented on the bug. "I think the warning is benign because the two gyp files
are equal."

If all locations from an embedder point to the new v8.gyp, I don't understand
why the warning wouldn't go away? Maybe some where missing?

[Michael Achenbach, 2016-04-26 08:52:38]

On 2016/04/25 23:28:43, adamk wrote:
> On 2016/04/25 21:05:26, Nico wrote:
> > I think this broke the world (silently).
> >
>
https://build.chromium.org/p/chromium.memory/builders/Linux%20Chromium%20OS%2...
> > e.g. has lots of 
> > 
> > ninja: warning: multiple rules generate gen/debug-support.cc. builds
involving
> > this target will not be correct; continuing anyway [-w dupbuild=warn]
> > 
> > (which is only not yet an error due to an accident). It started in that
build,
> > and that build has https://codereview.chromium.org/1915963002 which includes
> > this change.
> > 
> > This seems potentially very bad, so I'd recommend reverting this if the fix
> > isn't obvious.
> 
> I tried simply updating Chromium to use the new location of v8.gyp, but that
> didn't get rid of the warnings.

It's because of pdfium. If I update the locations in chromium and pdfium, the
warnings go away.

[Nico, 2016-04-26 13:44:04]

On 2016/04/26 07:25:06, Michael Achenbach wrote:
> On 2016/04/25 23:28:43, adamk wrote:
> > On 2016/04/25 21:05:26, Nico wrote:
> > > I think this broke the world (silently).
> > >
> >
>
https://build.chromium.org/p/chromium.memory/builders/Linux%20Chromium%20OS%2...
> > > e.g. has lots of 
> > > 
> > > ninja: warning: multiple rules generate gen/debug-support.cc. builds
> involving
> > > this target will not be correct; continuing anyway [-w dupbuild=warn]
> > > 
> > > (which is only not yet an error due to an accident). It started in that
> build,
> > > and that build has https://codereview.chromium.org/1915963002 which
includes
> > > this change.
> > > 
> > > This seems potentially very bad, so I'd recommend reverting this if the
fix
> > > isn't obvious.
> > 
> > I tried simply updating Chromium to use the new location of v8.gyp, but that
> > didn't get rid of the warnings.
> 
> I commented on the bug. "I think the warning is benign because the two gyp
files
> are equal."

In practice, the warning is benign only if both build commands for the same file
are identical (and only because ninja nowadays is careful to not end up with a
broken build graph like it did earlier in these cases). It's up to the generator
(gyp) to decide if it wants to be strict about duplicate edges or if it wants to
allow duplicate edges if build commands are equal -- gyp just doesn't handle
this at all and passes through everything, which means gyp can produce ninja
files that are broken. I think the right way to roll this out would've been to
change gyp to omit duplicate .o edges if they have the same build command, and
then land this.

> 
> If all locations from an embedder point to the new v8.gyp, I don't understand
> why the warning wouldn't go away? Maybe some where missing?

[Michael Achenbach, 2016-04-26 13:51:47]

> In practice, the warning is benign only if both build commands for the same
file
> are identical (and only because ninja nowadays is careful to not end up with a
> broken build graph like it did earlier in these cases). It's up to the
generator
> (gyp) to decide if it wants to be strict about duplicate edges or if it wants
to
> allow duplicate edges if build commands are equal -- gyp just doesn't handle
> this at all and passes through everything, which means gyp can produce ninja
> files that are broken. I think the right way to roll this out would've been to
> change gyp to omit duplicate .o edges if they have the same build command, and
> then land this.
> 

OK. "would've". Now the fixes landed already and we might never do this again ;)

But thanks for the explanation!