[PATCH] D82252: MachO: support `-syslibroot`

[Saleem Abdulrasool via Phabricator via llvm-commits]

compnerd added inline comments.


================
Comment at: lld/MachO/Driver.cpp:160
+    // NOTE: only absolute paths are re-rooted to syslibroot(s)
+    if (llvm::sys::path::is_absolute(path, llvm::sys::path::Style::posix)) {
+      for (auto root : roots) {
----------------
int3 wrote:
> compnerd wrote:
> > int3 wrote:
> > > Will this work on Windows given the use of `Style::posix`?
> > Yes, it should work on windows just as well.  I've been running the tests on Windows :)
> I guess my question then is how does it work? I was under the impression that we would want the style to be `native` since the path separators are different on Windows.
> 
> (I'm working on setting up a Windows VM so I can answer these things myself in the future 😅)
Windows paths are dark and full of terrors.  The Win32 APIs will re-normalize the paths and accept `/` and `\` (though not the NT APIs).  I opted for the posix representation to make the printing work out better.  We can always change this in the future.


================
Comment at: lld/test/MachO/syslibroot.test:21-22
+
+# NOTE: the match here is fuzzy because the default search paths exist on Linux
+# and macOS, but not on Windows.
+RUN: lld -flavor darwinnew -v -syslibroot /var/empty -syslibroot / 2>&1 | FileCheck %s -check-prefix CHECK-SYSLIBROOT-IGNORED
----------------
int3 wrote:
> compnerd wrote:
> > int3 wrote:
> > > which is the fuzzy match here? not sure I follow...
> > `/var/empty` is not being checked.
> ah I see. Maybe make that explicit in the comment?
> 
> Also, shouldn't we be able to portably check that `/var/empty` isn't present? My understanding is that the `-syslibroot /` will make `-syslibroot /var/empty` a no-op on all platforms... is that right?
Sure, I can make that more explicit in the comment.

The problem is that `-syslibroot /` will then pick up `/usr/lib` and `/usr/local/lib` which makes the test more brittle.  This was basically my attempt to force the test to be more uniform across Linux/BSD/Darwin/Windows.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D82252/new/

https://reviews.llvm.org/D82252