Bug 51327 - [GTK] minimal build fails at link time due to missing sqlite3 symbols
Summary: [GTK] minimal build fails at link time due to missing sqlite3 symbols
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords:
: 49208 (view as bug list)
Depends on:
Blocks:
 
Reported: 2010-12-20 04:28 PST by Philippe Normand
Modified: 2011-01-11 03:40 PST (History)
2 users (show)

See Also:


Attachments
proposed patch (2.61 KB, patch)
2010-12-20 05:17 PST, Philippe Normand
no flags Details | Formatted Diff | Diff
proposed patch (6.58 KB, patch)
2010-12-29 04:54 PST, Philippe Normand
mrobinson: review+
mrobinson: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Philippe Normand 2010-12-20 04:28:13 PST
WebCore/platform/sql/SQL* files should be included in the build only if the DATABASE feature is enabled. Especially SQLiteDatabase.cpp which is not guarded by that macro and includes sqlite3.h.

At configure time sqlite3 presence is checked only if one of the database-related features is enabled.
Comment 1 Philippe Normand 2010-12-20 05:17:14 PST
Created attachment 76989 [details]
proposed patch
Comment 2 Xan Lopez 2010-12-21 05:41:38 PST
Comment on attachment 76989 [details]
proposed patch

Any reason not to make the files have the proper guards?
Comment 3 Philippe Normand 2010-12-21 06:21:03 PST
(In reply to comment #2)
> (From update of attachment 76989 [details])
> Any reason not to make the files have the proper guards?

Well SQLiteDatabase is used in WebCore/page/GeolocationPositionCache.cpp, the appcache code, the icon database, WebCore/storage and probably more than git grep tells me. 

Now I think of another approach, what about keeping the GNUMakefile.am as it is and in configure.ac always check for sqlite3 presence. If it's not found and one or more features needing it are enabled, error out. If it's found and no features require it, platform/sql/SQL* will built anyway and not fail because the CFLAGS and LDFLAGS will be ok.

That'd be a less intrusive way to solve the problem, I believe.
Comment 4 Philippe Normand 2010-12-21 08:24:04 PST
Comment on attachment 76989 [details]
proposed patch

Will rework this
Comment 5 Philippe Normand 2010-12-29 04:54:35 PST
Created attachment 77611 [details]
proposed patch
Comment 6 Martin Robinson 2010-12-29 14:26:32 PST
Comment on attachment 77611 [details]
proposed patch

View in context: https://bugs.webkit.org/attachment.cgi?id=77611&action=review

Thanks! Please fix some of these really minor issues before landing this.

> configure.ac:844
> +# check if sqlite 3 is available. Error out only if one of the

Please fix the capitalization here before landing. :)

> configure.ac:863
> +   AC_MSG_ERROR([SQLite3 is required to enable Database support])

For instance, if the error was due to missing SQLite and the feature was offline web applications, this message would be incorrect. Do you mind making it more general (or more specific with if-else blocks) before landing?
Comment 7 Philippe Normand 2010-12-30 02:18:28 PST
Thanks Martin, I made the error message more general.

Committed r74792: <http://trac.webkit.org/changeset/74792>
Comment 8 Philippe Normand 2011-01-11 03:40:09 PST
*** Bug 49208 has been marked as a duplicate of this bug. ***