[PATCH] D48413: [llvm-objcopy] Add initial support for static libraries

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: test/tools/llvm-objcopy/basic-archive-copy.test:37
+# RUN: llvm-ar crsT %t.thin.a %t
+# RUN: llvm-objcopy %t.thin.a %t2.thin.a
+# RUN: cat %t.thin.a | FileCheck %s --check-prefix=VERIFY-THIN-ARCHIVE
----------------
Maybe worth checking that %t is the same as %t2?


================
Comment at: test/tools/llvm-objcopy/basic-archive-copy.test:110-118
+#INDEX-TABLE: Archive map
+#INDEX-TABLE-NEXT: foo in
+
+#NO-INDEX-TABLE-NOT: Archive map
+#NO-INDEX-TABLE-NOT: foo in
+
+#BAD-FORMAT: The file was not recognized as a valid object file
----------------
I think I find it easier if the checks are as near as reasonable to the place they're referenced from. In this case, the BAD-FORMAT and VERIFY-THIN-ARCHIVE lines are only used for one case each, so they could be moved to be with the corresponding RUN. Similarly, only the first part of the test uses the CHECK/INDEX-TABLE/NO-INDEX-TABLE patterns, so it might be worth moving them to where they are used.


================
Comment at: test/tools/llvm-objcopy/fail-no-output-directory.test:3
 # RUN: not llvm-objcopy %t no/such/dir 2>&1 | FileCheck %s
-# CHECK: failed to open no/such/dir
+# CHECK: 'no/such/dir': No such file or directory
 
----------------
So, yeah, you'll need to remove the bit after the colon here.


Repository:
  rL LLVM

https://reviews.llvm.org/D48413