Issue 4324 - Add Gradle build config

Issue 4324 - Add Gradle build config

The sources can be found in https://github.com/4ntoine/libadblockplus-android/commits/issue-4324

[anton, 2016-08-11 13:28:52]

https://codereview.adblockplus.org/29349720/diff/29349721/.hgignore
File .hgignore (right):

https://codereview.adblockplus.org/29349720/diff/29349721/.hgignore#newcode20
.hgignore:20: **/build
Gradle uses `build` as temporarily dir (like Maven uses `target`)

https://codereview.adblockplus.org/29349720/diff/29349721/gradlew
File gradlew (right):

https://codereview.adblockplus.org/29349720/diff/29349721/gradlew#newcode1
gradlew:1: #!/usr/bin/env bash
It's convenient to use gradle wrapper because it downloads gradle of required
version and performs all the stuff behind the scene

https://codereview.adblockplus.org/29349720/diff/29349721/libadblockplus-andr...
File libadblockplus-android-tests/build.gradle (right):

https://codereview.adblockplus.org/29349720/diff/29349721/libadblockplus-andr...
libadblockplus-android-tests/build.gradle:49: androidTestCompile
fileTree(include: ['*.jar'], dir: 'libs')
usually it's written like `androidTestCompile 'com.android.support.test:*`
(https://developer.android.com/training/testing/unit-testing/instrumented-unit...)
but we already having it for Ant in `libs` directory

https://codereview.adblockplus.org/29349720/diff/29349721/libadblockplus-andr...
File libadblockplus-android/build.gradle (right):

https://codereview.adblockplus.org/29349720/diff/29349721/libadblockplus-andr...
libadblockplus-android/build.gradle:1: apply plugin: 'com.android.model.library'
actually we're using experimental (1 year of being experimental) to be able to
compile the code with NDK

https://codereview.adblockplus.org/29349720/diff/29349721/libadblockplus-andr...
libadblockplus-android/build.gradle:6: libs(PrebuiltLibraries) {
In general it's overhead - i had to rewrite `Android.mk` with Gradle syntax so
it's becoming a bit more difficult to maintain both versions. But i believe it's
returned out by better integration between Android Studio as Gradle now "knows"
the dependencies better.

https://codereview.adblockplus.org/29349720/diff/29349721/libadblockplus-andr...
libadblockplus-android/build.gradle:34: ndk {
native module definition for Gradle (see overhead explanation above)

[diegocarloslima, 2016-11-03 15:13:23]

https://codereview.adblockplus.org/29349720/diff/29353124/.hgignore
File .hgignore (right):

https://codereview.adblockplus.org/29349720/diff/29353124/.hgignore#newcode20
.hgignore:20: **/build
Shouldn't we also ignore the .gradle folder?

https://codereview.adblockplus.org/29349720/diff/29353124/libadblockplus-andr...
File libadblockplus-android-tests/build.gradle (right):

https://codereview.adblockplus.org/29349720/diff/29353124/libadblockplus-andr...
libadblockplus-android-tests/build.gradle:24: versionName "1.3"
The same way we changed versionCode and versionName on manifest, we should
change it here too

https://codereview.adblockplus.org/29349720/diff/29353124/libadblockplus-andr...
File libadblockplus-android/build.gradle (right):

https://codereview.adblockplus.org/29349720/diff/29353124/libadblockplus-andr...
libadblockplus-android/build.gradle:47: versionName "1.3"
Change versionCode and versionName here too

[anton, 2016-11-07 07:05:35]

https://codereview.adblockplus.org/29349720/diff/29353124/.hgignore
File .hgignore (right):

https://codereview.adblockplus.org/29349720/diff/29353124/.hgignore#newcode20
.hgignore:20: **/build
On 2016/11/03 15:13:23, diegocarloslima wrote:
> Shouldn't we also ignore the .gradle folder?

Acknowledged.

https://codereview.adblockplus.org/29349720/diff/29353124/libadblockplus-andr...
File libadblockplus-android-tests/build.gradle (right):

https://codereview.adblockplus.org/29349720/diff/29353124/libadblockplus-andr...
libadblockplus-android-tests/build.gradle:24: versionName "1.3"
On 2016/11/03 15:13:23, diegocarloslima wrote:
> The same way we changed versionCode and versionName on manifest, we should
> change it here too

We did not change it yet, it will be changed in another issue. After all it will
be 1.0 (1):
https://github.com/4ntoine/libadblockplus-android/blob/issue-4399-4503/libadb...

https://codereview.adblockplus.org/29349720/diff/29353124/libadblockplus-andr...
File libadblockplus-android/build.gradle (right):

https://codereview.adblockplus.org/29349720/diff/29353124/libadblockplus-andr...
libadblockplus-android/build.gradle:47: versionName "1.3"
On 2016/11/03 15:13:23, diegocarloslima wrote:
> Change versionCode and versionName here too

Currently it's still 1.3 (359) in AndroidManifest.xml, pom.xml and gradle.build
and it will be changed in another issue changeset. After all it's 1.0 (1):
https://github.com/4ntoine/libadblockplus-android/blob/issue-4399-4503/libadb...

[anton, 2016-11-07 07:21:42]

https://codereview.adblockplus.org/29349720/diff/29361761/.hgignore
File .hgignore (right):

https://codereview.adblockplus.org/29349720/diff/29361761/.hgignore#newcode21
.hgignore:21: .gradle
took Diego's comment into account

[diegocarloslima, 2016-11-07 13:05:06]

https://codereview.adblockplus.org/29349720/diff/29353124/libadblockplus-andr...
File libadblockplus-android-tests/build.gradle (right):

https://codereview.adblockplus.org/29349720/diff/29353124/libadblockplus-andr...
libadblockplus-android-tests/build.gradle:24: versionName "1.3"
On 2016/11/07 07:05:34, anton wrote:
> On 2016/11/03 15:13:23, diegocarloslima wrote:
> > The same way we changed versionCode and versionName on manifest, we should
> > change it here too
> 
> We did not change it yet, it will be changed in another issue. After all it
will
> be 1.0 (1):
>
https://github.com/4ntoine/libadblockplus-android/blob/issue-4399-4503/libadb...

Acknowledged.

https://codereview.adblockplus.org/29349720/diff/29353124/libadblockplus-andr...
File libadblockplus-android/build.gradle (right):

https://codereview.adblockplus.org/29349720/diff/29353124/libadblockplus-andr...
libadblockplus-android/build.gradle:47: versionName "1.3"
On 2016/11/07 07:05:34, anton wrote:
> On 2016/11/03 15:13:23, diegocarloslima wrote:
> > Change versionCode and versionName here too
> 
> Currently it's still 1.3 (359) in AndroidManifest.xml, pom.xml and
gradle.build
> and it will be changed in another issue changeset. After all it's 1.0 (1):
>
https://github.com/4ntoine/libadblockplus-android/blob/issue-4399-4503/libadb...

Acknowledged.

[diegocarloslima, 2016-11-07 13:06:02]

On 2016/11/07 13:05:06, diegocarloslima wrote:
>
https://codereview.adblockplus.org/29349720/diff/29353124/libadblockplus-andr...
> File libadblockplus-android-tests/build.gradle (right):
> 
>
https://codereview.adblockplus.org/29349720/diff/29353124/libadblockplus-andr...
> libadblockplus-android-tests/build.gradle:24: versionName "1.3"
> On 2016/11/07 07:05:34, anton wrote:
> > On 2016/11/03 15:13:23, diegocarloslima wrote:
> > > The same way we changed versionCode and versionName on manifest, we should
> > > change it here too
> > 
> > We did not change it yet, it will be changed in another issue. After all it
> will
> > be 1.0 (1):
> >
>
https://github.com/4ntoine/libadblockplus-android/blob/issue-4399-4503/libadb...
> 
> Acknowledged.
> 
>
https://codereview.adblockplus.org/29349720/diff/29353124/libadblockplus-andr...
> File libadblockplus-android/build.gradle (right):
> 
>
https://codereview.adblockplus.org/29349720/diff/29353124/libadblockplus-andr...
> libadblockplus-android/build.gradle:47: versionName "1.3"
> On 2016/11/07 07:05:34, anton wrote:
> > On 2016/11/03 15:13:23, diegocarloslima wrote:
> > > Change versionCode and versionName here too
> > 
> > Currently it's still 1.3 (359) in AndroidManifest.xml, pom.xml and
> gradle.build
> > and it will be changed in another issue changeset. After all it's 1.0 (1):
> >
>
https://github.com/4ntoine/libadblockplus-android/blob/issue-4399-4503/libadb...
> 
> Acknowledged.

LGTM

[Felix Dahlke, 2016-11-18 07:17:43]

Looks good, just some questions and README comments.

Overall I'm not happy with duplicating build logic and dependencies, but then
again, I see the point of using Gradle and supporting Android Studio. We might
want to think about how long we are willing to support three different build
tools however.

https://codereview.adblockplus.org/29349720/diff/29361761/README.md
File README.md (right):

https://codereview.adblockplus.org/29349720/diff/29361761/README.md#newcode61
README.md:61: * Android SDK Build tools 22.0.1
Any reason we refer to a specific build tools version here, but not above when
it comes to building with Maven and Ant? Maybe we should move the SDK/NDK
requirement to a shared requirements section, so we don't have to duplicate it?

https://codereview.adblockplus.org/29349720/diff/29361761/README.md#newcode64
README.md:64: Edit 'buildToolsVersion' in 'build.gradle' files if necessary.
"files" - is there more than one? Also, does build.gradle have to be created? If
so, how should it look?

https://codereview.adblockplus.org/29349720/diff/29361761/README.md#newcode78
README.md:78: This will generate *.aar library artifact in
'libadblockplus-android/build/outputs/aar/' directory.
Unrelated, but I think it'd also be helpful to point out above in the Ant
section where the .aar is created if one builds via Ant. But that should go into
a separate commit, Noissue is fine.

https://codereview.adblockplus.org/29349720/diff/29361761/README.md#newcode78
README.md:78: This will generate *.aar library artifact in
'libadblockplus-android/build/outputs/aar/' directory.
Wording nit: "artifact in THE [...] directory."

https://codereview.adblockplus.org/29349720/diff/29361761/README.md#newcode125
README.md:125: Make sure you've created _local.properties_ file to build the
library (see above).
Wording nit: "you've created THE [...] file".

https://codereview.adblockplus.org/29349720/diff/29361761/README.md#newcode130
README.md:130: This will generate *.apk in
'libadblockplus-android-tests/build/outputs/apk/' directory.
Wording nit: "in THE [...] directory"

[anton, 2016-11-21 08:15:39]

https://codereview.adblockplus.org/29349720/diff/29361761/README.md
File README.md (right):

https://codereview.adblockplus.org/29349720/diff/29361761/README.md#newcode61
README.md:61: * Android SDK Build tools 22.0.1
On 2016/11/18 07:17:42, Felix Dahlke wrote:
> Any reason we refer to a specific build tools version here, but not above when
> it comes to building with Maven and Ant? Maybe we should move the SDK/NDK
> requirement to a shared requirements section, so we don't have to duplicate
it?

It's a good question and honestly i don't remember what was the reason exactly
(i think it was that 24.0.1 requires java 8 as android.jar is compiled with java
8). But it does not makes sense as after that a new version of gradle plugin was
released (moved from experimental ndk support to main branch) and now it
requires just the latest version (24.0.1), see
https://codereview.adblockplus.org/29361445/patch/29362070/29362134

https://codereview.adblockplus.org/29349720/diff/29361761/README.md#newcode64
README.md:64: Edit 'buildToolsVersion' in 'build.gradle' files if necessary.
On 2016/11/18 07:17:42, Felix Dahlke wrote:
> "files" - is there more than one? Also, does build.gradle have to be created?
If
> so, how should it look?

Yes, there are multiple `build.gradle` files - in each directory/Gradle module`.

https://codereview.adblockplus.org/29349720/diff/29361761/README.md#newcode78
README.md:78: This will generate *.aar library artifact in
'libadblockplus-android/build/outputs/aar/' directory.
On 2016/11/18 07:17:43, Felix Dahlke wrote:
> Unrelated, but I think it'd also be helpful to point out above in the Ant
> section where the .aar is created if one builds via Ant. But that should go
into
> a separate commit, Noissue is fine.

Acknowledged.

https://codereview.adblockplus.org/29349720/diff/29361761/README.md#newcode78
README.md:78: This will generate *.aar library artifact in
'libadblockplus-android/build/outputs/aar/' directory.
On 2016/11/18 07:17:43, Felix Dahlke wrote:
> Unrelated, but I think it'd also be helpful to point out above in the Ant
> section where the .aar is created if one builds via Ant. But that should go
into
> a separate commit, Noissue is fine.

Acknowledged.

[Felix Dahlke, 2016-11-21 08:55:34]

https://codereview.adblockplus.org/29349720/diff/29361761/README.md
File README.md (right):

https://codereview.adblockplus.org/29349720/diff/29361761/README.md#newcode61
README.md:61: * Android SDK Build tools 22.0.1
On 2016/11/21 08:15:39, anton wrote:
> On 2016/11/18 07:17:42, Felix Dahlke wrote:
> > Any reason we refer to a specific build tools version here, but not above
when
> > it comes to building with Maven and Ant? Maybe we should move the SDK/NDK
> > requirement to a shared requirements section, so we don't have to duplicate
> it?
> 
> It's a good question and honestly i don't remember what was the reason exactly
> (i think it was that 24.0.1 requires java 8 as android.jar is compiled with
java
> 8). But it does not makes sense as after that a new version of gradle plugin
was
> released (moved from experimental ndk support to main branch) and now it
> requires just the latest version (24.0.1), see
> https://codereview.adblockplus.org/29361445/patch/29362070/29362134

