Skip to content

Publish logins AAR to nalexander's personal bintray.#199

Merged
ncalexan merged 3 commits intomasterfrom
publish-logins-api
Sep 12, 2018
Merged

Publish logins AAR to nalexander's personal bintray.#199
ncalexan merged 3 commits intomasterfrom
publish-logins-api

Conversation

@ncalexan
Copy link
Member

@ncalexan ncalexan commented Aug 31, 2018 •
edited by rfk
Loading

Fixes #189

@ncalexan ncalexan force-pushed the publish-logins-api branch 2 times, most recently from c885527 to 8834bf8 Compare September 4, 2018 20:44
@vladikoff vladikoff added this to the Logins API - v1 milestone Sep 4, 2018
@rfk rfk added the in progress label Sep 5, 2018
@ncalexan ncalexan mentioned this pull request Sep 7, 2018
Closed
@ncalexan ncalexan force-pushed the publish-logins-api branch 3 times, most recently from 7bb945f to 19ac2fc Compare September 10, 2018 21:19
@ncalexan
Copy link
Member Author

ncalexan commented Sep 10, 2018

OK folks, I think this is ready for review. I have a healthy looking Logins AAR in my personal bintray repo at https://bintray.com/ncalexander/application-services -- sadly, that uses an untrusted cert (!).

I haven't tested the AAR in a sample application yet but I'm pretty confident.

@ncalexan
Copy link
Member Author

ncalexan commented Sep 10, 2018

The AAR itself is at https://dl.bintray.com/ncalexander/application-services/org/mozilla/sync15/logins/0.3.0/logins-0.3.0.aar (again, cert error). It looks like:

M Filemode      Length  Date         Time      File
- ----------  --------  -----------  --------  -----------------------------------
  -rw-r--r--       304   1-Feb-1980  00:00:00  AndroidManifest.xml
  -rw-r--r--     77501   1-Feb-1980  00:00:00  R.txt
  -rw-r--r--     73871   1-Feb-1980  00:00:00  classes.jar
  drwxr-xr-x         0   1-Feb-1980  00:00:00  jni/
  drwxr-xr-x         0   1-Feb-1980  00:00:00  jni/arm64-v8a/
  -rw-r--r--  114682888   1-Feb-1980  00:00:00  jni/arm64-v8a/libloginsapi_ffi.so
  drwxr-xr-x         0   1-Feb-1980  00:00:00  jni/armeabi-v7a/
  -rw-r--r--  113297988   1-Feb-1980  00:00:00  jni/armeabi-v7a/libloginsapi_ffi.so
  drwxr-xr-x         0   1-Feb-1980  00:00:00  jni/x86/
  -rw-r--r--  122422404   1-Feb-1980  00:00:00  jni/x86/libloginsapi_ffi.so
  drwxr-xr-x         0   1-Feb-1980  00:00:00  x86/
  drwxr-xr-x         0   1-Feb-1980  00:00:00  arm64-v8a/
  drwxr-xr-x         0   1-Feb-1980  00:00:00  armeabi-v7a/
- ----------  --------  -----------  --------  -----------------------------------
              350554956                         13 files

I'm pretty concerned that R.txt has things it shouldn't have in it, but I'll try to strip that down in follow-up -- we must be (incorrectly) depending on some Android support library with resources.

@ncalexan
Copy link
Member Author

ncalexan commented Sep 10, 2018

Those .so files do have symbols, BTW -- that's why they're so large. We can try to publish release and debug versions with different code optimization settings and strip settings if we want to do better in the future, but that can be follow-up.

thomcc
thomcc approved these changes Sep 11, 2018
View reviewed changes
Copy link
Contributor

@thomcc thomcc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

logins-api/android/gradle.properties Outdated
libUrl=https://github.com/mozilla/application-services
libVcsUrl=https://github.com/mozilla/application-services.git

libLicense 'Apache-2.0'
Copy link
Contributor

@thomcc thomcc Sep 11, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably should be MPL given the discussion the other day.

Copy link
Member Author

@ncalexan ncalexan Sep 12, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the reminder -- I will update, and perhaps I'll land #233 before this.

.taskcluster.yml
maxRunTime: 3600
deadline: "{{ '2 hours' | $fromNow }}"
maxRunTime: 7200
deadline: "{{ '4 hours' | $fromNow }}"
Copy link
Contributor

@thomcc thomcc Sep 11, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yikes, 4 hour build!

Copy link
Member Author

@ncalexan ncalexan Sep 12, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually a 2 hour build (maxRunTime). I think deadline is the sum of the time to schedule the job in TC and the time to actually complete, although I can't guarantee that.

vladikoff
vladikoff approved these changes Sep 12, 2018
View reviewed changes
Copy link
Contributor

@vladikoff vladikoff left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

r+!

nit: license needs to be MPL

@ncalexan
Pre: Bump version to 0.3.2.
835a823 
This just needs to be greater than 0.3.1, the last version I pushed in
testing.
@ncalexan
Publish logins AAR to nalexander's personal bintray.
bfbc662
@ncalexan
Post: Bump task timeout for build and publishing.
2760c36 
Building everything for three architectures just takes a long time --
15 minutes per architecture or more.

I investigated speeding this up, and I conclude that compiling the
architectures in parallel is both work (you need to work around cargo
a little bit, and you need to use a Make jobserver to limit the total
thread pool) and doesn't pay off (I witnessed only about a 5-10%
speed-up locally).  My guess is that it pays off even less in AWS,
where IO throughput is relatively low and context switching relatively
more expensive.
@ncalexan ncalexan merged commit a3bcf30 into master Sep 12, 2018
@ncalexan ncalexan deleted the publish-logins-api branch September 12, 2018 18:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers

@vladikoff vladikoff vladikoff approved these changes

@thomcc thomcc thomcc approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

Logins API - v1

Development

Successfully merging this pull request may close these issues.

4 participants

@ncalexan @vladikoff @thomcc @rfk