Add the ability to generate a depfile when running grit build.

Add the ability to generate a depfile when running grit build.

This is analogous to the ability of some compilers to generate depfiles
that contain the .h files a .cc file depends on when it compiles it.
This is useful for speeding up build systems because the input
dependencies don't need to be determined before compilation.

As there are no pre-determined outputs from a .grd file (all the outputs
are determined by the file contents) the depfile refers to itself and
acts as a stamp for the generated outputs.

BUG=297672

[Jói, 2013-10-04 09:47:11]

Neat.

I'd like to see a unit test for this added logic. Also a few nits below, nothing
major.

Cheers,
Jói

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

https://codereview.chromium.org/25683011/diff/1/grit/tool/build.py#newcode155
grit/tool/build.py:155: self.GenerateDepsfile(opts.input, deps_dir)
I can't find the word "depsfile" in general usage on the web. Can we use "deps
file" instead? That would make this function name GenerateDepsFile and would
affect a few of the variable names below that are of the form depsfile_xyz.

https://codereview.chromium.org/25683011/diff/1/grit/tool/build.py#newcode313
grit/tool/build.py:313: # Generate a depsfile that contains the imlicit
dependencies of the input grd.
This should be a docstring, i.e. enclosed in """...""" and coming right after
the def GenerateDepsfile line.

https://codereview.chromium.org/25683011/diff/1/grit/tool/build.py#newcode351
grit/tool/build.py:351: # Creates directories necessary to contain |file|.
Move to docstring.

https://codereview.chromium.org/25683011/diff/1/grit/tool/build.py#newcode352
grit/tool/build.py:352: def MakeDirectoriesTo(self, file):
This can be an @staticmethod and skip the |self| parameter. There is at least
one example of this in the file.

[tony, 2013-10-04 21:39:46]

Clever!

How is this going to work on Windows when using MSVS?  I thought MSVS doesn't
use .d files.

[brettw, 2013-10-04 21:40:45]

This will be a ninja-only thing.

[Nico, 2013-10-04 21:42:02]

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

https://codereview.chromium.org/25683011/diff/1/grit/tool/build.py#newcode155
grit/tool/build.py:155: self.GenerateDepsfile(opts.input, deps_dir)
On 2013/10/04 09:47:12, Jói wrote:
> I can't find the word "depsfile" in general usage on the web. Can we use "deps
> file" instead? That would make this function name GenerateDepsFile and would
> affect a few of the variable names below that are of the form depsfile_xyz.

It's called "depfile" in ninja:
http://martine.github.io/ninja/manual.html#_depfile That's probably a good name.
(i.e. drop the 's')

[tony, 2013-10-04 21:43:09]

On 2013/10/04 21:40:45, brettw wrote:
> This will be a ninja-only thing.

Does that mean this will be used by gn and not gyp?  I didn't think .gyp files
knew what the current generator is (it could, but I don't think that would solve
this problem).

It's fine to be gn only; I'm just curious.

[koz (OOO until 15th September), 2013-10-09 06:01:16]

Thanks for the feedback! I've added a test an responded to comments.

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

https://codereview.chromium.org/25683011/diff/1/grit/tool/build.py#newcode155
grit/tool/build.py:155: self.GenerateDepsfile(opts.input, deps_dir)
On 2013/10/04 21:42:02, Nico wrote:
> On 2013/10/04 09:47:12, Jói wrote:
> > I can't find the word "depsfile" in general usage on the web. Can we use
"deps
> > file" instead? That would make this function name GenerateDepsFile and would
> > affect a few of the variable names below that are of the form depsfile_xyz.
> 
> It's called "depfile" in ninja:
> http://martine.github.io/ninja/manual.html#_depfile That's probably a good
name.
> (i.e. drop the 's')

Renamed to depfile.

https://codereview.chromium.org/25683011/diff/1/grit/tool/build.py#newcode313
grit/tool/build.py:313: # Generate a depsfile that contains the imlicit
dependencies of the input grd.
On 2013/10/04 09:47:12, Jói wrote:
> This should be a docstring, i.e. enclosed in """...""" and coming right after
> the def GenerateDepsfile line.

Done.

https://codereview.chromium.org/25683011/diff/1/grit/tool/build.py#newcode351
grit/tool/build.py:351: # Creates directories necessary to contain |file|.
On 2013/10/04 09:47:12, Jói wrote:
> Move to docstring.

Done.

https://codereview.chromium.org/25683011/diff/1/grit/tool/build.py#newcode352
grit/tool/build.py:352: def MakeDirectoriesTo(self, file):
On 2013/10/04 09:47:12, Jói wrote:
> This can be an @staticmethod and skip the |self| parameter. There is at least
> one example of this in the file.

Done.

[Jói, 2013-10-10 11:11:32]

LGTM

[koz (OOO until 15th September), 2013-10-11 04:54:09]

On 2013/10/10 11:11:32, Jói wrote:
> LGTM

Thanks, Joi. I'm not a committer on grit, so can you commit this for me?

