[PATCH] D69146: [install-name-tool] Add first bits for install-name-tool

[Saleem Abdulrasool via Phabricator via llvm-commits]

compnerd added a comment.

I personally like this approach and patch.  I would give @mtrent a chance to look at it, but otherwise LGTM with some minor nits.



================
Comment at: llvm/tools/llvm-objcopy/CopyConfig.cpp:85
+   PARAM,          FLAGS,      INSTALL_NAME_TOOL_##GROUP,                                  \
+   INSTALL_NAME_TOOL_##ALIAS,  ALIASARGS,  VALUES},
+#include "InstallNameToolOpts.inc"
----------------
Probably should clang-format this block.


================
Comment at: llvm/tools/llvm-objcopy/CopyConfig.cpp:728
+  if (InputArgs.size() == 0) {
+    printHelp(T, errs(), "llvm-install-name-tool");
+    exit(1);
----------------
Do we want to keep with the tradition of naming identically to the tool being replaced?  This should probably be `llvm-install_name_tool` if so.


================
Comment at: llvm/tools/llvm-objcopy/CopyConfig.cpp:751
+                             Arg->getAsString(InputArgs).c_str());
+  for (auto Arg : InputArgs.filtered(INSTALL_NAME_TOOL_INPUT))
+    Positional.push_back(Arg->getValue());
----------------
A new line before this would be nice.


================
Comment at: llvm/tools/llvm-objcopy/InstallNameToolOpts.td:1
+include "llvm/Option/OptParser.td"
+
----------------
Add the LLVM copyright header please.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D69146