Skip to content

Fix some bugs that cause minor annoyances… - #22

Merged
savetheclocktower merged 13 commits into
masterfrom
fix-build-bugs
Sep 20, 2026
Merged

savetheclocktower merged 13 commits into
masterfrom
fix-build-bugs

Conversation

@savetheclocktower

Copy link
Copy Markdown

…during the build process.

Claude ferreted these out; the precipitating annoyance was the fact that yarn install in the Pulsar repo often somehow forces me to re-run the download-libiconv step. Claude doesn't know why the existing version gets removed during that process, but says that yarn build doesn't catch the omission because the are-we-still-fresh metadata doesn't know to check for libiconv.2.dylib. The fix is to move that file into the same directory as superstring.node.

Also, Claude spotted some bugs in the fetch-libiconv-61.sh script.

The proof here will be in the CI; if this works just as well in CI, then I'll consider it to be a lateral move at worst. These bugs would hardly ever surface during Pulsar builds because we're always starting from scratch, but fixing them may mean I have slightly fewer headaches.

…during the build process.

Claude ferreted these out; the precipitating annoyance was the fact that `yarn install` in the Pulsar repo often somehow forces me to re-run the download-`libiconv` step. Claude doesn't know why the existing version gets removed during that process, but says that `yarn build` doesn't catch the omission because the are-we-still-fresh metadata doesn't know to check for `libiconv.2.dylib`.

Also, Claude spotted some bugs in the `fetch-libiconv-61.sh` script.
@savetheclocktower

Copy link
Copy Markdown
Author

Since this PR was open, it became the testing ground for a bug I uncovered in superstring. It's possibly the reason why the Windows editor tests are crashing on ~40% of runs! This bug was pretty severe in the sense that it was possible to trigger a crash entirely from the JavaScript layer, so I didn't want to put it in another PR and queue it up behind this one.

While testing that bug, I also noticed that the test:native task wasn't actually doing anything on Windows and was vacuously reporting a zero exit code. Once I actually got those tests to run on Windows, it was clear that they had never been designed to run on Windows — C++ types and idioms that don't work with the standard Windows C++ toolchain, for instance. Those were annoying to troubleshoot because I don't have a Windows dev machine locally, so it was more like pushing and waiting for CI to tell me what was still broken.

Now that this is green, I'm gonna land it soon and then see if it fixes the Windows crashes (which, luckily, seem way more likely to happen in CI than in real life).

@savetheclocktower
savetheclocktower merged commit 6c5387e into master Sep 20, 2026
10 checks passed
@savetheclocktower
savetheclocktower deleted the fix-build-bugs branch September 20, 2026 05:50
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.

1 participant