[webkit-reviews] review granted: [Bug 67034] Teach scripts how to handle the building env. for the Chromium port on OS(ANDROID) : [Attachment 105355] Patch

[bugzilla-daemon at webkit.org]

Adam Barth <abarth at webkit.org> has granted Peter Beverloo
<peter at chromium.org>'s request for review:
Bug 67034: Teach scripts how to handle the building env. for the Chromium port
on OS(ANDROID)
https://bugs.webkit.org/show_bug.cgi?id=67034

Attachment 105355: Patch
https://bugs.webkit.org/attachment.cgi?id=105355&action=review

------- Additional Comments from Adam Barth <abarth at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=105355&action=review


> Tools/Scripts/update-webkit:103
> +    push @chromiumUpdateArgs, "--chromium-android" if isChromiumAndroid();

Thus flag can probably just be --android b/c update-webkit-chromium already
implies Chromium.

> Tools/Scripts/update-webkit-chromium:37
> +determineIsChromiumAndroid();

I see, but you're just sharing code.  Yeah, your way is better.

> Tools/Scripts/update-webkit-chromium:63
> +    # TODO(beverloo) Add code to automatically download the NDK

TODO(beverloo) => FIXME
^^^ WebKit's style is to use FIXME rather than TODO.

> Tools/Scripts/update-webkit-chromium:79
> +push @gclientArgs, "--deps=unix,android" if isChromiumAndroid();

We grab the unix deps, even on Mac?

> Tools/Scripts/webkitdirs.pm:1912
> +	   my $ndkBaseDir = getcwd() .
"/Source/WebKit/chromium/third_party/android-ndk-r6";

You sure getcwd() is right here?  I would have expected sourceDir().  Please
make sure this works when run from outside the root before landing.