Only computeRelativePath() on new members

[marksantaniello]

Summary

When using thin archives, and processing the same archive multiple times, we were mangling existing entries. The root cause is that we were calling computeRelativePath() more than once. Here, we only call it when adding new members to an archive.

Note that D27218 changes the way thin archives are printed, and will break the new unit test included here. Depending on which one lands first, the other will need to be slightly modified.

[davide]

LGTM. Do you need somebody to commit this on your behalf?

[marksantaniello]

Remove trailing whitespace

[marksantaniello]

Previous diff was a mistake. Got the revisions mixed up. Here I add the final newline to the new test.

[marksantaniello]

Thanks @davide, looks like I'm all set since @david2050 already did it.

include/llvm/Object/ArchiveWriter.h

 
 namespace llvm {
 
 struct NewArchiveMember {
   std::unique_ptr<MemoryBuffer> Buf;
   sys::TimePoint<std::chrono::seconds> ModTime;
   unsigned UID = 0, GID = 0, Perms = 0644;
 
+  bool IsNew = false;
   NewArchiveMember() = default;
   NewArchiveMember(MemoryBufferRef BufRef);
 
   static Expected<NewArchiveMember>
   getOldMember(const object::Archive::Child &OldMember, bool Deterministic);
 
   static Expected<NewArchiveMember> getFile(StringRef FileName,
                                             bool Deterministic);

lib/Object/ArchiveWriter.cpp

 Expected<NewArchiveMember>
 NewArchiveMember::getOldMember(const object::Archive::Child &OldMember,
                                bool Deterministic) {
   Expected<llvm::MemoryBufferRef> BufOrErr = OldMember.getMemoryBufferRef();
   if (!BufOrErr)
     return BufOrErr.takeError();
 
   NewArchiveMember M;
+  assert(M.IsNew == false);
   M.Buf = MemoryBuffer::getMemBuffer(*BufOrErr, false);
   if (!Deterministic) {
     auto ModTimeOrErr = OldMember.getLastModified();
     if (!ModTimeOrErr)
       return ModTimeOrErr.takeError();
     M.ModTime = ModTimeOrErr.get();
     Expected<unsigned> UIDOrErr = OldMember.getUID();
     if (!UIDOrErr)
ErrorOr<std::unique_ptr<MemoryBuffer>> MemberBufferOrErr =
       MemoryBuffer::getOpenFile(FD, FileName, Status.getSize(), false);
   if (!MemberBufferOrErr)
     return errorCodeToError(MemberBufferOrErr.getError());
 
   if (close(FD) != 0)
     return errorCodeToError(std::error_code(errno, std::generic_category()));
 
   NewArchiveMember M;
+  M.IsNew = true;
   M.Buf = std::move(*MemberBufferOrErr);
   if (!Deterministic) {
     M.ModTime = std::chrono::time_point_cast<std::chrono::seconds>(
         Status.getLastModificationTime());
     M.UID = Status.getUser();
     M.GID = Status.getGroup();
     M.Perms = Status.permissions();
   }
if (!useStringTable(Thin, Name))
       continue;
     if (StartOffset == 0) {
       printWithSpacePadding(Out, "//", 58);
       Out << "`\n";
       StartOffset = Out.tell();
     }
     StringMapIndexes.push_back(Out.tell() - StartOffset);
 
-    if (Thin)
+    if (Thin) {
+      if (M.IsNew)
         Out << computeRelativePath(ArcName, Path);
       else
+        Out << M.Buf->getBufferIdentifier();
+    } else
       Out << Name;
 
     Out << "/\n";
   }
   if (StartOffset == 0)
     return;
   if (Out.tell() % 2)
     Out << '\n';

test/Object/archive-thin-create.test

+RUN: mkdir -p %t
+RUN: cd %t
+RUN: mkdir -p foo
+RUN: touch foo/test1.o
+RUN: touch foo/test2.o
+RUN: llvm-ar qcT foo/libtest.a foo/test1.o
+RUN: llvm-ar qcT foo/libtest.a foo/test1.o
+RUN: llvm-ar qcT foo/libtest.a foo/test2.o
+RUN: llvm-ar t foo/libtest.a | FileCheck %s
+
+CHECK: test1.o
+CHECK: test1.o
+CHECK: test2.o
 No newline at end of file