Skip to content

Ignore u64ctrl's build directory - #780

Merged
GideonZ merged 1 commit into
GideonZ:test-mergefrom
barryw:fix-u64ctrl-gitignore
Aug 15, 2026
Merged

Ignore u64ctrl's build directory#780
GideonZ merged 1 commit into
GideonZ:test-mergefrom
barryw:fix-u64ctrl-gitignore

Conversation

@barryw

@barryw barryw commented Aug 13, 2026

Copy link
Copy Markdown

software/wifi/raw_u64/.gitignore and software/wifi/raw_c3/.gitignore each hold
build. software/u64ctrl, the third ESP32 project, has none.

Nothing has noticed because the CI image builds elsewhere, but building it locally
leaves about 1250 files of CMake output, object files and ELF images sitting
untracked in the working tree — one git add -A away from being committed. I came
close to doing exactly that while working on #778.

One line, matching what its two siblings already do.

No firmware change, so the hardware gate has nothing to say about it. For the record,
the full default E2E set passes on an Ultimate 64 Elite here, firmware 3.15, FPGA 123,
core 1.4B, 20 of 20 — attached to #778.

raw_u64 and raw_c3 each carry a .gitignore holding "build"; u64ctrl, the third
ESP32 project, does not. Nothing noticed because the CI image builds elsewhere,
but building it locally leaves 1250 files of CMake and object output sitting
untracked in the working tree, one "git add -A" away from being committed.
@GideonZ

GideonZ commented Aug 13, 2026

Copy link
Copy Markdown
Owner

@barryw Hey Barry! Thank you so much for fixing the same issue for the other two products as well!

Before merging, I was just wondering if I should merge PR 778, 779 and 780 in order?

Good thinking about that flag, making it volatile. One thing to note is that the ESP32 and ESP32-S3 are dual core CPUs. I think the tasks are running on the same core, but if they weren't, a volatile keyword is not enough, as it only guarantees that a value is read from memory every time. It doesn't guarantee cache coherency per se. I wonder how much would fall apart if I were to run the Ultimate application on two concurrent CPU cores. ;-) Especially because my Risc-V core does not support the atomic extension, nor any cache coherency protocol.

@barryw

barryw commented Aug 13, 2026

Copy link
Copy Markdown
Author

@GideonZ Thanks! On the merge order, and then on the volatile point, which is a better question than it first looks.

Order

#780 can go in whenever — it is green, and it cannot affect anything else. esp_depends.py only globs *.c *.h *.mk *.txt sdkconfig, so a .gitignore never enters the cache key. Its CI run skipped the ESP32 build for exactly that reason.

#779 before #778, but there is a prerequisite that is yours rather than mine: the runner's ESP-IDF Python environment is out of date and both PRs are failing on it. Neither failure compiles a line of the changes under review:

Requirement 'idf-component-manager~=2.2' was not met. Installed version: 2.1.2
Requirement 'esp-idf-nvs-partition-gen~=0.1.9' was not met. Installed version: 0.1.6
To install the missing packages, please run "install.sh"
make: *** [Makefile:26: esp32_raw_u64] Error 1

/opt/build-tools is bind-mounted :ro in every docker invocation in build.yml, so this cannot be repaired from inside the workflow — install.sh needs running on the runner host against /opt/build-tools/espressif.

Worth knowing why it surfaced only now. The ESP32 build step has been skipped on every recent test-merge run — cache hit each time — so the environment has been broken for some unknown stretch with nothing to reveal it. These two PRs are the first in a while to invalidate that cache, and it failed immediately. That is #779's argument arriving as evidence: the stale key was not only serving old binaries, it was concealing a build environment that no longer works.

One consequence worth flagging: merging #779 will make the ESP32 build run more often, because the key will finally invalidate on raw_u64 changes. Until the runner is fixed, that means red more often rather than less. Fixing the runner first, then merging, gets the benefit without the noise.

A correction to something I wrote on #778

I said there that #779 needed to land first or #778's CI would not rebuild the module it is meant to be testing. That was true when Chris observed it, when #778 only touched raw_u64. It is not true of #778 as it now stands: the port to raw_c3 and u64ctrl touches files that are in the current dependency list, so #778 does invalidate the cache and does rebuild everything, raw_u64 included. Correct by side effect rather than because the key works. A raw_u64-only change still would not.

On volatile

You are right, and it applies to more of this than I had checked before you raised it. Both dual-core targets are SMP builds here:

project target FreeRTOS
raw_u64 esp32 # CONFIG_FREERTOS_UNICORE is not set
u64ctrl esp32s3 # CONFIG_FREERTOS_UNICORE is not set
raw_c3 esp32c3 CONFIG_FREERTOS_UNICORE=y

and both tasks are created with plain xTaskCreaterpc_dispatch.c:334, wifi_modem.c:754 — which under ESP-IDF's SMP FreeRTOS means tskNO_AFFINITY. So the writer and the reader genuinely can be on different cores on two of the three. Your instinct was correct; I had assumed same-core without checking, which was not good enough.

Why I still think volatile holds for this particular variable: neither project sets CONFIG_SPIRAM=y, so the flag is a plain static in internal SRAM, and the caches on ESP32 and ESP32-S3 sit in front of external flash and PSRAM rather than internal RAM — there is no line that can go stale. A bool load and store are single-byte accesses, so nothing tears, and the usage is set / clear / read with no read-modify-write and no second piece of data whose visibility depends on the flag's ordering.

That is an argument that it works, not that it is robust. It leans on three facts that are true today and are not written down anywhere. If you would rather it not depend on them, atomic_bool from <stdatomic.h> is a drop-in on both Xtensa targets and costs nothing there. Happy to make that change if you prefer it; it is a one-line diff per project.

Your closing question is the more interesting one, and the answer is different for your own core than for the ESP32. Without the atomic extension, C11 atomics degrade to libatomic calls or interrupt masking, and with no coherency protocol anything shared would need explicit flushes at well-defined points rather than a keyword. On the current single-core application that cost is invisible, which is precisely what would make the transition expensive — the places that quietly rely on "only one thing runs at a time" are not marked, and a fair number of them will be in the UI and filesystem layers rather than anywhere obviously concurrency-shaped. Worth its own issue if you ever get serious about it.

@GideonZ
GideonZ merged commit 4b7130e into GideonZ:test-merge Aug 15, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants