[PATCH v2 3/3] xfree86: Remove libxorg convenience library to speed up build

[Dan Nicholson]

On Fri, Jun 17, 2011 at 8:27 AM, Daniel Stone <daniel at fooishbar.org> wrote:
> Hi,
>
> On Tue, Jun 14, 2011 at 11:02:15PM -0700, Dan Nicholson wrote:
>> libxorg.la served to collect all the Xorg convenience libraries into one
>> massive archive to link into Xorg. This made things easy for symbol
>> resolution, but it tremendously slowed down the build since each change
>> caused libxorg.la to be rebuilt. This is an extremely slow process of
>> extracting all the objects from the sub-libraries and recombining them.
>>
>> Instead, the archives are linked directly into Xorg. The order of the
>> libraries had to be tweaked a bit to make symbols resolve correctly with
>> the lower level code moving later in the link command.
>
> My hero! This makes _such_ a difference to my builds.  Seriously.

Yeah, I'd never given a lot of thought why the convenience library of
convenience libraries would be so painful. I think xts has a lot of
this same pattern that slows things down.

I noticed one other thing when I started to try removing
libxservertest.la yesterday. Since sdksyms references symbols in all
libraries, you pretty much need to make it an object of the program or
do the conglomeration convenience library trick. I tried to make
libxservertest.la be just sdksyms.c, but then I was getting undefined
references back and forth with libloader. So, it seems you need to
either make it an object of each test program, which kind of sucks
because sdksyms.c is generated in a different directory and you need
to pass a -I for almost every directory in the tree. The alternative
is just to make a stub sdksyms for test that just has an empty
xorg_symbols[]. What do you think?

>> -libxorg_la_LIBADD = \
>> +Xorg_LDADD = \
>> +            $(MAIN_LIB) \
>>              $(XSERVER_LIBS) \
>>              loader/libloader.la \
>> -            os-support/libxorgos.la \
>>              common/libcommon.la \
>> +            os-support/libxorgos.la \
>>              parser/libxf86config_internal.la \
>>              dixmods/libdixmods.la \
>>              modes/libxf86modes.la \
>> @@ -58,14 +57,11 @@ libxorg_la_LIBADD = \
>>              ddc/libddc.la \
>>              i2c/libi2c.la \
>>              dixmods/libxorgxkb.la \
>> +            $(XORG_LIBS) \
>>              $(top_builddir)/mi/libmi.la \
>>              $(top_builddir)/os/libos.la \
>> -            @XORG_LIBS@
>> -
>> -libxorg_la_DEPENDENCIES = $(libxorg_la_LIBADD)
>> -
>> -Xorg_DEPENDENCIES = libxorg.la
>> -Xorg_LDADD = $(MAIN_LIB) libxorg.la $(XORG_SYS_LIBS) $(XSERVER_SYS_LIBS)
>> +            $(XORG_SYS_LIBS) \
>> +            $(XSERVER_SYS_LIBS)
>
> Are you fine with the following patch, which readds the _DEPENDENCIES
> magic to relink Xorg whenever any of the relevant source files have
> changed? If so, I'll integrate that into my tree and send out a pull
> request for 1.11 when a couple of smaller changes have been reviewed,
> including a dependencies patch for test/ which makes sure it gets
> rebuilt as well.

Sure, I should have added a comment. I looked at the automake manual
and it seems that if you don't specify _DEPENDENCIES, it will use
_LDADD with the external libs removed.

http://www.gnu.org/software/automake/manual/automake.html#Linking

Hmm, actually looking at what's generated, I think automake can't
resolve through the variables and ends up using only the local .la
files explicitly specified in _LDADD. Here's what it looks like for me
in hw/xfree86/Makefile:

am__DEPENDENCIES_1 =
#am__DEPENDENCIES_2 =  \
#       $(am__DEPENDENCIES_1)
Xorg_DEPENDENCIES = $(am__DEPENDENCIES_1) $(am__DEPENDENCIES_1) \
        loader/libloader.la common/libcommon.la \
        os-support/libxorgos.la parser/libxf86config_internal.la \
        dixmods/libdixmods.la modes/libxf86modes.la \
        ramdac/libramdac.la ddc/libddc.la i2c/libi2c.la \
        dixmods/libxorgxkb.la $(am__DEPENDENCIES_1) \
        $(top_builddir)/mi/libmi.la $(top_builddir)/os/libos.la \
        $(am__DEPENDENCIES_1) $(am__DEPENDENCIES_1) \
        $(am__DEPENDENCIES_2)

So, yeah you're patch looks right. I'll add it back in and send the
merge request.

--
Dan