Bug 49208 – [Gtk] Build fails when features are disabled (--disable-database, --disable-svg, etc.)

• Status: RESOLVED
• Resolution: DUPLICATE
• Component: WebCore Misc.

[Michael Smith]

Hi,

webkit-gtk fails to build when database storage and/or SVG are disabled. I have a patch to fix the build in r68244; looking at the affected files, I think it'll apply to trunk.

The affected flags are --disable-database, --disable-dom-storage, --disable-offline-web-applications, and --disable-svg.

I'm using the following configure flags:
                --disable-debug \
                --disable-channel-messaging \
                --disable-database \
                --disable-datalist \
                --disable-dom-storage \
                --disable-eventsource \
                --disable-filters \
                --disable-geolocation \
                --disable-icon-database \
                --disable-javascript-debugger \
                --disable-offline-web-applications \
                --disable-svg \
                --disable-svg-animation \
                --disable-svg-as-image \
                --disable-svg-fonts \
                --disable-svg-foreign-object \
                --disable-svg-use \
                --disable-video \

The stripped-down builds get broken once in a while - previous fixes include #37761, r53645, and r52245. (Changesets breaking the builds include r64151, r58801, and r67392.) --disable-database is especially prone to breaking. I think it'd be worth it to have a buildbot slave doing builds with features disabled.

[Michael Smith]

Created attachment 73284 [details]
Patch to fix --disable-database, --disable-svg, etc.

[Alexey Proskuryakov]

Comment on attachment 73284 [details]
Patch to fix --disable-database, --disable-svg, etc.

>-#include "JSSQLStatementErrorCallback.h"
> 
>#if ENABLE(DATABASE)
>
>+#include "JSSQLStatementErrorCallback.h"

This is not the right way to fix this. Preprocessor guards should be in header files, so that one doesn't need to to at extra lengths in .cpp files. That also means that generated header files need to be generated even for disabled features.

Not marking r-, because changing derived files generator is somewhat more challenging, but please consider fixing this the right way. And yes, there is quite a bit of precedent for preprocessor guards being in .cpp, unfortunately.

[Michael Smith]

Comment on attachment 73284 [details]
Patch to fix --disable-database, --disable-svg, etc.

It looks like most or all of the relevant .idl files have conditionals set, so this may not be too difficult - I think all I've got to do is go into GNUmakefile.am and move the webkit_built_sources from ENABLE_DATABASE up to the main block.

[Michael Smith]

Created attachment 73405 [details]
Fix build with --disable-database

I've reworked the patch. It's mostly just moving chunks around in GNUmakefile.am now.

Unfortunately, since I'm using r68244, I suspect the GNUmakefile.am hunks won't apply without some manual help.

[Alexey Proskuryakov]

Thanks! Leaving for a Gtk person to review.

+        * ../../../../../GNUmakefile.am: Always generate derived sources
+		for database and SVG features.
+        * ../../../../../platform/sql/SQLValue.cpp:
+        * ../../../../../platform/sql/SQLValue.h:
+        * ../../../../../platform/sql/SQLiteAuthorizer.cpp:
+        * ../../../../../platform/sql/SQLiteDatabase.cpp:
+        * ../../../../../platform/sql/SQLiteDatabase.h:
+        * ../../../../../platform/sql/SQLiteFileSystem.cpp:
+        * ../../../../../platform/sql/SQLiteFileSystem.h:
+        * ../../../../../platform/sql/SQLiteStatement.cpp:
+        * ../../../../../platform/sql/SQLiteStatement.h:
+        * ../../../../../platform/sql/SQLiteTransaction.cpp:
+        * ../../../../../platform/sql/SQLiteTransaction.h: Add feature guards.

There should be no backpaths here.

[Michael Smith]

Created attachment 73409 [details]
Fix build with --disable-database

Oops. backpaths fixed.

[Martin Robinson]

Comment on attachment 73409 [details]
Fix build with --disable-database

Looks good, though this doesn't apply cleanly to ToT. Perhaps I'll have some time later today to update it.

[Eric Seidel (no email)]

Ping? :)  Seems this should be landed or r-'d for being out of date.

[Michael Smith]

Unfortunately, any patch I make to GNUmakefile.am is likely to be out of date as it changes so frequently. Perhaps someone with commit access would be willing to fix it up?

[Martin Robinson]

Philippe, do you know what the status of this bug is after your recent changes?

[Philippe Normand]

(In reply to comment #10)
> Philippe, do you know what the status of this bug is after your recent changes?

Looks like a dupe of bug 51327
Mike, can you please check your build is fine now? My patch fixed the minimal build: build-webkit --minimal

[Philippe Normand]

Please reopen if needed

*** This bug has been marked as a duplicate of bug 51327 ***