Allow depfile to depend on a Stamp file instead of the default first input.

Allow depfile to depend on a Stamp file instead of the default first input.

BUG=466315

[knn, 2015-03-11 20:35:42]

knn@chromium.org changed reviewers:
+ brettw@chromium.org, thakis@chromium.org

[knn, 2015-03-11 20:35:42]

Grit target builds are broken when the first output of the grd file changes.
This will allow non-breaking changes to occur in the future.

[knn, 2015-03-11 20:51:50]

knn@chromium.org changed reviewers:
+ bauerb@chromium.org

[knn, 2015-03-11 23:16:52]

knn@chromium.org changed reviewers:
+ cjhopman@chromium.org

[Nico, 2015-03-12 18:31:06]

Thanks. Can you add a short test to build_test.py?

(I'm trying to change ninja to treat an edge as dirty instead of erroring on an
unexpected outname in the .d file: https://github.com/martine/ninja/pull/930
This will likely make it into ninja 1.6.)

[knn, 2015-03-12 20:21:31]

On 2015/03/12 18:31:06, Nico (traveling) wrote:
> Thanks. Can you add a short test to build_test.py?
> 
> (I'm trying to change ninja to treat an edge as dirty instead of erroring on
an
> unexpected outname in the .d file: https://github.com/martine/ninja/pull/930
> This will likely make it into ninja 1.6.)

Wow, Thanks! I thought that would not be possible considering that this was a
long standing issue.

Added unittest.

[Nico, 2015-03-12 21:34:25]

lgtm

https://codereview.chromium.org/997813004/diff/40001/grit/tool/build.py
File grit/tool/build.py (right):

https://codereview.chromium.org/997813004/diff/40001/grit/tool/build.py#newco...
grit/tool/build.py:122: generated will depend on the depfile itself instead of
the
this is kinda weird

[cjhopman, 2015-03-12 21:38:35]

https://codereview.chromium.org/997813004/diff/40001/grit/tool/build.py
File grit/tool/build.py (right):

https://codereview.chromium.org/997813004/diff/40001/grit/tool/build.py#newco...
grit/tool/build.py:122: generated will depend on the depfile itself instead of
the
On 2015/03/12 21:34:24, Nico (traveling) wrote:
> this is kinda weird

Yeah, and it'll need to be changed if we change gn actions to use ninja's
depsmode. I'd say you're better off using the version in patchset 1.

[knn, 2015-03-13 12:13:43]

On 2015/03/12 21:38:35, cjhopman wrote:
> https://codereview.chromium.org/997813004/diff/40001/grit/tool/build.py
> File grit/tool/build.py (right):
> 
>
https://codereview.chromium.org/997813004/diff/40001/grit/tool/build.py#newco...
> grit/tool/build.py:122: generated will depend on the depfile itself instead of
> the
> On 2015/03/12 21:34:24, Nico (traveling) wrote:
> > this is kinda weird
> 
> Yeah, and it'll need to be changed if we change gn actions to use ninja's
> depsmode. I'd say you're better off using the version in patchset 1.

Thanks Chris! Did not know about deps mode before this.

[cjhopman, 2015-03-14 00:36:55]

lgtm

[Nico, 2015-03-23 18:59:35]

https://codereview.chromium.org/997813004/diff/60001/grit/tool/build_unittest.py
File grit/tool/build_unittest.py (right):

https://codereview.chromium.org/997813004/diff/60001/grit/tool/build_unittest...
grit/tool/build_unittest.py:330: self.assertTrue(second_mtime > first_mtime)
this fails for me:

======================================================================
FAIL: testGenerateDepFileWithDependOnStamp
(grit.tool.build_unittest.BuildUnittest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Volumes/MacintoshHD2/src/grit-i18n/grit/tool/build_unittest.py", line
330, in testGenerateDepFileWithDependOnStamp
    self.assertTrue(second_mtime > first_mtime)
AssertionError: False is not true


You probably can't assume that 2s (stat resolution on win) pass when running
this twice?

Here's a hack you can use: set the mtime before rebuilding to something very
old, and then check that it has changed (example:
https://codereview.chromium.org/757213003/diff/60001/grit/tool/build_unittest.py
)

[knn, 2015-03-23 19:28:15]

https://codereview.chromium.org/997813004/diff/60001/grit/tool/build_unittest.py
File grit/tool/build_unittest.py (right):

https://codereview.chromium.org/997813004/diff/60001/grit/tool/build_unittest...
grit/tool/build_unittest.py:330: self.assertTrue(second_mtime > first_mtime)
On 2015/03/23 18:59:35, Nico wrote:
> this fails for me:
> 
> ======================================================================
> FAIL: testGenerateDepFileWithDependOnStamp
> (grit.tool.build_unittest.BuildUnittest)
> ----------------------------------------------------------------------
> Traceback (most recent call last):
>   File "/Volumes/MacintoshHD2/src/grit-i18n/grit/tool/build_unittest.py", line
> 330, in testGenerateDepFileWithDependOnStamp
>     self.assertTrue(second_mtime > first_mtime)
> AssertionError: False is not true
> 
> 
> You probably can't assume that 2s (stat resolution on win) pass when running
> this twice?
> 
> Here's a hack you can use: set the mtime before rebuilding to something very
> old, and then check that it has changed (example:
>
https://codereview.chromium.org/757213003/diff/60001/grit/tool/build_unittest.py
> )

Oh I see :(
Thanks!

[Nico, 2015-03-25 16:48:04]

r188

[knn, 2015-03-25 16:53:06]

On 2015/03/25 16:48:04, Nico wrote:
> r188

Thanks!