adafruit / circuitpython

CircuitPython - a Python implementation for teaching coding with microcontrollers
https://circuitpython.org
Other
4.1k stars 1.22k forks source link

ci_set_matrix: Builds no boards in specific circumstance #9468

Open jepler opened 3 months ago

jepler commented 3 months ago

CircuitPython version

origin/main (pull request)

Code/REPL

GITHUB_EVENT_NAME=pull_request BASE_SHA=901dd228cbdb2069e66fae8c2108d44466ab1e7f HEAD_SHA=d335a10574a1eaf78c94e566399d96150f269636 GITHUB_SHA=7dd3bf81e44bfb73880cc34d418bf7ed09223bad python tools/ci_set_matrix.py

Behavior

Should build many boards, but actually builds none

Description

Encountered while building https://github.com/adafruit/circuitpython/pull/9467

Additional information

I was unable to determine what the intersection_update call is intended to do but in this case it gives an empty set of changed files to consider:

diff --git a/tools/ci_set_matrix.py b/tools/ci_set_matrix.py
index d050d8e44c..a105cacbe6 100755
--- a/tools/ci_set_matrix.py
+++ b/tools/ci_set_matrix.py
@@ -91,8 +91,12 @@ if len(sys.argv) > 1:
 elif compute_diff:
     print("Using files list by computing diff")
     changed_files = git_diff("$BASE_SHA...$HEAD_SHA")
+    print(f"{changed_files=}")
     if os.environ.get("GITHUB_EVENT_NAME") == "pull_request":
-        changed_files.intersection_update(git_diff("$GITHUB_SHA~...$GITHUB_SHA"))
+        print("intersection update (pull request)")
+        other_files = git_diff("$GITHUB_SHA~...$GITHUB_SHA")
+        changed_files.intersection_update(other_files)
+        print(f"{other_files=} {changed_files=}")
 else:
     print("Using files list in CHANGED_FILES")
     changed_files = set(json.loads(os.environ.get("CHANGED_FILES") or "[]"))
