See my other response.  Maybe we don’t even need a substitution at all?<br><div class="gmail_quote"><div dir="ltr">On Mon, Dec 11, 2017 at 12:24 PM Benoit Belley via Phabricator <<a href="mailto:reviews@reviews.llvm.org">reviews@reviews.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">belleyb added inline comments.<br>
<br>
<br>
================<br>
Comment at: test/lit.cfg.py:52-57<br>
+if platform.system() in ['Windows']:<br>
+    config.substitutions.append(('dos2unix', 'sed -b "s/\r$//"'))<br>
+    config.substitutions.append(('unix2dos', 'sed -b "s/\r*$/\r/"'))<br>
+else:<br>
+    config.substitutions.append(('dos2unix', "sed $'s/\r$//'"))<br>
+    config.substitutions.append(('unix2dos', "sed $'s/\r*$/\r/'"))<br>
----------------<br>
caoz wrote:<br>
> zturner wrote:<br>
> > Since the user has `sed` already, why wouldn't they have the actual tool `dos2unix` and `unix2dos`?<br>
> Thanks Zachary. dos2unix and unix2dos aren't installed by default on Linux/Mac. However, if the user can be assumed to have them, I can remove these substitutions.<br>
@zturner According to <a href="https://llvm.org/docs/GettingStarted.html#software" rel="noreferrer" target="_blank">https://llvm.org/docs/GettingStarted.html#software</a>, we shouldn't be relying on `dos2unix` nor `unix2dos` to be present on a build machine.<br>
<br>
Note that the `$'string'` syntax (ANSI-C quotes) is a `bash` extension. According to the page mentioned above, the LLVM builds should only be relying on having a Bourne shell (`sh`). But, are there still any *nix system out there where `/bin/sh` isn't a link for `\bin\bash` ? I.e. Is relying on `bash+sed` fine here ?<br>
<br>
A fully portable solution would be to write python scripts for `dos2unix.py` and `unix2dos.py`. That way, one would not be relying on build tools that LLVM isn't already using.<br>
<br>
Any advice on how to proceed ?<br>
<br>
<br>
<br>
<br>
<a href="https://reviews.llvm.org/D41081" rel="noreferrer" target="_blank">https://reviews.llvm.org/D41081</a><br>
<br>
<br>
<br>
</blockquote></div>