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

[Alexander Shaposhnikov via Phabricator via llvm-commits]

alexshap marked an inline comment as done.
alexshap added inline comments.


================
Comment at: llvm/tools/llvm-objcopy/llvm-objcopy.cpp:320-322
+  const bool IsStrip = Stem.contains("strip");
+  const bool IsInstallNameTool = (Stem.contains("install-name-tool") ||
+                                  Stem.contains("install_name_tool"));
----------------
MaskRay wrote:
> rupprecht wrote:
> > Once we've gone past 2 tools, I think we should have an enum instead -- and maybe add something to `StringSwitch` so you could write:
> > 
> > ```
> > ToolType Tool = StringSwitch(Stem)
> >   .Contains("strip", ToolType::Strip)
> >   .Contains("install-name-tool", ToolType::InstallNameTool)
> >   .Contains("install_name_tool", ToolType::InstallNameTool)
> >   .Default(ToolType::Objcopy);
> > ```
> It seems many comments are not addressed.
> 
> This one regressed D54193 (llvm-strip-9 is not recognized). It was my problem that I did not add a test because I did not know I could use a trick like D71301 to make testing work on at least non-Windows OSes.
> 
> Please check D76562
just curious - which comments are not addressed ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69146