[PATCH] D65372: [llvm-objcopy] Add support for response files in llvm-strip and llvm-objcopy

[James Henderson via Phabricator via llvm-commits]

jhenderson accepted this revision.
jhenderson added a comment.
This revision is now accepted and ready to land.

LGTM, with a couple of nits. I'm okay with the refactoring being left to a different change, as long as you don't leave it a long time.



================
Comment at: llvm/test/tools/llvm-objcopy/ELF/response-file.test:8
+# RUN: llvm-readobj -S %t.o | FileCheck %s
+# RUN: llvm-readobj -S %t2.o | FileCheck %s
+# RUN: cmp %t.o %t2.o
----------------
Strictly, you no longer need the second llvm-readobj line, since the cmp makes it redundant.


================
Comment at: llvm/tools/llvm-objcopy/llvm-objcopy.cpp:316
+
+  // Expand response files.
+  SmallVector<const char *, 20> NewArgv(argv, argv + argc);
----------------
Perhaps worth adding a TODO note saying that this is duplicated code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65372