make: remove hacks used for shared-object builds

make: remove hacks used for shared-object builds

The previous rules made our shared objects include *every* other
library they transitively depended on.  Now that we're moving to
a world where shared objects have well-defined APIs, instead assume
the dependencies are correct for shared objects just like they
are for static objects.
Committed: http://code.google.com/p/gyp/source/detail?r=918

[Evan Martin, 2011-05-02 21:55:11]

Note that this breaks the shared library build, but makes the componetns build
work.

[Michael Moss, 2011-05-02 22:39:28]

http://codereview.chromium.org/6912005/diff/2001/pylib/gyp/generator/make.py
File pylib/gyp/generator/make.py (right):

http://codereview.chromium.org/6912005/diff/2001/pylib/gyp/generator/make.py#...
pylib/gyp/generator/make.py:217: cmd_solink = $(LINK.$(TOOLSET)) -shared
$(GYP_LDFLAGS) $(LDFLAGS.$(TOOLSET)) -Wl,-soname=$(@F) -o $@ -Wl,--whole-archive
$(filter-out FORCE_DO_CMD, $^) -Wl,--no-whole-archive $(LIBS)
Is --whole-archive advisable for all types of shared objects? For instance, what
about loadable-modules? This may be what we want for Chrome components, but will
it add unwanted bloat in the general case?

[Evan Martin, 2011-05-02 22:55:52]

On 2011/05/02 22:39:28, Michael Moss wrote:
>
http://codereview.chromium.org/6912005/diff/2001/pylib/gyp/generator/make.py#...
> pylib/gyp/generator/make.py:217: cmd_solink = $(LINK.$(TOOLSET)) -shared
> $(GYP_LDFLAGS) $(LDFLAGS.$(TOOLSET)) -Wl,-soname=$(@F) -o $@
-Wl,--whole-archive
> $(filter-out FORCE_DO_CMD, $^) -Wl,--no-whole-archive $(LIBS)
> Is --whole-archive advisable for all types of shared objects? For instance,
what
> about loadable-modules? This may be what we want for Chrome components, but
will
> it add unwanted bloat in the general case?

That is a very good question, and I've gone back and forth on it a few times in
my head, but I think the answer is that we're fine.

The reason we need --whole-archive is to handle a case like this: we build some
library out of smaller pieces in gyp, libfoo_interface1.a libfoo_interface2.a,
and then link those together at the end into libfoo.so.  In that case, without
whole-archive, we would drop any symbols exported in the sublibraries, which is
not what we want.

What you're asking about is the reverse problem: could we be exposing too much? 
And in that case, the only time it would make a difference is when
libfoo_interface1.a is explicitly exporting symbols.  But we never export
symbols except when we intend to export them, the default is private.  (With the
major exception of the components build, but there we aren't concerned about
bloat.)

[Michael Moss, 2011-05-02 23:09:12]

On 2011/05/02 22:55:52, Evan Martin wrote:
> On 2011/05/02 22:39:28, Michael Moss wrote:

> The reason we need --whole-archive is to handle a case like this: we build
some
> library out of smaller pieces in gyp, libfoo_interface1.a libfoo_interface2.a,
> and then link those together at the end into libfoo.so.  In that case, without
> whole-archive, we would drop any symbols exported in the sublibraries, which
is
> not what we want.
> 
> What you're asking about is the reverse problem: could we be exposing too
much? 

Not just exposing too much, but also simply linking too much. From the man page:

"include every object file in the archive in the link [...] forcing every object
to be included in the resulting shared library"

This might not be what users want in every shared_library or loadable_module
they build.

> And in that case, the only time it would make a difference is when
> libfoo_interface1.a is explicitly exporting symbols.  But we never export
> symbols except when we intend to export them, the default is private.  (With
the
> major exception of the components build, but there we aren't concerned about
> bloat.)

I think you've addressed the issue from a Chrome perspective, but my concern
here is more for developers who are using gyp for other projects. At the very
least, it seems like it could break current expectations.

[Evan Martin, 2011-05-02 23:55:23]

On 2011/05/02 23:09:12, Michael Moss wrote:
> On 2011/05/02 22:55:52, Evan Martin wrote:
> > On 2011/05/02 22:39:28, Michael Moss wrote:
> 
> > The reason we need --whole-archive is to handle a case like this: we build
> some
> > library out of smaller pieces in gyp, libfoo_interface1.a
libfoo_interface2.a,
> > and then link those together at the end into libfoo.so.  In that case,
without
> > whole-archive, we would drop any symbols exported in the sublibraries, which
> is
> > not what we want.
> > 
> > What you're asking about is the reverse problem: could we be exposing too
> much? 
> 
> Not just exposing too much, but also simply linking too much. From the man
page:
> 
> "include every object file in the archive in the link [...] forcing every
object
> to be included in the resulting shared library"
> 
> This might not be what users want in every shared_library or loadable_module
> they build.

