Bug 1807317 Comment 4 Edit History

Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.

I agree with :jcristau. We have zipaligned APKs since bug 1335370 which was 6 years ago. 

> I determined that autograph (our signing server) is not zipaligning fenix APKs and 
I'm not sure how this was determined but that doesn't sound correct to me. Maybe there was a regression in 2021, but as :jcristau said, we're calling `zipalign` these days. Moreover, like said in bug 1335370 comment 0, Google Play refuses APKs that aren't signed. Therefore, this potential regression in 2021 would have to match another regression on Google Play's end. 

If people remember how things were in 2021, I'm happy to discuss details. In the meantime and based on the facts above, I assume there was an error in the original bug report. 

> discussion in #releaseduty-mobile indicates we may not be zipaligning before sending the APK to autograph for signing

This is actually on purpose. Signing APKs with the v1 scheme[1] actually injects some files in the APK. An APK file is a actually a zip archive. Adding files to a zip changes the archive. Zipaligning is actually a step that modifies the archive for performance reasons[2]. Therefore, there is no point of optimizing the archive before adding the last missing files. That's why we don't `zipalign` files before sending them for signing. 

> One thing that we might want to change is we don't currently seem to pass -p to zipalign, which https://developer.android.com/studio/command-line/zipalign.html recommends to page-align .so files.

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 to pass the `-p` flag.

[1] https://source.android.com/docs/security/features/apksigning#v1
[2] https://developer.android.com/studio/command-line/zipalign.html
[3] https://github.com/mozilla-releng/scriptworker-scripts/blob/df61f036ab5fddf56c7a34e4516db8cd2a7dde9d/signingscript/src/signingscript/sign.py#L667
I agree with :jcristau. We have zipaligned APKs since bug 1335370 which was 6 years ago. 

> I determined that autograph (our signing server) is not zipaligning fenix APKs and 
I'm not sure how this was determined but that doesn't sound correct to me. Maybe there was a regression in 2021, but as :jcristau said, we're calling `zipalign` these days. Moreover, like said in bug 1335370 comment 0, Google Play refuses APKs that aren't signed. Therefore, this potential regression in 2021 would have to match another regression on Google Play's end. 

If people remember how things were in 2021, I'm happy to discuss details. In the meantime and based on the facts above, I assume there was an error in the original bug report. 

> discussion in #releaseduty-mobile indicates we may not be zipaligning before sending the APK to autograph for signing

This is actually on purpose. Signing APKs with the v1 scheme[1] actually injects some files in the APK. An APK file is a actually a zip archive. Adding files to a zip changes the archive. Zipaligning is actually a step that modifies the archive for performance reasons[2]. Therefore, there is no point of optimizing the archive before adding the last missing files. That's why we don't `zipalign` files before sending them for signing. 

> One thing that we might want to change is we don't currently seem to pass -p to zipalign, which https://developer.android.com/studio/command-line/zipalign.html recommends to page-align .so files.

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.

[1] https://source.android.com/docs/security/features/apksigning#v1
[2] https://developer.android.com/studio/command-line/zipalign.html
[3] https://github.com/mozilla-releng/scriptworker-scripts/blob/df61f036ab5fddf56c7a34e4516db8cd2a7dde9d/signingscript/src/signingscript/sign.py#L667

Back to Bug 1807317 Comment 4