Apologies James, this fixes the case issues.

Changes made in response to James' comments

Thanks Paul.

After Paul's comment, a custom message can now be used to replace the generic one.

Hi James,

Removed Clangs use of the same message.

Hi sbc100, thanks for highlighting this inconsistency, I think wasm-ld and clang need updating too. Would it be worth the effort to have some kind of shared implementation for this?

ping

• Removed unused function removeHyphens
• Fixed matchFlagWithArg so 3 hyphens are no longer excepted before an argument
• getRspQuoting now uses ArraryRef instead of SmallVector

ping

ping

I agree with mstorsjo, matching what what LLVM is documented to require seems best.
LGTM

Thanks Maskray for your comment regarding marking comments as "done", I'll look out for that in future.

Inlined removeHyphens function

ping

ping

The use of python in the test response.test was failing on my Windows machine with Python 3. This patch fixes the issue.

• Use single quotes in tests
• Clarify use of echo in response.test

ping

• Removed --rsp-quoting=default
  • Reverted rename of Options
  • Removed use of using

ping

Fixed:

• Redundant pair of braces
• space after ,
• indent the continuation lines

• Use using instead of typedef
• removeHyphens is now called from matchFlagWithArg
• Removed Argv reference argument from getRspQuoting
• Fix response.test that failed on linux

• Pulled out the MatchFlagWithArg lambda expression into static function, and done the same with some code that handles hyphen removal.
• Added test cases to response.test.
• Removed the python call in response-utf8.test. I would prefer the file comparison but it should be sufficient to show the archive is functioning properly.

Added rsp-quoting switch as thopre requested.

This looks good to me, although with llvm-ar there is a balance between adding useful functionality and keeping command compatibility with other archivers. I've added Rui as a reviewer in case.

Rui makes good points that I struggle to argue with. I'll abandon this review as their is no interest.

Fixed two format issues.

Renamed "Long Options" to "Other" and moved it below the other options, removed bad link to gnu-ar and updated with changes from D69087.

LGTM

Fix nits, add missing options, remove old limitation and add information regarding deterministic archives.

LGTM

Updated llvm-ar doc to clarify non-windows behaviour and why windows is not Unicode compliant.

Due to updating the command guide I have not added the case insensitivity details to the llvm-ar help text. Would it be preferred to have these details in both?

Update llvm-ar command guide with case insensitivity details, and include a test for archived files with paths for names.

I added the XFAIL to 3 llvm-ar tests I added earlier in the year, due to them failing on Darwin systems. See below:

In D68472#1698154, @thopre wrote:

In D68472#1697691, @hubert.reinterpretcast wrote:

Thanks for adding the BOM. With the BOM, would it make sense to leave mri-utf8.test as the name of the file?

I think the testfile name should reflect what is being tested since that's the test identifier (ie. when a test fails lit prints the relative filepath) so the fact that the file is encoded in UTF-8 is irrelevant. Here the test is about llvm-ar handling non ascii filename, as the first comment explains it. How is the <pound sign>.txt file encoded would make a bit more sense as a name but then as I mentioned AFAIK the filename is encoded in UTF-16 on Windows anywat. In summary, I think the renaming is warranted.

This functions on Windows fine.

Removed unneeded const, swapped use of ifndef and added comment to comparePaths function.

I reverted this change due to the clang-x64-windows-msvc build bot failure.

You are correct that the locale is required to pass on linux. I had some trouble with this test as the behaviour of python in this area differs between linux / windows and python 2 / python 3. For example this fix appears to be fine for linux, however Windows with python 2 fails:

Updated after Edd and Ruiu's suggestion.

In D68033#1689605, @MaskRay wrote:

I asked because I want to make sure this patch is put up for practical use, not for a compatibility corner case that may rarely matter. I think most people may realize case insensitiveness on file systems is a bad idea. If my understanding is correct, llvm-lib used by Windows. llvm-ar is an ELF tool and it should match a generic ELF platform as close as possible and such platform disparity should be as little as possible.

Updated after rupprecht's suggestion.

My understanding is that although NTFS is case sensitive, the windows API makes file operations case insensitive by default.

This is failing on the sphinx build bot:
lab.llvm.org:8011/builders/llvm-sphinx-docs

There are many tests in this area that use llvm-ar to test the underlying library. It may be better for these also to be in the llvm-ar test directory and have unit tests for specifically testing ArchiveWriter?