(In reply to Johan Lorenzo [:jlorenzo] from comment #4) > Great call! While the rest of the bug is invalid I think we can still improve this part. This is a `good-first-bug`. I'm happy to mentor it. It should just be a matter of changing this line[3] to pass the `-p` flag. > > [...] > [3] https://github.com/mozilla-releng/scriptworker-scripts/blob/df61f036ab5fddf56c7a34e4516db8cd2a7dde9d/signingscript/src/signingscript/sign.py#L667 I'm sorry. I took a closer look at this code and I just realized I was wrong. `zip_align_apk()` is now dead code. We've let autograph handle zipalignment since bug 1618531[1]. Autograph does so by shelling out to Google's `apksigner`[2]. It's not crystal clear per the Android documentation but apksigner does take care of zipaligning APKs[3]: > Caution: If you sign your APK using apksigner and make further changes to the APK, the APK's signature is invalidated. If you use zipalign to align your APK, use it before signing the APK. I checked locally with the latest Fenix Nightly and even `.so` files are already zipaligned: ``` $ zipalign -c -p -v 4 ~/Downloads/target.apk Verifying alignment of /Users/johanlorenzo/Downloads/target(1).apk (4)... 87 META-INF/com/android/build/gradle/app-metadata.properties (OK - compressed) 196 assets/dexopt/baseline.prof (OK) 4420 assets/dexopt/baseline.profm (OK) 4776 classes.dex (OK - compressed) 4256229 classes2.dex (OK - compressed) 6516802 lib/arm64-v8a/libfreebl3.so (OK - compressed) 6766328 lib/arm64-v8a/libipcclientcerts.so (OK - compressed) 6940951 lib/arm64-v8a/libjnidispatch.so (OK - compressed) 6983977 lib/arm64-v8a/liblgpllibs.so (OK - compressed) 6996122 lib/arm64-v8a/libmegazord.so (OK - compressed) 10782708 lib/arm64-v8a/libmozavcodec.so (OK - compressed) 10873254 lib/arm64-v8a/libmozavutil.so (OK - compressed) 10955651 lib/arm64-v8a/libmozglue.so (OK - compressed) 11452530 lib/arm64-v8a/libnss3.so (OK - compressed) 12447413 lib/arm64-v8a/libnssckbi.so (OK - compressed) 12736306 lib/arm64-v8a/libplugin-container.so (OK - compressed) 12802000 lib/arm64-v8a/libsentry-android.so (OK - compressed) 12807202 lib/arm64-v8a/libsentry.so (OK - compressed) 13184537 lib/arm64-v8a/libsoftokn3.so (OK - compressed) 13292843 lib/arm64-v8a/libxul.so (OK - compressed) [...] ``` Therefore, there's no action needed to zipalign `.so` files. I'm sorry for leading you to write a patch, :royang! I had forgotten about this until today. That said, there's one more thing to do on the RelEng side: we should get rid of this dead code. I'll file a follow-up bug for it. [1] https://github.com/mozilla-releng/scriptworker-scripts/commit/8155d6299a3917be35a02eda81ab239705f9a497 [2] https://github.com/mozilla-services/autograph/blob/0b2dcedd1738720c8642a3fd65b2793991c3709a/signer/apk2/apk2.go#L148 [3] https://developer.android.com/tools/apksigner
Bug 1807317 Comment 6 Edit History
Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.
(In reply to Johan Lorenzo [:jlorenzo] from comment #4) > Great call! While the rest of the bug is invalid I think we can still improve this part. This is a `good-first-bug`. I'm happy to mentor it. It should just be a matter of changing this line[3] to pass the `-p` flag. > > [...] > [3] https://github.com/mozilla-releng/scriptworker-scripts/blob/df61f036ab5fddf56c7a34e4516db8cd2a7dde9d/signingscript/src/signingscript/sign.py#L667 I'm sorry. I took a closer look at this code and I just realized I was wrong. `zip_align_apk()` is now dead code. We've let autograph handle zipalignment since bug 1618531[1]. Autograph does so by shelling out to Google's `apksigner`[2]. It's not crystal clear per the Android documentation but apksigner does take care of zipaligning APKs[3]: > Caution: If you sign your APK using apksigner and make further changes to the APK, the APK's signature is invalidated. If you use zipalign to align your APK, use it before signing the APK. I checked locally with the latest Fenix Nightly and even `.so` files are already zipaligned: ``` $ zipalign -c -p -v 4 ~/Downloads/target.apk Verifying alignment of /Users/johanlorenzo/Downloads/target(1).apk (4)... 87 META-INF/com/android/build/gradle/app-metadata.properties (OK - compressed) 196 assets/dexopt/baseline.prof (OK) 4420 assets/dexopt/baseline.profm (OK) 4776 classes.dex (OK - compressed) 4256229 classes2.dex (OK - compressed) 6516802 lib/arm64-v8a/libfreebl3.so (OK - compressed) 6766328 lib/arm64-v8a/libipcclientcerts.so (OK - compressed) 6940951 lib/arm64-v8a/libjnidispatch.so (OK - compressed) 6983977 lib/arm64-v8a/liblgpllibs.so (OK - compressed) 6996122 lib/arm64-v8a/libmegazord.so (OK - compressed) 10782708 lib/arm64-v8a/libmozavcodec.so (OK - compressed) 10873254 lib/arm64-v8a/libmozavutil.so (OK - compressed) 10955651 lib/arm64-v8a/libmozglue.so (OK - compressed) 11452530 lib/arm64-v8a/libnss3.so (OK - compressed) 12447413 lib/arm64-v8a/libnssckbi.so (OK - compressed) 12736306 lib/arm64-v8a/libplugin-container.so (OK - compressed) 12802000 lib/arm64-v8a/libsentry-android.so (OK - compressed) 12807202 lib/arm64-v8a/libsentry.so (OK - compressed) 13184537 lib/arm64-v8a/libsoftokn3.so (OK - compressed) 13292843 lib/arm64-v8a/libxul.so (OK - compressed) [...] ``` Therefore, there's no action needed to zipalign `.so` files. I'm sorry for leading you to write a patch, :royang! I had forgotten about this until today. That said, there's one more thing to do on the RelEng side: we should get rid of this dead code. I filed bug 1828876 for it. [1] https://github.com/mozilla-releng/scriptworker-scripts/commit/8155d6299a3917be35a02eda81ab239705f9a497 [2] https://github.com/mozilla-services/autograph/blob/0b2dcedd1738720c8642a3fd65b2793991c3709a/signer/apk2/apk2.go#L148 [3] https://developer.android.com/tools/apksigner