Skip to content
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

line ending changed after running not-owned test #268

Closed
BurtHarris opened this issue Aug 1, 2017 · 9 comments
Closed

line ending changed after running not-owned test #268

BurtHarris opened this issue Aug 1, 2017 · 9 comments

Comments

@BurtHarris
Copy link
Contributor

BurtHarris commented Aug 1, 2017

Before running tests, my checked-out copy of test\fixtures\not-owned\not-owned.txt has a Windows style line break, and is thus is 14 bytes long. After running the tests, the line break is changed to Unix style, now only 13 bytes long.

Git notices the change, but reproducing it may require the core.autocrlf setting on windows. A simple test case for this specific behavior might be justified.

Does this imply that vinyl-fs is imposing Unix line break conventions on files it outputs? I think that's undesirable.

@BurtHarris
Copy link
Contributor Author

BurtHarris commented Aug 1, 2017

P.S. This may explain why I get many other test failures using Windows style files. #266 Tools should be robust to line break styles, and preserve what they find.

@BurtHarris
Copy link
Contributor Author

BurtHarris commented Aug 1, 2017

Full log from test pass w/ Windows style line endings...

test.log.txt

Ironically, the log file has Unix style line endings. ;-)

For some further irony, it looks like the _ .dest() with custom owner_ tests have been skipped, but somehow they changed the file.

@erikkemperman
Copy link
Member

@BurtHarris I think all of these are related to your core.autocrlf setting. I'm drafting something that I hope will resolve all of this stuff in one go -- just need a little time.

@BurtHarris
Copy link
Contributor Author

OK, that sounds good.

@erikkemperman
Copy link
Member

erikkemperman commented Aug 1, 2017

@BurtHarris Could you please try

git clone https://github.com/gulpjs/vinyl-fs -b gitattributes

and see if your line ending issues go away? Thanks!

EDITed: I pasted an ssh remote from my shell, but that's not going to work for you, so changed it to http :-)

@erikkemperman
Copy link
Member

For what it's worth, vinyl-fs the tool (as opposed to vinyl-fs the git repo) doesn't enforce any line ending style. It's just that this fixture is in the repo with unix style \n and thus we are comparing its contents as such:
https://github.com/gulpjs/vinyl-fs/blob/master/test/utils/test-constants.js#L38

@BurtHarris
Copy link
Contributor Author

BurtHarris commented Aug 1, 2017

Yes, that helps, thanks @erikkemperman. After cloning that branch (into a separate directory) the working tree has Unix style line-endings, which avoids this problem, and #263 as well. [It interesting that I can't get the same effect by doing a git checkout gitattributes (unless I delete all files in the working tree.)]

I'd still suggest that a similar change be made to .editorconfig as part of the fix, that will help to maintain consistency.

I had misread the test code and thought it was reading content from the file system rather than from the string constant in test-constants.js. Given how the test is implemented, I agree the code being tested isn't enforcing a particular style. Thanks for taking the time to point that out.

There are still a few tests failures on my machine, but I'll detail the current set under Issue #266, most now seem to be related to symlink.

@BurtHarris
Copy link
Contributor Author

BurtHarris commented Aug 2, 2017

P.S. I found details on windows-specific git settings that indicate that the core.autocrlf is getting set to true as a default on git for Windows distribution. There's a lengthy discussion in git-for-windows/git#881 about this.

@BurtHarris BurtHarris changed the title line ending changed by not-owned test line ending changed after running not-owned test Aug 2, 2017
@erikkemperman
Copy link
Member

@BurtHarris It's expected that just updating gitattributes doesn't work, this setting affects the transfer between the client and the remote, which has already been done for an existing clone. That's why I asked you to make a new one.

I've adapted the .editorconfig per your suggestion and made a PR.

I'll close this issue now, I propose we continue discussion on the others.

erikkemperman added a commit to erikkemperman/vinyl-fs that referenced this issue Oct 4, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants