Don't write deps that GN already knows about to .d files

Don't write deps that GN already knows about to .d files

It causes extra targets to be compiled when GN args change, and a target
has logic like:

target("foo") {
  if (arg) {
    deps = [":A"]
  } else {
    deps = [":B"]
  }
}

BUG=589311
Committed: https://crrev.com/f5c084da4d9de1d51dd62ab425e6933148874ad0
Cr-Commit-Position: refs/heads/master@{#380704}

[agrieve, 2016-02-24 02:58:47]

Description was changed from

==========
Don't write deps that GN already knows about to .d files

It causes extra targets to be compiled when GN args change, and a target
has logic like:

target("foo") {
  if (arg) {
    deps = [":A"]
  } else {
    deps = [":B"]
  }
}

BUG=589311
==========

to

==========
Don't write deps that GN already knows about to .d files

It causes extra targets to be compiled when GN args change, and a target
has logic like:

target("foo") {
  if (arg) {
    deps = [":A"]
  } else {
    deps = [":B"]
  }
}

BUG=589311
==========

[agrieve, 2016-02-24 02:58:47]

agrieve@chromium.org changed reviewers:
+ michaelbai@chromium.org

[agrieve, 2016-02-24 02:59:32]

On 2016/02/24 02:58:47, agrieve wrote:
> mailto:agrieve@chromium.org changed reviewers:
> + mailto:michaelbai@chromium.org

ptal. Might sit on this and test it out locally for a few days to make sure it
doesn't break anything (would appreciate if you would all do the same).

[michaelbai, 2016-02-24 17:24:15]

On 2016/02/24 02:59:32, agrieve wrote:
> On 2016/02/24 02:58:47, agrieve wrote:
> > mailto:agrieve@chromium.org changed reviewers:
> > + mailto:michaelbai@chromium.org
> 
> ptal. Might sit on this and test it out locally for a few days to make sure it
> doesn't break anything (would appreciate if you would all do the same).

How will this be used in your plan?

[agrieve, 2016-02-24 17:28:34]

On 2016/02/24 17:24:15, michaelbai wrote:
> On 2016/02/24 02:59:32, agrieve wrote:
> > On 2016/02/24 02:58:47, agrieve wrote:
> > > mailto:agrieve@chromium.org changed reviewers:
> > > + mailto:michaelbai@chromium.org
> > 
> > ptal. Might sit on this and test it out locally for a few days to make sure
it
> > doesn't break anything (would appreciate if you would all do the same).
> 
> How will this be used in your plan?

This is what's currently causing deps that GN already knows about to be written
into .d files. With this change, only extra python scripts are written into .d.
I'm going under the assumption that all other input deps are captured by GN.
I've inspected several spots and believe this to be true, but still want to be a
bit cautious (by using this myself for a while).

Did that make sense?

[michaelbai, 2016-03-07 23:01:40]

On 2016/02/24 17:28:34, agrieve wrote:
> On 2016/02/24 17:24:15, michaelbai wrote:
> > On 2016/02/24 02:59:32, agrieve wrote:
> > > On 2016/02/24 02:58:47, agrieve wrote:
> > > > mailto:agrieve@chromium.org changed reviewers:
> > > > + mailto:michaelbai@chromium.org
> > > 
> > > ptal. Might sit on this and test it out locally for a few days to make
sure
> it
> > > doesn't break anything (would appreciate if you would all do the same).
> > 
> > How will this be used in your plan?
> 
> This is what's currently causing deps that GN already knows about to be
written
> into .d files. With this change, only extra python scripts are written into
.d.
> I'm going under the assumption that all other input deps are captured by GN.
> I've inspected several spots and believe this to be true, but still want to be
a
> bit cautious (by using this myself for a while).
> 
> Did that make sense?

This patch didn't fix the issue, I 
1. applied this patch
2. ran gn args ..., saw targets generated.
3. then ninja -C ...

got same error.

[agrieve, 2016-03-08 03:13:25]

On 2016/03/07 23:01:40, michaelbai wrote:
> On 2016/02/24 17:28:34, agrieve wrote:
> > On 2016/02/24 17:24:15, michaelbai wrote:
> > > On 2016/02/24 02:59:32, agrieve wrote:
> > > > On 2016/02/24 02:58:47, agrieve wrote:
> > > > > mailto:agrieve@chromium.org changed reviewers:
> > > > > + mailto:michaelbai@chromium.org
> > > > 
> > > > ptal. Might sit on this and test it out locally for a few days to make
> sure
> > it
> > > > doesn't break anything (would appreciate if you would all do the same).
> > > 
> > > How will this be used in your plan?
> > 
> > This is what's currently causing deps that GN already knows about to be
> written
> > into .d files. With this change, only extra python scripts are written into
> .d.
> > I'm going under the assumption that all other input deps are captured by GN.
> > I've inspected several spots and believe this to be true, but still want to
be
> a
> > bit cautious (by using this myself for a while).
> > 
> > Did that make sense?
> 
> This patch didn't fix the issue, I 
> 1. applied this patch
> 2. ran gn args ..., saw targets generated.
> 3. then ninja -C ...
> 
> got same error.

You'd need to clear out your .d files:
  find out/Debug -name "*.d" -delete

Then, this patch will prevent the problem from happening again (should be able
to switch back and forth)

[agrieve, 2016-03-09 16:28:11]

On 2016/03/08 03:13:25, agrieve wrote:
> On 2016/03/07 23:01:40, michaelbai wrote:
> > On 2016/02/24 17:28:34, agrieve wrote:
> > > On 2016/02/24 17:24:15, michaelbai wrote:
> > > > On 2016/02/24 02:59:32, agrieve wrote:
> > > > > On 2016/02/24 02:58:47, agrieve wrote:
> > > > > > mailto:agrieve@chromium.org changed reviewers:
> > > > > > + mailto:michaelbai@chromium.org
> > > > > 
> > > > > ptal. Might sit on this and test it out locally for a few days to make
> > sure
> > > it
> > > > > doesn't break anything (would appreciate if you would all do the
same).
> > > > 
> > > > How will this be used in your plan?
> > > 
> > > This is what's currently causing deps that GN already knows about to be
> > written
> > > into .d files. With this change, only extra python scripts are written
into
> > .d.
> > > I'm going under the assumption that all other input deps are captured by
GN.
> > > I've inspected several spots and believe this to be true, but still want
to
> be
> > a
> > > bit cautious (by using this myself for a while).
> > > 
> > > Did that make sense?
> > 
> > This patch didn't fix the issue, I 
> > 1. applied this patch
> > 2. ran gn args ..., saw targets generated.
> > 3. then ninja -C ...
> > 
> > got same error.
> 
> You'd need to clear out your .d files:
>   find out/Debug -name "*.d" -delete
> 
> Then, this patch will prevent the problem from happening again (should be able
> to switch back and forth)

