From 787c424cfc1e07c8881713ee5c1a6e26f6d6bf07 Mon Sep 17 00:00:00 2001
From: Angus Gratton <angus@redyak.com.au>
Date: Thu, 31 Oct 2024 10:47:47 +1100
Subject: [PATCH] tools/ci.sh: Fix reference commit for code size comparison.

Previously the code size comparison was between the merge base (i.e. where
the PR branched), and the generated merge commit into master.  If the PR
branch was older than current master, this meant the size comparison could
incorrectly include changes already merged on master but missing from the
PR branch.

This commit changes it to compare the generated merge commit against
current master, i.e. the size impact if this PR was to be merged.

This commit also disables running the code size check on "push", it now
only runs on pull_request events.

This work was funded through GitHub Sponsors.

Signed-off-by: Angus Gratton <angus@redyak.com.au>
---
 .github/workflows/code_size.yml |  1 -
 tools/ci.sh                     | 44 +++++++++++++++++++++------------
 2 files changed, 28 insertions(+), 17 deletions(-)

diff --git a/.github/workflows/code_size.yml b/.github/workflows/code_size.yml
index 163d4455c..672619337 100644
--- a/.github/workflows/code_size.yml
+++ b/.github/workflows/code_size.yml
@@ -1,7 +1,6 @@
 name: Check code size
 
 on:
-  push:
   pull_request:
     paths:
       - '.github/workflows/*.yml'
diff --git a/tools/ci.sh b/tools/ci.sh
index 0b4b4e11c..db6f1cce0 100755
--- a/tools/ci.sh
+++ b/tools/ci.sh
@@ -69,25 +69,37 @@ function ci_code_size_build {
     PORTS_TO_CHECK=bmusxpdv
     SUBMODULES="lib/asf4 lib/berkeley-db-1.xx lib/btstack lib/cyw43-driver lib/lwip lib/mbedtls lib/micropython-lib lib/nxp_driver lib/pico-sdk lib/stm32lib lib/tinyusb"
 
-    # starts off at either the ref/pull/N/merge FETCH_HEAD, or the current branch HEAD
-    git checkout -b pull_request # save the current location
-    git remote add upstream https://github.com/micropython/micropython.git
-    git fetch --depth=100 upstream master
-    # If the common ancestor commit hasn't been found, fetch more.
-    git merge-base upstream/master HEAD || git fetch --unshallow upstream master
+    # Default GitHub pull request sets HEAD to a generated merge commit
+    # between PR branch (HEAD^2) and base branch (i.e. master) (HEAD^1).
+    #
+    # We want to compare this generated commit with the base branch, to see what
+    # the code size impact would be if we merged this PR.
+    REFERENCE=$(git rev-parse --short HEAD^1)
+    COMPARISON=$(git rev-parse --short HEAD)
+
+    echo "Comparing sizes of reference ${REFERENCE} to ${COMPARISON}..."
+    git log --oneline $REFERENCE..$COMPARISON
+
+    function code_size_build_step {
+        COMMIT=$1
+        OUTFILE=$2
+        IGNORE_ERRORS=$3
+
+        echo "Building ${COMMIT}..."
+        git checkout --detach $COMMIT
+        git submodule update --init $SUBMODULES
+        git show -s
+        tools/metrics.py clean $PORTS_TO_CHECK
+        tools/metrics.py build $PORTS_TO_CHECK | tee $OUTFILE || $IGNORE_ERRORS
+    }
+
     # build reference, save to size0
     # ignore any errors with this build, in case master is failing
-    git checkout `git merge-base --fork-point upstream/master pull_request`
-    git submodule update --init $SUBMODULES
-    git show -s
-    tools/metrics.py clean $PORTS_TO_CHECK
-    tools/metrics.py build $PORTS_TO_CHECK | tee ~/size0 || true
+    code_size_build_step $REFERENCE ~/size0 true
     # build PR/branch, save to size1
-    git checkout pull_request
-    git submodule update --init $SUBMODULES
-    git log upstream/master..HEAD
-    tools/metrics.py clean $PORTS_TO_CHECK
-    tools/metrics.py build $PORTS_TO_CHECK | tee ~/size1
+    code_size_build_step $COMPARISON ~/size1 false
+
+    unset -f code_size_build_step
 }
 
 ########################################################################################