This is an archive of the discontinued LLVM Phabricator instance.

[llvm-ar] Prevent automatic conversion from thin to full archive
ClosedPublic

Authored by gbreynoo on Feb 1 2022, 3:56 AM.

Details

Summary

llvm-ar silently converts a thin archive to a regular archive when you specify a modification operation (e.g. 'r') without the 'T' modifier. This is probably not what is normally intended, since it would be very easy to forget the modifier. If a user is trying to convert between thin and full archives then they can explicitly use the 'L' command to create a new archive.

Diff Detail

Event Timeline

gbreynoo created this revision.Feb 1 2022, 3:56 AM
gbreynoo requested review of this revision.Feb 1 2022, 3:56 AM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 1 2022, 3:56 AM

I believe Gnu-ar commands without T on thin archives do not auto convert to "full" archives:

  • Move and Delete work as intended
  • Replace and Quick Append errors

I think a consistent approach would be best, so I'm happy to just go with an error if that is preferred.

MaskRay accepted this revision.Feb 1 2022, 4:45 PM

May be worth stating that GNU ar rejects 'r' and 'q' for a thin archive if 'T'/--thin is not specified.

% ar r a.a b.o
ar: Cannot convert existing thin library a.a to normal format

I do not have a strong opinion whether llvm-ar 'r' and 'q' should report a similar error.
Supporting the operation in the absence of explicit 'T'/--thin seems useful.

llvm/test/tools/llvm-ar/full-to-thin-archive.test
12

Prefer two-dash long options for FileCheck. And you already used the form for --check-prefixes:)

llvm/test/tools/llvm-ar/thin-to-full-archive.test
7

ditto

27

If a positive pattern can serve the same purpose as a negative pattern. I'd use the positive one since a negative one may go stale without being noticed.

llvm/tools/llvm-ar/llvm-ar.cpp
658
This revision is now accepted and ready to land.Feb 1 2022, 4:45 PM
MaskRay added inline comments.Feb 1 2022, 4:46 PM
llvm/test/tools/llvm-ar/full-to-thin-archive.test
10