feat: Add code signing for Android release artefacts#2724
feat: Add code signing for Android release artefacts#2724JohananOppongAmoateng wants to merge 9 commits intobeeware:mainfrom
Conversation
freakboy3742
left a comment
There was a problem hiding this comment.
Thanks for the PR. I've given a partial review; the general shape of things looks good, but there's some reorganisation that will be required, and I'm not 100% certain about some of the details in the implementation (specifically, if the -P arguments you're referring to are standard features, or something we'll need to accomodate).
I haven't done a full review of the test cases yet, mostly because they'll see a bunch of changes from the reorganisation.
|
|
||
| * The project root directory | ||
| * The project's `.android/` subfolder | ||
| * `~/.android/` |
There was a problem hiding this comment.
~ is UNIX-specific terminology - something like:
| * `~/.android/` | |
| * The `.android/` folder in the user's home directory |
would be preferred here.
| > [!IMPORTANT] | ||
| > Keep your keystore file secure and backed up. If you lose it, you will not be able to publish updates to your app on the Play Store. |
There was a problem hiding this comment.
This isn't the syntax we use for callouts. See the admonitions and notes section of our style guide.
| $ briefcase package android --identity /path/to/my-release-key.jks | ||
| ``` | ||
|
|
||
| You can also supply the alias and passwords non-interactively: |
There was a problem hiding this comment.
This is the first mention of alias and passwords; this might need some clarification:
| You can also supply the alias and passwords non-interactively: | |
| If you are using a keystore that has been created with an alias or password, you can also supply those properties at the command line: |
Also - what's the behavior if you interactively select an keystore from a list, but that keystore requires an alias or password?
| def _keytool(self) -> Path: | ||
| """Path to the keytool executable bundled with the JDK.""" | ||
| ext = ".exe" if self.tools.host_os == "Windows" else "" | ||
| return self.tools.java.java_home / "bin" / f"keytool{ext}" |
There was a problem hiding this comment.
These keytool calls seem out of place here. They should be part of the android_sdk integrations. (Although the signing tools ship with java, it's very much tied to android signing, so we might as well keep it there).
I'd suggest pulling these tools into a standalone object (a separate signing namespace) so android_sdk.signing.create_keystore() is how you create the initial keystore, etc.
| f"-Pandroid.injected.signing.store.file={signing_config.keystore_path!s}", | ||
| f"-Pandroid.injected.signing.store.password={signing_config.store_password}", | ||
| f"-Pandroid.injected.signing.key.alias={signing_config.alias}", | ||
| f"-Pandroid.injected.signing.key.password={signing_config.key_password}", |
There was a problem hiding this comment.
Are these standard configuration points, or do we need to modify the Android gradle template to support these?
There was a problem hiding this comment.
These work with the android gradle plugin so we dont need to modify the gradle template
| "debug-apk": ("assembleDebug", "apk/debug/app-debug.apk"), | ||
| }[app.packaging_format] | ||
|
|
||
| build_type, build_artefact_path = { |
There was a problem hiding this comment.
Why have you removed the wait_bar usage here?
|
@freakboy3742 i will address all your concerns later bogged down with work now |
Add code signing for Android release artefacts Closes #1268
PR Checklist: