Page MenuHomePhabricator

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

Authored by ikudrin on Sep 21 2015, 10:59 PM.

Details

Summary

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

Diff Detail

Event Timeline

ikudrin updated this revision to Diff 35345.Sep 21 2015, 10:59 PM
ikudrin retitled this revision from to [mach-o] Support Windows line endings for -filelist parameter..
ikudrin updated this object.
ikudrin added a project: lld.
ikudrin added a subscriber: llvm-commits.

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.

lhames edited edge metadata.Sep 22 2015, 10:46 AM

Hi ikudrin,

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

Cheers,
Lang.

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?

ikudrin abandoned this revision.Oct 22 2015, 10:43 PM
lhames added a subscriber: lhames.Oct 22 2015, 11:07 PM

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 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?