-
-
Notifications
You must be signed in to change notification settings - Fork 365
Fixed line endings in 'largest-series-product' files #461
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
Conversation
Mmm. Okay, I'm reading git's messages wrongly. Thanks! I'll see and check what really happened. |
Okay, this time I'm pretty sure I fixed them. @jpreese could you corroborate? :) |
I think I would just need to do some research into this topic. Right now, all of my line endings are in CRLF. So from my point of view, it doesn't make a whole lot of sense to merge this in. Two files would have LF line endings, and the rest would be CRLF. Maybe @ErikSchierboom or @robkeim are more versed in Git and line endings across operating systems? |
Wait, what? Really? My guess is that, since
That must mean that But yeah, given how the repo's files are in your computer, we need experts' advice here... because I don't know what's up either 🤷♂️ |
Well, actually this makes total sense, as this is precisely what the And now for the problem @felix91gr is having. Are you having that problem on checkout or only when running the test generator? One problem might be that our test generator doesn't output line endings in the host OS'es native line ending format. For example, at a couple of places we use a hardcoded My suggestion is as follows: @felix91gr can you confirm that the problem only happens after running the test generator? |
I just generated all of the tests and looked at some \n cases with the ValueFormatter, and all of my newlines were still CRLF. We output \n a lot for string variables, but that shouldn't impact be a true LF in the file itself. |
@ErikSchierboom so this is what I get when I just do checkout of git clone https://github.com/exercism/csharp/ csharp_checkout
Cloning into 'csharp_checkout'...
remote: Counting objects: 4990, done.
remote: Compressing objects: 100% (34/34), done.
remote: Total 4990 (delta 15), reused 27 (delta 9), pack-reused 4947
Receiving objects: 100% (4990/4990), 1.67 MiB | 1.23 MiB/s, done.
Resolving deltas: 100% (3021/3021), done.
Checking connectivity... done.
~/D/P/exercism $ cd csharp_checkout/
~/D/P/e/csharp_checkout (master) $ git status
On branch master
Your branch is up-to-date with 'origin/master'.
Changes not staged for commit:
(use "git add <file>..." to update what will be committed)
(use "git checkout -- <file>..." to discard changes in working directory)
modified: exercises/largest-series-product/Example.cs
modified: exercises/largest-series-product/LargestSeriesProductTest.cs
no changes added to commit (use "git add" and/or "git commit -a")
~/D/P/e/csharp_checkout (master) $ file exercises/largest-series-product/Example.cs
exercises/largest-series-product/Example.cs: ASCII text, with CRLF line terminators
~/D/P/e/csharp_checkout (master) $
file exercises/largest-series-product/LargestSeriesProductTest.cs
exercises/largest-series-product/LargestSeriesProductTest.cs: ASCII text, with very long lines, with CRLF line terminators
~/D/P/e/csharp_checkout (master) $ The Okay, now after running the generators: ~/D/P/e/csharp_checkout (master) $ cd generators/
~/D/P/e/c/generators (master) $ dotnet run
[17:11:01 INF] Re-generating test classes...
[17:11:01 INF] Updating repository to latest version...
[17:11:06 INF] Updated repository to latest version.
[17:11:06 INF] Generated tests for acronym exercise in ../exercises/acronym/AcronymTest.cs
[17:11:06 INF] Generated tests for allergies exercise in ../exercises/allergies/AllergiesTest.cs
.
.
.
[17:11:07 INF] Generated tests for complex-numbers exercise in ../exercises/complex-numbers/ComplexNumbersTest.cs
.
.
.
[17:11:07 INF] Generated tests for isogram exercise in ../exercises/isogram/IsogramTest.cs
[17:11:07 INF] Generated tests for kindergarten-garden exercise in ../exercises/kindergarten-garden/KindergartenGardenTest.cs
// Notice how the LargestSeriesProduct isn't generated
// because the exercise generator doesn't exist yet.
[17:11:07 INF] Generated tests for leap exercise in ../exercises/leap/LeapTest.cs
.
.
.
[17:11:07 INF] Generated tests for saddle-points exercise in ../exercises/saddle-points/SaddlePointsTest.cs
.
.
.
[17:11:08 INF] Generated tests for wordy exercise in ../exercises/wordy/WordyTest.cs
[17:11:08 INF] Re-generated test classes.
~/D/P/e/c/generators (master) $ cd ..
~/D/P/e/csharp_checkout (master) $ git status
On branch master
Your branch is up-to-date with 'origin/master'.
Changes not staged for commit:
(use "git add <file>..." to update what will be committed)
(use "git checkout -- <file>..." to discard changes in working directory)
modified: exercises/complex-numbers/ComplexNumbersTest.cs
modified: exercises/isogram/IsogramTest.cs
modified: exercises/largest-series-product/Example.cs
modified: exercises/largest-series-product/LargestSeriesProductTest.cs
modified: exercises/saddle-points/SaddlePointsTest.cs
no changes added to commit (use "git add" and/or "git commit -a")
~/D/P/e/csharp_checkout (master) $
file exercises/largest-series-product/LargestSeriesProductTest.cs
exercises/largest-series-product/LargestSeriesProductTest.cs: ASCII text, with very long lines, with CRLF line terminators
~/D/P/e/csharp_checkout (master) $ file exercises/largest-series-product/Example.cs
exercises/largest-series-product/Example.cs: ASCII text, with CRLF line terminators
~/D/P/e/csharp_checkout (master) $
file exercises/complex-numbers/ComplexNumbersTest.cs
exercises/complex-numbers/ComplexNumbersTest.cs: ASCII text, with CRLF, LF line terminators
~/D/P/e/csharp_checkout (master) $ file exercises/isogram/IsogramTest.cs
exercises/isogram/IsogramTest.cs: ASCII text
~/D/P/e/csharp_checkout (master) $ file exercises/saddle-points/SaddlePointsTest.cs
exercises/saddle-points/SaddlePointsTest.cs: ASCII text, with CRLF, LF line terminators As you can see, the tests are generated weirdly. We have a variety of line endings:
Also, have in mind that the
|
If we want to squash this problem once and for all (normalizing on
Does that seem reasonable? |
@robkeim I agree with the approach, though I'm still lost on how it only impacts a select few. @ErikSchierboom wasn't able to reproduce on his Mac, which doesn't use CRLF either. Have we pinpointed the source of the problem? I'd love to be able to reproduce it on my end. Even if I need to spin up a docker container if this only is a linux problem. |
Well that's an easy question, as we haven't always had a I like the approach by @robkeim, but perhaps we can do one other modification. We can use the steps proposed to normalize all the line-endings in the Git repository. However, once that is committed I would then like to see if changing the line-ending format back to The first step will thus be to normalize the line-endings in Git. The following document might help: https://help.github.com/articles/dealing-with-line-endings/#refreshing-a-repository-after-changing-line-endings |
Another document about that problem: You're just another carriage return line feed in the wall |
FWIW the linked article actually has the LF normalization approach crossed out. I'm still a little lost on exactly what the problem is. Bits and pieces make sense, but there's some details that seem unclear.
How is @felix91gr getting CRLF on a Linux machine without even running the generators? I can see how this can be problematic when forcing \r\n in a generator. The generator is adding the \r\n whereas if the user on a Linux box were to do that, it would only be \n. So text=auto is trying to change it to LF. Which kind of leads to my next question, if I understand how gitattribute works, if we're on a Windows machine with * text=auto, does Git actually store the files as LF? And when we check these files out in Windows they are made into CRLF, only for our machines. |
I have no idea how to check that. |
When I get some time, I'm going to try and spin up a Linux container and see if I can reproduce this. Though it's a little strange that it doesn't break on your Mac either. |
Regarding files that are problematic before running the generator(Erik's)
Yes! That seems to be exactly what's happenning. Check this out: So, at least those files should be able to be changed such that they are normalized automatically to I guess that answers @jpreese's question :) Other things(jpreese's)
Please do! Even if it's unlikely that my setup has a unique bug (I'm using an almost pure Ubuntu derivative), we should be trying to reproduce this on as many machines as possible to, as you say, pinpoint the source.
Yeah... weird. But Macs are sometimes magical like that. Or maybe just a Mac user saw this problem a long time ago and made a PR to the Mac branch of Regarding the files generated with
|
@jpreese, have you downloaded the latest commit of this PR? Maybe we could check if Windows's |
So, on a windows machine, when you commit, does gitattributes turn all CRLF's into LF? That makes a little more sense where the commit hash contains changes to the file in LF format.. but before the time of gitattributes, it was committed as CRLF? I guess I'm just stuck on the fact that right now LargestSeriesProduct is CRLF (as is everything else).. so when @felix91gr pulls this down, his line endings should be changed to LF anyway. Why is this one file special. |
Our guess is that it's because those two files were created before the If this hypothesis is true, two things should be happenning:
Wanna check? :) |
Test resultsNormalization worked here :) git clone https://github.com/felix91gr/csharp csharp_check_LF
Cloning into 'csharp_check_LF'...
remote: Counting objects: 4981, done.
remote: Compressing objects: 100% (12/12), done.
remote: Total 4981 (delta 4), reused 10 (delta 4), pack-reused 4965
Receiving objects: 100% (4981/4981), 1.67 MiB | 857.00 KiB/s, done.
Resolving deltas: 100% (3022/3022), done.
Checking connectivity... done.
~/D/P/exercism $ cd csharp_check_LF
~/D/P/e/csharp_check_LF (master) $ git checkout fix-line-endings-1-commit
Branch fix-line-endings-1-commit set up to track remote branch fix-line-endings-1-commit from origin.
Switched to a new branch 'fix-line-endings-1-commit'
~/D/P/e/csharp_check_LF (fix-line-endings-1-commit) $ git status
// No complains! YES!
On branch fix-line-endings-1-commit
Your branch is up-to-date with 'origin/fix-line-endings-1-commit'.
nothing to commit, working directory clean
~/D/P/e/csharp_check_LF (fix-line-endings-1-commit) $
file exercises/largest-series-product/LargestSeriesProductTest.cs
// No CRLF! :D
exercises/largest-series-product/LargestSeriesProductTest.cs: ASCII text, with very long lines
~/D/P/e/csharp_check_LF (fix-line-endings-1-commit) $
file exercises/largest-series-product/Example.cs
// No CRLF either! :)
exercises/largest-series-product/Example.cs: ASCII text If point (1.) works on your computer @jpreese, then we've solved the mystery for the |
Ok so if you checkout the |
Yup! In
Yes, as far as I can test without running over all of the files one by one. My EDITWell, also the |
When you say "git seems happy", that means that the files that you have checked, use strictly LF? |
No... what I mean by that is that git doesn’t complain. When I invoke git status after a checkout, it should say “working directory clean, no changes to commit”. A checkout of master introduces changes in those two files without me doing anything. And I can’t cancel those changes either. A checkout of this branch however, doesn’t trigger those changes. And it also leaves me with LF endings where they should be. If this change is good, it should be similar for you:
|
Okay, so if I understand correctly, this PR the issues? |
It should, yeah. I just really don't see how the timing of a gitattributes file impacts these files. Before the gitattributes, this file and all of the other files had CRLF. When you introduce gitattributes, it still has that CRLF. Windows users commit as CRLF, Linux users commit as LF. But the endings need to be stored on orgin in one of those (unless it just flips back and forth the gitattributes ignores the line endings so it doesn't show up in the diff?). Theres just some processes happening on the back end that I'm missing. I'll probably just have to spend some time playing with different OS's and commits with gitattributes, I personally still don't quite understand how this is occuring in the first place. |
@jpreese I know what you mean. Git is kind of a mess... Linus Torvalds laughed at SVN, saying he programmed git in two weeks and still managed to kick their ass. But such speed comes with a lot of “magic” like this and bad ergonomics like the gitless researchers have explored. I have no answer as to why this is happenning. But maybe we can fix it... try this PR and tell me what you see :) |
Yeah, it fixes it. But I'm a stickler for knowing why its a problem in the first place. I'm not saying we can't merge it, I just need to figure out the source of the problem. |
Oh, shit dawg! Good thing you tested it. Then there must be something weird in my setup, let's see:
~ $ cat /etc/*-release
DISTRIB_ID="elementary"
DISTRIB_RELEASE=0.4.1
DISTRIB_CODENAME=loki
DISTRIB_DESCRIPTION="elementary OS 0.4.1 Loki"
NAME="elementary OS"
VERSION="0.4.1 Loki"
ID="elementary"
ID_LIKE=ubuntu
PRETTY_NAME="elementary OS 0.4.1 Loki"
VERSION_ID="0.4.1"
HOME_URL="http://elementary.io/"
SUPPORT_URL="http://elementary.io/support/"
BUG_REPORT_URL="https://github.com/elementary/"
VERSION_CODENAME=loki
UBUNTU_CODENAME=loki
cat: /etc/upstream-release: Is a directory |
Now that I’ve looked it up, the latest git is 2.14. Shhhhhhhhhhhhhhhhhhit. I have the 2.7.4! XD that’s probably the problem. Which one do you have? |
So assuming that fixes it for you, this makes a lot more sense to me. I still have no idea why that version of git was picky. I could try downgrading my Git on my Ubuntu container down to 2.7.4 and see if it errors there, but at least its a Git version thing. What I was trying to say above is that your situation just didn't make any sense at all. @ErikSchierboom couldn't replicate, I couldn't replicate, even on a Linux box. So it must be with how old versions of Git handles .gitattributes |
You did it, man. You found the answer. And now... rm -rf csharp_clean/
~/D/P/exercism $ git clone https://github.com/exercism/csharp/ csharp_clean
Cloning into 'csharp_clean'...
remote: Counting objects: 4990, done.
remote: Compressing objects: 100% (34/34), done.
remote: Total 4990 (delta 15), reused 27 (delta 9), pack-reused 4947
Receiving objects: 100% (4990/4990), 1.67 MiB | 1.85 MiB/s, done.
Resolving deltas: 100% (3021/3021), done.
~/D/P/exercism $ cd csharp_clean
~/D/P/e/csharp_clean (master) $ git status
On branch master
Your branch is up-to-date with 'origin/master'.
nothing to commit, working tree clean
|
So, whatever with this PR? Or should we merge it? |
Yup, indeed. What we have left is to check the generated files, because maybe there’s an issue there (with the Environment.Newline or something). But the issue with the preexisting files is no more :3 thanks, jp! (I’ll be back tomorrow. Night!’) |
Great research work! |
Yeah my next step is to see what changed between the versions. Again, to me, I'm curious how gitattributes knows that the LargestSeries file was added before it. I'm curious if gitattributes does something to the commit hash, so it knows that it should transform it. So LargestSeries is a problem because it was added before gitattributes and doesn't have that special something. I don't know how else gitattributes would know about it. This is all me talking aloud 🤣 Though I'm pretty sure there are more files than just those two that were added before gitattributes, unless they've just been updated over time. Who knows. |
I know right? Go home git, you’re drunk 😆
There are no more in that directory, I’ve checked them. But maybe there are in the other folders. Do you know of a quick way to check which files were last modified before a certain date? There are too many to be checking by hand :v |
Good detective work tracking this down @felix91gr and @jpreese! @felix91gr this should give you a list of the last modification date for all of the files: The output format seems to be already sorted by date so that should give you what you're looking for. |
Thank you, @robkeim! Using that script + regexes, I made this list of files that were last modified before or at the same time than
Now I'm gonna loop over them and see what |
Final verdict on old filesLooping over all of the files that preceded
And those are all of them. So, @jpreese: the error appears to occur only if:
Do you agree with me? |
Yeah, if you wanted to dig some more that'd be great. I won't be able to check it out till later. I'm curious if the date that gitattributes was added matters. There's just too much of a coincidence. |
After the above investigations, this should be resolved with newer versions of Git. I'd prefer to just stay with auto. |
👍🏻
…On Mon, Oct 9, 2017 at 10:12 PM John Reese ***@***.***> wrote:
Closed #461 <#461>.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#461 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/ALNBJ4RV9TdmQT5idESueQVUYPD4pmsbks5sqsR6gaJpZM4PuhSs>
.
|
As per #452
Now they have just
LF