multiple outputs aren't (yet?) supported by depslog

[jvp...@g.rit.edu]

I know this has been brought up on the mailing list before. However, I think I'd like to try to resolve this issue. I'm running into the above error when trying to generate precompiled headers under MSVC. To build precompiled headers under MSVC, you need to do something like the following:

cl.exe dummy.cpp /Ycheader.hpp

This produces two output files: header.pch and dummy.obj. The former is used when compiling all the other translation units into object files, and the latter is used during the linking step to link in any symbols defined in header.hpp. This means that, for completeness, the Ninja build step should have two outputs. Since this is a compilation step, you'd want to add /showIncludes to generate header deps. Hence the error message. In the short term, I can just treat the build step as having a single output, but that's not ideal.

How hard would it be to fix this in Ninja?

- Jim

[Evan Martin]

The basic problem of dependencies loading is: for a given build rule,
quickly look up the set of header files it depends on.
The strangely hard part is coming up with a unique ID for the rule!

When each build rule references its own "foo.o.d" deps file, it's easy
to map that to a path and the data in the path. When we eliminated
these separate deps files, we no longer had an easy key to look up the
data from, so in the depslog database I went with the single output
file path as the key.

This is why it doesn't work with multiple outputs. It was a bad
design, I am sorry. My only excuse is that I picked it knowing that
we could change it later.

Some options to fix this:
- make the key instead be the *set* of output files
- require build rules with multiple outputs to specify some sort of
key (could even reuse the "depfile" string perhaps)

Here are some docs on the dependency log format:
https://github.com/ninja-build/ninja/blob/master/src/deps_log.h#L29

The first of those options, where the key be a set of outputs is
appealing from a user standpoint, but a bit hard to implement because
the format is designed to be dense, where we only have 32 bits for the
key. I can imagine schemes (all problems can be solved using another
layer of indirection) but they'd be some work. It wasn't clear if you
were volunteering to fix it... :)

> --
> 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.

[Nico Weber]

In addition to what Evan said: While this isn't supported yet, you can work around the limitation by only giving the pch edge a single output, and by giving all edges that use the pch an implicit dependency on that output, for example:

build obj/base/base/precompile.cc.o: cxx ../../build/precompile.cc || obj/base/base.inputdeps.stamp

  cflags_cc = ${cflags_cc} /Ycbuild/precompile.h

build obj/base/base/barrier_closure.obj: cxx ../../base/barrier_closure.cc | obj/base/base/precompile.cc.o

That's how gyp and gn deal with this in the Chromium build.

(On non-Windows, you can also say "deps=" to disable depslog for infrequent edges with multiple outputs, but that doesn't help you on Windows if you need "deps=msvc".)

[jvp...@g.rit.edu]

On Wednesday, March 30, 2016 at 10:28:37 AM UTC-5, Evan Martin wrote:

This is why it doesn't work with multiple outputs.  It was a bad
design, I am sorry.  My only excuse is that I picked it knowing that
we could change it later.

No worries! There's not much sense in coding up support for a feature until it's clear that it's needed, after all.

 

Some options to fix this:
- make the key instead be the *set* of output files
- require build rules with multiple outputs to specify some sort of
key (could even reuse the "depfile" string perhaps)

As you say, the former seems like a much more usable way of doing things, and probably isn't *that* hard to implement. The latter would work, but isn't really that much simpler than Nico's workaround (I'm already doing something very similar).

I actually wonder if the key could just be the first output, as it currently is. This would fall flat on its face if you changed the order of the outputs, but I'm struggling to think of a time that would actually happen. It looks like the only thing that would need to change for this, aside from removing a few checks, would be to enhance `ninja -t deps` to take an output, find the edge that owns it, and grab that edge's *first* output to pass to GetDeps for lookup.

Does that sound sane, or do you think it would be too brittle?

 

It wasn't clear if you were volunteering to fix it... :)

I'd be happy to write a patch! Once we agree on a basic strategy, I'll try to put something together and write a test.

- Jim

[Jason Gunthorpe]

On Tuesday, March 29, 2016 at 7:27:37 PM UTC-6, jvp...@g.rit.edu wrote:

I know this has been brought up on the mailing list before. However, I think I'd like to try to resolve this issue. I'm running into the above error when trying to generate precompiled headers under MSVC. To build precompiled headers under MSVC, you need to do something like the following:

I would also like to add my voice that this is something needed. My case is code generators that produce multiple outputs from complex (ie like #include) inputs where the generator script supports gcc deps file writing.

One special wrinkle is that the generator script use a compare and update scheme and thus uses restat=1, which complicated building a work around.

I'd like to write:

rule gen

   command = generate --in $in --out_cc a.cc --out_h a.h --deps a.cc.d

   depsfile = a.cc.d

   deps=gcc

   restat=1

build a.cc a.h : gen a.inc

My workaround is then:

rule gen

   command = generate --in $in --out_cc a.cc --out_h a.h --deps a.cc.d && touch .stamp.a.cc.d

   depsfile = a.cc.d

   deps=gcc

   restat=1

build .stamp.a.cc.d : gen a.inc

rule dummy

  command = true

  restat=1

build a.cc a.h : dummy .stamp.a.cc.d

I just finished converting a fairly large and complex build scheme over to using ninja as the back end, and this is pretty much the only thing I've found that isn't prefect, thanks for a great tool!

Regarding, Evan's points - for my case, I'm perfectly happy if ninja requires depfile to be a stable and globally unique key to load the deps log, my generator scripts guarantee that.

Alternatively, I'd also be happy enough if the key was just the first output, that is what the above work around basically does. Again, my generator scripts produce a stable first output.

It would also be OK to just clone the deps list for every output, that overhead would not break my use cases..

Jason

[Sergey Semushin]

The sad thing I noticed is that if .pch file is not dirty, cl.exe /showIncludes will not produce any includes in output and that's why ninja /t clean may produce erroneous dependencies if pch.obj is treated as the only output. The soultion seems to be to treat .pch file as actual output and .obj as discardable side effect, adding implicit dependency of every obj file and linking on .pch, which of course adds a lot of ugliness to build specification.

[Dan Willemsen]

We're running into problems with this in Android as well, and while it can probably be worked around, it seems like all the workarounds have downsides that we'd prefer to avoid.

With multiple outputs, there's no difference between loading deps from depfiles on disk or from the depslog -- they both require the first output to be the one named in the depfile. Changing the order of these outputs would break loading the depfile as well as not working with the depslog. As long as that holds true, I don't see any reason to allow depfiles but ban depslog.

So the ban could be lifted and the first output could just be considered special like it is today with depfiles. Or more practically, the outputs / implicit outputs split could be used to say that implicit outputs aren't considered when reading deps, so the outputs list should be used, which can still be restricted to a single entry. I've uploaded a patch for this here: https://github.com/ninja-build/ninja/pull/1236

Alternatively I could switch the key over to the depfile string as well, that seems like it would work well, but would invalidate every current depslog. (I could envision some problems if two rules used the same depfile, but the same problems would exist without the depslog too)

- Dan

--