Bug 31325 – [Android] Android port lacks Makefiles

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Steve Block]

Android port lacks Makefiles.

[Steve Block]

Created attachment 43119 [details]
Patch 1 for Bug 31325

[Sam Weinig]

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.

[Steve Block]

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

[Steve Block]

Created attachment 43309 [details]
Patch 3 for Bug 31325

[Steve Block]

Sam, have you had chance to look at my updated patch?

[Steve Block]

Created attachment 43513 [details]
Patch 4 for Bug 31325

Update to reflect recently removed Canvas source files.

[Eric Seidel (no email)]

I'm surprised that Andriod would use Make instead of sharing Chromium's GYP files.

[Adam Barth]

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.

[Steve Block]

> 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.

[Adam Barth]

Attachment 43513 [details] passed the style-queue

[Adam Barth]

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.

[Steve Block]

> 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

[WebKit Commit Bot]

Comment on attachment 43513 [details]
Patch 4 for Bug 31325

Clearing flags on attachment: 43513

Committed r51858: <http://trac.webkit.org/changeset/51858>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.