From 49d875b674f8e634ee5b8820cb869669bdd0a95b Mon Sep 17 00:00:00 2001 From: rory Date: Thu, 27 Jun 2024 18:23:18 -0700 Subject: [PATCH 01/13] Add verifyPatches PR check --- .github/workflows/verifyPatches.yml | 25 +++++++++++++++++++++++++ scripts/verifyPatches.sh | 29 +++++++++++++++++++++++++++++ 2 files changed, 54 insertions(+) create mode 100644 .github/workflows/verifyPatches.yml create mode 100755 scripts/verifyPatches.sh diff --git a/.github/workflows/verifyPatches.yml b/.github/workflows/verifyPatches.yml new file mode 100644 index 000000000000..3fd8f6e406c0 --- /dev/null +++ b/.github/workflows/verifyPatches.yml @@ -0,0 +1,25 @@ +name: Verify patches + +on: + pull_request: + types: [opened, synchronize] + branches: [main] + paths: ['patches', 'package.json', 'package-lock.json'] + +concurrency: + group: verify-patches-${{ format('{0}-{1}', github.ref, github.sha) || github.ref }} + cancel-in-progress: true + +jobs: + verifyPatches: + runs-on: ubuntu-latest + name: Verify patches + steps: + - name: Checkout + uses: actions/checkout@v4 + + - name: Setup Node + uses: ./.github/actions/composite/setupNode + + - name: Verify patches + run: ./scripts/verifyPatches.sh diff --git a/scripts/verifyPatches.sh b/scripts/verifyPatches.sh new file mode 100755 index 000000000000..dbc0eeb293c7 --- /dev/null +++ b/scripts/verifyPatches.sh @@ -0,0 +1,29 @@ +#!/bin/bash + +SCRIPTS_DIR=$(dirname "${BASH_SOURCE[0]}") +source "$SCRIPTS_DIR/shellUtils.sh" + +# Run patch-package and capture its output and exit code +OUTPUT="$(npx patch-package --error-on-fail 2>&1)" +EXIT_CODE=$? + +# Check if the output contains the warning message +echo "$OUTPUT" | grep -q "patch-package detected a patch file version mismatch" +WARNING_FOUND=$? + +# Determine the final exit code +if [ $EXIT_CODE -eq 0 ]; then + if [ $WARNING_FOUND -eq 0 ]; then + # patch-package succeeded but warning was found + error "It looks like you upgraded a dependency without upgrading the patch. Please review the patch, determine if it's still needed, and port it to the new version of the dependency." + exit 1 + else + # patch-package succeeded and no warning was found + success "patch-package succeeded without errors or warnings" + exit 0 + fi +else + # patch-package failed + error "patch-package failed to apply a patch" + exit $EXIT_CODE +fi From 5187fe6b251fc7a694dfeb9e8d64b89b5f34024f Mon Sep 17 00:00:00 2001 From: rory Date: Thu, 27 Jun 2024 18:44:20 -0700 Subject: [PATCH 02/13] Preserve output --- scripts/verifyPatches.sh | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/scripts/verifyPatches.sh b/scripts/verifyPatches.sh index dbc0eeb293c7..d3b1748b3bbe 100755 --- a/scripts/verifyPatches.sh +++ b/scripts/verifyPatches.sh @@ -4,7 +4,10 @@ SCRIPTS_DIR=$(dirname "${BASH_SOURCE[0]}") source "$SCRIPTS_DIR/shellUtils.sh" # Run patch-package and capture its output and exit code -OUTPUT="$(npx patch-package --error-on-fail 2>&1)" +TEMP_OUTPUT="$(mktemp)" +npx patch-package --error-on-fail 2>&1 | tee "$TEMP_OUTPUT" +OUTPUT="$(cat "$TEMP_OUTPUT")" +rm -f "$TEMP_OUTPUT" EXIT_CODE=$? # Check if the output contains the warning message From e37a91d26dbb65ca7118c2f4914fc5f0a56a90d3 Mon Sep 17 00:00:00 2001 From: rory Date: Thu, 27 Jun 2024 19:03:04 -0700 Subject: [PATCH 03/13] Update version number in react-native-live-markdown patch --- ...5.patch => @expensify+react-native-live-markdown+0.1.88.patch} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename patches/{@expensify+react-native-live-markdown+0.1.85.patch => @expensify+react-native-live-markdown+0.1.88.patch} (100%) diff --git a/patches/@expensify+react-native-live-markdown+0.1.85.patch b/patches/@expensify+react-native-live-markdown+0.1.88.patch similarity index 100% rename from patches/@expensify+react-native-live-markdown+0.1.85.patch rename to patches/@expensify+react-native-live-markdown+0.1.88.patch From bf2b11b0dd7a78099157648027a0b8ccd9032b56 Mon Sep 17 00:00:00 2001 From: rory Date: Thu, 27 Jun 2024 19:04:42 -0700 Subject: [PATCH 04/13] Fix funky patch name in react-native-keyboard-controller --- ....patch.patch => react-native-keyboard-controller+1.12.2.patch} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename patches/{react-native-keyboard-controller+1.12.2.patch.patch => react-native-keyboard-controller+1.12.2.patch} (100%) diff --git a/patches/react-native-keyboard-controller+1.12.2.patch.patch b/patches/react-native-keyboard-controller+1.12.2.patch similarity index 100% rename from patches/react-native-keyboard-controller+1.12.2.patch.patch rename to patches/react-native-keyboard-controller+1.12.2.patch From 1845b09d10a3c96bbfba09bca7324db2affe8207 Mon Sep 17 00:00:00 2001 From: rory Date: Thu, 27 Jun 2024 19:08:40 -0700 Subject: [PATCH 05/13] Correctly preserve exit status using PIPESTATUS --- scripts/verifyPatches.sh | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/scripts/verifyPatches.sh b/scripts/verifyPatches.sh index d3b1748b3bbe..32990e62a960 100755 --- a/scripts/verifyPatches.sh +++ b/scripts/verifyPatches.sh @@ -6,16 +6,16 @@ source "$SCRIPTS_DIR/shellUtils.sh" # Run patch-package and capture its output and exit code TEMP_OUTPUT="$(mktemp)" npx patch-package --error-on-fail 2>&1 | tee "$TEMP_OUTPUT" +EXIT_CODE=${PIPESTATUS[0]} OUTPUT="$(cat "$TEMP_OUTPUT")" rm -f "$TEMP_OUTPUT" -EXIT_CODE=$? # Check if the output contains the warning message echo "$OUTPUT" | grep -q "patch-package detected a patch file version mismatch" WARNING_FOUND=$? # Determine the final exit code -if [ $EXIT_CODE -eq 0 ]; then +if [ "$EXIT_CODE" -eq 0 ]; then if [ $WARNING_FOUND -eq 0 ]; then # patch-package succeeded but warning was found error "It looks like you upgraded a dependency without upgrading the patch. Please review the patch, determine if it's still needed, and port it to the new version of the dependency." @@ -28,5 +28,5 @@ if [ $EXIT_CODE -eq 0 ]; then else # patch-package failed error "patch-package failed to apply a patch" - exit $EXIT_CODE + exit "$EXIT_CODE" fi From 7b9ceefb246921a7b7a5b391a8e34976c4c1e208 Mon Sep 17 00:00:00 2001 From: rory Date: Fri, 28 Jun 2024 08:56:24 -0700 Subject: [PATCH 06/13] Preserve colorization in output --- scripts/verifyPatches.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/verifyPatches.sh b/scripts/verifyPatches.sh index 32990e62a960..9665d31b7b99 100755 --- a/scripts/verifyPatches.sh +++ b/scripts/verifyPatches.sh @@ -5,7 +5,7 @@ source "$SCRIPTS_DIR/shellUtils.sh" # Run patch-package and capture its output and exit code TEMP_OUTPUT="$(mktemp)" -npx patch-package --error-on-fail 2>&1 | tee "$TEMP_OUTPUT" +script -q /dev/null npx patch-package --error-on-fail 2>&1 | tee "$TEMP_OUTPUT" EXIT_CODE=${PIPESTATUS[0]} OUTPUT="$(cat "$TEMP_OUTPUT")" rm -f "$TEMP_OUTPUT" From 526f18f46c2cca27374812022a1ca10f76cffa94 Mon Sep 17 00:00:00 2001 From: rory Date: Fri, 28 Jun 2024 09:04:07 -0700 Subject: [PATCH 07/13] Rename script to applyPatches and remove the PR check --- .github/workflows/verifyPatches.yml | 25 ------------------- scripts/{verifyPatches.sh => applyPatches.sh} | 0 2 files changed, 25 deletions(-) delete mode 100644 .github/workflows/verifyPatches.yml rename scripts/{verifyPatches.sh => applyPatches.sh} (100%) diff --git a/.github/workflows/verifyPatches.yml b/.github/workflows/verifyPatches.yml deleted file mode 100644 index 3fd8f6e406c0..000000000000 --- a/.github/workflows/verifyPatches.yml +++ /dev/null @@ -1,25 +0,0 @@ -name: Verify patches - -on: - pull_request: - types: [opened, synchronize] - branches: [main] - paths: ['patches', 'package.json', 'package-lock.json'] - -concurrency: - group: verify-patches-${{ format('{0}-{1}', github.ref, github.sha) || github.ref }} - cancel-in-progress: true - -jobs: - verifyPatches: - runs-on: ubuntu-latest - name: Verify patches - steps: - - name: Checkout - uses: actions/checkout@v4 - - - name: Setup Node - uses: ./.github/actions/composite/setupNode - - - name: Verify patches - run: ./scripts/verifyPatches.sh diff --git a/scripts/verifyPatches.sh b/scripts/applyPatches.sh similarity index 100% rename from scripts/verifyPatches.sh rename to scripts/applyPatches.sh From 502b3807c190818ed084e95bc88eace53cd81e74 Mon Sep 17 00:00:00 2001 From: rory Date: Fri, 28 Jun 2024 09:04:50 -0700 Subject: [PATCH 08/13] Check for a more generic warning message --- scripts/applyPatches.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts/applyPatches.sh b/scripts/applyPatches.sh index 9665d31b7b99..6c14e6a339c0 100755 --- a/scripts/applyPatches.sh +++ b/scripts/applyPatches.sh @@ -10,8 +10,8 @@ EXIT_CODE=${PIPESTATUS[0]} OUTPUT="$(cat "$TEMP_OUTPUT")" rm -f "$TEMP_OUTPUT" -# Check if the output contains the warning message -echo "$OUTPUT" | grep -q "patch-package detected a patch file version mismatch" +# Check if the output contains a warning message +echo "$OUTPUT" | grep -q "Warning:" WARNING_FOUND=$? # Determine the final exit code From 54ebd8b9adea7ea74ea012ee938678ad38e733b7 Mon Sep 17 00:00:00 2001 From: rory Date: Fri, 28 Jun 2024 09:04:59 -0700 Subject: [PATCH 09/13] Improve comments --- scripts/applyPatches.sh | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/scripts/applyPatches.sh b/scripts/applyPatches.sh index 6c14e6a339c0..b9ff6c9e1b19 100755 --- a/scripts/applyPatches.sh +++ b/scripts/applyPatches.sh @@ -1,9 +1,14 @@ #!/bin/bash +# This script is a simple wrapper around patch-package that fails if any errors or warnings are detected. +# This is useful because patch-package does not fail on errors or warnings by default, +# which means that broken patches are easy to miss, and leads to developer frustration and wasted time. + SCRIPTS_DIR=$(dirname "${BASH_SOURCE[0]}") source "$SCRIPTS_DIR/shellUtils.sh" -# Run patch-package and capture its output and exit code +# Run patch-package and capture its output and exit code, while still displaying the original output to the terminal +# (we use `script -q /dev/null` to preserve colorization in the output) TEMP_OUTPUT="$(mktemp)" script -q /dev/null npx patch-package --error-on-fail 2>&1 | tee "$TEMP_OUTPUT" EXIT_CODE=${PIPESTATUS[0]} From 197d68a33502b435f77554a131edacdd791eb70d Mon Sep 17 00:00:00 2001 From: rory Date: Fri, 28 Jun 2024 09:06:29 -0700 Subject: [PATCH 10/13] Run applyPatches in postInstall --- scripts/postInstall.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts/postInstall.sh b/scripts/postInstall.sh index 339fdf25cb10..dbe107e26623 100755 --- a/scripts/postInstall.sh +++ b/scripts/postInstall.sh @@ -4,8 +4,8 @@ ROOT_DIR=$(dirname "$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")" &> /dev/null && pwd)") cd "$ROOT_DIR" || exit 1 -# Run patch-package -npx patch-package +# Apply packages using patch-package +scripts/applyPatches.sh # Install node_modules in subpackages, unless we're in a CI/CD environment, # where the node_modules for subpackages are cached separately. From cd6b97de768e49ce2d43d4dfe10ebb0d4d6999ec Mon Sep 17 00:00:00 2001 From: rory Date: Fri, 28 Jun 2024 09:08:43 -0700 Subject: [PATCH 11/13] Pet peeve: add newline to output --- scripts/applyPatches.sh | 2 ++ 1 file changed, 2 insertions(+) diff --git a/scripts/applyPatches.sh b/scripts/applyPatches.sh index b9ff6c9e1b19..31e1b7f50e6a 100755 --- a/scripts/applyPatches.sh +++ b/scripts/applyPatches.sh @@ -19,6 +19,8 @@ rm -f "$TEMP_OUTPUT" echo "$OUTPUT" | grep -q "Warning:" WARNING_FOUND=$? +printf "\n"; + # Determine the final exit code if [ "$EXIT_CODE" -eq 0 ]; then if [ $WARNING_FOUND -eq 0 ]; then From debcf7a6591f99f0702ee86e5a45df6d5e695db5 Mon Sep 17 00:00:00 2001 From: rory Date: Fri, 28 Jun 2024 09:50:25 -0700 Subject: [PATCH 12/13] Fail postInstall if any command fails --- scripts/postInstall.sh | 2 ++ 1 file changed, 2 insertions(+) diff --git a/scripts/postInstall.sh b/scripts/postInstall.sh index dbe107e26623..e18ca77a6824 100755 --- a/scripts/postInstall.sh +++ b/scripts/postInstall.sh @@ -1,5 +1,7 @@ #!/bin/bash +set -e + # Go to project root ROOT_DIR=$(dirname "$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")" &> /dev/null && pwd)") cd "$ROOT_DIR" || exit 1 From 84585b6960f59e86a940ac45d0f2dd6bcb339100 Mon Sep 17 00:00:00 2001 From: rory Date: Fri, 28 Jun 2024 09:51:25 -0700 Subject: [PATCH 13/13] Add comment --- scripts/postInstall.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/scripts/postInstall.sh b/scripts/postInstall.sh index e18ca77a6824..782c8ef5822c 100755 --- a/scripts/postInstall.sh +++ b/scripts/postInstall.sh @@ -1,5 +1,6 @@ #!/bin/bash +# Exit immediately if any command exits with a non-zero status set -e # Go to project root