Bug 59892 - [android] Add Android support to the CMake buildsystem.
Summary: [android] Add Android support to the CMake buildsystem.
Status: RESOLVED WONTFIX
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other Other
: P2 Normal
Assignee: Holger Freyther
URL:
Keywords:
Depends on:
Blocks: 60907
  Show dependency treegraph
 
Reported: 2011-05-01 14:13 PDT by Holger Freyther
Modified: 2012-02-13 14:45 PST (History)
5 users (show)

See Also:


Attachments
Patch (8.51 KB, patch)
2011-05-01 14:19 PDT, Holger Freyther
abarth: review-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Holger Freyther 2011-05-01 14:13:53 PDT
[android] Add Android support to the CMake buildsystem.
Comment 1 Holger Freyther 2011-05-01 14:19:01 PDT
Created attachment 91845 [details]
Patch
Comment 2 Adam Barth 2011-05-11 21:04:45 PDT
Ah, I clearly read the patches in the wrong order.
Comment 3 Adam Barth 2011-05-11 21:07:27 PDT
Comment on attachment 91845 [details]
Patch

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

Cool.  Does this actually build?

> Source/WebCore/CMakeListsAndroid.txt:2
> +  "${JAVASCRIPTCORE_DIR}/wtf/text"

WebCore shouldn't be including directly from this directory.  Even if it did, it shouldn't be in an Android-specific include block.

> Source/cmake/OptionsAndroid.cmake:34
> +WEBKIT_FEATURE(ENABLE_AS_IMAGE "Enable SVG as image" DEFAULT ON SVG)
> +WEBKIT_FEATURE(ENABLE_BLOB "Enable blob slice" DEFAULT OFF)
> +WEBKIT_FEATURE(ENABLE_CHANNEL_MESSAGING "Enable channel messaging" DEFAULT ON)
> +WEBKIT_FEATURE(ENABLE_DATABASE "Enable database" DEFAULT ON)
> +WEBKIT_FEATURE(ENABLE_DATAGRID "Enable datagrid" DEFAULT OFF)
> +WEBKIT_FEATURE(ENABLE_DATALIST "Enable datalist" DEFAULT ON HTML)
> +WEBKIT_FEATURE(ENABLE_DATA_TRANSFER_ITEMS "Enable data transfer items" DEFAULT OFF)
> +WEBKIT_FEATURE(ENABLE_DOM_STORAGE "Enable DOM storage" DEFAULT ON)
> +WEBKIT_FEATURE(ENABLE_EVENTSOURCE "Enable event source" DEFAULT ON)
> +WEBKIT_FEATURE(ENABLE_FAST_MALLOC "Enable TCmalloc instead of system's allocator" DEFAULT ON)
> +WEBKIT_FEATURE(ENABLE_FAST_MOBILE_SCROLLING "Enable fast mobile scrolling" DEFAULT ON)
> +WEBKIT_FEATURE(ENABLE_FILTERS "Enable SVG filters" DEFAULT ON SVG)

Does all this stuff have to be duplicated in all these files?
Comment 4 Adam Barth 2011-05-11 21:07:49 PDT
+steveblock
Comment 5 Adam Barth 2011-05-11 21:08:17 PDT
Comment on attachment 91845 [details]
Patch

R- for the errant wtf/text include path.
Comment 6 Steve Block 2011-05-12 02:25:06 PDT
Comment on attachment 91845 [details]
Patch

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

I'm a little confused about your overall plan. As I'm sure you're aware, the Android port is not yet fully upstreamed, and the upstreamed portion alone won't currently build. It certainly won't build with features enabled other than those currently in Platform.h and config.h. Do you intend to fix this? It would help if there was a meta bug to capture your larger intent.

> Source/cmake/OptionsAndroid.cmake:4
> +ADD_DEFINITIONS(-DWEBCORE_NAVIGATOR_VENDOR="moiji-mobile.com")

!!

I guess we shouldn't be specifying a vendor at all. What do other ports do?

