Skip to content

Commit a5bfbd4

Browse files
committed
fix(rules): improve linker PATH construction for Windows and macOS
Improved `get_linking_path` to ensure essential system paths are available during the linking stage, fixing build failures on Windows and macOS (Intel). - Included `C:\Windows\System32` and `C:\Windows` for Windows builds. - Implemented path deduplication to keep the `PATH` environment clean. - Added robust directory extraction handling both `/` and `\` separators. - Updated Starlark unit tests to verify the new path construction logic. Fixes #11
1 parent f99d130 commit a5bfbd4

2 files changed

Lines changed: 49 additions & 13 deletions

File tree

scala_native/private/rules/target_triple.bzl

Lines changed: 28 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -98,14 +98,33 @@ def get_linking_path(host_path_separator, clang_path, ar_path):
9898
Returns:
9999
A string containing the PATH environment variable.
100100
"""
101-
paths = [
102-
clang_path.rpartition("/")[0],
103-
ar_path.rpartition("/")[0],
104-
]
105-
106-
# Add standard paths on POSIX for tools that might be needed by the linker
101+
# Use a dict to deduplicate paths while preserving order
102+
unique_paths = {}
103+
104+
def add_path(p):
105+
if p and p not in unique_paths:
106+
unique_paths[p] = True
107+
108+
def get_dir(p):
109+
# Bazel usually uses forward slashes even on Windows, but check for both.
110+
# rpartition returns (before, separator, after)
111+
for sep in ["/", "\\"]:
112+
before, s, _ = p.rpartition(sep)
113+
if s:
114+
return before
115+
return ""
116+
117+
add_path(get_dir(clang_path))
118+
add_path(get_dir(ar_path))
119+
120+
# Add standard paths for tools that might be needed by the linker
107121
# discovery but are not part of the hermetic toolchain (e.g. system libs)
108122
if host_path_separator == ":":
109-
paths.extend(["/usr/bin", "/bin"])
110-
111-
return host_path_separator.join(paths)
123+
add_path("/usr/bin")
124+
add_path("/bin")
125+
elif host_path_separator == ";":
126+
# Standard Windows system paths
127+
add_path("C:\\Windows\\System32")
128+
add_path("C:\\Windows")
129+
130+
return host_path_separator.join(unique_paths.keys())

tests/unit/target_triple_test.bzl

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -320,13 +320,13 @@ _platform_flags_none_triple_test = unittest.make(
320320

321321
def _linking_path_posix_test_impl(ctx):
322322
env = unittest.begin(ctx)
323-
# POSIX uses ':' and includes /usr/bin, /bin
323+
# POSIX uses ':' and includes /usr/bin, /bin. It should also deduplicate.
324324
result = get_linking_path(
325325
":",
326326
"/path/to/llvm/bin/clang",
327327
"/path/to/llvm/bin/ar",
328328
)
329-
asserts.equals(env, "/path/to/llvm/bin:/path/to/llvm/bin:/usr/bin:/bin", result)
329+
asserts.equals(env, "/path/to/llvm/bin:/usr/bin:/bin", result)
330330
return unittest.end(env)
331331

332332
_linking_path_posix_test = unittest.make(
@@ -335,13 +335,13 @@ _linking_path_posix_test = unittest.make(
335335

336336
def _linking_path_windows_test_impl(ctx):
337337
env = unittest.begin(ctx)
338-
# Windows uses ';' and should NOT include /usr/bin, /bin
338+
# Windows uses ';' and should include System32, and deduplicate.
339339
result = get_linking_path(
340340
";",
341341
"C:/llvm/bin/clang.exe",
342342
"C:/llvm/bin/llvm-ar.exe",
343343
)
344-
asserts.equals(env, "C:/llvm/bin;C:/llvm/bin", result)
344+
asserts.equals(env, "C:/llvm/bin;C:\\Windows\\System32;C:\\Windows", result)
345345
return unittest.end(env)
346346

347347
_linking_path_windows_test = unittest.make(
@@ -362,6 +362,22 @@ _linking_path_different_dirs_test = unittest.make(
362362
_linking_path_different_dirs_test_impl,
363363
)
364364

365+
def _linking_path_dedup_test_impl(ctx):
366+
env = unittest.begin(ctx)
367+
# Test that duplicate paths are removed even if they come from different sources
368+
result = get_linking_path(
369+
":",
370+
"/usr/bin/clang",
371+
"/usr/bin/ar",
372+
)
373+
# /usr/bin is added by clang/ar paths AND by default POSIX paths, should only appear once.
374+
asserts.equals(env, "/usr/bin:/bin", result)
375+
return unittest.end(env)
376+
377+
_linking_path_dedup_test = unittest.make(
378+
_linking_path_dedup_test_impl,
379+
)
380+
365381
# ===========================================================================
366382
# Test suite entry point
367383
# ===========================================================================
@@ -404,4 +420,5 @@ def target_triple_test_suite(name):
404420
_linking_path_posix_test,
405421
_linking_path_windows_test,
406422
_linking_path_different_dirs_test,
423+
_linking_path_dedup_test,
407424
)

0 commit comments

Comments
 (0)