[PATCH] D68839: [lit] Fix internal diff's --strip-trailing-cr and use it

Joel E. Denny via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Oct 11 14:04:29 PDT 2019


jdenny added a comment.

In D68839#1706504 <https://reviews.llvm.org/D68839#1706504>, @rnk wrote:

> I looked into these tests, and it seems that they would've failed if it were not for Python's universal newline translator thing. When I diff the files in question with gnu diff from git bash, they appear to be different without `-w` or `--strip-trailing-cr`. So, your fix makes lit's diff more like gnu diff, and fixes the tests to work in that mode.


Thanks for verifying.  I only had access to logs, so I was doing some guesswork.

> The only downside is that this is one more way for tests to pass on Linux but fail on Windows out of the box.

Yes, I debated whether the old behavior was desirable, but ultimately I concluded that consistency with external diffs is more important to avoid surprising behavior.

> However, if we want to address that, I think we should fix it by adding a lit substitution for "\bdiff\b " to add `--strip-trailing-cr` on Windows.

Agreed.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68839/new/

https://reviews.llvm.org/D68839





More information about the llvm-commits mailing list