Support import libraries for Windows ninja

Support import libraries for Windows ninja

There's 2 outputs for a shared library link on Windows, the .dll and a .lib to
link other targets against. Split up the link step to generate these and link
against the .lib rather than .dll.
Committed: https://code.google.com/p/gyp/source/detail?r=1224

[Evan Martin, 2012-02-21 18:10:47]

weak LGTM, maybe thakis has input

https://chromiumcodereview.appspot.com/9424042/diff/3001/pylib/gyp/generator/...
File pylib/gyp/generator/ninja.py (right):

https://chromiumcodereview.appspot.com/9424042/diff/3001/pylib/gyp/generator/...
pylib/gyp/generator/ninja.py:691: extra_link_deps.add(target.import_library or
target.binary)
Would it make more sense for target.Linkable() to just return the correct thing
or None here?

https://chromiumcodereview.appspot.com/9424042/diff/3001/pylib/gyp/generator/...
pylib/gyp/generator/ninja.py:750: self.target.binary, self.target.import_library
= output
This makes me feel a little funny. I wonder if it'd be better to just fill in
self.target.import_library inside the WriteLink() function?  (This is fine too
if you prefer it.)

https://chromiumcodereview.appspot.com/9424042/diff/3001/pylib/gyp/generator/...
pylib/gyp/generator/ninja.py:1139: description='LINK_DLL $dll',
FWIW, the description is free text.  So you can use "LINK(DLL) or "LINK DLL" if
you want, underscores aren't necessary.

https://chromiumcodereview.appspot.com/9424042/diff/3001/test/lib/TestGyp.py
File test/lib/TestGyp.py (right):

https://chromiumcodereview.appspot.com/9424042/diff/3001/test/lib/TestGyp.py#...
test/lib/TestGyp.py:461: result.append('lib')
I wonder why this is different.  Maybe we should unify all the platforms.  (Not
in this change, obviously.)

[Nico, 2012-02-21 18:20:54]

https://chromiumcodereview.appspot.com/9424042/diff/3001/pylib/gyp/generator/...
File pylib/gyp/generator/ninja.py (right):

https://chromiumcodereview.appspot.com/9424042/diff/3001/pylib/gyp/generator/...
pylib/gyp/generator/ninja.py:691: extra_link_deps.add(target.import_library or
target.binary)
Why is this needed? It looks like the build step will create both
target.import_library and target.binary, so the link dep on target.binary should
suffice, right?

https://chromiumcodereview.appspot.com/9424042/diff/3001/pylib/gyp/generator/...
pylib/gyp/generator/ninja.py:919: type_in_output_root += ['shared_library']
Is there a test for this?

[Evan Martin, 2012-02-21 18:23:45]

https://chromiumcodereview.appspot.com/9424042/diff/3001/pylib/gyp/generator/...
File pylib/gyp/generator/ninja.py (right):

https://chromiumcodereview.appspot.com/9424042/diff/3001/pylib/gyp/generator/...
pylib/gyp/generator/ninja.py:691: extra_link_deps.add(target.import_library or
target.binary)
On 2012/02/21 18:20:54, Nico wrote:
> Why is this needed? It looks like the build step will create both
> target.import_library and target.binary, so the link dep on target.binary
should
> suffice, right?

extra_link_deps are specifically the files that need to be on the command line
to the linker.  So here he's passing foobar.lib instead of foobar.dll because
that's what the linker wants.

[Nico, 2012-02-21 18:27:40]

https://chromiumcodereview.appspot.com/9424042/diff/3001/pylib/gyp/generator/...
File pylib/gyp/generator/ninja.py (right):

https://chromiumcodereview.appspot.com/9424042/diff/3001/pylib/gyp/generator/...
pylib/gyp/generator/ninja.py:691: extra_link_deps.add(target.import_library or
target.binary)
On 2012/02/21 18:23:45, Evan Martin wrote:
> On 2012/02/21 18:20:54, Nico wrote:
> > Why is this needed? It looks like the build step will create both
> > target.import_library and target.binary, so the link dep on target.binary
> should
> > suffice, right?
> 
> extra_link_deps are specifically the files that need to be on the command line
> to the linker.  So here he's passing foobar.lib instead of foobar.dll because
> that's what the linker wants.

I see. Is it possible to make target.binary the import library on windows then?

[scottmg1, 2012-02-21 18:30:38]

On 2012/02/21 18:27:40, Nico wrote:
>
https://chromiumcodereview.appspot.com/9424042/diff/3001/pylib/gyp/generator/...
> File pylib/gyp/generator/ninja.py (right):
> 
>
https://chromiumcodereview.appspot.com/9424042/diff/3001/pylib/gyp/generator/...
> pylib/gyp/generator/ninja.py:691: extra_link_deps.add(target.import_library or
> target.binary)
> On 2012/02/21 18:23:45, Evan Martin wrote:
> > On 2012/02/21 18:20:54, Nico wrote:
> > > Why is this needed? It looks like the build step will create both
> > > target.import_library and target.binary, so the link dep on target.binary
> > should
> > > suffice, right?
> > 
> > extra_link_deps are specifically the files that need to be on the command
line
> > to the linker.  So here he's passing foobar.lib instead of foobar.dll
because
> > that's what the linker wants.
> 
> I see. Is it possible to make target.binary the import library on windows
then?

