ninja: split out the logic for linking from target writing

ninja: split out the logic for linking from target writing

There was a bunch of code in WriteTarget that only applied when
writing out something involving the linker.  Split this out as
a step from separating output_binary from output.

Committed: http://code.google.com/p/gyp/source/detail?r=1147

[Evan Martin, 2012-01-11 23:11:43]

One question about something I may have broken: what does it mean to have a mac
bundle that doesn't have a link step?

[Evan Martin, 2012-01-11 23:12:38]

http://codereview.chromium.org/9107028/diff/1/pylib/gyp/generator/ninja.py
File pylib/gyp/generator/ninja.py (left):

http://codereview.chromium.org/9107028/diff/1/pylib/gyp/generator/ninja.py#ol...
pylib/gyp/generator/ninja.py:617: output_binary = self.ComputeOutput(spec,
type='none')
These two lines are the ones I'm not sure about -- we're writing out a special
stamp file when we have a mac bundle that has no linker input (aka .o files).

[Nico, 2012-01-11 23:27:20]

On 2012/01/11 23:11:43, Evan Martin wrote:
> One question about something I may have broken: what does it mean to have a
mac
> bundle that doesn't have a link step?

It's just a collection of resource files in that case. The only example in
chromium that I'm aware of is
http://codesearch.google.com/codesearch#OAMlx_jo-ck/src/chrome/app/policy/pol...
.  I added a reduced version of that as a gyp test in
http://code.google.com/p/gyp/source/detail?r=1136 .



