[PATCH] D84206: [llvm-libtool-darwin] Add support for -filelist option

[James Henderson via Phabricator via llvm-commits]

jhenderson added a comment.

Documentation for the new option?



================
Comment at: llvm/test/tools/llvm-libtool-darwin/filelist.test:28
+# RUN: rm -rf %t/dirname && mkdir -p %t/dirname
+# RUN: rm -f %t-input1.o && rm -f %basename_t.tmp-input1.o
+# RUN: yaml2obj %S/Inputs/input1.yaml -o %t/dirname/%basename_t.tmp-input1.o
----------------
Why are you trying to delete some files here?


================
Comment at: llvm/test/tools/llvm-libtool-darwin/filelist.test:32
+
+## Check that error is thrown on passing just filelist:
+# RUN: not llvm-libtool-darwin -static -o %t.lib -filelist %t.files.txt 2>&1 | \
----------------
I'm not sure this comment is accurate for what the error is indicating. It needs to provide more context.


================
Comment at: llvm/test/tools/llvm-libtool-darwin/filelist.test:66
+
+## Check that error is thrown when file in filelist doesn't exist (without dir):
+# RUN: echo 'no-such-file' > %t.invalid-list.txt
----------------
This seems like essentially a duplicate of the earlier error case?


================
Comment at: llvm/test/tools/llvm-libtool-darwin/filelist.test:94
+# RUN: not llvm-libtool-darwin -static -o %t.lib -filelist %t.space-line.txt 2>&1 | \
+# RUN:   FileCheck %s --check-prefix=FILE-ERROR -DFILE=' '
----------------
Additional possible test-cases:
1) Multiple file lists specified.
2) File is in working directory, but a directory name is specified.
3) Directory exists but does not contain the requested file.


================
Comment at: llvm/tools/llvm-libtool-darwin/llvm-libtool-darwin.cpp:47
+    FileList("filelist",
+             cl::desc("Pass in file containing a list of filenames."),
+             cl::value_desc("listfile[,dirname]"), cl::cat(LibtoolCategory));
----------------
Nit: your -o option doesn't include a trailing full stop in its description. Be consistent.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84206