Summary: | Use of C++ compiler flags in CFLAGS | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Rodney Dawes <dobey> | ||||||||
Component: | WebCore Misc. | Assignee: | Nobody <webkit-unassigned> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | mrowe | ||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | PC | ||||||||||
OS: | All | ||||||||||
Attachments: |
|
Description
Rodney Dawes
2008-02-04 08:26:02 PST
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. |