[PATCH] D116979: [llvm-ar] Add --thin for creating a thin archive

James Henderson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Jan 11 01:45:45 PST 2022


jhenderson added inline comments.


================
Comment at: llvm/docs/CommandGuide/llvm-ar.rst:204-205
 
- When creating or modifying an archive, this option specifies that the
- ``archive`` will be thin. By default, archives are not created as thin
- archives and when modifying a thin archive, it will be converted to a regular
- archive.
+ Deprecated alias for ``--thin``. In many ar implementations ``T`` has a
+ different meaning from X/Open System interface.
 
----------------
"from" isn't the right word here, and because of that, I'm struggling to understand the meaning of this sentence. I'd suggest rewording.

I'm also somewhat opposed to deprecating the existing form. We have many users who are using thin archives, and will be using the T form to generate their archives. Deprecation implies that the option will be removed in a few releases time, but that would break all those users. Yes, it's a small change for them to make, but I don't really see the benefit. Most people probably don't care about the portability aspects of their build scripts.

Another related issue is that T is a modifier that is grouped with other operations and modifiers like 'r' and 'c', to form `Trc` or similar, whereas `--thin` is a long-form option. I suspect most llvm-ar users are more familiar with the short-form versions, and don't even know that llvm-ar has a long-form.


================
Comment at: llvm/docs/CommandGuide/llvm-ar.rst:284-287
+ When creating or modifying an archive, this option specifies that the
+ ``archive`` will be thin. By default, archives are not created as thin
+ archives and when modifying a thin archive, it will be converted to a regular
+ archive.
----------------
Text isn't wrapped properly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116979



More information about the llvm-commits mailing list