<div dir="ltr">Thanks for the quick response Rui. To clarify, I agree with the scheme you suggest with a call to remove_dots(). I also consider conversion to absolute paths a requirement so the path roots can be compared. This implementation was based of the existing use of path iterators when taking the above into account, is there an alternate implementation you would prefer?</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, Mar 20, 2019 at 5:57 PM Rui Ueyama <<a href="mailto:ruiu@google.com">ruiu@google.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div dir="ltr"></div><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, Mar 20, 2019 at 10:54 AM Owen Reynolds via Phabricator <<a href="mailto:reviews@reviews.llvm.org" target="_blank">reviews@reviews.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">gbreynoo marked 2 inline comments as done.<br>
gbreynoo added a comment.<br>
<br>
The problem with working out the paths is if either are in the form `foo/../bar/`. I don't believe your above suggestion would work for paths with internal `../`.<br></blockquote><div><br></div><div>You can easily remove them using remove_dots().<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
<br>
================<br>
Comment at: lib/Object/ArchiveWriter.cpp:499-500<br>
 std::string computeArchiveRelativePath(StringRef From, StringRef To) {<br>
-  if (sys::path::is_absolute(From) || sys::path::is_absolute(To))<br>
-    return To;<br>
+  auto getDotlessAbsolutePath =<br>
+      [](SmallVectorImpl<char> &P) -> std::error_code {<br>
+    std::error_code Ret = sys::fs::make_absolute(P);<br>
----------------<br>
ruiu wrote:<br>
> This lambda doesn't capture anything, you probably should consider moving this outside of this function.<br>
The lambda was to keep it local to this function, i'd rather keep it unless others really disagree. <br>
<br>
<br>
================<br>
Comment at: lib/Object/ArchiveWriter.cpp:514-515<br>
<br>
-  StringRef DirFrom = sys::path::parent_path(From);<br>
-  auto FromI = sys::path::begin(DirFrom);<br>
-  auto ToI = sys::path::begin(To);<br>
+  SmallString<128> PathTo = To;<br>
+  SmallString<128> DirFrom = sys::path::parent_path(From);<br>
+  if (getDotlessAbsolutePath(PathTo) || getDotlessAbsolutePath(DirFrom))<br>
----------------<br>
ruiu wrote:<br>
> Create instances of SmallString and populate their contents as a side-effect seems a bit awkward. You could instead make your helper function (which is currently named `getDotlessAbsolutePath`) to return an std::string.<br>
Made a change as you've suggested but the use of SmallString is due to remove__dots() and I didn't want a second conversion to std::string. <br>
<br>
<br>
<br>
<br>
<br>
CHANGES SINCE LAST ACTION<br>
  <a href="https://reviews.llvm.org/D59491/new/" rel="noreferrer" target="_blank">https://reviews.llvm.org/D59491/new/</a><br>
<br>
<a href="https://reviews.llvm.org/D59491" rel="noreferrer" target="_blank">https://reviews.llvm.org/D59491</a><br>
<br>
<br>
<br>
</blockquote></div></div>
</blockquote></div>