Using files list by computing diff
changed_files={'ports/espressif/common-hal/wifi/__init__.c', 'ports/espressif/boards/lolin_c3_pico/sdkconfig', 'ports/espressif/mpconfigport.mk', 'ports/espressif/boards/vidi_x/sdkconfig', 'shared-bindings/socketpool/Socket.h', 'shared-bindings/socketpool/SocketPool.h', 'shared-bindings/socketpool/SocketPool.c', 'shared-bindings/audiocore/RawSample.c', 'shared-module/audiocore/RawSample.c', 'shared-module/audiocore/RawSample.h', 'locale/fr.po', 'locale/cs.po', 'locale/el.po', 'ports/espressif/boards/vidi_x/mpconfigboard.mk', 'locale/de_DE.po', 'locale/sv.po', 'shared-bindings/analogbufio/BufferedIn.c', 'shared-bindings/socketpool/Socket.c', 'ports/espressif/esp-idf-config/sdkconfig.defaults', 'shared-bindings/wifi/Radio.c', 'locale/tr.po', 'ports/espressif/boards/lolin_c3_pico/mpconfigboard.mk', 'docs/shared_bindings_matrix.py', 'ports/espressif/common-hal/socketpool/SocketPool.c', 'locale/hi.po', 'docs/library/sys.rst', 'ports/raspberrypi/common-hal/analogbufio/BufferedIn.c', 'ports/raspberrypi/common-hal/socketpool/__init__.h', 'ports/espressif/common-hal/wifi/Radio.c', 'shared-bindings/audiocore/RawSample.h', 'ports/espressif/common-hal/wifi/__init__.h', 'ports/espressif/common-hal/analogbufio/BufferedIn.c', 'locale/ru.po', 'tests/circuitpython-manual/live_audio/analogbufio_bufferedin_loop.py', 'ports/espressif/boards/vidi_x/pins.c', 'ports/raspberrypi/common-hal/socketpool/Socket.c', 'ports/raspberrypi/common-hal/socketpool/Socket.h', 'locale/it_IT.po', 'ports/espressif/boards/waveshare_esp32_s3_tiny/sdkconfig', 'ports/raspberrypi/common-hal/socketpool/SocketPool.h', 'supervisor/shared/web_workflow/web_workflow.c', 'locale/ko.po', 'locale/pt_BR.po', 'ports/espressif/boards/vidi_x/mpconfigboard.h', 'ports/espressif/boards/waveshare_esp32_s3_tiny/mpconfigboard.mk', 'shared-bindings/wifi/Radio.h', 'ports/raspberrypi/common-hal/analogbufio/BufferedIn.h', 'locale/nl.po', 'tests/circuitpython-manual/live_audio/mix.py', 'ports/raspberrypi/common-hal/wifi/Radio.c', 'shared-bindings/wifi/__init__.c', 'locale/ja.po', 'locale/zh_Latn_pinyin.po', 'shared-bindings/socketpool/__init__.c', 'locale/ID.po', 'locale/en_GB.po', 'locale/es.po', 'ports/espressif/common-hal/socketpool/__init__.h', 'ports/raspberrypi/common-hal/socketpool/SocketPool.c', 'shared-bindings/analogbufio/BufferedIn.h', 'ports/espressif/common-hal/socketpool/Socket.c', 'locale/circuitpython.pot', 'locale/fil.po', 'locale/pl.po', 'py/circuitpy_mpconfig.mk', 'ports/espressif/boards/vidi_x/board.c', 'ports/raspberrypi/common-hal/socketpool/__init__.c'}
intersection update (pull request)
other_files={'ports/raspberrypi/boards/bradanlanestudio_explorer_rp2040/board.c', 'ports/espressif/common-hal/rotaryio/IncrementalEncoder.c', 'ports/nordic/common-hal/_bleio/__init__.c', 'ports/espressif/common-hal/countio/Counter.c', 'ports/espressif/common-hal/frequencyio/FrequencyIn.c', 'conf.py', 'py/modsys.c'} changed_files=set()
Using jobs list in LAST_FAILED_JOBS
::group::Log: changed_files
set()
::endgroup::
::group::Log: last_failed_jobs
{}
::endgroup::
Running: conditionally
Building docs: False
Would set GitHub actions output docs to 'False'
Building windows: False
Would set GitHub actions output windows to 'False'
Building boards: False
Would set GitHub actions output ports to '{}'

@microdev1 pinging because I hope you can enlighten and maybe address the problem.

jepler commented 3 months ago

The specific $GITHUB_SHA seems to have been a temporary merge commit created for building the PR; I pushed it to jepler/git_ref_value_pr9468 because it is now otherwise missing/unreachable as far as I can tell.

microdev1 commented 2 months ago

Hey, diffs between merge commits can get quite complex and the current implementation doesn't handle them very well. Here, the runs should have been scheduled according to the changes introduced in the 9.1.x branch since it diverged. Hopefully, the following sheds some light on this:

env_var commit ref
BASE_SHA https://github.com/adafruit/circuitpython/commit/901dd228cbdb2069e66fae8c2108d44466ab1e7f 9.1.x
GITHUB_SHA~ https://github.com/adafruit/circuitpython/commit/bd834182c43e46606124ce63a604afd91b6568cf main
HEAD_SHA https://github.com/adafruit/circuitpython/commit/d335a10574a1eaf78c94e566399d96150f269636 main ← 9.1.x
GITHUB_SHA https://github.com/adafruit/circuitpython/commit/7dd3bf81e44bfb73880cc34d418bf7ed09223bad main ← (main ← 9.1.x)
diff variable description
BASE_SHA...HEAD_SHA changed_files changes in main since 9.1.x was branched off
GITHUB_SHA~...GITHUB_SHA other_files changes in 9.1.x since it was branched off
microdev1 commented 2 months ago

In a typical pull request scenario, intersection_update gives intersection of changed files between the following two sets:

  1. The changes between the current PR commit head and the last commit that had CI runs base.
  2. The changes being introduced in the PR.

Suppose a new file was added in the PR in an earlier commit but was removed in the current commit, any such files will be excluded by taking intersection of the two sets.