[PATCH] D71302: [llvm-ar] Improve tool selection logic

[Jordan Rupprecht via Phabricator via llvm-commits]

rupprecht added a comment.

In D71302#1778352 <https://reviews.llvm.org/D71302#1778352>, @MaskRay wrote:

> In D71302#1778295 <https://reviews.llvm.org/D71302#1778295>, @rupprecht wrote:
>
> > > Update the heuristic to make all the following work as intended:
> > > 
> > > llvm-ar-9
> > >  llvm-ranlib.exe
> > > arm-pokymllib32-linux-gnueabi-llvm-ar
> > > arm-pokymllib32-linux-gnueabi-llvm-ar-9
> >
> > This was manually verified, but can you add a test for it?
> >  You could probably write a lit test like:
> >
> >   # RUN: mkdir -p bin
> >   # RUN: cp llvm-ar bin/llvm-ar-9
> >   # RUN: bin/llvm-ar-9 --help | FileCheck %s --check-prefix=AR
> >   # RUN: cp llvm-ar bin/llvm-ranlib.exe
> >   # RUN: bin/llvm-ranlib.exe --help | FileCheck %s --check-prefix=RANLIB
> >   ...
> >
>
>
> The path of llvm-ar is not known.


lit substitutes llvm-ar with the full path; see `llvm/test/tools/llvm-ar/case-detection.test`

> Even if it does, I don't know copying an executable to a directory that may be mounted with the noexec flag will work...

Hmm, fair point. There's probably some other trick that would work... would creating a symlink work on a noexec mount?



================
Comment at: llvm/tools/llvm-ar/llvm-ar.cpp:1184
+  auto Is = [](StringRef Tool) {
+    auto I = Stem.rfind(Tool);
+    return I != StringRef::npos &&
----------------
MaskRay wrote:
> rupprecht wrote:
> > Needs to be lower-cased (+ a test case -- e.g. `LLVM-AR.EXE` should work)
> I think LLVM-AR.EXE is a DOSism that we don't need to care in 2019.
Per D44808, the lowercasing is needed


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71302