[PATCH] D33575: [llvm-ar] Make llvm-lib behave more like the MSVC archiver

[Zachary Turner via Phabricator via llvm-commits]

zturner added a comment.

Why do we even need `WriteObjPaths`?  Can't we just standardize on `true` for writing, but still support `false` for reading?



================
Comment at: llvm/lib/Object/ArchiveWriter.cpp:207-208
 // Compute the relative path from From to To.
-static std::string computeRelativePath(StringRef From, StringRef To) {
+static StringRef computeRelativePath(StringRef From, StringRef To,
+                                     SmallString<128> &Relative) {
   if (sys::path::is_absolute(From) || sys::path::is_absolute(To))
----------------
For some reason I thought we had an llvm function to do this, essentially mimicing the behavior of `os.path.commonprefix` in python.  I guess I was wrong though.


================
Comment at: llvm/lib/Object/ArchiveWriter.cpp:220-222
+  // Use forward slashes everywhere so that our archives look the same on all
+  // platforms.
+  // FIXME: Make it possible for 'lld-link.exe /lib' and 'llvm-lib.exe' to make
----------------
Is this what we want?  If we're producing an archive to be used on Windows, it seems like we would want it to be as close to MSVC's behavior as possible, which is going to be to use backslashes (on Windows anyway).  It seems like this could confuse MSVC's linker if we feed it archives that have forward slashes.


================
Comment at: llvm/lib/Object/ArchiveWriter.cpp:224
+  // archives with backslashes in them.
+  sys::path::Style Style = sys::path::Style::posix;
 
----------------
I don't think this is entirely what you want.  This means more than just "use forward slashes".  It means "use the entire posix path grammar".  Valid filename characters, drive letter parsing, etc all depend on this.  I suspect we'll run into subtle changes with this.  If you really just want to use slashes, use the native host path style, and at the end write `replace(str, '\\', '/');`


https://reviews.llvm.org/D33575