| Summary: | build-webkit --no-indexed-database fails due to missing guards | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Emanuele Aina <emanuele.aina> | ||||||
| Component: | New Bugs | Assignee: | Nobody <webkit-unassigned> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | aestes, andersca, andrunko, ap, bdakin, beidson, benjamin, cgarcia, commit-queue, darin, enrica, hyatt, kling, mcatanzaro, mitz, mjs, ossy, sam, simon.fraser, thorton | ||||||
| Priority: | P3 | ||||||||
| Version: | 528+ (Nightly build) | ||||||||
| Hardware: | PC | ||||||||
| OS: | Linux | ||||||||
| Attachments: |
|
||||||||
Created attachment 256014 [details]
Patch
Is it useful for anything to enable DatabaseProcess without IndexedDB? If no, it would be better to unify these guards. > Is it useful for anything to enable DatabaseProcess without IndexedDB?
> If no, it would be better to unify these guards.
As far as I can tell, at the moment IndexedDB seems to be the only user of DatabaseProcess, so merging those guards would make sense as the split will likely bitrot again.
On the other hand the DatabaseProcess code seems to be structured to handle multiple users, so in future we could need to split them again, redoing some work which has already been done.
I just needed to make the whole thing build (at the moment I'm stuck to a GCC 4.8 toolchain and DatabaseProcess is the only thing that requires GCC 4.9) so both solutions work for me. :)
Created attachment 257252 [details] Patch Rebased on current trunk (r187148) Anders or Sam, what do you think about this patch? Do we need DatabaseProcess if IndexedDB feature is disabled? - If yes, we need a patch like this to fix this build configuration. - If no, we should unify the two guard or disable DatabaseProcess automatically if IndexedDB is disabled. (In reply to comment #5) > Anders or Sam, what do you think about this patch? > > Do we need DatabaseProcess if IndexedDB feature is disabled? > - If yes, we need a patch like this to fix this build configuration. > - If no, we should unify the two guard or disable DatabaseProcess > automatically if IndexedDB is disabled. just a note: We need one of these changes to fix the --minimal build configuration on EFL to be able to catch broken ifdef guards again. (In reply to comment #5) > Anders or Sam, what do you think about this patch? > > Do we need DatabaseProcess if IndexedDB feature is disabled? > - If yes, we need a patch like this to fix this build configuration. > - If no, we should unify the two guard or disable DatabaseProcess > automatically if IndexedDB is disabled. ping? (In reply to comment #7) > (In reply to comment #5) > > Anders or Sam, what do you think about this patch? > > > > Do we need DatabaseProcess if IndexedDB feature is disabled? > > - If yes, we need a patch like this to fix this build configuration. > > - If no, we should unify the two guard or disable DatabaseProcess > > automatically if IndexedDB is disabled. > > ping? The DatabaseProcess is only needed for IndexedDB right now. That may not always be the case however, so I prefer having two guards. (In reply to comment #8) > (In reply to comment #7) > > (In reply to comment #5) > > > Anders or Sam, what do you think about this patch? > > > > > > Do we need DatabaseProcess if IndexedDB feature is disabled? > > > - If yes, we need a patch like this to fix this build configuration. > > > - If no, we should unify the two guard or disable DatabaseProcess > > > automatically if IndexedDB is disabled. > > > > ping? > > The DatabaseProcess is only needed for IndexedDB right now. That may not > always be the case however, so I prefer having two guards. The proposed patch leaves the two guards untouched and adds the missing guards to be able to build with disabled indexedDB support and enabled DatabaseProcess. So could you possibly review this patch? ping? (In reply to comment #8) > (In reply to comment #7) > > (In reply to comment #5) > > > Anders or Sam, what do you think about this patch? > > > > > > Do we need DatabaseProcess if IndexedDB feature is disabled? > > > - If yes, we need a patch like this to fix this build configuration. > > > - If no, we should unify the two guard or disable DatabaseProcess > > > automatically if IndexedDB is disabled. > > > > ping? > > The DatabaseProcess is only needed for IndexedDB right now. That may not > always be the case however, so I prefer having two guards. Could you review this patch? I don't think we need owners to approve build fixes? A good argument could be made that ENABLE_DATABASE_PROCESS should subsume the ENABLE_INDEXED_DATABASE option, but for the time being that's not the case, and it can always be changed later.... Comment on attachment 257252 [details] Patch Rejecting attachment 257252 [details] from commit-queue. Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-03', 'land-attachment', '--force-clean', '--non-interactive', '--parent-command=commit-queue', 257252, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit Last 500 characters of output: ', 'dcommit', '--rmdir']" exit_code: 139 cwd: /Volumes/Data/EWS/WebKit Committing to http://svn.webkit.org/repository/webkit/trunk ... Authentication realm: <http://svn.webkit.org:80> Mac OS Forge Password for 'commit-queue@webkit.org': Authentication realm: <http://svn.webkit.org:80> Mac OS Forge Username: error: git-svn died of signal 11 Failed to run "['git', 'svn', 'dcommit', '--rmdir']" exit_code: 139 cwd: /Volumes/Data/EWS/WebKit Updating OpenSource Current branch master is up to date. Full output: http://webkit-queues.webkit.org/results/132788 Comment on attachment 257252 [details] Patch Clearing flags on attachment: 257252 Committed r189244: <http://trac.webkit.org/changeset/189244> All reviewed patches have been landed. Closing bug. |
At the moment IndexedDatabase is the only user of the DatabaseProcess, which means that the usefulness of enabling the latter without the former is very low. They are however protected by different guards, ENABLE(INDEXED_DATABASE) and ENABLE(DATABASE_PROCESS) respectively, while it is technically possible to end up in that situation it's likely that everyone either disable them both or enable them. In such a situation, inevitably the two guards grew intermingled, with different levels on consistency. Unfortunately, turning off ENABLE(INDEXED_DATABASE) while keeping ENABLE(DATABASE_PROCESS) is exactly what WebKit ends up doing when specifying the --no-indexed-database flag on `build-webkit` invocation, leading to build failures like the one below: $ Tools/Scripts/build-webkit --gtk --release --no-indexed-database [...] In file included from ../../Source/WebKit2/DatabaseProcess/DatabaseProcess.cpp:27:0: ../../Source/WebKit2/DatabaseProcess/DatabaseProcess.h:56:70: error: ‘UniqueIDBDatabaseIdentifier’ does not name a type PassRefPtr<UniqueIDBDatabase> getOrCreateUniqueIDBDatabase(const UniqueIDBDatabaseIdentifier&); ^ In file included from ../../Source/WebKit2/DatabaseProcess/DatabaseProcess.cpp:27:0: ../../Source/WebKit2/DatabaseProcess/DatabaseProcess.h:90:98: error: ‘SecurityOriginData’ was not declared in this scope void deleteWebsiteDataForOrigins(WebCore::SessionID, uint64_t websiteDataTypes, const Vector<SecurityOriginData>& origins, uint64_t callbackID); [...]