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

[Alexander Shaposhnikov via Phabricator via llvm-commits]

alexshap added inline comments.


================
Comment at: test/tools/llvm-objcopy/basic-archive-copy.test:1
+# RUN: yaml2obj %s > %t
+
----------------
jhenderson wrote:
> jhenderson wrote:
> > jakehehrlich wrote:
> > > Can you add a test that has multiple objects in it? You can use one of the binaries from Input or you can duplicate the same binary.
> > I'd like to see one more test case using a switch that results in some modification: the first member can be modified, but the second cannot (e.g. because it is a text file). This should result in an error and no manipulation of the first object either, I think (though I again could be persuaded that all members should be manipulated if possible).
> Has this case been done? I'm expecting to see an extra case in strip-debug/strip-all, to show that something would indeed be changed.
again, as it has already been discussed (am i missing smth ?) - if an archive contains a text file the full thing won't be modified, and the tool will return an error. A test for that has already been added. (this matches the behavior of binutils strip). So "something would indeed be changed." - that's not the case! (i have a feeling that we are discussing the same question for the 2nd or 3rd time)


================
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
----------------
alexshap wrote:
> jhenderson wrote:
> > jhenderson wrote:
> > > Maybe worth checking that %t is the same as %t2?
> > Have I missed something? I don't see this being done anywhere.
> > 
> > For clarity, since the comment has moved, I was referring to checking the object file in the thin archive.
> because of the location of your comment i thought you were referring to comparing the archives, and that was added (please, have a look at the line 42, I'm kinda tired to be honest).  Comparing %t and %t2 doesn't provide value - it's already done in the test basic-copy.test, so I won't add it here, sorry.
okay, %t and %t2 are not compared exactly like this in basic-copy.test, instead, readobj  + text matching is used there. i can add it here


Repository:
  rL LLVM

https://reviews.llvm.org/D48413