[PATCH] D83002: [llvm-libtool-darwin] Add support for -static option

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: llvm/test/tools/llvm-libtool-darwin/create-static-lib.test:25
+
+## Check that output archive is in MachO format:
+# RUN: llvm-objdump --macho --archive-headers %t.lib | \
----------------
If I'm not mistaken, the canonical name is "Mach-O" (we use MachO for variable and tool names etc, but the file format is technically Mach-O, so we should use that in comments etc).


================
Comment at: llvm/test/tools/llvm-libtool-darwin/create-static-lib.test:51
+## The warning is not yet implemented for llvm-libtool-darwin.
+# RUN: llvm-libtool-darwin -static -o %t.lib %t-input1.o %t-input2.o %t-input1.o
+# RUN: llvm-ar t %t.lib | \
----------------
sameerarora101 wrote:
> jhenderson wrote:
> > It might be worth adding a `2&>1` and check the output is empty here, to flag up if a warning starts getting emitted. That way, it points to where to add testing for the warning.
> ok, added
> ```
> # RUN: llvm-libtool-darwin -static -o %t.lib %t-input1.o %t-input2.o %t-input1.o 2>&1 | \
> # RUN:   FileCheck %s --allow-empty --implicit-check-not={{.}}
> ```
> I hope `--alow-empty` is the right way to go about empty input files (found it here http://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20140804/229905.html). The flag is not there in the documentation
Sounds like a documentation bug. Feel free to update the doc if you get a minute. By default, FileCheck emits an error if the input is empty, but this can be overridden with the use of `--allow-empty`. That in turn has got me wondering whether `--allow-empty` should be replaced by a new option that also implies `--implicit-check-not`. I've raised that on llvm-dev.


================
Comment at: llvm/test/tools/llvm-libtool-darwin/help-message.test:10
 # RUN: llvm-libtool-darwin --help-list | \
-# RUN:   FileCheck -check-prefixes=LIBTOOL-USAGE,LIST %s --match-full-lines
+# RUN:   FileCheck -check-prefixes=LIBTOOL-USAGE,LIST %s --match-full-lines --implicit-check-not="--safepoint-ir-verifier-print-only"
 
----------------
sameerarora101 wrote:
> added `--implicit-check-not="--safepoint-ir-verifier-print-only` as we know the headers won't be present because of `LIST-NOT` checks below.
Sounds reasonable, but I'd get rid of the `--` bit of the implicit-check-not pattern, so that it doesn't matter whether options are printed in the help with one or two dashes.


================
Comment at: llvm/tools/llvm-libtool-darwin/llvm-libtool-darwin.cpp:52
+
+  // Throw error if not in MachO format.
+  if (!isa<object::MachOObjectFile>(**ObjOrErr))
----------------
Mach-O (same as mentioned elsewhere).


================
Comment at: llvm/tools/llvm-libtool-darwin/llvm-libtool-darwin.cpp:73
+
+  // Verify that Member is a MachO object file.
+  if (Error E = verifyMachOObject(*NMOrErr))
----------------
Ditto.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83002