[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