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

James Henderson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Oct 29 02:13:52 PDT 2019


jhenderson added inline comments.


================
Comment at: llvm/test/tools/llvm-objcopy/MachO/install-name-tool-add-rpath.test:1
+# This test checks adding a new LC_RPATH load command to a MachO binary.
+
----------------
Most of our newer tests in the binutils have been using '##' to help distinguish comments from lit/FileCheck directives. Please make the same change here too.


================
Comment at: llvm/test/tools/llvm-objcopy/MachO/install-name-tool-add-rpath.test:15
+
+# RUN: not llvm-install-name-tool -add_rpath @executable_path/. %t.i386 2>&1 | FileCheck --check-prefix=DUPLICATE-RPATH %s
+# DUPLICATE-RPATH: duplicate load command
----------------
This and the next RUN line could probably do with being broken up over two lines:


```
# RUN: not llvm-install-name-tool -add_rpath @executable_path/. %t.i386 2>&1 \
# RUN:  | FileCheck --check-prefix=DUPLICATE-RPATH %s
```


================
Comment at: llvm/test/tools/llvm-objcopy/MachO/install-name-tool-help-message.test:7-10
+# INSTALL-NAME-TOOL-USAGE:  USAGE: llvm-install-name-tool
+# INSTALL-NAME-TOOL-USAGE:  @FILE
+
+# UNKNOWN-ARG:    unknown argument '{{-+}}abcabc'
----------------
You can probably get rid of the excess spaces between the CHECK name and the pattern, i.e. `# UNKNOWN-ARG: unknown argument` etc.


================
Comment at: llvm/tools/llvm-objcopy/CopyConfig.cpp:766
+    return createStringError(errc::invalid_argument,
+                             "llvm-install-name-tool expects a single input file");
+  Config.InputFilename = Positional[0];
----------------
This line still doesn't look to be clang-formatted.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D69146





More information about the llvm-commits mailing list