Your patch (I haven't looked at it yet) passes all tests except of this one:

/usr/bin/python test/mac/gyptest-depend-on-bundle.py
Missing files:
`/Volumes/MacintoshHD2/src/gyp-rw/out/testworkarea/testgyp.uA9TnG/depend-on-bundle/out/Default/my_bundle.framework/my_bundle'
FAILED test at line 320 of
/Volumes/MacintoshHD2/src/gyp-rw/test/lib/TestCommon.py (must_exist)
	from line 119 of /Volumes/MacintoshHD2/src/gyp-rw/test/lib/TestGyp.py
(built_file_must_exist)
	from line 31 of test/mac/gyptest-depend-on-bundle.py


Here are the commands that are run with your patch:

hummer:foo thakis$ ~/src/chrome-git/src/third_party/ninja/ninja -C out/Default/
-v dependent_on_bundle
ninja: Entering directory `out/Default/'
[1/4] gcc -MMD -MF obj/my_bundle.bundle.o.d   -fasm-blocks -mpascal-strings -Os
-gdwarf-2 -arch i386   -c ../../bundle.c -o obj/my_bundle.bundle.o
[2/4] gcc -MMD -MF obj/dependent_on_bundle.executable.o.d   -fasm-blocks
-mpascal-strings -Os -gdwarf-2 -arch i386   -c ../../executable.c -o
obj/dependent_on_bundle.executable.o
[3/4] g++ -shared -arch i386 -L. -o my_bundle.framework/Versions/A/my_bundle
obj/my_bundle.bundle.o 
[4/4] g++ -arch i386 -L. -o dependent_on_bundle
obj/dependent_on_bundle.executable.o my_bundle.framework/Versions/A/my_bundle 


Here's what happens without your patch:

hummer:foo thakis$ cd ..
hummer:gyp-rw thakis$ svn revert -R .
Reverted 'pylib/gyp/generator/ninja.py'
cd hummer:gyp-rw thakis$ cd foo
hummer:foo thakis$ rm -rf out/
hummer:foo thakis$ GYP_GENERATORS=ninja ../gyp --depth .
hummer:foo thakis$ ~/src/chrome-git/src/third_party/ninja/ninja -C out/Default/
-v dependent_on_bundle
ninja: Entering directory `out/Default/'
[1/7] ./gyp-mac-tool copy-bundle-resource ../../English.lproj/InfoPlist.strings
my_bundle.framework/Versions/A/Resources/English.lproj/InfoPlist.strings
[2/7] ./gyp-mac-tool copy-info-plist ../../Info.plist
my_bundle.framework/Versions/A/Resources/Info.plist
[3/7] gcc -MMD -MF obj/my_bundle.bundle.o.d   -fasm-blocks -mpascal-strings -Os
-gdwarf-2 -arch i386   -c ../../bundle.c -o obj/my_bundle.bundle.o
[4/7] gcc -MMD -MF obj/dependent_on_bundle.executable.o.d   -fasm-blocks
-mpascal-strings -Os -gdwarf-2 -arch i386   -c ../../executable.c -o
obj/dependent_on_bundle.executable.o
[5/7] g++ -shared -arch i386 -L. -o my_bundle.framework/Versions/A/my_bundle
obj/my_bundle.bundle.o 
[6/7] ./gyp-mac-tool package-framework my_bundle.framework A; touch
my_bundle.framework
[7/7] g++ -arch i386 -L. -o dependent_on_bundle
obj/dependent_on_bundle.executable.o my_bundle.framework/Versions/A/my_bundle 

It looks like dependent_on_bundle loses its dependency on the my_bundle bundle.

[Evan Martin, 2012-01-11 23:34:52]

On 2012/01/11 23:27:20, Nico wrote:
> It looks like dependent_on_bundle loses its dependency on the my_bundle
bundle.

Thanks, I diffed the generated files and found:
---
out/testworkarea/testgyp.CLfkxU/depend-on-bundle//out/Default/obj/dependent_on_bundle.ninja	2012-01-11
15:33:19.382634567 -0800
+++
out/testworkarea/testgyp.M0NEcX/depend-on-bundle//out/Default/obj/dependent_on_bundle.ninja	2012-01-11
15:33:59.038635197 -0800
@@ -9,4 +9,4 @@
 ldflags = 
 libs = 
 build dependent_on_bundle: link obj/dependent_on_bundle.executable.o $
-    lib/libmy_bundle.so
+    lib/libmy_bundle.so || lib/libmy_bundle.so

So it is definitely my fault.  Will stare at it.

[Evan Martin, 2012-01-11 23:35:21]

On 2012/01/11 23:34:52, Evan Martin wrote:
> On 2012/01/11 23:27:20, Nico wrote:
> > It looks like dependent_on_bundle loses its dependency on the my_bundle
> bundle.
> 
> Thanks, I diffed the generated files and found:
> ---
>
out/testworkarea/testgyp.CLfkxU/depend-on-bundle//out/Default/obj/dependent_on_bundle.ninja	2012-01-11
> 15:33:19.382634567 -0800
> +++
>
out/testworkarea/testgyp.M0NEcX/depend-on-bundle//out/Default/obj/dependent_on_bundle.ninja	2012-01-11
> 15:33:59.038635197 -0800
> @@ -9,4 +9,4 @@
>  ldflags = 
>  libs = 
>  build dependent_on_bundle: link obj/dependent_on_bundle.executable.o $
> -    lib/libmy_bundle.so
> +    lib/libmy_bundle.so || lib/libmy_bundle.so
> 
> So it is definitely my fault.  Will stare at it.

Wait a sec, those two should be equivalent!  Hmmmmm

[Evan Martin, 2012-01-11 23:36:27]

On 2012/01/11 23:35:21, Evan Martin wrote:
obj/dependent_on_bundle.executable.o $
> > -    lib/libmy_bundle.so
> > +    lib/libmy_bundle.so || lib/libmy_bundle.so
> > 
> > So it is definitely my fault.  Will stare at it.
> 
> Wait a sec, those two should be equivalent!  Hmmmmm

Prolly need to use a real Mac, sorry for hte spam.

[Nico, 2012-01-11 23:37:52]

On 2012/01/11 23:36:27, Evan Martin wrote:
> On 2012/01/11 23:35:21, Evan Martin wrote:
> obj/dependent_on_bundle.executable.o $
> > > -    lib/libmy_bundle.so
> > > +    lib/libmy_bundle.so || lib/libmy_bundle.so
> > > 
> > > So it is definitely my fault.  Will stare at it.
> > 
> > Wait a sec, those two should be equivalent!  Hmmmmm
> 
> Prolly need to use a real Mac, sorry for hte spam.

I can give you an account on my mac if you want.

[Evan Martin, 2012-01-12 02:53:16]

seems to pass on linux and mac

http://codereview.chromium.org/9107028/diff/1003/pylib/gyp/generator/ninja.py
File pylib/gyp/generator/ninja.py (right):

http://codereview.chromium.org/9107028/diff/1003/pylib/gyp/generator/ninja.py...
pylib/gyp/generator/ninja.py:604: implicit_deps.add(output)
From here to "if linkable" above is different than the left side.  Otherwise
it's intended to be all the same, with "output_uses_linker" no longer necessary
because all code in this function is for the linker codepath.

[Nico, 2012-01-12 06:24:33]

LGTM!

http://codereview.chromium.org/9107028/diff/1003/pylib/gyp/generator/ninja.py
File pylib/gyp/generator/ninja.py (right):

http://codereview.chromium.org/9107028/diff/1003/pylib/gyp/generator/ninja.py...
pylib/gyp/generator/ninja.py:602: if 'lastchange' in binary:
'lastchange' is probably a stamp file, so i think it would be clearer if you
said "in output" here (binary = output in that case; but you're adding output to
implicit_deps 2 lines below)

http://codereview.chromium.org/9107028/diff/1003/pylib/gyp/generator/ninja.py...
pylib/gyp/generator/ninja.py:642: output =
self.WriteCollapsedDependencies('foobar', final_deps)[0]
nit: Maybe there's a better name than 'foobar'.