robocop isn't signed properly from buildbot builds

RESOLVED FIXED

Status

defect
RESOLVED FIXED
8 years ago
8 years ago

People

(Reporter: jmaher, Assigned: jmaher)

Tracking

Trunk
ARM
Android
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(4 attachments, 2 obsolete attachments)

while trying to get robocop to run from tinderbox builds, I see this in logcat:
W/ActivityManager(  109): Permission Denial: starting instrumentation ComponentInfo{org.mozilla.roboexample.test/android.test.InstrumentationTestRunner} from pid=6046, uid=6046 not allowed because package org.mozilla.roboexample.test does not have a signature matching the target org.mozilla.fennec

Here is where we do the signing for robocop.apk:
http://mxr.mozilla.org/mozilla-central/source/build/mobile/robocop/Makefile.in#132

Here is where we do the signing for fennec.apk:
http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/installer/packager.mk#357

Is it possible that we are not defining JARSIGNER for the mobile/build/robocop builds, and we are for the toolkit/mozapps/installer?
Is robocop packaged during make package?
That will fix it.
I won't have time to look into this until Monday.  Right now robocop is build during the 'make' process and not the 'make package'.  Maybe it is an easy change to do, if anybody knows some magic.
I created this patch last night and it generated a robocop.apk in the build directory and at make package time.  Unfortunately when I ran robocop tests, it failed with the same permission denied error.

Is there something I am doing wrong in the patch?

Here is the try server run if logs will help:
https://tbpl.mozilla.org/?tree=Try&rev=f42a113d739f
ok, after looking into this further, we use mozpass.py to wrap jarsigner on the production machines.  I set this up locally using my debug cert, but inside the mozpass.py toolchain.  I was able to build, install and test just fine.

Next I pushed my patch to try and it generated a fennec/robocop set of apks and those failed to run with the same error.  After talking with bear, I ran some verification scripts:
jmaher@jmaher-MacBookPro:~/mozilla/tools/release/signing$ ./verify-android-signature.sh --tools-dir=/home/jmaher/mozilla/tools/ --apk=/home/jmaher/Downloads/fennec-12.0a1.en-US.android-arm.apk -n
TOOLS DIR: /home/jmaher/mozilla/tools/
Archive:  /home/jmaher/Downloads/fennec-12.0a1.en-US.android-arm.apk
  inflating: META-INF/NIGHTLY.DSA    
Valid
jmaher@jmaher-MacBookPro:~/mozilla/tools/release/signing$ ./verify-android-signature.sh --tools-dir=/home/jmaher/mozilla/tools/ --apk=/home/jmaher/Downloads/robocop.apk -nTOOLS DIR: /home/jmaher/mozilla/tools/
Archive:  /home/jmaher/Downloads/robocop.apk
  inflating: META-INF/NIGHTLY.DSA    
Valid
jmaher@jmaher-MacBookPro:~/mozilla/tools/release/signing$


it appears that both are signed the same.  I am not sure why this is failing, but I need somebody else who understands the certificates to look into this.
blassey, can you or somebody from mobile development look into this?
aki mentioned that I don't have the sharedId in my .xml file.  Pushing to try and seeing what happens.
this is pretty simple once you realize that you can have >1 key in a .apk.  When testing locally, I found that I didn't always have a key in robocop-raw.apk, so I added a " & echo ''" so we wouldn't error out during make.  Hacky, so suggest something better please!  Otherwise this final patch is running on try while a similar patch build and tested fine last night.
Assignee: nobody → jmaher
Attachment #590473 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #591476 - Flags: review?(blassey.bugs)
Attachment #591476 - Flags: review?(aki)
Attachment #591476 - Flags: review?(aki) → review+
Comment on attachment 591476 [details] [diff] [review]
sign robocop with the same key as fennec and only 1 key (1.0)

Review of attachment 591476 [details] [diff] [review]:
-----------------------------------------------------------------

::: testing/testsuite-targets.mk
@@ +297,2 @@
>  	$(NSINSTALL) $(DEPTH)/build/mobile/robocop/fennec_ids.txt $(PKG_STAGE)/mochitest
> +	zip -q -d 'META-INF/*' $(DIST)/robocop-raw.apk & echo ''

what's the & echo '' for?
Attachment #591476 - Flags: review?(blassey.bugs) → review+
sorry for the re-review, but this is much different than the last patch.  All the logic is in toolkit/mozapps/installer/packager.mk and will be built during 'make package' instead of 'make package-tests'.  

Tested on try and ran locally, all looks good.
Attachment #591476 - Attachment is obsolete: true
Attachment #591764 - Flags: review?(blassey.bugs)
Attachment #591764 - Flags: review?(aki)
note to hwine (and other releng folks): this creates a file robocop.apk inside the upload folder (same locations as fennec.apk and fennec.tests.zip).  There will NOT be a tests.zip:/bin/robocop.apk anymore.  We will need to fix our existing code to handle this new location.  Here is an example:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/jmaher@mozilla.com-d7dc53a1af3f/try-android/
Comment on attachment 591764 [details] [diff] [review]
sign robocop with the same key as fennec and only 1 key (2.0)

Review of attachment 591764 [details] [diff] [review]:
-----------------------------------------------------------------

I feel much better about this patch
Attachment #591764 - Flags: review?(blassey.bugs) → review+
Comment on attachment 591764 [details] [diff] [review]
sign robocop with the same key as fennec and only 1 key (2.0)

