[native] Clean up `detect_abis.sh`
ClosedPublic
Actions

Authored by • abosh on Aug 16 2022, 1:02 PM.

Details

Reviewers

atul
• jon
• karol

Commits

rCOMMb836238ee3e1: [native] Clean up `detect_abis.sh`

Summary

Relevant Linear issue here. D4717 was my original code to fix this, but it ended up breaking yarn react-native run-android.

Turns out the problematic code was writing if [[ ! $found ]] instead of if [[ "$found" = false ]] since in bash ! returns true for any nonempty string (and found is nonempty even when it's equal to false). Once I changed that, yarn react-native run-android builds successfully.

Test Plan

Along with ShellCheck, ran yarn react-native run-android successfully.

Diff Detail

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

• abosh created this revision.Aug 16 2022, 1:02 PM

Herald added subscribers: • adrian, tomek, ashoat. · View Herald TranscriptAug 16 2022, 1:02 PM

• abosh added inline comments.Aug 16 2022, 1:05 PM

native/android/app/bash/detect_abis.sh
12–17 ↗	(On Diff #15680)	This code loops through `ABIS` to see if `ABI` is in there already, setting `found` to `true` if it's found.
19–20 ↗	(On Diff #15680)	This is where the code was breaking `yarn react-native run-android` before. Now that `found` is compared to the string `false` instead of using `[[ ! $found ]]`, `ABI` is only added to `ABIS` if it wasn't found in there before.

Harbormaster completed remote builds in B11398: Diff 15680.Aug 16 2022, 1:12 PM

• abosh requested review of this revision.Aug 16 2022, 1:12 PM

Would prefer to use associative arrays, but can't guarantee bash4+ is available until after move to nix dev

native/android/app/bash/detect_abis.sh
19–20 ↗	(On Diff #15680)	You should always quote a value when testing, as if it's unset, then it will likely not be what you intended `[[ -z "$found" ]];`