Closed Bug 1252928 Opened 8 years ago Closed 8 years ago

Use in-tree Fennec Gradle configuration rather than special dependencies configuration in Android Gradle dependency fetching task

Categories

(Firefox Build System :: Android Studio and Gradle Integration, defect)

defect
Not set
normal

Tracking

(firefox48 fixed)

RESOLVED FIXED
Tracking Status
firefox48 --- fixed

People

(Reporter: nalexander, Assigned: nalexander)

References

Details

Attachments

(3 files)

I landed https://dxr.mozilla.org/mozilla-central/source/testing/docker/android-gradle-build/project to quickly get dependency fetching into the tree.  We'd much rather use the actual project in https://dxr.mozilla.org/mozilla-central/source/mobile/android/app/build.gradle and add sdk-manager-plugin there.  This ticket tracks doing that work.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=51198adaacdb shows this working.
Assignee: nobody → nalexander
Status: NEW → ASSIGNED
This is a version produced locally from
https://github.com/JakeWharton/sdk-manager-plugin/commit/1d07987217120fffdc0ce1291d73e974310d7d92.
I used |./gradlew install| and then copied the pieces out of ~/.m2/repository.

Local builders should never download using this plugin, since |mach configure|
ensures that all the relevant pieces can already be found.  It's just useful in
automation; and as an eventual replacement for chunks of |mach bootstrap|.

Review commit: https://reviewboard.mozilla.org/r/38813/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/38813/
Attachment #8728145 - Flags: review?(s.kaspari)
This container formerly defined a special Gradle project for fetching
dependencies.  This patch lays the ground-work to use the in-tree
Gradle project instead.  Using the in-tree project looks like first
starting a local Nexus repository to collect downloaded dependencies
(before.sh); then running a build which populates that repository; and
then packaging up the downloaded dependencies (after.sh).  The patch
after this will define the build which populates the repository.

Sadly there's no easy way to *inherit* from desktop-build, so this is
a copy-paste-modify clone.

Review commit: https://reviewboard.mozilla.org/r/38815/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/38815/
Attachment #8728146 - Flags: review?(dustin)
The existing task's outcome is best achieved with a special purpose
build task, so here it is.

Review commit: https://reviewboard.mozilla.org/r/38817/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/38817/
Attachment #8728147 - Flags: review?(dustin)
Attachment #8728145 - Flags: review?(s.kaspari) → review+
Comment on attachment 8728145 [details]
MozReview Request: Bug 1252928 - Part 1: Use Jake Wharton's sdk-manager-plugin in Fennec's Gradle configuration. r?sebastian

https://reviewboard.mozilla.org/r/38813/#review35573
Comment on attachment 8728146 [details]
MozReview Request: Bug 1252928 - Part 2: Turn android-gradle-build into a desktop-build clone. r?dustin

https://reviewboard.mozilla.org/r/38815/#review35597
Attachment #8728146 - Flags: review?(dustin) → review+
Attachment #8728147 - Flags: review?(dustin)
Comment on attachment 8728147 [details]
MozReview Request: Bug 1252928 - Part 3: Replace android-gradle-build-dependencies with android-api-15-gradle-dependencies. r?dustin

https://reviewboard.mozilla.org/r/38817/#review35595

::: testing/taskcluster/scripts/misc/repackage-jdk-centos.sh:27
(Diff revision 1)
> +ln -s /etc/pki/java/cacerts java_home/jre/lib/security/cacerts

It occurs to me, tar can do this too:
  http://www.gnu.org/software/tar/manual/html_node/dereference.html
(assuming the link is correc to begin with)

::: testing/taskcluster/tasks/builds/android_api_15_gradle_dependencies.yml:60
(Diff revision 1)
> +        /home/worker/bin/after.sh &&

I like this!

That's minor - if you end up doing another round of review, maybe switch to use `tar --dereference`, but otherwise this is fine.
(In reply to Dustin J. Mitchell [:dustin] from comment #7)
> Comment on attachment 8728147 [details]
> MozReview Request: Bug 1252928 - Part 3: Replace
> android-gradle-build-dependencies with android-api-15-gradle-dependencies.
> r?dustin
> 
> https://reviewboard.mozilla.org/r/38817/#review35595
> 
> ::: testing/taskcluster/scripts/misc/repackage-jdk-centos.sh:27
> (Diff revision 1)
> > +ln -s /etc/pki/java/cacerts java_home/jre/lib/security/cacerts
> 
> It occurs to me, tar can do this too:
>   http://www.gnu.org/software/tar/manual/html_node/dereference.html
> (assuming the link is correc to begin with)
> 
> :::
> testing/taskcluster/tasks/builds/android_api_15_gradle_dependencies.yml:60
> (Diff revision 1)
> > +        /home/worker/bin/after.sh &&
> 
> I like this!
> 
> That's minor - if you end up doing another round of review, maybe switch to
> use `tar --dereference`, but otherwise this is fine.

Just for the next person: there's a subtle difference between making the symlink absolute and using --dereference.  Making the symlink absolute lets the consuming system set the cacerts; using --dereference takes the producing system's cacerts and sets them in stone.  Until further notice, I prefer the former.
Blocks: 1255227
https://hg.mozilla.org/mozilla-central/rev/d316d6160a5f
https://hg.mozilla.org/mozilla-central/rev/022cdd61e207
https://hg.mozilla.org/mozilla-central/rev/063fb6e87b4e
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
Component: Build Config → Build Config & IDE Support
Product: Core → Firefox for Android
Target Milestone: mozilla48 → ---
Product: Firefox for Android → Firefox Build System
You need to log in before you can comment on or make changes to this bug.