Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

normalise and enforce correct line endings #61

Closed
adamralph opened this Issue · 9 comments

3 participants

@adamralph
Owner

See https://help.github.com/articles/dealing-with-line-endings

I've still seen a few files cropping up with mixed line endings even when using autocrlf=true, which is strange.

I think if we do a one-off normalisation and enforcement using gitattributes then we should kill this problem once and for all.

Note that we need to run a one-off build on the build server with 'clean all files in checkout directory before build' switched on after making this change.

@philippdolder

looks like a solution to me.
@danielmarbach just did that for @appccelerate. see danielmarbach/appccelerate@afa0b34

@danielmarbach

Hy, yes gitattributes solves this. And it let's you define how git should treat various files in the repo. Important is this

$ define gitattributes in .gitattributes
$ rm .git/index # Remove the index to force git to
$ git reset # re-scan the working directory
$ git status # Show files that will be normalized
$ git add -u
$ git add .gitattributes
$ git commit -m "Introduce end-of-line normalization"

Done

@adamralph
Owner

Thanks @philippdolder and @danielmarbach.

BTW - the workflow suggested by GitHub is the following. I guess it would have an equivalent outcome.

$ git add .gitattributes
$ git commit -m "Add gitattributes"
$ git rm --cached -r .
$ git reset --hard
$ git add .
$ git status
$ git commit -m "Normalize line endings"
@adamralph
Owner

I guess the only thing we need to decide now is what our policy is.

Before the proposal to add gitattributes, we'd decided to stick with autocrlf=true to save any awkward transition and risk of people not making the switch. However, if we are going to bake the setting into the repo then we have the freedom to choose whatever policy we want.

My vote still goes with no line ending tampering at all (equivalent of autocrlf=false). I used to prefer autocrlf=true but I had a discussion with the http://nancyfx.org/ guys about this, who are of the strong opinion that false is best, and they persuaded me to their view. More on this at #36 (comment)

If I'm not mistaken, with gitattributes this could be achieved by the file contents being simply

* binary

I.e. treat every file as binary and don't attempt to change any line endings. I think the simplicity of this setting is an advantage in itself, i.e. if wanted to use line ending conversion then we'd have the maintenance overhead of a file such as https://github.com/danielmarbach/appccelerate/blob/master/.gitattributes

@adamralph
Owner

OK, this doesn't work

# .gitattributes
* binary

because binary is a macro which expands to -text -diff. The -diff attribute tells git not to perform text diffs, which is definitely not desired.

However

# .gitattributes
* -text

does the trick.

From http://git-scm.com/docs/gitattributes#_effects

Unsetting the text attribute on a path tells git not to attempt any end-of-line conversion upon checkin or checkout.

I followed the steps suggested by GitHub and now I have files with both types of line endings in my clone, i.e. no conversions have taken place and I see the real line endings which are in the repo.

I also have no further changes to commit since no normalization at all is required. This is another advantage of this approach.

@adamralph adamralph referenced this issue from a commit in adamralph/FakeItEasy
@adamralph adamralph #61 added .gitattributes 636cf08
@adamralph
Owner

To clarify, the above commit is just a spike. This issue won't be Ready until we've reached agreement on our policy.

@philippdolder

I'm in for that way of treating line endings

@danielmarbach
@adamralph
Owner

Something to note regarding this. If we do switch off line ending conversion we'll have to do a one off fix of any files which contains a mix of LF and CRLF. Otherwise VS seems to decide to align the line endings of it's own accord when you edit a file and this can be very disruptive.

@adamralph adamralph was assigned
@adamralph adamralph referenced this issue from a commit in adamralph/FakeItEasy
@adamralph adamralph #61 added .gitattributes d5927fb
@adamralph adamralph referenced this issue from a commit in adamralph/FakeItEasy
@adamralph adamralph #61 converted all LF files to CRLF
 - moved all binaries out of repo into a temp folder
 - replaced CRLF with LF in all files
 - replaced LF with CRLF in all files
 - moved binaries back into repo
26801c8
@adamralph adamralph referenced this issue from a commit in adamralph/FakeItEasy
@adamralph adamralph #61 updated CONTRIBUTING.md with new EOL policy 43c963e
@adamralph adamralph closed this in #117
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.