Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

[updates] Fix app.manifest occasionally missing on android #19731

Conversation

Kudo
Copy link
Contributor

@Kudo Kudo commented Oct 27, 2022

Why

there are some reported issues that the app.manifest is missing from output builds.
close ENG-2610

How

i cannot reliably reproduce the problem and the hypothesis is about building task sequence. the pr tries changing the task dependency to mergeReleaseResources which would be earlier than processReleaseResources. this is also aligned with react-native implementation.

Test Plan

updates e2e ci passed (based on #19730)

Checklist

@linear
Copy link

linear bot commented Oct 27, 2022

ENG-2610 Investigate expo-updates manifest not getting embedded in Android app with a lot of assets

A user reported that their Android app built with EAS Build was crashing due to the app.manifest ending up missing from the final APK. I added some logging to our createManifest.js script and it looks like we do successfully write app.manifest (see line 463 and onwards in gradlew phase). So it is somewhere after writing the file to android/app/build/generated/assets/expo-updates/release/app.manifest where this falls apart - possibly related to the [intermediates](https://github.com/expo/expo/blob/fd06ac0fc2296b3441d1e48896fded19883d8a43/packages/expo-updates/scripts/create-manifest-android.gradle#L66-L71) directory (@eric pointed out he wasn't super confident in this approach so it may be worth digging into more).

Example repo: https://gitlab.epsteindidntcommitsuici.de/eguillaumin/catamaps/-/tree/0c153891cad36928726eb73c3fa40c6efeb08eb0

I wasn't able to reproduce this on my machine, only on EAS Build. So the repro steps are to clone the exact commit provided above and then run eas build -p android --profile preview and inspect the resulting APK (apktool d -s file.apk -o outdir) - notice that app.manifest is missing from assets.

Note: the user was able to resolve this issue by compressing png to webp, so it does seem related to the size of assets. You can also verify that excluding the source by replacing App.js with some basic "Hello, world!" example (thus preventing importing the assets) will resolve the issue.

@Kudo Kudo marked this pull request as ready for review October 27, 2022 18:05
@Kudo Kudo removed the request for review from esamelson October 27, 2022 18:05
@brentvatne
Copy link
Member

@douglowder - what do you think about this? can we land it for sdk 47? we've seen a handful of reports about this recently so it would be nice to land this if possible

@douglowder
Copy link
Contributor

@brentvatne yes this seems safe to land.

@brentvatne brentvatne merged commit 8286c4c into @kudo/fix-update-e2e Oct 28, 2022
@brentvatne brentvatne deleted the @kudo/eng-2610-investigate-expo-updates-manifest-not branch October 28, 2022 01:46
Kudo added a commit that referenced this pull request Nov 9, 2022
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants