Skip to content
This repository has been archived by the owner on Jan 18, 2024. It is now read-only.

[upload] Added support for tar.gz files #2504

Merged
merged 12 commits into from
Aug 27, 2020

Conversation

EvanBacon
Copy link
Contributor

@EvanBacon EvanBacon commented Aug 26, 2020

Why

  • Expo EAS produces tar.gz files that need to be extracted before they can be uploaded. Because of this expo upload:ios --url <artifact> doesn't work. Likewise expo upload:ios --path </Downloads/artifact> can also be easy to mess up.
  • Because the error that gets thrown Failed to upload the standalone app to the app store. Zip end of central directory signature not found is part of Fastlane we can't throw a more useful error in an elegant fashion.
  • Legacy code from BaseUploader to the submission service utils. This unifies the logic a bit more.

How

  • Check if the file path ends with tar.gz - this is pretty fragile.
  • Extract the tar and search its contents for an ipa, apk, or aab file.
  • Rename a destination path to have the correct extension and move the app file to that path.
  • Continue uploading using the new extracted file.
  • Refactored the export command to use tar + got instead of targz + axios - logic is also tested now.

Test Plan

  • Wrote unit tests to ensure that the tar extracting works as expecting from remote URLs or local paths.
  • Manual testing for iOS:
    • expo upload:ios --path /path/to/eas-output.tar.gz
    • expo upload:ios --url http://128.0.0.1:3000/app.tar.gz
    • expo upload:ios --path /path/to/app.ipa
    • expo upload:ios --path http://128.0.0.1:3000/app.ipa

Copy link
Contributor

@dsokal dsokal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm requesting changes mostly because of the addition of a new package for uncompressing tar archives. We already have targz installed so we should either use it or refactor Expo CLI to use the new one and remove the old one.

packages/expo-cli/package.json Show resolved Hide resolved
packages/expo-cli/src/commands/upload/BaseUploader.ts Outdated Show resolved Hide resolved
packages/expo-cli/src/commands/upload/BaseUploader.ts Outdated Show resolved Hide resolved
packages/expo-cli/src/commands/upload/utils.ts Outdated Show resolved Hide resolved
@dsokal
Copy link
Contributor

dsokal commented Aug 26, 2020

Also, heads up that this code is a legacy. We already refactored/reimplemented Android uploads so they use Submission Service. The code lives in src/commands/upload/submission-service/. I'm not saying you should move it there (or start implementing iOS uploads with Submission Service) but in my opinion, it's not worth spending too much time on improving the old legacy command that we'll eventually rewrite entirely.

@EvanBacon
Copy link
Contributor Author

Didn't take too long to implement this and I think it'll end up being valuable since I'm sure people will attempt to use it with EAS (I already did).

@dsokal dsokal requested a review from wkozyra95 August 26, 2020 09:05
@EvanBacon EvanBacon changed the title [upload:ios] Added support for tar.gz files [upload] Added support for tar.gz files Aug 26, 2020
Copy link
Contributor

@dsokal dsokal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. Please update the changelog!

packages/expo-cli/src/commands/export.ts Outdated Show resolved Hide resolved
@EvanBacon EvanBacon merged commit a107b63 into master Aug 27, 2020
@EvanBacon EvanBacon deleted the @evanbacon/upload/support-eas-urls branch August 27, 2020 21:30
async function createTemporaryDirectoryForExtractionAsync(): Promise<string> {
// Since we may need to rename the destination path,
// add everything to a folder which can be nuked to ensure we don't accidentally use an old build with the same name.
const destinationFolder = join(os.tmpdir(), 'expo-submission-service');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@EvanBacon with this change, only one single process can upload one build artefact at a time, since otherwise the file will be deleted from under that process. E.g. trying to run expo upload:ios and expo upload:android in parallel will have one of them fail with either:

Error setting value '/tmp/expo-submission-service/foo-xxxxxx-signed.aab' for option 'aab'
Could not find aab file at path '/tmp/expo-submission-service/foo-xxxxxx-signed.aab'

or:

Failed to upload the standalone app to the app store.
Could not find file at path '/tmp/expo-submission-service/foo-xxxxxx-archive.ipa'

We have build scripts that submits several builds at the same time, and this is currently breaking our workflow. Would it be possible to use a new folder each time, instead of always using expo-submission-service?

I'll send a pull request with that change, but I'm also happy to consider any other approach to solve this 👍

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants