[mach-o] Support Windows line endings for -filelist parameter.

[ikudrin]

Summary

On Windows, text files have "\r\n" line ensings, so the trailing '\r' should be removed after splitting an input file by '\n'.

[ikudrin]

This patch fixes a failing test mach-o/filelist.yaml on Windows if a git mirror is used and git has default settings for crlf.

The alternative is to just fix a test issue by adding a corresponding .gitattribute file, see the atteched patch.

lld-motest-alt.patch244 BDownload

[lhames]

Hi ikudrin,

This looks good, it just needs a test case. You should be able to extend test/mach-o/filelist.yaml.

Cheers,
Lang.

[ikudrin]

It actually fixes an existent test case (test/mach-o/filelist.yaml) on Windows + git mirror. I can't imagine any suitable additional tests. Any suggestions?

[lhames]

Hi Igor,

Apologies for not getting to this earlier. I meant that it should be
possible to add a filelist with carriage returns in it to reproduce the
failure.

Are you still seeing the failure on the windows bots?

Cheers,
Lang.

[ikudrin]

In D13046#273717, @lhames wrote:

Hi Igor,

Apologies for not getting to this earlier. I meant that it should be
possible to add a filelist with carriage returns in it to reproduce the
failure.

Are you still seeing the failure on the windows bots?

Cheers,
Lang.

Hi Lang,

I didn't see the failure on any bots, I suppose they use svn to get the sources. I met the problem only on my Windows machine with git (on Windows, git usually converts line endings of text files during checkouts). But since then I've moved to Linux which doesn't have such a problem.

I think that if we add such a file as a test, it won't add any significant value, because the important property of the test (CR character at the end of line) will be invisible and the test could be changed unintentionally. Moreover, git will continue to treat this file as a text and can change line endings, depending on git's settings (we can avoid this situation by using .gitattributes file, but it makes things even more complicated, I think).

As a result, I can't imagine how to create a good test for this situation. May be the fact that this change doesn't broke anything is enough?

lld/trunk/lib/Driver/DarwinLdDriver.cpp

static std::error_code loadFileList(StringRef fileListPath,
   ErrorOr<std::unique_ptr<MemoryBuffer>> mb =
                                         MemoryBuffer::getFileOrSTDIN(filePath);
   if (std::error_code ec = mb.getError())
     return ec;
   StringRef buffer = mb->get()->getBuffer();
   while (!buffer.empty()) {
     // Split off each line in the file.
     std::pair<StringRef, StringRef> lineAndRest = buffer.split('\n');
-    StringRef line = lineAndRest.first;
+    // On Windows, line endings can be "\r\n", not just "\n".
+    StringRef line = lineAndRest.first.rtrim("\r");
     StringRef path;
     if (!dirName.empty()) {
       // If there is a <dir> then prepend dir to each line.
       SmallString<256> fullPath;
       fullPath.assign(dirName);
       llvm::sys::path::append(fullPath, Twine(line));
       path = ctx.copy(fullPath.str());
     } else {