[Webkit-unassigned] [Bug 16669] autotools update and fixes

[bugzilla-daemon at webkit.org]

http://bugs.webkit.org/show_bug.cgi?id=16669


jmalonzo at gmail.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #18171|0                           |1
        is obsolete|                            |
  Attachment #18172|                            |review?
               Flag|                            |




------- Comment #3 from jmalonzo at gmail.com  2007-12-29 18:36 PDT -------
Created an attachment (id=18172)
 --> (http://bugs.webkit.org/attachment.cgi?id=18172&action=view)
Fix database inclusion and few fixes to the patch

Hi Darin

> +#if ENABLE(DATABASE)
> +#include "Database.h"
> +#endif
>
> This change is wrong. The file "Database.h" already has an #if 
> ENABLE(DATABASE) around its contents. We don't want to ifdef it in both 
> places.

Removed the ifdef in Database.h. See explanation below.

> +#if ENABLE(DATABASE)
> +#include "DatabaseTracker.h"
> +#endif
>
> We need to be consistent. The ENABLE #if statements are inside Database.h and
> should go inside DatabaseTracker.h too. Or we could go the other way and 
> change how we handle optional things like database.

I went the other way around to make Database consistent with the other
features, since Database ifdefs are already there, it's just a matter of adding
a guard before including Database.h, which pretty much what the other features
are doing (e.g. include xpath headers only if xpath is enabled). 

> ENABLE_XBL should not be in the makefile. XBL was an experiment done more than
> two years ago. It doesn't make sense to have it commented-out code in there
> with a FIXME.

Removed XBL from the makefile/configure scripts.

Also, ideally we should be using DerivedSources.make but i had some issues with
the existing makefile (e.g., vpath mangling, some make constructs do not work
well with automake) as well as the risk of making untested changes that affects
the other platforms (i dont have a mac for example). But anyway, patches are
always welcome if somebody wants to work on it.

Thanks for the review!


-- 
Configure bugmail: http://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.