[PULL] Server build fixes

Dan Nicholson dbn.lists at gmail.com
Sat Feb 26 11:05:20 PST 2011


On Sat, Feb 26, 2011 at 10:31 AM, Keith Packard <keithp at keithp.com> wrote:
> On Sat, 26 Feb 2011 15:33:20 +0100, Cyril Brulebois <kibi at debian.org> wrote:
>
>> I fear that the sdksyms part is broken. If one builds with some
>> options different than the ones used to generate the tarball, the file
>> isn't generated again, leading to such issues.
>
> Looks like sdksyms.c should be stuck in nodist_libloader_la_SOURCES so
> that it doesn't get distributed. Nothing has changed here in a long
> time, so I'm a bit confused why this cropped up so soon with 1.10.0;
> the sdksyms.c stuff was added over two years ago.
>
> A 'make clean' before 'make' will resolve the issue for the existing
> 1.10 package.
>
>> A wild guess would be the “touch” added to configure leading to an
>> up-to-date target. Also, I'm not sure it's a good idea to ship
>> sdksyms.c in the tarball? (It was shipped already in rc3 and before,
>> but that didn't lead to any issues.)
>
> I read through the automake docs and they suggest placing a dependency
> on the generated configuration header file. We've got several, but
> there's the auto-generated do-not-use-config.h which holds all of the
> cpp defines which sdksyms.c may use.
>
> I think this is what we want:
>
> diff --git a/hw/xfree86/loader/Makefile.am b/hw/xfree86/loader/Makefile.am
> index 7f386cc..0e5b304 100644
> --- a/hw/xfree86/loader/Makefile.am
> +++ b/hw/xfree86/loader/Makefile.am
> @@ -12,18 +12,21 @@ EXTRA_DIST = \
>        loaderProcs.h \
>        sdksyms.sh
>
> +nodist_libloader_la_SOURCES = \
> +       sdksyms.c
> +
>  libloader_la_SOURCES = \
>        loader.c \
>        loaderProcs.h \
>        loadext.c \
>         loadmod.c \
> -       os.c \
> -       sdksyms.c
> +       os.c
> +
>  libloader_la_LIBADD = $(DLOPEN_LIBS)
>
>  CLEANFILES = sdksyms.c sdksyms.dep
>
> -sdksyms.dep sdksyms.c: sdksyms.sh
> +sdksyms.dep sdksyms.c: sdksyms.sh $(top_builddir)/include/do-not-use-config.h
>        CPP='$(CPP)' AWK='$(AWK)' $(srcdir)/sdksyms.sh $(top_srcdir) $(AM_CFLAGS) $(CFLAGS) $(INCLUDES)
>
>  SDKSYMS_DEP = sdksyms.dep

Oh, I didn't see this before I sent. This seems like a useful thing,
but I think there really needs to be a rule that sdksyms.c depends on
sdksyms.dep in case the dep file gets updated out-of-band from running
sdksyms.sh. nodisting the file will probably be good enough, but your
patch can't hurt.

Reviewed-by: Dan Nicholson <dbn.lists at gmail.com>

--
Dan


More information about the xorg-devel mailing list