> Source/cmake/OptionsAndroid.cmake:59
> +WEBKIT_FEATURE(ENABLE_XSLT "Enable XSLT" DEFAULT ON)

I'm not sure exactly what your intent is here. Are you trying to replicate the set of feature enables that you removed from Platform.h and config.h in Bug 59889 ? It might be easier to follow if these were combined in a single patch to move these flags from code to the build system.
Comment 7 Holger Freyther 2011-05-12 08:14:34 PDT
(In reply to comment #3)
> (From update of attachment 91845 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=91845&action=review
> 
> Cool.  Does this actually build?
> 
> > Source/WebCore/CMakeListsAndroid.txt:2
> > +  "${JAVASCRIPTCORE_DIR}/wtf/text"
> 
> WebCore shouldn't be including directly from this directory.  Even if it did, it shouldn't be in an Android-specific include block.

Okay, both CE and Efl did it in their file. I agree... if it is necessary it should be shared and if it is not necessary I will remove it.



> > Source/cmake/OptionsAndroid.cmake:34
...
> > +WEBKIT_FEATURE(ENABLE_FAST_MOBILE_SCROLLING "Enable fast mobile scrolling" DEFAULT ON)
> > +WEBKIT_FEATURE(ENABLE_FILTERS "Enable SVG filters" DEFAULT ON SVG)

It appears to be. The reasoning could be to have different defaults on different platforms (e.g. SVG being mostly off Foo and XSLT always on for Bar). I will attempt to consult with one of the authors of the CMake build support.
Comment 8 Adam Barth 2011-05-12 10:06:05 PDT
> It appears to be. The reasoning could be to have different defaults on different platforms (e.g. SVG being mostly off Foo and XSLT always on for Bar). I will attempt to consult with one of the authors of the CMake build support.

Thanks.  It's not a super big deal either way, and it certainly doesn't need to block your work.
Comment 9 Holger Freyther 2011-05-13 07:52:36 PDT
(In reply to comment #6)
> (From update of attachment 91845 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=91845&action=review
> 
> I'm a little confused about your overall plan. As I'm sure you're aware, the Android port is not yet fully upstreamed, and the upstreamed portion alone won't currently build. It certainly won't build with features enabled other than those currently in Platform.h and config.h. Do you intend to fix this? It would help if there was a meta bug to capture your larger intent.

Has my email to webkit-dev helped? The short answer is.
- I want build-webkit --android to work with a NDK (and external libs for Skia)
- I will provide a build-bot to keep it working
- I will start with DRT and will be unlikely to make this a core builder



> 
> > Source/cmake/OptionsAndroid.cmake:4
> > +ADD_DEFINITIONS(-DWEBCORE_NAVIGATOR_VENDOR="moiji-mobile.com")

It will default to Apple Inc.
Comment 10 Steve Block 2011-05-16 06:28:27 PDT
Yes, your email did help, though you didn't explicitly mention your plans regarding all of the components of the Android port that are not yet upstreamed. Please CC me on all Android changes so we can make sure to avoid conflicts between your work and the existing Android port.

I still think that a meta-but would be useful to track your work.
Comment 11 Holger Freyther 2011-05-16 10:51:17 PDT
(In reply to comment #10)
> Yes, your email did help, though you didn't explicitly mention your plans regarding all of the components of the Android port that are not yet upstreamed. Please CC me on all Android changes so we can make sure to avoid conflicts between your work and the existing Android port.

CC: Sure, I can do that. We could also ask for an android component if it does not exist yet.

conflict: My current plan is to get everything out of tree into the tree (including WebKit/android) but then again my Agenda is different to the "Google Android". Then I'm also not aware of the current merge status but I try to pick up as much as possible. In general I think an upstream project should not show much sympathy toward out of tree forks.

irc: It would be nice if we could chat a bit on IRC.


> 
> I still think that a meta-but would be useful to track your work.

So, I would add each of my android bugs to this bug?
Comment 12 Eric Seidel (no email) 2012-02-13 14:45:56 PST
Android Chromium is the new future.