[PATCH] D33575: [llvm-ar] Make llvm-lib behave more like the MSVC archiver

[Reid Kleckner via llvm-commits]

On Thu, May 25, 2017 at 4:46 PM, Rafael Avila de Espindola via llvm-commits
<llvm-commits at lists.llvm.org> wrote:

>
> > ===================================================================
> > --- llvm/lib/Object/COFFImportFile.cpp
> > +++ llvm/lib/Object/COFFImportFile.cpp
> > @@ -516,10 +516,9 @@
> >          OF.createShortImport(*Name, E.Ordinal, ImportType, NameType));
> >    }
> >
> > -  std::pair<StringRef, std::error_code> Result =
> > -      writeArchive(Path, Members, /*WriteSymtab*/ true,
> object::Archive::K_GNU,
> > -                   /*Deterministic*/ true, /*Thin*/ false);
> > -
> > +  std::pair<StringRef, std::error_code> Result = writeArchive(
> > +      Path, Members, /*WriteSymtab=*/true, /*WriteObjPaths=*/true,
> > +      object::Archive::K_GNU, /*Deterministic=*/true, /*Thin=*/false);
>
> This changes WriteObjPaths for COFF import files. Is that intentional?
>

The names we invent for import files don't have path separators in them, so
there should be no behavior change either way here. By passing true, we
don't needlessly run sys::path::filename on them. Are you suggesting that
maybe it will cause us to not use the short strtab format? I guess I'll
change it back. Most DLL names are < 16 characters.

> -static bool useStringTable(bool Thin, StringRef Name) {
> > -  return Thin || Name.size() >= 16;
> > +static bool useStringTable(bool WriteObjPaths, StringRef Name) {
> > +  // Force the use of the string table if we're writing full paths.
> > +  return WriteObjPaths || Name.size() >= 16;
>
> I think you want to keep Thin in here. That is a format difference of
> thin archives:
>
> https://sourceware.org/ml/binutils/2008-03/msg00150.html


Thin should imply WriteObjPaths. I added an assertion to that effect, but I
can remove it and thread Thin back here as well if you like.


> > zturner added a comment.
> >
> > Why do we even need `WriteObjPaths`?  Can't we just standardize on
> `true` for writing, but still support `false` for reading?
> >
>
> That is an interesting idea. Can you measure what the impact in on a
> clang build?
>

I think if people care about the size of their static archives they should
use thin archives, which always have long relative paths to the object
files. It doesn't seem worth deviating from binutils ar here without a
really good reason.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170525/2afb032b/attachment.html>