Hm, I see.

To make this concrete, let's say you're building libfoo.so, and it has an
accompanying foo.c as well as a libbar.a that it depends on (in the gyp sense).

If foo.c uses functions from libbar.a, then they necessarily need to be included
in the resulting .so, and that happens regardless of the whole-archive flag.

If libbar.a exposes any functions intended for users of libfoo.so, then you must
use whole-archive.  But that also includes all other code in libbar.a.
If libbar.a does not expose any functions intended for users of libfoo.so, then
you do not need whole-archive.  Any .o files within libbar.a that are
unreferenced are dropped.

So the only extra stuff we include with whole-archive are the functions that
foo.c doesn't use.  I did a test where I created a large bar.c and put it in
libbar.a.

Two tests: one were foo.c referred to the big stuff, one where it didn't.

Without a reference to big stuff in libbar:
- normal link: small .so
- whole-archive, big .so
- whole-archive and gc-sections (dead code elimination), small .so

With a reference to big stuff in libbar:
- all .so were big.

I think this means that we are ok as long as we eliminate dead code.  What do
you think?

[Michael Moss, 2011-05-03 02:25:38]

> Two tests: one were foo.c referred to the big stuff, one where it didn't.
> 
> Without a reference to big stuff in libbar:
> - normal link: small .so
> - whole-archive, big .so
> - whole-archive and gc-sections (dead code elimination), small .so

Yeah, this is pretty much the situation I was worried about. And while code
elimination deals with it (thanks for verifying that), what does that mean for
this change? Does --gc-sections become a default? I'm guessing not, since I
think that would mess up exactly the situation you are trying to address with
Chrome. And if it's not the default, then we just expect users to know "if you
don't want cruft if your shared libs, don't forget to build with --gc-sections"?

That gets back to my concern over the change in expectations, but maybe good
coding hygiene already dictates using --gc-sections where appropriate and we can
write this off fixing an unintended side-effect that people shouldn't be relying
on in the first place (with an appropriate comment somewhere in the gyp docs).
That's the long-winded way of saying:

LGTM

[Nico, 2011-05-25 03:22:51]

Hi Evan,

this breaks the linux build. See http://codereview.chromium.org/6987004/ :

  SOLINK(target) out/Debug/obj.target/ppapi/libppapi_example.so
/usr/bin/ld:
out/Debug/obj.target/native_client/src/trusted/platform_qualify/libplatform_qual_lib.a(out/Debug/obj.target/native_client/src/trusted/platform_qualify/../../../../platform_qual_lib/native_client/src/trusted/platform_qualify/arch/x86/nacl_cpuwhitelist.o):
in function NaCl_ThisCPUIsWhitelisted:nacl_cpuwhitelist.c(.text+0x474):error:
undefined reference to 'NaClCPUDataGet'
/usr/bin/ld:
out/Debug/obj.target/native_client/src/trusted/platform_qualify/libplatform_qual_lib.a(out/Debug/obj.target/native_client/src/trusted/platform_qualify/../../../../platform_qual_lib/native_client/src/trusted/platform_qualify/arch/x86/nacl_cpuwhitelist.o):
in function NaCl_ThisCPUIsWhitelisted:nacl_cpuwhitelist.c(.text+0x480):error:
undefined reference to 'GetCPUIDString'
/usr/bin/ld:
out/Debug/obj.target/native_client/src/trusted/platform_qualify/libplatform_qual_lib.a(out/Debug/obj.target/native_client/src/trusted/platform_qualify/../../../../platform_qual_lib/native_client/src/trusted/platform_qualify/arch/x86/nacl_cpuwhitelist.o):
in function NaCl_ThisCPUIsBlacklisted:nacl_cpuwhitelist.c(.text+0x4df):error:
undefined reference to 'NaClCPUDataGet'
/usr/bin/ld:
out/Debug/obj.target/native_client/src/trusted/platform_qualify/libplatform_qual_lib.a(out/Debug/obj.target/native_client/src/trusted/platform_qualify/../../../../platform_qual_lib/native_client/src/trusted/platform_qualify/arch/x86/nacl_cpuwhitelist.o):
in function NaCl_ThisCPUIsBlacklisted:nacl_cpuwhitelist.c(.text+0x4eb):error:
undefined reference to 'GetCPUIDString'
collect2: ld returned 1 exit status
make: ***
[out/Debug/obj.target/native_client/src/trusted/plugin/libppGoogleNaClPluginChrome.so]
Error 1

It's well possible that this is due to broken gyp files. Happens only on Linux;
blocks gyp rolls.

[Nico, 2011-05-25 20:02:13]

Doesn't look like it: http://codereview.chromium.org/7069020/ (this is with a
revision from ~1.5 h ago).

Who do I need to ping to get the fix in?

(replying here instead of in gmail where I can see your reply to this because
gmail 500s for me)