Thanks for all the feedback. Regarding an error handler I think the introduction of an error handler could be left for another time.

Updated the test in line with suggested changes.

In D65743#1619508, @rupprecht wrote:

I think it'd be good to also write a test that verifies the internal representation is always forward slashes on windows by running FileCheck directly on the thin archive instead of using llvm-ar t to look at it (e.g. see thin-archive.test). Or do we already have a test for that?

Hi James,

Thanks rupprecht, I'll fix my clone before committing.

I have not included a test for this change due to how large the test files would need to be.

Simmilarly to D63197, I'm not sure if it's preferable to make this test XFAIL: darwin or explicitly call llvm-ar with --format=gnu. What do you think jfb?

I believe the test failure is based on an issue with output, specific to the darwin format:

Thanks phosek and jfb, I will investigate this macOS issue.

Fixed a reapplied:
rG5d5078e341f5: [llvm-ar] Reapply Fix relative thin archive path handling

Thanks for the comments MaskRay, I have updated the revision.

After MaskRay's suggestion I have removed all use of rm -f %t-*

Reverted in 363239 due to buildbot test failure

LGTM

Changes after rupprecht's comments.

Thanks hintonda and gribozavr, I will look into that build issue before I recommit.

I've attempted to make "computeArchiveRelativePath" clearer and errors are now propagated to the caller to deal with rather than just returning the "To" argument.

Hi Rui, thanks for the feedback. I'm unsure what changes would better fit what we have outlined in past comments:

If the reasons for these changes are unclear I'll be happy to clarify.

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 ../.

In response to rupprecht:

Update after Rui's comment.