[PATCH] D91891: [lld-macho] Don't warn on non-existent system libraries

[Saleem Abdulrasool via Phabricator via llvm-commits]

compnerd requested changes to this revision.
compnerd added inline comments.
This revision now requires changes to proceed.


================
Comment at: lld/test/MachO/syslibroot.test:31
-CHECK-SYSLIBROOT-IGNORED: /usr/lib
-CHECK-SYSLIBROOT-IGNORED: /usr/local/lib
-
----------------
thakis wrote:
> thakis wrote:
> > smeenai wrote:
> > > thakis wrote:
> > > > This patch is also fixing this test ffailing on one of my macs, in a fairly normal setup (commandline tools installed, xcode not installed, m1).
> > > I don't quite understand this. A final syslibroot of just `/` is supposed to ignore any previously specified roots (a hack which we inherit from ld64), and I believe that's what this test was for. I don't see how your code change affects this test?
> > Without my patch, the output of the command under test is (on my linux box):
> > 
> > ```
> > $ out/gn/bin/lld -flavor darwinnew -v -syslibroot /var/empty -syslibroot /
> > lld: warning: directory not found for option -F/Library/Frameworks
> > lld: warning: directory not found for option -F/System/Library/Frameworks
> > LLD 12.0.0
> > Library search paths:
> > 	/usr/lib
> > 	/usr/local/lib
> > Framework search paths:
> > ```
> > 
> > After my change, it is just:
> > 
> > ```
> > $ out/gn/bin/lld -flavor darwinnew -v -syslibroot /var/empty -syslibroot /
> > LLD 12.0.0
> > Library search paths:
> > 	/usr/lib
> > 	/usr/local/lib
> > Framework search paths:
> > ```
> > 
> > Previously, the "/usr/local/lib" in the CHECK-SYSLIBROOT-IGNORED line matched against the "warning: directory not found" line, but now that the warning is gone the test fails instead. I don't know if it was supposed to match against that warning line (the "mach here is fuzzy" comment isn't quite clear to me), but after this change it can't match the warning line any longer.
> (Sorry, this output was for the other test further. But I'm guessing the test here currently passes on Windows for the same reason -- and it fails on my m1 mac because it has /usr/lib but not /usr/local/lib so it currently emits a warning for /usr/local/lib and then prints /usr/lib as search path, and that means /usr/local/lib and /usr/lib are not in the order the CHECK lines here currently expect. For the M1, this is indeed independent of my patch – the test fails at head, without the change.)
The match there was to prevent the accidental match.  I think that we should retain the test, but remove the match against the warning.  We should verify that we ignore the non-default-search paths here.


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

https://reviews.llvm.org/D91891