Currently, the autotools build adds -fno-rtti and -fvisibility-inlines-hidden to CFLAGS, but they are only valid for C++ as gcc warns during the build. We also need to add -I$(srcdir)/WebCore to the global_cppflags so that the proper config.h gets found by code under WebCore. The attached patch fixes these issues.
Created attachment 18908 [details] Patch to resolve CFLAGS vs. CXXFLAGS issues
Comment on attachment 18908 [details] Patch to resolve CFLAGS vs. CXXFLAGS issues Doh. Was just pointed out that global_cppflags is for CPPFLAGS not CXXFLAGS, and the content of global_cppflags confused me.
Comment on attachment 18908 [details] Patch to resolve CFLAGS vs. CXXFLAGS issues >Index: ChangeLog >=================================================================== >--- ChangeLog (revision 29961) >+++ ChangeLog (working copy) >@@ -1,3 +1,13 @@ >+2008-02-04 Rodney Dawes <dobey@wayofthemonkey.com> >+ >+ Reviewed by NOBODY (OOPS!!) >+ >+ Only use -fno-rtti and $(SYMBOL_VISIBILITY_INLINES) with >+ global_cppflags as gcc complains they aren't valid for C >+ Add -I$(srcdir)/WebCore to global_cppflags to get the right config.h >+ >+ * GNUmakefile.am: >+ > 2008-02-04 Alp Toker <alp@atoker.com> > > Rubber-stamped by Mark Rowe. >Index: GNUmakefile.am >=================================================================== >--- GNUmakefile.am (revision 29961) >+++ GNUmakefile.am (working copy) >@@ -47,9 +47,11 @@ EXTRA_DIST := > > # > # Global flags to CPP >-global_cppflags := >+global_cppflags := \ >+ $(SYMBOL_VISIBILITY_INLINES) cppflags is for preprocessor options, not C++ flags. This won't help. Better solution is to clean up the flags and layout of the build modules.
Created attachment 18909 [details] Updated patch that adds and uses global_cxxflags
Comment on attachment 18909 [details] Updated patch that adds and uses global_cxxflags >Index: JavaScriptCore/GNUmakefile.am >=================================================================== >--- JavaScriptCore/GNUmakefile.am (revision 29961) >+++ JavaScriptCore/GNUmakefile.am (working copy) >@@ -149,14 +149,14 @@ Programs_minidom_SOURCES = \ > JavaScriptCore/API/NodeList.c \ > JavaScriptCore/API/minidom.c > Programs_minidom_CPPFLAGS = $(global_cppflags) >-Programs_minidom_CXXFLAGS = $(global_cflags) >+Programs_minidom_CXXFLAGS = $(globacl_cxxflags) $(global_cflags) Typo. Please test before requesting review.
Created attachment 18910 [details] Fix typo
Comment on attachment 18910 [details] Fix typo Looks good
Comment on attachment 18910 [details] Fix typo It seems that this patch will cause JavaScriptCore to use WebCore's config.h. Why is that any better than WebCore using JavaScriptCore's config.h?
Comment on attachment 18910 [details] Fix typo Clearing r+, I missed the -I addition which doesn't look right.
The -IWebCore and -I JavaScriptCore's in global_cppflags look incorrect. There are no headers in the top-level JavaScriptCore folder, and JavaScriptCore should definitely not be looking for any headers inside WebCore. If there's an issue there that needs fixed it should be addressed separately. A patch without that change would be great to get landed.
I landed the relevant portion of this in r29986.