runtime: fix issues with nr_cpus()#26679
runtime: fix issues with nr_cpus()#26679programmingkidx wants to merge 3 commits intovlang:masterfrom
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6a8c21a31e
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| VFLAGS += -cflags "$(CFLAGS)" | ||
| VFLAGS += -ldflags -L$(LEGACYLIBS)/lib | ||
| VFLAGS += -ldflags -lMacportsLegacySupport | ||
| VFLAGS += -cflags $(LEGACYLIBS)/lib/libMacportsLegacySupport.a |
There was a problem hiding this comment.
Keep Macports legacy archive in linker flags
Passing libMacportsLegacySupport.a through -cflags places the archive in the compile-options section of V's C command, which in practice is emitted before the generated source/object; for static archives this ordering can prevent symbol resolution on affected toolchains (the legacy macOS path this change targets) and cause link failures. I checked vlib/v/help/build/build-c.txt and it explicitly documents that -ldflags are appended after every other C option, while vlib/v/builder/cc.v builds commands with cflags before sources and ldflags at the end, so this archive should remain in -ldflags rather than -cflags.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
👎
Your suggestion sounds good, but it does not work. I would see C compilation errors when doing that.
runtime's nr_cpus() directly returns the value returned by sysconf(). Problem with this approach is it does not handle error conditions where sysconf() returns -1. To correct this problem a check is made to see if the value returned by sysconf() is greater than 1. If this check fails then return 1 to indicate there is 1 CPU.
The problem encountered with sysconf() was encountered on Mac OS 10.4. To fix this problem on Mac OS 10.4 the Macport legacy library needs to be statically linked to the v binary so its implementation of functions like sysconf() will be used instead of libc's version.