cli icon indicating copy to clipboard operation
cli copied to clipboard

refactor: unify APK building and installing process

Open krizzu opened this issue 4 years ago • 21 comments

Summary:

Hey!

While working on #1038, I've noticed that there are two different ways to build and install APK: one with --deviceId provided and one without. This PR unifies that process.

What has changed

  • Moved all methods related to APK build and install to buildAndInstallApk.ts (including utility functions).

  • Removed old and unused code.

  • Exported two new functions :

    • buildApk - builds APK using provided variants.
    • installApk - checks if APK has been build for specified variant. If it is, use adb to install. Otherwise use gradle's install* task to build and install.

To Do:

  • [x] cleanup
  • [x] tests

Test Plan:

Green CI.

krizzu avatar Mar 17 '20 11:03 krizzu

How's it going, do you need any help here?

thymikee avatar Mar 19 '20 10:03 thymikee

Hey, working on Jest tests and manual testing too - should be here soonish

krizzu avatar Mar 19 '20 10:03 krizzu

This one is ready for some reviews, thanks!

krizzu avatar Mar 23 '20 16:03 krizzu

@thymikee @Esemesek Any idea why those doctor e2e tests fails?

krizzu avatar Apr 20 '20 07:04 krizzu

Azure fails with "EPERM: operation not permitted, mkdir 'D:'" when we try to create a temporary directory for testing, maybe something changed there? Sounds like Azure changing something, would be good to verify this in other PRs

thymikee avatar Apr 20 '20 08:04 thymikee

I think this test fails randomly. We consider Windows failures as non-blocking since most of them are just flakiness in the Azure environment.

Esemesek avatar Apr 20 '20 13:04 Esemesek

Do you feel like giving this one a go @thymikee @Esemesek before shipping? Since it's ready and failures are non-related, I'd love to have a final approval from your side.

grabbou avatar May 26 '20 20:05 grabbou

Since this code is ready to go and this is our second attempt and rewriting this relatively major part of run-android, I am going to take it over and do the last round of testing myself.

I am hoping we can ship it next week, it should resolve some really annoying issues.

grabbou avatar Sep 17 '20 13:09 grabbou

@thymikee: i have rebased the branch of this pr with the master into this branch https://github.com/tokyodrift1993/react-native-community-cli/tree/feature/buildApkFixes

driiftkiing avatar Jun 30 '21 11:06 driiftkiing

@driiftkiing can you send a PR amending this one? :)

thymikee avatar Jun 30 '21 12:06 thymikee

@driiftkiing can you send a PR amending this one? :)

just creating a new one? or something else?

driiftkiing avatar Jun 30 '21 12:06 driiftkiing

Yup, a new one. Unless @Krizzu is still interested in pursuing this, but let's assume he's not and make sure to attribute him for the work done when merging

thymikee avatar Jun 30 '21 14:06 thymikee

@driiftkiing @thymikee Been a while 🤖 I just rebased the code, tests are passing. I'm yet to test this, making sure that it works properly.

krizzu avatar Jun 30 '21 17:06 krizzu

Ran some tests on custom android project, trying different configuration of build variants (with and without flavors) and it works. Would love to have someone else do the sanity check as well 🙏

krizzu avatar Jun 30 '21 19:06 krizzu

Thanks @Krizzu, I am going to do another pass at it this week and ship it!

grabbou avatar Jul 06 '21 13:07 grabbou

I will do also some tests, if i got the time

driiftkiing avatar Jul 07 '21 12:07 driiftkiing

Thanks for the review @andriytsaryov. @grabbou I guess we can finally go ahead and merge it

thymikee avatar Jul 12 '21 12:07 thymikee

Yes, that's on my todo list for tomorrow. When ready, I will create a new release.

grabbou avatar Jul 15 '21 14:07 grabbou

I'd love to be able to run

npx react-native run-android --variant=developmentDebug --deviceId 931X1X9JW

and have it work. This PR seems to solve that. Anything holding this up getting merged in?

pas256 avatar Feb 04 '22 05:02 pas256

No, @pas256, this PR will be pulled in soon, once we merge a PR removing link and unlink.

grabbou avatar Feb 04 '22 16:02 grabbou

if anyone is interested to use this PR until it is merged:

  • created patch: https://gist.github.com/driiftkiing/09f3bd7bb125e6436a5caba7834f3a99
    • used for yarn v3
    • can be used like:
    diff --git a/package.json b/package.json
    --- a/package.json
    +++ b/package.json
       "resolutions": {
    +    "@react-native-community/cli-platform-android": "patch:@react-native-community/cli-platform-android@npm:6.3.0#.yarn/patches/@react-native-community-cli-platform-android-npm-6.3.0-1b9d17ef3a",
       },
    
  • source: https://github.com/tokyodrift1993/react-native-community-cli/tree/krizzu/buildApkFixes--merged-v6.3.0
  • merged v6.3.0
  • (fixed lint & tests)
  • should be compatible with [email protected]


command to run:

yarn react-native run-android --variant=developmentDebug

driiftkiing avatar Feb 07 '22 11:02 driiftkiing

There hasn't been any activity on this pull request in the past 3 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 7 days.

github-actions[bot] avatar Nov 28 '22 03:11 github-actions[bot]

#1739 closes this one

TMisiukiewicz avatar Nov 28 '22 12:11 TMisiukiewicz

🫡

krizzu avatar Dec 06 '22 15:12 krizzu