Fix 1117: Use realpath in stage1 Darwin os_self_exe_path #1128
Conversation
binary132
force-pushed
the
fix-1117-macos-realpath
branch
from
cff95ed
to
1596ff4
Compare
src/os.cpp
Outdated
| Buf *resolve_tmp = buf_alloc_fixed(PATH_MAX); | ||
|
|
||
| // Fill it with the real resolved path. | ||
| char *real_path = realpath(buf_ptr(path_tmp), buf_ptr(resolve_tmp)); |
There was a problem hiding this comment.
Instead of resolve_tmp, we should resize out_path and use it directly. And then we need to buf_resize(out_path, strlen(buf_ptr(out_path))) to make the length correct.
Can you confirm this patch works for you?
|
Thanks for the patch, one little fixup and then it should be good to merge. |
Issue: ziglang#1117 The macOS stage1 Zig compiler should look in Zig's real absolute path for the Zig stdlib, but os_self_exe_path looks in its path as returned by _NSGetExecutablePath, which may be a symlink. This means that a symlinked Zig cannot find the Zig stdlib. This patch fixes the issue by resolving the _NSGetExecutablePath result to the real path using realpath() before copying the result to the output path.
binary132
force-pushed
the
fix-1117-macos-realpath
branch
from
0fa01b4
to
c7057bd
Compare
| buf_resize(out_path, u32_len); | ||
| int ret2 = _NSGetExecutablePath(buf_ptr(out_path), &u32_len); | ||
|
|
||
| // Make a buffer having room for the temp path. |
There was a problem hiding this comment.
This might just be me but I personally don't really like comments that just say what the code on the line below does.
As a rule of thumb, comments should explain what won't be obvious from just looking at the code, not restate the code in prose.
edit: hadn't noticed this has been merged between starting and finishing the review.
There was a problem hiding this comment.
The justification is that it makes code faster to read.
| assert(ret2 == 0); | ||
|
|
||
| // Resolve the real path from that. | ||
| buf_resize(out_path, PATH_MAX); |
There was a problem hiding this comment.
Probably not relevant to Zig but in libuv we allocate 2*PATH_MAX to work around a bug in old versions of Apple's libc where the expanded path is sometimes > PATH_MAX.
Issue: #1117
The macOS stage1 Zig compiler should look in Zig's real absolute path
for the Zig stdlib, but
os_self_exe_pathlooks in its path as returnedby
_NSGetExecutablePath, which may be a symlink. This means that asymlinked Zig cannot find the Zig stdlib.
This patch fixes the issue by resolving the
_NSGetExecutablePathresultto the real path using
realpath()before copying the result to theoutput path.