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

[Sameer Arora via Phabricator via llvm-commits]

sameerarora101 marked 15 inline comments as done.
sameerarora101 added inline comments.


================
Comment at: llvm/test/tools/llvm-libtool-darwin/filelist-error-blank-lines.test:9
+# RUN: echo %t-input2.o > %t.blank-line.txt
+# RUN: echo -ne "\n"   >> %t.blank-line.txt
+# RUN: not llvm-libtool-darwin -static -o %t.lib -filelist %t.blank-line.txt 2>&1 | \
----------------
jhenderson wrote:
> Could you just use `echo '' >> %t.blank-line.txt` to get your blank line, so that this can run on Windows?
oh nice, thanks!


================
Comment at: llvm/test/tools/llvm-libtool-darwin/filelist-error-blank-lines.test:15
+# RUN: echo %t-input2.o    > %t.space-line.txt
+# RUN: echo -ne "     \n" >> %t.space-line.txt
+# RUN: not llvm-libtool-darwin -static -o %t.lib -filelist %t.space-line.txt 2>&1 | \
----------------
jhenderson wrote:
> Similarly, here, use `echo -n "      "` here. Why the `-n` at all in fact in this case?
Yup, works without `-n`.


================
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=' '
----------------
jhenderson wrote:
> sameerarora101 wrote:
> > jhenderson wrote:
> > > 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.
> > 3rd case is already there:
> > ```
> > ## Check that error is thrown when file in filelist doesn't exist (with dir):
> > # RUN: rm -rf %t/Invalid-Dir && mkdir -p %t/Invalid-Dir
> > # RUN: echo 'no-such-file' > %t.invalid-list.txt
> > # RUN: not llvm-libtool-darwin -static -o %t.lib -filelist %t.invalid-list.txt,%t/Invalid-Dir 2>&1 | \
> > # RUN:   FileCheck %s --check-prefix=FILE-ERROR -DFILE=%t/Invalid-Dir/no-such-file
> > ```
> >  I have modified the comment to make it more clear. Added the first 2 cases now. Thanks.
> Thanks. What about one for the directory not existing at all?
Added now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84206