Please validate that this advice on depfile usage is correct

[agr...@chromium.org]

I'm working on updating the help text for GN's depfile variable, and wanted to see if the following guidance makes sense:

1. A depfile should be used only when a target depends on files that are

not already specified by a target's inputs and sources. Likewise,

depfiles should specify only those dependencies not already included

in sources or inputs.

Rationale for not including edges that already exist within ninja files:

 - Dependencies can change, and the ones listed in depfiles are always one build out-of-date. So, it's best to keep them to the minimal set (aka, those not already listed in .ninja files)

2. Although depfiles are created by an action, they should not be listed

in the action's outputs unless another target will use the file as an input.

[Evan Martin]

Another way to look at it: a depfile is only needed if you don't know
the full set of dependencies before running the command, and running
the command generates more dependency information.

I think it'd be harmless to list the depfile as an output. Now that
you mention it, it's kind of an oversight, but it's never mattered.

> --
> You received this message because you are subscribed to the Google Groups
> "ninja-build" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to ninja-build...@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.

[Andrew Grieve]

Thanks Evan.

The problem we were finding with it being listed as an output, was that GN targets that depended on the action now list the .d file as an input, and so changes to the .d file would trigger a rebuild of dependent targets. I'm having a tough time thinking why this is an issue, but it ended up causing ninja to be upset because of a circular dependency when one of the dependencies in GN was changed.

> email to ninja-build+unsubscribe@googlegroups.com.

[Dirk Pranke]

As I had mentioned to Andrew on the original CL, I believe ninja's current implementation is such that a (gcc) depfile is transient: the compiler

generates them, and ninja reads them and then deletes them, storing the information in .ninja_deps instead. If ninja didn't take into account

the fact that the depfile was also an output, this wouldn't work, and I don't know if it does (and is required to)?

Though obviously it would be easy enough to test this, it's more a question of whether this is supposed to be supported or not.

-- Dirk

[Andrew Grieve]

Just tested, and I don't see ninja deleting .d files locally.

Also of note is that when .d files cause an issue like a circular dependency, deleting them all with "find -name *.d -delete" causes ninja to fix up the dependencies.

[Evan Martin]

On Fri, Sep 16, 2016 at 11:17 AM, Andrew Grieve <agr...@chromium.org> wrote:

> The problem we were finding with it being listed as an output, was that GN
> targets that depended on the action now list the .d file as an input,

Ah, this is a GN thing then. Ninja itself doesn't require a
downstream build target to list all the outputs as inputs. I think at
the GN level it would likely make more sense to not list the depfile
as an output, but I don't know much about GN.

[Evan Martin]

On Fri, Sep 16, 2016 at 12:05 PM, Dirk Pranke <dpr...@chromium.org> wrote:
> As I had mentioned to Andrew on the original CL, I believe ninja's current
> implementation is such that a (gcc) depfile is transient: the compiler
> generates them, and ninja reads them and then deletes them, storing the
> information in .ninja_deps instead. If ninja didn't take into account
> the fact that the depfile was also an output, this wouldn't work, and I
> don't know if it does (and is required to)?

I had completely forgotten about this feature, heh. If you specify
deps=gcc (which you should -- it's opting into the faster behavior),
Ninja will delete the file after reading it.

See here:
https://ninja-build.org/manual.html#_deps

I think this is another argument for not listing depfiles as outputs
-- they're effectively just a temporary file.

[Andrew Grieve]

Tried adding deps=gcc, but it results in:

ninja: error: toolchain.ninja:92: multiple outputs aren't (yet?) supported by depslog; bring this up on the mailing list if it affects you

Here's the ninja targets:

rule __android_webview_android_webview_java__compile_java__javac___build_toolchain_android_clang_arm__rule
  command = python ../../build/android/gyp/javac.py --depfile=gen/android_webview/android_webview_java__compile_java__javac.d --jar-path=gen/android_webview/android_webview_java__compile_java.javac.jar --java-srcjars=\[\"gen/android_webview/native/aw_permission_request_resource.srcjar\"\] --java-srcjars=@FileArg\(gen/android_webview/android_webview_java.build_config$:javac$:srcjars\) --classpath=@FileArg\(gen/android_webview/android_webview_java.build_config$:javac$:interface_classpath\) --bootclasspath=lib.java/android.interface.jar --java-version=1.7 --chromium-code=1 @gen/android_webview/android_webview_java.sources
  description = ACTION //android_webview:android_webview_java__compile_java__javac(//build/toolchain/android:clang_arm)
  restat = 1

build gen/android_webview/android_webview_java__compile_java.javac.jar gen/android_webview/android_webview_java__compile_java.javac.jar.md5.stamp: __android_webview_android_webview_java__compile_java__javac___build_toolchain_android_clang_arm__rule | obj/android_webview/android_webview_java__compile_java__javac.inputdeps.stamp
  depfile = gen/android_webview/android_webview_java__compile_java__javac.d 

  deps = gcc

[Evan Martin]

https://github.com/ninja-build/ninja/issues/1184 has some more on this
issue. Leaving deps= off should be fine.

[Andrew Grieve]

Gotcha. Just tried changing GN to add deps=gcc only when there is just one output listed and that seemed to work. Seems like it's probably not worth special-casing this unless we run into some performance issues with our builds.

[Nico Weber]

(I'm pretty sure gn uses deps=gcc for compilations, and that's the most common edge type in practice.)

[Andrew Grieve]

It does. An actually - if you have an advice on how to avoid circular dependencies caused by depfiles when dependencies are inverted within .ninja files, that's the issue that started me asking all these questions :S.

https://bugs.chromium.org/p/chromium/issues/detail?id=639042

[Andrew Grieve]

nvm - Nico pointed out on the bug that it's already a known Ninja issue: https://github.com/ninja-build/ninja/issues/664