Nit: I think this would read better for me if it said

+MAKE_PACKAGE    = $(PREPARE_PACKAGE) && $(INNER_MAKE_PACKAGE) && $(INNER_ROBOCOP_PACKAGE)

and removed the first && from INNER_ROBOCOP_PACKAGE.  But I think moving to make package is the right move here; we may want to do the same with sutagent.
Attachment #591764 - Flags: review?(aki) → review+
Sorry, I had to back this out on inbound along with bug 713970:
https://hg.mozilla.org/integration/mozilla-inbound/rev/547cea3b54fd

...because one of them caused some build bustage:
https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=f151cccff911
Joel: do you still need me to pull a try slave for you to diagnose this error/iterate more quickly?
:coop, not anymore, I think I got it to stick this time around.

inbound:
https://hg.mozilla.org/integration/mozilla-inbound/rev/c593ef0f4353
Comment on attachment 592168 [details] [diff] [review]
detect robocop.apk

Thanks for the quick turn, Aki!
Attachment #592168 - Flags: review?(hwine) → review+
landed on m-c:
https://hg.mozilla.org/mozilla-central/rev/80cc0dcc7fec
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Depends on: 722172
Depends on: 722118
This breaks android single locale repacks:

java.io.FileNotFoundException: /builds/slave/m-cen-andrd-l10n-2/mozilla-central/obj-l10n/mobile/android/locales/../../../dist/../build/mobile/robocop/robocop.ap_ does not exist
make[1]: *** [repackage-zip] Error 1
make[1]: Leaving directory `/builds/slave/m-cen-andrd-l10n-2/mozilla-central/obj-l10n/mobile/android/locales'
make: *** [repackage-zip-gl] Error 2
command: END (17.74s elapsed)

Error creating locale 'gl': Command '['make', 'installers-gl']' returned non-zero exit status 2
Traceback (most recent call last):
  File "/builds/slave/m-cen-andrd-l10n-2/scripts/scripts/l10n/nightly-mobile-repacks.py", line 192, in <module>
    options.platform, stage_platform, mobileDirName, en_us_binary_url)
  File "/builds/slave/m-cen-andrd-l10n-2/scripts/scripts/l10n/nightly-mobile-repacks.py", line 74, in createRepacks
    raise RepackError("At least one repack failed, see above")
__main__.RepackError: At least one repack failed, see above
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(In reply to Aki Sasaki [:aki] from comment #21)
> This breaks android single locale repacks:

blassey is going to help joel look at the l10n repacks.
I have been able to repack locally and I don't see the problem.  It could be I am using the wrong mozharness config file.  I noticed that there was a problem when building without --enable-tests which has the same error.  This was fixed already:
https://bugzilla.mozilla.org/show_bug.cgi?id=722118#c0

Is there a chance that these are working now?
No, you're testing the wrong thing.
Single locale is broken.
Multilocale works.
You're testing multilocale which works.
You can't test single locale until I'm done with my script.
My script may or may not be slowed by the fact that robocop will break it.
Gave Joel instructions on how to repack single locale builds.

I think the proper fix may be:

1. back out robocop packaging
2. go back to robocop in |make build|
3. land JARSIGNER fix in mozilla-{aurora,beta,release} - bug 611648
4. releng enables JARSIGNER in |make build|
5. verify that robocop is signed properly in |make build|
In looking at the single locale repack issues with fennec and robocop, it appears that we don't do a 'make -f client.mk build', but instead try to do the 'make package' steps.  While this is fine, we never end up with our robocop stuff.

There are 2 solutions I have found:
1) add a 'ac_add_options --disable-tests' to ALL the mozconfigs used for the repacks.  This is about 12.
2) add a check for existence of the ROBOCOP_PATH in the block where we define INNER_ROBOCOP_PACKAGE.  This is the patch I have uploaded and does a simple check for the directory to exist.

Both of these solutions solve the problem, we could take one of these or maybe something else.
Attachment #593972 - Flags: feedback?(ted.mielczarek)
Attachment #593972 - Flags: feedback?(blassey.bugs)
Attachment #593976 - Flags: review?(catlee) → review+
(In reply to Joel Maher (:jmaher) from comment #26)
> There are 2 solutions I have found:
> 1) add a 'ac_add_options --disable-tests' to ALL the mozconfigs used for the
> repacks.  This is about 12.
I see that aki landed the fixed and it is live now according to the maintenance page [1].

What is next? Is this closed and we move now to bug 719443?

[1] https://wiki.mozilla.org/ReleaseEngineering:Maintenance
Comment on attachment 593972 [details] [diff] [review]
an option to fix the makefile to solve the repack issue

Review of attachment 593972 [details] [diff] [review]:
-----------------------------------------------------------------

not sure whether to f+ or f- this, so just going to clear it with the comment

::: toolkit/mozapps/installer/packager.mk
@@ +343,1 @@
>  UPLOAD_EXTRA_FILES += robocop.apk

why wouldn't it exist if test are enabled? This looks like a recipe for a hard to find failure
Attachment #593972 - Flags: feedback?(blassey.bugs)
can we close this bug?
Status: REOPENED → RESOLVED
Closed: 8 years ago8 years ago
Resolution: --- → FIXED
Attachment #593972 - Flags: feedback?(ted.mielczarek)
You need to log in before you can comment on or make changes to this bug.