Add depfile support to android build scripts

Add depfile support to android build scripts

This adds the --depfile option to a bunch of android's build scripts
(the ones used by gn). It also adds a simple function in build_utils to
calculate the paths of the imported python files.

Currently, the written depfiles just contain that list of python files
(which is much more accurate than the way this is handled in gyp).

BUG=359249
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=279546

[cjhopman, 2014-06-23 15:08:21]

nyquist: *

cced thakis for my comments about ninja depfile handling

https://codereview.chromium.org/341823003/diff/20001/build/config/android/int...
File build/config/android/internal_rules.gni (right):

https://codereview.chromium.org/341823003/diff/20001/build/config/android/int...
build/config/android/internal_rules.gni:14: outputs = [depfile, invoker.output]
afaict, ninja expects the depfile to specify the dependencies for the first
output :/. I would really prefer to just be able to write a list of paths that
ninja would add to the inputs.

Also, is it possible that ninja doesn't treat the depfile as an input to the
action? I expect that if it did, it couldn't also be specified as an output. If
it isn't treated as an input, that is rather unexpected.

[Nico, 2014-06-23 15:32:00]

The reason the depfile format is like it is is because it's the same format make
uses for depfiles. That way, there's one fewer file format in the world. (And
gcc already had a switch to write depfiles in this format.)

What do you mean "treats as input"? If the depfile is touched, I think the rule
isn't rerun (with the reasoning probably being that the depfile should only be
written by the edge that lists is as depfile).

https://codereview.chromium.org/341823003/diff/20001/build/android/gyp/util/b...
File build/android/gyp/util/build_utils.py (right):

https://codereview.chromium.org/341823003/diff/20001/build/android/gyp/util/b...
build/android/gyp/util/build_utils.py:230: p for p in abs_module_paths if
p.startswith(chromium_src)]
See thread ".d files and absolute paths" on gn-dev where the conclusion was """
I would expect .d files to allow me to follow the same rules I do for
writing .ninja files. GN's current rules are to write relative paths
for anything inside the source root ("src/") (probably excepting the
case where the build directory is not inside the source root), and
absolute paths other times.
"""

https://codereview.chromium.org/341823003/diff/20001/build/android/gyp/util/b...
build/android/gyp/util/build_utils.py:237: return
list(set(non_system_module_paths))
set() means this won't have a deterministic order

[cjhopman, 2014-06-23 16:07:20]

On 2014/06/23 15:32:00, Nico (away) wrote:
> The reason the depfile format is like it is is because it's the same format
make
> uses for depfiles. That way, there's one fewer file format in the world. (And
> gcc already had a switch to write depfiles in this format.)

Yeah, I know why it's that way. As a user of ninja, it's (slightly)
inconvenient. The most inconvenient part is that ninja requires the depfile to
refer to the first output of the action which necessarily couples the
implementation of the action (the .py script) and the specification (the .gn
file). I should update `gn help depfile` to clearly state that the depfile must
specify deps for the first output of the action.

> 
> What do you mean "treats as input"? If the depfile is touched, I think the
rule
> isn't rerun (with the reasoning probably being that the depfile should only be
> written by the edge that lists is as depfile).

Yeah, I would want the action to be rerun when the depfile is touched.
Basically, I want to be able to write a (very) conservative depfile when
generating (or regenerating) the build files that the action itself then
rewrites to be more specific when it first runs. The workaround for ninja not
treating the depfile as an input is very straightforward, so it's not really a
big deal.

[Nico, 2014-06-23 16:10:46]

On Mon, Jun 23, 2014 at 9:07 AM, <cjhopman@chromium.org> wrote:

> On 2014/06/23 15:32:00, Nico (away) wrote:
>
>> The reason the depfile format is like it is is because it's the same
>> format
>>
> make
>
>> uses for depfiles. That way, there's one fewer file format in the world.
>> (And
>> gcc already had a switch to write depfiles in this format.)
>>
>
> Yeah, I know why it's that way. As a user of ninja, it's (slightly)
> inconvenient. The most inconvenient part is that ninja requires the
> depfile to
> refer to the first output of the action which necessarily couples the
> implementation of the action (the .py script) and the specification (the
> .gn
> file). I should update `gn help depfile` to clearly state that the depfile
> must
> specify deps for the first output of the action.


What's worse, ninja checks that the name matches in non-deps mode but
doesn't in deps mode (with a TODO for doing so) :-/


>
>
>
>  What do you mean "treats as input"? If the depfile is touched, I think the
>>
> rule
>
>> isn't rerun (with the reasoning probably being that the depfile should
>> only be
>> written by the edge that lists is as depfile).
>>
>
> Yeah, I would want the action to be rerun when the depfile is touched.
> Basically, I want to be able to write a (very) conservative depfile when
> generating (or regenerating) the build files that the action itself then
> rewrites to be more specific when it first runs. The workaround for ninja
> not
> treating the depfile as an input is very straightforward, so it's not
> really a
> big deal.
>
>
>
> https://codereview.chromium.org/341823003/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[cjhopman, 2014-06-23 17:26:56]

https://codereview.chromium.org/341823003/diff/20001/build/android/gyp/util/b...
File build/android/gyp/util/build_utils.py (right):

https://codereview.chromium.org/341823003/diff/20001/build/android/gyp/util/b...
build/android/gyp/util/build_utils.py:230: p for p in abs_module_paths if
p.startswith(chromium_src)]
On 2014/06/23 15:32:00, Nico (away) wrote:
> See thread ".d files and absolute paths" on gn-dev where the conclusion was
"""
> I would expect .d files to allow me to follow the same rules I do for
> writing .ninja files. GN's current rules are to write relative paths
> for anything inside the source root ("src/") (probably excepting the
> case where the build directory is not inside the source root), and
> absolute paths other times.
> """

Done.

https://codereview.chromium.org/341823003/diff/20001/build/android/gyp/util/b...
build/android/gyp/util/build_utils.py:237: return
list(set(non_system_module_paths))
On 2014/06/23 15:32:00, Nico (away) wrote:
> set() means this won't have a deterministic order

Done.

[nyquist, 2014-06-23 18:41:58]

base/android and build/android lgtm

https://codereview.chromium.org/341823003/diff/50010/build/android/gyp/util/b...
File build/android/gyp/util/build_utils.py (right):

https://codereview.chromium.org/341823003/diff/50010/build/android/gyp/util/b...
build/android/gyp/util/build_utils.py:244: help='Path to depfile. This should be
specified as the '
Should 'should' be 'must'?

[cjhopman, 2014-06-24 18:12:53]

brettw: build/config OWNERS

https://codereview.chromium.org/341823003/diff/50010/build/android/gyp/util/b...
File build/android/gyp/util/build_utils.py (right):

https://codereview.chromium.org/341823003/diff/50010/build/android/gyp/util/b...
build/android/gyp/util/build_utils.py:244: help='Path to depfile. This should be
specified as the '
On 2014/06/23 18:41:57, nyquist wrote:
> Should 'should' be 'must'?

Done. I read this as "'Should' should be 'must'."

[brettw, 2014-06-24 19:44:27]

build/config LGTM

[cjhopman, 2014-06-24 20:15:12]

The CQ bit was checked by cjhopman@chromium.org

[commit-bot: I haz the power, 2014-06-24 20:17:05]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/cjhopman@chromium.org/341823003/90001

[commit-bot: I haz the power, 2014-06-24 23:38:35]

Change committed as 279546