[PATCH] D130458: [llvm-objcopy] Support --{,de}compress-debug-sections for zstd

Tue Jul 26 00:42:17 PDT 2022

MaskRay added inline comments.

================
Comment at: llvm/test/tools/llvm-objcopy/ELF/compress-debug-sections-zstd.test:2
+# REQUIRES: zstd
+## Test --compress-debug-sections=zstd and uncompression.
+
----------------
jhenderson wrote:
> To me, "decompression" sounds much more natural than "uncompression" (FWIW, I'm not sure I'd ever use the verb "uncompress").
> 
> Later on, `UNCOMPRESSED` should probably be the more specific `DECOMPRESSED` (see https://english.stackexchange.com/a/56483), since it's a reference to something that has undergone the reverse of compression.
Thanks for the reference. It matches the terminology in https://datatracker.ietf.org/doc/html/rfc8478#section-2 , too. I fixed some places to use "decompress" but forgot this one.

================
Comment at: llvm/test/tools/llvm-objcopy/ELF/compress-debug-sections-zstd.test:29-30
+
+## --compress-debug-sections does not update a compressed section. Its compression
+## type does not change.
+# RUN: llvm-objcopy --compress-debug-sections=zstd %t-zstd %t-zstd-zstd
----------------
jhenderson wrote:
> This comment makes me think it might be good to have a test case that shows we don't convert a zstd section to a zlib section if specified with `--compress-debug-sections=zlib` and vice versa.
`# RUN: %if zlib ` provides the test.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130458