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

Keith Smiley via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed May 4 17:48:59 PDT 2022


keith added a comment.

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

> I don't think we ought to modify the archive reader like you've done, because we don't need to tell the difference between BSD/DARWIN when reading, only while writing. Thus, going through the trouble of distinguishing them in the reader is wasted work. Instead, please add a separate function, which ar and objcopy can call before constructing an ArchiveWriter, which returns the more "refined" Kind.

Sounds fair, I'll take a stab at this. I naturally gravitated towards doing it as early as possible since I was worried the Format might be read and used somehow, such that changing it later might introduce some incompatibilities. I will try to take this path instead.

> Also, while your goal of improving the heuristics seems reasonable, perhaps we ought to have a --archive-format option for objcopy, so that users can be explicit if they desire. (We already have --format for ar, and for the cross-compiling case it's basically required -- there's simply no other way to choose the correct behavior for archives with no files, like `llvm-ar rcs x.a`).

>From a UX perspective I hope we can come to a reasonable patch here instead so folks don't have to know about these details in this case. In the case of no files I don't think the decision really matters for this use case at least, although I get that issue in general.

>From this comment I also realized I could probably apply the "is first object macho" heuristic only to the decision about what format is needed for 64 bit archives, and therefore I could potentially take the path of removing K_DARWIN if we felt like applying the other darwin specific workarounds to standard BSD archives was also acceptable, do you have any thoughts there? I believe there wouldn't be any functional downsides, as it worked this way in the past before as well, but it adds some unnecessary behavior for non-macho K_BSD archives.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124895



More information about the llvm-commits mailing list