[PATCH] D99363: [Windows] Turn off text mode in TableGen and Rewriter to stop CRLF translation

Tue Mar 30 04:56:15 PDT 2021

abhina.sreeskantharajan added a comment.

In D99363#2656913 <https://reviews.llvm.org/D99363#2656913>, @aganea wrote:

> In D99363#2653476 <https://reviews.llvm.org/D99363#2653476>, @abhina.sreeskantharajan wrote:
>
>> There were a lot of similar patches so reverting all of them might be more work than isolating the change that caused it and reverting that. It seems that the patch you initially commented on did not contain the problematic change since reverting the change doesn't fix your issue. I created the following patch https://reviews.llvm.org/D99426 based on @rnk suggestion. I created a new flag for OF_TextWithCRLF on Windows and made sure my most recent text changes use the OF_Text flag while all other uses were changed to OF_TextWithCRLF. This should solve any CRLF issues that were introduced recently by my patches. If you have time, would you be able to test if that patch fixes your issue?
>
> I've applied https://reviews.llvm.org/D99426#2656738 over rGc4d5b956170dd85941c1c2787abaa2e01575234c <https://reviews.llvm.org/rGc4d5b956170dd85941c1c2787abaa2e01575234c> but I'm still seeing the issue in https://reviews.llvm.org/D96363#2650460.

Ok, I missed the change in llvm/tools/dsymutil/DwarfLinkerForBinary.cpp.

In D99363#2656913 <https://reviews.llvm.org/D99363#2656913>, @aganea wrote:

> In D99363#2653476 <https://reviews.llvm.org/D99363#2653476>, @abhina.sreeskantharajan wrote:
>
>> There were a lot of similar patches so reverting all of them might be more work than isolating the change that caused it and reverting that. It seems that the patch you initially commented on did not contain the problematic change since reverting the change doesn't fix your issue. I created the following patch https://reviews.llvm.org/D99426 based on @rnk suggestion. I created a new flag for OF_TextWithCRLF on Windows and made sure my most recent text changes use the OF_Text flag while all other uses were changed to OF_TextWithCRLF. This should solve any CRLF issues that were introduced recently by my patches. If you have time, would you be able to test if that patch fixes your issue?
>
> I've applied https://reviews.llvm.org/D99426#2656738 over rGc4d5b956170dd85941c1c2787abaa2e01575234c <https://reviews.llvm.org/rGc4d5b956170dd85941c1c2787abaa2e01575234c> but I'm still seeing the issue in https://reviews.llvm.org/D96363#2650460.

Sorry, I realized the change for DwarfLinkerForBinary.cpp was missing. This was only other file from https://reviews.llvm.org/D96363 that was using OF_TextWithCRLF instead of OF_Text. Please let me know if this latest patch https://reviews.llvm.org/file/data/2jljo4tfl5aiisvwpzg2/PHID-FILE-egbpcbhz3t7b7a2tcjka/D99426.diff fixes your issue. If not, I will revert the remaining changes in my old commit to unblock you.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99363