Going for it!

[agrieve, 2016-03-09 16:28:15]

The CQ bit was checked by agrieve@chromium.org

[commit-bot: I haz the power, 2016-03-09 16:28:29]

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

[commit-bot: I haz the power, 2016-03-09 16:28:30]

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

[agrieve, 2016-03-09 16:28:31]

The CQ bit was unchecked by agrieve@chromium.org

[commit-bot: I haz the power, 2016-03-09 16:28:31]

No L-G-T-M from a valid reviewer yet. 
CQ run can only be started by full committers or once the patch has
received an L-G-T-M from a full committer.
Even if an L-G-T-M may have been provided, it was from a non-committer,
_not_ a full super star committer.
See http://www.chromium.org/getting-involved/become-a-committer
Note that this has nothing to do with OWNERS files.

[agrieve, 2016-03-09 16:28:53]

On 2016/03/09 16:28:31, commit-bot: I haz the power wrote:
> No L-G-T-M from a valid reviewer yet. 
> CQ run can only be started by full committers or once the patch has
> received an L-G-T-M from a full committer.
> Even if an L-G-T-M may have been provided, it was from a non-committer,
> _not_ a full super star committer.
> See http://www.chromium.org/getting-involved/become-a-committer
> Note that this has nothing to do with OWNERS files.

