[PATCH] D158901: [llvm-rc] Continue to use Argv[0] to resolve executable path

Martin Storsjö via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Aug 25 23:40:58 PDT 2023


mstorsjo added a comment.

Sorry for the breakage, and thanks for making a patch!

This issue actually came up already a couple days ago, in https://github.com/llvm/llvm-project/issues/64927, and I considered doing something pretty much like this there as well - but the submitter managed to work around the issue quite neatly.

I’ve got mainly one suggestion; could you flip the nesting of the two loops? I’d like it to prefer `<triple>-clang` in parent(argv[0]) over `clang` in parent(MainExecPath).

As a side note, on windows, argv[0] is overwritten with the equivalent of MainExecPath by InitLLVM, but not on Unix.

Also, it’s nice to hear that llvm-rc actually is in use within Google - I’ve had a feeling that not many actually use this tool.



================
Comment at: llvm/tools/llvm-rc/llvm-rc.cpp:139
-    // First look for the tool with all potential names in the specific
-    // directory of Argv0, if known
-    for (const auto *Name :
----------------
I see I had forgotten to update this comment here, as it is slightly wrong now - can you reword it s as part of this patch?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158901



More information about the llvm-commits mailing list