Hmm, maybe, I'd have to try it. It feels a bit weird because the lib is sort of
an incidental side effect as far as the final build is concerned. i.e.
deploy/install/copy/postbuilds might get confused then? Not sure.

[Nico, 2012-02-21 18:38:54]

> Hmm, maybe, I'd have to try it. It feels a bit weird because the lib is sort
of
> an incidental side effect as far as the final build is concerned. i.e.
> deploy/install/copy/postbuilds might get confused then? Not sure.

These don't exist yet :-)

If it works, it would at least delay having to add stuff to Target, and the
strange "isinstance(..., list)" becomes unnecessary too.

[scottmg, 2012-02-21 18:45:46]

I'll see what it's like to use .binary for the implib instead of the dll.

https://chromiumcodereview.appspot.com/9424042/diff/3001/pylib/gyp/generator/...
File pylib/gyp/generator/ninja.py (right):

https://chromiumcodereview.appspot.com/9424042/diff/3001/pylib/gyp/generator/...
pylib/gyp/generator/ninja.py:750: self.target.binary, self.target.import_library
= output
On 2012/02/21 18:10:47, Evan Martin wrote:
> This makes me feel a little funny. I wonder if it'd be better to just fill in
> self.target.import_library inside the WriteLink() function?  (This is fine too
> if you prefer it.)

Yeah, that would be a little tidier.

But, I have to modify output in WriteLink anyway so that
self.ninja.build(output, ...) writes both as outputs of the link step. If an exe
depends on the dll target, then it requires the .lib, so there needs to be a
build rule that outputs that.

https://chromiumcodereview.appspot.com/9424042/diff/3001/pylib/gyp/generator/...
pylib/gyp/generator/ninja.py:919: type_in_output_root += ['shared_library']
On 2012/02/21 18:20:54, Nico wrote:
> Is there a test for this?

test/library/gyptest-shared.py

https://chromiumcodereview.appspot.com/9424042/diff/3001/pylib/gyp/generator/...
pylib/gyp/generator/ninja.py:1139: description='LINK_DLL $dll',
On 2012/02/21 18:10:47, Evan Martin wrote:
> FWIW, the description is free text.  So you can use "LINK(DLL) or "LINK DLL"
if
> you want, underscores aren't necessary.

Done.

[scottmg, 2012-02-21 23:12:06]

On 2012/02/21 18:45:46, scottmg wrote:
> I'll see what it's like to use .binary for the implib instead of the dll.

Now using .binary to be the import lib. It's a smaller patch, but still feel a
bit weird about calling the import library the "binary". Meh?

[Evan Martin, 2012-02-21 23:22:48]

On 2012/02/21 23:12:06, scottmg wrote:
> On 2012/02/21 18:45:46, scottmg wrote:
> > I'll see what it's like to use .binary for the implib instead of the dll.
> 
> Now using .binary to be the import lib. It's a smaller patch, but still feel a
> bit weird about calling the import library the "binary". Meh?

We also call the .a the "binary", which doesn't much feel like a binary to me. 
Maybe a better name would have been "linker_output".

LGTM in any case

[Nico, 2012-02-22 00:46:23]

https://chromiumcodereview.appspot.com/9424042/diff/9001/pylib/gyp/generator/...
File pylib/gyp/generator/ninja.py (right):

https://chromiumcodereview.appspot.com/9424042/diff/9001/pylib/gyp/generator/...
pylib/gyp/generator/ninja.py:729: output = [output, import_lib]
Is this last line still needed? Could you do

  output = import_lib

instead, and then move the

  self.target.binary = output

line below this? (or go back to `return output` even?)

[scottmg, 2012-02-22 00:53:42]

https://chromiumcodereview.appspot.com/9424042/diff/9001/pylib/gyp/generator/...
File pylib/gyp/generator/ninja.py (right):

https://chromiumcodereview.appspot.com/9424042/diff/9001/pylib/gyp/generator/...
pylib/gyp/generator/ninja.py:729: output = [output, import_lib]
On 2012/02/22 00:46:23, Nico wrote:
> Is this last line still needed? Could you do
> 
>   output = import_lib
> 
> instead, and then move the
> 
>   self.target.binary = output
> 
> line below this? (or go back to `return output` even?)

Yes, still required because there has to be a build rule that outputs both the
lib and the dll so both outputs have to be passed to self.ninja.build(output,
...). (Is that what you meant?)

[Nico, 2012-02-22 00:55:12]

> Yes, still required because there has to be a build rule that outputs both the
> lib and the dll so both outputs have to be passed to self.ninja.build(output,
> ...). (Is that what you meant?)

Does anything actually need a build rule for the dll? (Maybe an actions copy
step?)

But LGTM either way.