[PATCH] D67144: [llvm-objcopy] Default --output-target to --input-target when unspecified

[James Henderson via Phabricator via llvm-commits]

jhenderson added a comment.

Would you mind explaining the purpose of the -B switch now? I'm just struggling to grasp it a little at the moment.



================
Comment at: test/tools/llvm-objcopy/ELF/binary-input-and-output.test:11
 
-# -I binary -O binary preserves payload through an intermediate object file
-# RUN: llvm-objcopy -I binary -B i386:x86-64 %t.txt %t.o
-# RUN: llvm-objcopy -O binary %t.o %t.3.txt
+## -B is a NOP when -I and -O are specified.
+# RUN: llvm-objcopy -I binary -B i386:x86-64 -O binary %t.txt %t.3.txt
----------------
Nit: are specified -> are both specified.


================
Comment at: test/tools/llvm-objcopy/ELF/binary-input-and-output.test:15
+
+# -I binary -O binary preserves payload through an intermediate object file
+# RUN: llvm-objcopy -I binary -O elf64-x86-64 %t.txt %t.o
----------------
Would you also mind adding a second hash to the existing comments in this file for consistency throughout? Happy for that to be a separate NFC commit if you want, or in this one.

If you do that, please add the missing trailing full-stops too.


================
Comment at: test/tools/llvm-objcopy/ELF/binary-input-and-output.test:21
+
+## -B is a NOP when -I and -O are specified.
+# RUN: llvm-objcopy -I binary -B i386:x86-64 -O elf64-x86-64 %t.txt %t.2.o
----------------
Nit: are specified -> are both specified.

Perhaps also worth adding "for non-binary output" or similar, to distinguish this case from the above similar one.


================
Comment at: test/tools/llvm-objcopy/ELF/binary-input-error.test:3-4
 
-# RUN: not llvm-objcopy -I binary %t.txt %t.o 2>&1 \
-# RUN:   | FileCheck %s --check-prefix=MISSING-BINARY-ARCH
-
----------------
This is valid behaviour now, right? We need a test case to show this works.


================
Comment at: test/tools/llvm-objcopy/ELF/binary-input.test:4
 # RUN: cp %t.x-txt %t-copy.txt
-# RUN: llvm-objcopy -I binary -B i386:x86-64 %t.x-txt %t.o
+# RUN: llvm-objcopy -I binary -B i386:x86-64 -O elf64-x86-64 %t.x-txt %t.o
 # RUN: llvm-readobj --sections --symbols %t.o | FileCheck %s
----------------
I think you can remove the -B here, right?


================
Comment at: test/tools/llvm-objcopy/ELF/new-symbol-visibility.test:4
 
-# RUN: llvm-objcopy -I binary -B i386:x86-64 %s %t.unspecified
+# RUN: llvm-objcopy -I binary -B i386:x86-64 -O elf64-x86-64 %s %t.unspecified
 # RUN: llvm-readelf -s %t.unspecified | FileCheck %s --check-prefix=BINARY -DVISIBILITY=DEFAULT
----------------
Can you remove the -B here and elsewhere in this test too?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D67144