-
Notifications
You must be signed in to change notification settings - Fork 39
feat: allow adhoc for android #640
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: allow adhoc for android #640
Conversation
|
@LukasMod is attempting to deploy a commit to the Callstack Team on Vercel. A member of the Team first needs to authorize it. |
thymikee
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! would love to see a video of a working demo :)
ce89ae9 to
f571064
Compare
|
I tested with fork LukasMod#1 (same as this one + dist and some changes in package.json to make it executable from project after referencing it in package.json). Test command: terminal logs: android.adhoc.mp4Additionally I updated link to rock docs in index template (old one: https://rockjs.dev/docs/cli#ad-hoc-distribution). I used private bucket for tests only (bucket with public access), its already removed to avoid extra costs. |
|
Here is example with expensify app: Expensify/App#76061 (comment)
|
| const nodeApk = await import('node-apk'); | ||
| const { Apk } = nodeApk.default || nodeApk; | ||
| const apk = new Apk(binaryPath); | ||
| const manifest = await apk.getManifestInfo(); | ||
| apk.close(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
node-apk adds a lot of install size: https://packagephobia.com/result?p=node-apk. Can we instead use zip command to get the manifest? similar to how we use it in re-sign
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we could use fast-xml-parser (one we use in apple platform) to parse the manifest
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will test aapt (locally works). zip + fast-xml-parser was not working because AndroidManifest is binary in this case.
thymikee
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! Thanks for contributing that. I left a comment to consider, but no biggie
03664c8 to
111ffb3
Compare
| try { | ||
| const aaptPath = findAapt(); | ||
|
|
||
| const output = execFileSync(aaptPath, ['dump', 'badging', binaryPath], { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we have spawn for running commands, let's use that
thymikee
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚀
No description provided.