Skip to content

fix(build): bootstrap libraries before compiling emu#163

Open
Ticed wants to merge 1 commit intoNERVsystems:masterfrom
Ticed:fix/macos-build-bootstrap
Open

fix(build): bootstrap libraries before compiling emu#163
Ticed wants to merge 1 commit intoNERVsystems:masterfrom
Ticed:fix/macos-build-bootstrap

Conversation

@Ticed
Copy link
Copy Markdown

@Ticed Ticed commented Apr 4, 2026

Reduced scope per review: this PR now contains only the two fixes that are ready to merge.

QUICKSTART.md had a stale path (X/arm64/bin) that would leave readers with a broken PATH.
emu/port/portmkfile was missing the devprof.$O: $RUNT dependency, so devprof.c would not be rebuilt when runt.h regenerates.

The macOS library-bootstrap work will follow in a separate PR. That follow-up
will address the review notes by removing libfreetype from the loop, dropping the redundant SYSTARG export, and extracting the duplicated bootstrap logic into a shared scripts/bootstrap-libs.sh helper.

Copy link
Copy Markdown

@pdfinn pdfinn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @Ticed, thanks for this — really appreciate you taking the time to dig into the build system and put together a fix. This is a real problem: the macOS build scripts aren't self-contained like the Linux one, and a fresh clone + ./build-macos-headless.sh shouldn't leave someone staring at linker errors.

Two of these fixes are clean and I'd like to get them merged right away:

  • QUICKSTART.md typo — good catch, X/arm64/binMacOSX/arm64/bin
  • devprof.$O: $RUNT in portmkfiledevprof.c includes runt.h but was missing the dependency. Correct fix.

For the library bootstrap, I have a few suggestions before we merge that part:

  1. libfreetype doesn't need to be built. If you look at the emu/MacOSX/emu config, freetype is commented out in the lib section — it's not linked into either headless or SDL3 builds. Building it is harmless but unnecessary.

  2. The SYSTARG export is redundant. mkconfig already sets SYSTARG=$SYSHOST, so the build system picks it up automatically. No harm done, but it's not needed.

  3. The library loop is duplicated across three files. Identical 23-line blocks in build-macos-headless.sh, build-macos-sdl3.sh, and build_test.sh will be a maintenance headache. Would you be open to extracting that into a shared script (something like scripts/bootstrap-libs.sh) that all three source? Our CI already handles this as discrete steps — take a look at the macos-arm64 job in .github/workflows/ci.yml for how we structure the build order.

Suggestion: Would you be up for splitting this into two PRs? I can merge the QUICKSTART + portmkfile fixes right now, and we can iterate on the bootstrap work separately. That way the good stuff lands immediately. Happy to help with the shared-script approach if you want to pair on it.

One more thing — the QUICKSTART and portmkfile fixes are really upstream improvements that would benefit all InferNode users. Would you consider submitting those to infernode-os/infernode as well? That's the canonical repo; our fork here carries product-specific changes. Totally fine to keep the bootstrap PR here since it's about our macOS build scripts specifically.

Thanks again for contributing — finding the devprof dependency and the QUICKSTART typo shows you were reading carefully.

@Ticed Ticed force-pushed the fix/macos-build-bootstrap branch from 58261c3 to afa19d2 Compare April 16, 2026 21:55
@sonarqubecloud
Copy link
Copy Markdown

@Ticed
Copy link
Copy Markdown
Author

Ticed commented Apr 16, 2026

Thanks for the thorough review, @pdfinn. I force-pushed this PR down to the two changes you called out as ready to merge: the QUICKSTART.md path fix and the devprof.$O: $RUNT
dependency in emu/port/portmkfile.

I’ll send the macOS bootstrap changes as a separate PR with your suggestions folded in: remove libfreetype from the loop, drop the redundant SYSTARG export, and pull the
duplicated bootstrap logic into a shared script.

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