-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[tests] Checkout sdf and usda files with LF for cross-platform unit testing. #848
[tests] Checkout sdf and usda files with LF for cross-platform unit testing. #848
Conversation
Hey @asluk - from the changelog, it looks like you may need to rebase your change? |
Looks like you already did it - thanks! |
Is the LF (no CRLF) requirement documented (I can't find it)? If not, it would be nice to make a note in the FAQ, in addition to this fix. (https://graphics.pixar.com/usd/docs/USD-Frequently-Asked-Questions.html) |
Filed as internal issue #USD-5285. |
I don't think the LF requirement is documented anywhere, but the source of it is in the explicit "\n" writes here https://github.com/PixarAnimationStudios/USD/blob/dev/pxr/usd/lib/sdf/textFileFormat.cpp#L192 |
Definitely in favor of text files on all platforms being written with
single ^J characters as the newlines. Readers should ignore ^M characters
in the input so that text editors that don't obey this can still be used.
Writing different files on different platforms is causing unnecessary
differences and makes cross-platform operation more difficult.
…On Wed, Jul 24, 2019 at 7:57 AM Aaron Luk ***@***.***> wrote:
I don't think the LF requirement is documented anywhere, but the source of
it is in the explicit "\n" writes here
https://github.com/PixarAnimationStudios/USD/blob/dev/pxr/usd/lib/sdf/textFileFormat.cpp#L192
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#848>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAJMCEL4A7M6ZRTWO363GUDQBBUWJANCNFSM4HME5S5Q>
.
|
+1 on modifying .gitattributes in this way to ensure LF when git cloning. |
…ecked out with LF for cross-platform consistency.
69a70cd
to
26b9c6e
Compare
Hi all, this would be a nice one to get into 20.05, as it gets all tests passing on Windows. Thanks! |
^M should be considered a whitespace character in usda. Code on one platform should not add whitespace to the end of every line, as it makes unnecessary differences between files written on different platforms. There is very little software left on Windows that cannot deal with just ^J as a newline. |
Description of Change(s)
Ensures that sdf and usda test inputs and baselines use LF on all platforms; otherwise testSdfParsing* and testUsdFlatten* fail on Windows.
Fixes Issue(s)