Bug 31325 - [Android] Android port lacks Makefiles
Summary: [Android] Android port lacks Makefiles
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Android Android
: P2 Normal
Assignee: Steve Block
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-11-10 16:24 PST by Steve Block
Modified: 2009-12-08 08:56 PST (History)
7 users (show)

See Also:


Attachments
Patch 1 for Bug 31325 (133.66 KB, patch)
2009-11-12 18:05 PST, Steve Block
sam: review-
Details | Formatted Diff | Diff
Patch 2 for Bug 31325 (95.68 KB, patch)
2009-11-16 06:34 PST, Steve Block
no flags Details | Formatted Diff | Diff
Patch 3 for Bug 31325 (95.67 KB, patch)
2009-11-16 08:19 PST, Steve Block
no flags Details | Formatted Diff | Diff
Patch 4 for Bug 31325 (95.00 KB, patch)
2009-11-19 10:16 PST, Steve Block
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Steve Block 2009-11-10 16:24:57 PST
Android port lacks Makefiles.
Comment 1 Steve Block 2009-11-12 18:05:51 PST
Created attachment 43119 [details]
Patch 1 for Bug 31325
Comment 2 Sam Weinig 2009-11-13 10:33:36 PST
Comment on attachment 43119 [details]
Patch 1 for Bug 31325

There seems to be a lot of duplication between the *jsc.mk and *.v8.mk files.  That should really be avoided since we really don't want to have to update both when adding new files.  The copyright date also seems out of date.


> Index: WebCore/WebCorePrefix.h
> ===================================================================
> --- WebCore/WebCorePrefix.h	(revision 50854)
> +++ WebCore/WebCorePrefix.h	(working copy)
> @@ -62,6 +62,18 @@
>  #include <pthread.h>
>  #endif // defined(WIN32) || defined(_WIN32)
>  
> +#if defined(ANDROID)
> +#ifdef __cplusplus
> +// Must come before include of alorithm.
> +#define PREFIX_FOR_WEBCORE 1
> +#define EXPORT __attribute__((visibility("default")))
> +#endif
> +// We use a single set of include directories when building WebKit and
> +// JavaScriptCore. Since WebCore/ is included before JavaScriptCore/, we include
> +// JavaScriptCore/config.h explicitly here to make sure it gets picked up.

The "we" is unclear here.  You should probably use a more descriptive term like "Android".


r- to address the duplication.
Comment 3 Steve Block 2009-11-16 06:34:19 PST
Created attachment 43304 [details]
Patch 2 for Bug 31325

> There seems to be a lot of duplication between the *jsc.mk and *.v8.mk files. 
> That should really be avoided since we really don't want to have to update both
> when adding new files.  The copyright date also seems out of date.
Fixed

> The "we" is unclear here.  You should probably use a more descriptive term like
> "Android".
Fixed
Comment 4 Steve Block 2009-11-16 08:19:34 PST
Created attachment 43309 [details]
Patch 3 for Bug 31325
Comment 5 Steve Block 2009-11-19 04:53:35 PST
Sam, have you had chance to look at my updated patch?
Comment 6 Steve Block 2009-11-19 10:16:08 PST
Created attachment 43513 [details]
Patch 4 for Bug 31325

Update to reflect recently removed Canvas source files.
Comment 7 Eric Seidel (no email) 2009-11-29 13:18:30 PST
I'm surprised that Andriod would use Make instead of sharing Chromium's GYP files.
Comment 8 Adam Barth 2009-11-29 18:29:23 PST
Is it at all possible to avoid having yet another file that lists all the other files in the project?  This approach doesn't really scale.
Comment 9 Steve Block 2009-11-30 03:00:06 PST
> I'm surprised that Andriod would use Make instead of sharing Chromium's GYP
> files.
The entire Android platform uses Make as its build system, so using something different for WebKit isn't trivial. We've looked into using GYP and it's something we'd like to do in the long term, but for now our focus is on unforking our version of WebKit and getting an Android build-bot set up.

> Is it at all possible to avoid having yet another file that lists all the other
> files in the project?  This approach doesn't really scale.
As above, we'd like to share the GYP files in the long term.
Comment 10 Adam Barth 2009-11-30 12:26:15 PST
Attachment 43513 [details] passed the style-queue
Comment 11 Adam Barth 2009-12-08 08:29:07 PST
Comment on attachment 43513 [details]
Patch 4 for Bug 31325

Mostly a rubber stamp because I don't understand the build system, but it seems useful to have make files for your port.

1) I'm surprised at the 2-clause BSD.  I thought we used 3-clause

2) I'd like to strongly encourage you to use GYP.  It's easier for every port to use its own build system, but its a tragedy of the commons.

3) Does you build system integrate with build-webkit?  I'd encourage you to either make build-webkit do the right thing automagically or add a --android flag analogous to the --chromium flag.
Comment 12 Steve Block 2009-12-08 08:40:23 PST
> 2) I'd like to strongly encourage you to use GYP.  It's easier for every port
> to use its own build system, but its a tragedy of the commons.
As I mentioned earlier, this is something we'd like to do in the long term.

> 3) Does you build system integrate with build-webkit?  I'd encourage you to
> either make build-webkit do the right thing automagically or add a --android
> flag analogous to the --chromium flag.
This is part of our short-term plan. See https://bugs.webkit.org/show_bug.cgi?id=32276
Comment 13 WebKit Commit Bot 2009-12-08 08:56:06 PST
Comment on attachment 43513 [details]
Patch 4 for Bug 31325

Clearing flags on attachment: 43513

Committed r51858: <http://trac.webkit.org/changeset/51858>
Comment 14 WebKit Commit Bot 2009-12-08 08:56:14 PST
All reviewed patches have been landed.  Closing bug.