[Jói, 2013-10-11 09:54:19]

On 2013/10/11 04:54:09, koz wrote:
> On 2013/10/10 11:11:32, Jói wrote:
> > LGTM
> 
> Thanks, Joi. I'm not a committer on grit, so can you commit this for me?

Landed as r137.

[Nico, 2013-12-31 01:07:42]

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

https://codereview.chromium.org/25683011/diff/10001/grit/tool/build.py#newcod...
grit/tool/build.py:350: outfile.writelines(depfile_contents)
I'm currently looking at using this in gyp, and I'm wondering: Why does the flag
take a directory, not a path? As is, the generator or the gyp file will need to
know that the .d file is in os.path.join(dep_dir_path, grd_filename + '.d').
It'd be slightly nicer and simpler everywhere, if grit just got a --depfile
parameter instead of --dep-dir, no? Why was this design chosen?

[Nico, 2013-12-31 01:41:55]

On 2013/12/31 01:07:42, Nico wrote:
> https://codereview.chromium.org/25683011/diff/10001/grit/tool/build.py
> File grit/tool/build.py (right):
> 
>
https://codereview.chromium.org/25683011/diff/10001/grit/tool/build.py#newcod...
> grit/tool/build.py:350: outfile.writelines(depfile_contents)
> I'm currently looking at using this in gyp, and I'm wondering: Why does the
flag
> take a directory, not a path? As is, the generator or the gyp file will need
to
> know that the .d file is in os.path.join(dep_dir_path, grd_filename + '.d').
> It'd be slightly nicer and simpler everywhere, if grit just got a --depfile
> parameter instead of --dep-dir, no? Why was this design chosen?

An example, to make this more concrete:

Let's say I change this in grit_action.gypi:

+  'ninja_depfile': '<(grit_out_dir)/<(grit_grd_file).d',
   'outputs': [
     '<!@pymod_do_main(grit_info <@(grit_defines) <@(grit_additional_defines) '
         '--outputs \'<(grit_out_dir)\' '
@@ -35,7 +40,9 @@
              '-f', '<(grit_resource_ids)',
              '-o', '<(grit_out_dir)',
              '<@(grit_defines)',
-             '<@(grit_additional_defines)' ],
+             '<@(grit_additional_defines)',
+             '--dep-dir=<(grit_out_dir)',
+             ],

Now, net/net.gyp sets

            'grit_grd_file': 'base/net_resources.grd',

so --dep-dir becomes ../out/Release/gen/net (note: no "base") but ninja_depfile
evaluates to "gen/net/base/net_resources.grd.d" (note: "base/"). So the gyp file
would really have to say ` 'ninja_depfile':
'<(grit_out_dir)/os.path.basename(<(grit_grd_file)).d',` which is slightly
annoying to express. With a --depfile  parameter, things would Just Work I
think.

[Nico, 2013-12-31 06:19:52]

On 2013/12/31 01:41:55, Nico wrote:
> On 2013/12/31 01:07:42, Nico wrote:
> > https://codereview.chromium.org/25683011/diff/10001/grit/tool/build.py
> > File grit/tool/build.py (right):
> > 
> >
>
https://codereview.chromium.org/25683011/diff/10001/grit/tool/build.py#newcod...
> > grit/tool/build.py:350: outfile.writelines(depfile_contents)
> > I'm currently looking at using this in gyp, and I'm wondering: Why does the
> flag
> > take a directory, not a path? As is, the generator or the gyp file will need
> to
> > know that the .d file is in os.path.join(dep_dir_path, grd_filename + '.d').
> > It'd be slightly nicer and simpler everywhere, if grit just got a --depfile
> > parameter instead of --dep-dir, no? Why was this design chosen?
> 
> An example, to make this more concrete:
> 
> Let's say I change this in grit_action.gypi:
> 
> +  'ninja_depfile': '<(grit_out_dir)/<(grit_grd_file).d',
>    'outputs': [
>      '<!@pymod_do_main(grit_info <@(grit_defines) <@(grit_additional_defines)
'
>          '--outputs \'<(grit_out_dir)\' '
> @@ -35,7 +40,9 @@
>               '-f', '<(grit_resource_ids)',
>               '-o', '<(grit_out_dir)',
>               '<@(grit_defines)',
> -             '<@(grit_additional_defines)' ],
> +             '<@(grit_additional_defines)',
> +             '--dep-dir=<(grit_out_dir)',
> +             ],
> 
> Now, net/net.gyp sets
> 
>             'grit_grd_file': 'base/net_resources.grd',
> 
> so --dep-dir becomes ../out/Release/gen/net (note: no "base") but
ninja_depfile
> evaluates to "gen/net/base/net_resources.grd.d" (note: "base/"). So the gyp
file
> would really have to say ` 'ninja_depfile':
> '<(grit_out_dir)/os.path.basename(<(grit_grd_file)).d',` which is slightly
> annoying to express. With a --depfile  parameter, things would Just Work I
> think.

Oh, I guess the base dir for stuff in the depfile needs to be specified somehow
too…