[PATCH] D118693: [llvm-ar] Prevent automatic conversion from thin to full archive

[Fangrui Song via Phabricator via llvm-commits]

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

May be worth stating that GNU ar rejects 'r' and 'q' for a thin archive if 'T'/--thin is not specified.

  % ar r a.a b.o
  ar: Cannot convert existing thin library a.a to normal format

I do not have a strong opinion whether llvm-ar 'r' and 'q' should report a similar error.
Supporting the operation in the absence of explicit 'T'/--thin seems useful.



================
Comment at: llvm/test/tools/llvm-ar/full-to-thin-archive.test:12
+# RUN: llvm-ar -TqcL %t/thin.a %t/archive.a
+# RUN: FileCheck --check-prefixes=THIN -input-file=%t/thin.a %s
+
----------------
Prefer two-dash long options for FileCheck. And you already used the form for --check-prefixes:)


================
Comment at: llvm/test/tools/llvm-ar/thin-to-full-archive.test:6
+# RUN: llvm-ar -Trc %tthin.a %S/Inputs/a.txt
+# RUN: FileCheck --check-prefixes=THIN -input-file=%tthin.a %s
+
----------------
ditto


================
Comment at: llvm/test/tools/llvm-ar/thin-to-full-archive.test:26
+
+CHECK-NOT: !<thin>
----------------
If a positive pattern can serve the same purpose as a negative pattern. I'd use the positive one since a negative one may go stale without being noticed.



================
Comment at: llvm/tools/llvm-ar/llvm-ar.cpp:655
+
+  // Avoid converting an existing thin archive to a regular one
+  if (!AddLibrary && M.getParent()->isThin())
----------------



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118693