Err.. right, no l-g-t-m yet :P

[jbudorick, 2016-03-09 16:31:07]

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

[jbudorick, 2016-03-09 16:31:08]

https://codereview.chromium.org/1730993002/diff/1/build/android/gyp/util/buil...
File build/android/gyp/util/build_utils.py (left):

https://codereview.chromium.org/1730993002/diff/1/build/android/gyp/util/buil...
build/android/gyp/util/build_utils.py:502: WriteDepfile(options.depfile,
python_deps + input_paths)
What happened to input_paths?

[agrieve, 2016-03-11 15:21:13]

https://codereview.chromium.org/1730993002/diff/1/build/android/gyp/util/buil...
File build/android/gyp/util/build_utils.py (left):

https://codereview.chromium.org/1730993002/diff/1/build/android/gyp/util/buil...
build/android/gyp/util/build_utils.py:502: WriteDepfile(options.depfile,
python_deps + input_paths)
On 2016/03/09 16:31:08, jbudorick wrote:
> What happened to input_paths?

That's the fix. Have a look at the new comment I added about depfile_deps.

[jbudorick, 2016-03-11 15:24:17]

lgtm

https://codereview.chromium.org/1730993002/diff/1/build/android/gyp/util/buil...
File build/android/gyp/util/build_utils.py (left):

https://codereview.chromium.org/1730993002/diff/1/build/android/gyp/util/buil...
build/android/gyp/util/build_utils.py:502: WriteDepfile(options.depfile,
python_deps + input_paths)
On 2016/03/11 15:21:13, agrieve wrote:
> On 2016/03/09 16:31:08, jbudorick wrote:
> > What happened to input_paths?
> 
> That's the fix. Have a look at the new comment I added about depfile_deps.

definitely skipped right through that comment the first time.

[agrieve, 2016-03-11 15:27:01]

The CQ bit was checked by agrieve@chromium.org

[commit-bot: I haz the power, 2016-03-11 15:27:26]

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

[commit-bot: I haz the power, 2016-03-11 17:08:13]

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

[commit-bot: I haz the power, 2016-03-11 17:08:14]

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

[agrieve, 2016-03-11 18:49:14]

The CQ bit was checked by agrieve@chromium.org

[commit-bot: I haz the power, 2016-03-11 18:49:56]

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

[commit-bot: I haz the power, 2016-03-11 19:39:47]

Description was changed from

==========
Don't write deps that GN already knows about to .d files

It causes extra targets to be compiled when GN args change, and a target
has logic like:

target("foo") {
  if (arg) {
    deps = [":A"]
  } else {
    deps = [":B"]
  }
}

BUG=589311
==========

to

==========
Don't write deps that GN already knows about to .d files

It causes extra targets to be compiled when GN args change, and a target
has logic like:

target("foo") {
  if (arg) {
    deps = [":A"]
  } else {
    deps = [":B"]
  }
}

BUG=589311
==========

[commit-bot: I haz the power, 2016-03-11 19:39:48]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-03-11 19:41:47]

Description was changed from

==========
Don't write deps that GN already knows about to .d files

It causes extra targets to be compiled when GN args change, and a target
has logic like:

target("foo") {
  if (arg) {
    deps = [":A"]
  } else {
    deps = [":B"]
  }
}

BUG=589311
==========

to

==========
Don't write deps that GN already knows about to .d files

It causes extra targets to be compiled when GN args change, and a target
has logic like:

target("foo") {
  if (arg) {
    deps = [":A"]
  } else {
    deps = [":B"]
  }
}

BUG=589311

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

[commit-bot: I haz the power, 2016-03-11 19:41:48]

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