Add Macports legacy software options to builder#26691
Add Macports legacy software options to builder#26691programmingkidx wants to merge 6 commits intovlang:masterfrom
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a719f46a3f
ℹ️ 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".
vlib/v/builder/cc.v
Outdated
| ccoptions.args << "-I./thirdparty/legacy/include/LegacySupport/" | ||
| ccoptions.args << "./thirdparty/legacy/lib/libMacportsLegacySupport.a" |
There was a problem hiding this comment.
These new flags use ./thirdparty/legacy/..., which is resolved from the caller's current working directory rather than the V installation/repo root. setup_ccompiler_options runs for normal v builds from arbitrary project folders, so on Darwin <= 8 this will often point to missing files and fail with header/library not found errors. Please build these paths from @VEXEROOT/v.pref.vroot (and quote them) like other thirdparty references.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
This is always a good idea. Just in case, somehow, the current directory gets changed, such that ./thirdparty is no longer correct.
vlib/v/builder/cc.v
Outdated
| // Mac OS 10.4 and older requires Macports legacy software to build programs | ||
| if user_darwin_version <= 8 { | ||
| ccoptions.args << "-I./thirdparty/legacy/include/LegacySupport/" | ||
| ccoptions.args << "./thirdparty/legacy/lib/libMacportsLegacySupport.a" |
There was a problem hiding this comment.
The archive is added to ccoptions.args, which are emitted before source inputs in only_compile_args; that makes the static .a appear too early in the final command. On toolchains with order-sensitive static linking, the linker can scan libMacportsLegacySupport.a before the generated object and still report unresolved references (the exact issue this patch is trying to fix). This archive should be appended via linker flags so it is passed in link position.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
👎 It works just fine on Mac OS 10.4.
There was a problem hiding this comment.
Perhaps, but wouldn't it be better to have the second line start with -L to ensure it is linked rather than included?
There was a problem hiding this comment.
I tried this and used this code:
ccoptions.args << "-L ./thirdparty/legacy/lib/libMacportsLegacySupport.a"
The result was the ld command failed to find the the symbols _backtrace and _backtrace_symbols_fd when trying to build a program.
|
Leaving this one up to you @medvednikov |
|
@JalonSolov I implemented a change that I think you will find agreeable. What do you think? |
Co-authored-by: JalonSolov <JalonSolov@gmail.com>
Since we are using @VEXEROOT directly we don't need the vexe_path variable.
|
Close/re-open to run CI with latest V. |
|
You need to run |
2 participants
V on Mac OS 10.4 and older requires the MacPorts legacy software in order to build a program. If it is not included then error messages about missing functions like backtrace() are displayed. To fix this problem, this patch has the MacPorts legacy' include folder and library path sent to the C compiler.