[PATCH] D124895: [Object] Fix updating darwin archives

[Keith Smiley via Phabricator via llvm-commits]

keith added a comment.

In D124895#3494149 <https://reviews.llvm.org/D124895#3494149>, @jyknight wrote:

> In D124895#3492632 <https://reviews.llvm.org/D124895#3492632>, @keith wrote:
>
>> In D124895#3491724 <https://reviews.llvm.org/D124895#3491724>, @jyknight wrote:
>>
>>> 
>
> Of course, if ld64 is ever removed in favor of lld, we could stop worrying about most of these issues

I won't hold my breath :(

> and just write normal BSD (or even GNU/GNU64!) archives...except for one bit. The workaround which would need to remain is the duplicate-filename+timestamp avoidance. That's necessitated by the format used for "N_OSO" entries in a linked binary, which is needed so that lldb and dsymutil can lookup archive members to get debug info (as explained in a long comment in ArchiveWriter.cpp.)

My assumption if it did come down to this is we could safely apply this behavior to all BSD archives, since it's still deterministic and I don't think would have significant downsides? The only thing I wonder is how many places do something like `timestamp == 0` to mean something special?



================
Comment at: llvm/lib/Object/ArchiveWriter.cpp:633-634
 
+  if (Kind == object::Archive::K_BSD && !NewMembers.empty() &&
+      NewMembers[0].isMacho())
+    Kind = object::Archive::K_DARWIN;
----------------
jyknight wrote:
> I don't think this function should be doing this -- I'd like for the two callers in llvm/lib/ObjCopy/Archive.cpp (where it does `deepWriteArchive(..., Ar.kind(), ...)`) and llvm/tools/llvm-ar/llvm-ar.cpp (where it does `Kind = OldArchive->kind();`) to pass a different Kind (computed using some common function placed somewhere appropriate).
> 
> 
> 
What do you think about the trade off of new places not automatically given this behavior if we do that?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124895