Bug 20244

Summary:	Bad -I ordering can break build
Product:	libxklavier	Reporter:	Daniel Macks <dmacks>
Component:	General	Assignee:	Sergey V. Udaltsov <svu>
Status:	RESOLVED FIXED	QA Contact:	Sergey V. Udaltsov <svu>
Severity:	normal
Priority:	medium	Keywords:	NEEDINFO
Version:	unspecified
Hardware:	All
OS:	All
Whiteboard:
i915 platform:		i915 features:

Description Daniel Macks 2009-02-21 11:40:56 UTC

Various libxklavier-3.8 Makefiles pass -I for local, build-, and/or src-dir paths after those flags for global (external support) paths. That can break compiling building libxklavier because an unrelated or incompatible or broken .h installed on the system might mask the expected one in the source tarball. Should always pass all local -I before any global ones.

On the other hand, some of those local -I are not needed anyway. Autotools automatically pass -I. and "the -I needed to find config.h" via DEFAULT_INCLUDES.

Comment 1 Sergey V. Udaltsov 2009-02-21 12:25:19 UTC

In the Makefile.am I see CFLAGS include local dirs first. What is your proposal for CFLAGS?

Comment 2 Daniel Macks 2009-02-21 13:00:45 UTC

libxklavier-3.8/libxklavier/Makefile.am:

>AM_CFLAGS=-Wall -DDATA_DIR=\"$(datadir)/$(PACKAGE)\" \
>  -I. -I$(includedir) $(X_CFLAGS) -I$(top_srcdir) \
>  $(XML_CFLAGS) $(GLIB_CFLAGS) $(XINPUT_CFLAGS) \
>  $(LIBXKBFILE_PRESENT_CFLAG) \
>  $(ENABLE_XKB_SUPPORT_CFLAG) \
>  $(ENABLE_XMODMAP_SUPPORT_CFLAG)

libxklavier-3.8/tests/Makefileam:

>AM_CFLAGS=-Wall -Werror -I$(includedir) $(X_CFLAGS) -I$(top_srcdir) $(GLIB_CFLAGS)

In both cases, the local -I$(top_srcdir) after the global $(X_CFLAGS). 
Actually actually, -I and -D flags for normal compiling should be in AM_CPPFLAGS or INCLUDES instead of AM_CFLAGS (those former are restricted to the compiler whereas AM_CFLAGS is also passed to the linker where -I and -D are harmless but useless).

Comment 3 Sergey V. Udaltsov 2009-02-21 14:10:22 UTC

Right, I was looking at libxklavier subdir. Ok, moving -I$(top_srcdir) in front of global X_CFLAGS would make sense, I'll do that.

Comment 4 Sergey V. Udaltsov 2009-02-21 14:23:04 UTC

Done! Please check from CVS.

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.