So how I understand that, it works just fine with the latest version? Why
specify the SDK build tools version here then?

https://codereview.adblockplus.org/29349720/diff/29361761/README.md#newcode64
README.md:64: Edit 'buildToolsVersion' in 'build.gradle' files if necessary.
On 2016/11/21 08:15:39, anton wrote:
> On 2016/11/18 07:17:42, Felix Dahlke wrote:
> > "files" - is there more than one? Also, does build.gradle have to be
created?
> If
> > so, how should it look?
> 
> Yes, there are multiple `build.gradle` files - in each directory/Gradle
module`.

Acknowledged.

[anton, 2016-11-21 10:11:07]

On 2016/11/21 08:55:34, Felix Dahlke wrote:
> https://codereview.adblockplus.org/29349720/diff/29361761/README.md
> File README.md (right):
> 
> https://codereview.adblockplus.org/29349720/diff/29361761/README.md#newcode61
> README.md:61: * Android SDK Build tools 22.0.1
> On 2016/11/21 08:15:39, anton wrote:
> > On 2016/11/18 07:17:42, Felix Dahlke wrote:
> > > Any reason we refer to a specific build tools version here, but not above
> when
> > > it comes to building with Maven and Ant? Maybe we should move the SDK/NDK
> > > requirement to a shared requirements section, so we don't have to
duplicate
> > it?
> > 
> > It's a good question and honestly i don't remember what was the reason
exactly
> > (i think it was that 24.0.1 requires java 8 as android.jar is compiled with
> java
> > 8). But it does not makes sense as after that a new version of gradle plugin
> was
> > released (moved from experimental ndk support to main branch) and now it
> > requires just the latest version (24.0.1), see
> > https://codereview.adblockplus.org/29361445/patch/29362070/29362134
> 
> So how I understand that, it works just fine with the latest version? Why
> specify the SDK build tools version here then?

`Build.gradle` requires build tools version (`buildToolsVersion`) to be
specified explicitly, but Ant and Maven just use the latest available found in
the sdk folder.

> 
> https://codereview.adblockplus.org/29349720/diff/29361761/README.md#newcode64
> README.md:64: Edit 'buildToolsVersion' in 'build.gradle' files if necessary.
> On 2016/11/21 08:15:39, anton wrote:
> > On 2016/11/18 07:17:42, Felix Dahlke wrote:
> > > "files" - is there more than one? Also, does build.gradle have to be
> created?
> > If
> > > so, how should it look?
> > 
> > Yes, there are multiple `build.gradle` files - in each directory/Gradle
> module`.
> 
> Acknowledged.

[Felix Dahlke, 2016-11-21 11:26:33]

I see, so we have to make sure the user installs the exact version we specify
there. Yuck! Anyway, LGTM then.

[anton, 2016-11-21 12:29:51]

https://codereview.adblockplus.org/29349720/diff/29361761/README.md
File README.md (right):

https://codereview.adblockplus.org/29349720/diff/29361761/README.md#newcode78
README.md:78: This will generate *.aar library artifact in
'libadblockplus-android/build/outputs/aar/' directory.
On 2016/11/18 07:17:43, Felix Dahlke wrote:
> Unrelated, but I think it'd also be helpful to point out above in the Ant
> section where the .aar is created if one builds via Ant. But that should go
into
> a separate commit, Noissue is fine.

When compiling with Ant no aar is created. It just uses good-old copy to copy
compiled classes and resources to the application. For this the app should refer
to the library in project.properties