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

Fangrui Song via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Jan 12 01:57:45 PST 2022


MaskRay 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.
 
----------------
jhenderson wrote:
> MaskRay wrote:
> > jhenderson wrote:
> > > "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.
> > Reworded.
> > 
> > The deprecation semantic here is soft - no diagnostics. I think it will keep this way for a while. The Linux kernel uses it ([[ https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=a5967db9af51a84f5e181600954714a9e4c69f1f | kbuild: allow architectures to use thin archives instead of ld -r ]]) since 2016. 
> > 
> > elfutils has had the XSI -T since 2007. FreeBSD's and macOS's may be earlier. elfutils is used a lot on Linux, too, though its usage, compared with binutils, is fairly small. The thin/XSI conflict is unfortunate, and llvm-ar taking 'T' has some issues outside Linux (https://github.com/llvm/llvm-project/issues/25899#issuecomment-1009388242). If you find packages using T for thin archives, I'd still suggest that you try migrating away, if the available version (binutils>=2.38, llvm-ar>=14) is not a problem.
> > 
> > If you still think "Deprecated alias for --thin" is a problem, how about "Alias for --thin for compatibility"?
> Rather than deprecating the llvm-ar meaning of -T, perhaps the elfutils version should be deprecated? Truncating the filename hardly seems like useful functionality to me... I noticed that @emaste has turned off the -T functionality in FreeBSD ar, which means there's only macOS and elfutils remaining as other archivers (and including our proprietary archiver, I know of three current archivers that use T for thin archives - our version, llvm-ar and GNU ar).
> 
> llvm-ar is designed primarily to be a drop-in replacement for GNU ar. I guess I'm okay if we deprecate the option, if the GNU binutils community wish to deprecate it on their side (though I would question if that community has really considered this heavily), and we can then decide whether to remove it in the future, if they choose to. If they don't there's no point in deprecating it in llvm-ar.
The 2004 edition of https://pubs.opengroup.org/onlinepubs/009696899/utilities/ar.html mentioned -T, so this was indeed a binutils created portability issue. I think it is unfair to say that elfutils T is the one to be deprecated....

As of binutils 2.38, T has been deprecated (just without diagnostic). I hope folks can find users of T for thin archives can help migrate them to --thin. After a while, binutils T can introduce a warning and perhaps after few years T can be removed.


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