Wireshark mailing list archives

Re: Git and line endings


From: Bálint Réczey <balint () balintreczey hu>
Date: Tue, 4 Feb 2014 15:20:51 +0100

Hi,

2014-02-04 Pascal Quantin <pascal.quantin () gmail com>:
2014-02-04 Bálint Réczey <balint () balintreczey hu>:

Hi,


2014-02-04 Pascal Quantin <pascal.quantin () gmail com>:



2014-02-04 Graham Bloice <graham.bloice () trihedral com>:

On 4 February 2014 11:18, Pascal Quantin <pascal.quantin () gmail com>
wrote:

2014-02-04 Graham Bloice <graham.bloice () trihedral com>:


On 3 February 2014 22:50, Pascal Quantin <pascal.quantin () gmail com>
wrote:

Hi all,

with subversion we were using the native eol-style property. Now
that
we moved to git, would it make sense to commit a .gitattributes file
with
text=auto to avoid any issue between Linux and Windows development
boxes?
I faced tonight an issue with asn2wrs.py that generated the ASN.1
dissector with CRLF line endings on my Windows machine (due to the
use of
Python open() function) while my checkout had LF line endings,
leading to
completely different files from git point of view.


There seems to be a lot of confusion around this aspect, possibly
because Windows is usually a second class citizen in the git world.

Some other useful links:


http://stackoverflow.com/questions/170961/whats-the-best-crlf-handling-strategy-with-git
https://help.github.com/articles/dealing-with-line-endings
http://adaptivepatchwork.com/2012/03/01/mind-the-end-of-your-line

Even when the .gitattributes is correctly set it seems that someone
will
have to re-normalise the repo (as in the GitHub article).


Yes, I played a bit with .gitattributes after sending my email and
could
easily shoot me in the foot...
I guess we cannot (should not) expect to have all Windows contributors
to
set the core.autocrlf=true setting before cloning Wireshark.
Still I do not see any easy solution:
- do one big commit introducing .gitattributes and re-normalising the
repo, OR
- put a big fat warning somewhere (in the documentation ?) stating
that
Windows users should set core.autocrlf=true before cloning, OR
- modify all scripts like asn2wrs.py so that they are less smart and
ouput LF line endings (assuming it is possible) and expect peaople
that they
did not activate autocrlf (sigh...)

Only first option seems good to me (maybe there is a 4th option that I
missed?).
I just did a dry run and following the re-normalize procedure
described
on github only wireshark.sln file is impacted so it is not a massive
commit.
If there is a consensus I will do the change.



I know others differ, but IMHO the wireshark.sln file could be deleted
anyway. If the re-normalisation isn't too bad then go for that.

+1


I'm a bit confused by the asn2wrs issues though.  If they produce the
platform specific line endings for their output, then won't those be
normalised by git on commit (if the correct settings for git are
applied)?

On Linux asn1wrs.py produces LF line endings, while on Windows it
produces
CRLF line endings.
Without having .gitattributes or core.autocrlf setting set to true
locally,
my working directory is having files with LF line endings but as soon as
I
want to update an ASN.1 dissector the file written is with CRLF. I have
to
manually change it back to LF...
On Linux, both the working copy and asn2wrs.py output have LF line
endings.
I expect that we will have similar issues with other scripts than
asn2wrs.py, thus my idea to do what seems to be the equivalent of the
subversion eol-style native property.

Pascal.

PS: I do not like that much playing with line ending conversion like
this,
but this is how our repository was working previously and I do not
master
all the subtleties to get rid of it
I made a quick test and converting every file with CRLF-s to LF using
the script at

http://stackoverflow.com/questions/1510798/trying-to-fix-line-endings-with-git-filter-branch-but-having-no-luck/1511273#1511273

Yielded a diff reasonable size:
commit 57bdccf83b9a6fc2d8db34e526d6635cd0eae207
Author: Balint Reczey <balint () balintreczey hu>
Date:   Tue Feb 4 14:10:41 2014 +0100

    Fixed crlf issue

 adns_dll.dep                              |  170
+++++++++++++-------------
 adns_dll.rc                               |  220
+++++++++++++++++-----------------
 epan/dissectors/packet-isakmp.c           |    2 +-
 epan/dissectors/pidl/initshutdown.cnf     |    2 +-
 epan/enterprise-numbers                   |    6 +-
 image/expert_indicators.svg               |  854

+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++------------------------------------------------------------------
 image/filetap.rc.in                       |   68 +++++------
 image/toolbar/capture_interfaces_16.svg   |   82 ++++++-------
 image/toolbar/capture_interfaces_24.svg   |   86 +++++++-------
 image/toolbar/capture_options_16.svg      |   58 ++++-----
 image/toolbar/capture_options_24.svg      |   60 +++++-----
 image/toolbar/capture_options_alt1_16.svg |   74 ++++++------
 image/toolbar/capture_options_alt1_24.svg |   74 ++++++------
 image/toolbar/capture_pause_16.svg        |   66 +++++------
 image/toolbar/capture_pause_24.svg        |   66 +++++------
 image/toolbar/capture_restart_16.svg      |   76 ++++++------
 image/toolbar/capture_restart_16_alt1.svg |   56 ++++-----
 image/toolbar/capture_restart_16_alt2.svg |   62 +++++-----
 image/toolbar/capture_restart_16_alt3.svg |   56 ++++-----
 image/toolbar/capture_restart_16_alt4.svg |   32 ++---
 image/toolbar/capture_restart_24.svg      |   74 ++++++------
 image/toolbar/capture_restart_24_alt1.svg |   56 ++++-----
 image/toolbar/capture_restart_24_alt2.svg |   62 +++++-----
 image/toolbar/capture_restart_24_alt3.svg |   56 ++++-----
 image/toolbar/capture_restart_24_alt4.svg |   32 ++---
 image/toolbar/capture_stop_16_alt1.svg    |   50 ++++----
 image/toolbar/capture_stop_24_alt1.svg    |   50 ++++----
 plugins/opcua/plugin.rc.in                |   68 +++++------
 profiles/Bluetooth/colorfilters           |    2 +-
 profiles/Classic/colorfilters             |    2 +-
 wireshark.sln                             |  350
+++++++++++++++++++++++++++---------------------------
 31 files changed, 1486 insertions(+), 1486 deletions(-)

Funny, with a simple .gitattributes containing only "* text=auto" and the
commands given in
https://help.github.com/articles/dealing-with-line-endings#re-normalizing-a-repository
on Windows, only wireshark.sln was modified. But with the recipe from
http://git-scm.com/docs/gitattributes I get the same result as you.
OK, let's go with "* text=auto" and don't convert the files now.
As Graham suggested wireshark.sln can stay CRLF, since it won't be
ever used on other OS-es.



I think we should do that and regarding the generated files we should
simply remove them from the repo.

This can be handled separately (if this is an issue at all): with
.gitattributes ther is no more issue as Windows clones are in CRLF mode and
Linux ones are in LF mode.
I think storing generated files is an issue on its own and I agree
with handling it separately.



If some of the developers does not set line ending properly on Windows
it cause no harm because
those problems can be easily spotted on Gerrit possibly by automated
scripts


Let cross fingers ;)
+1  :)

Cheers,
Balint
___________________________________________________________________________
Sent via:    Wireshark-dev mailing list <wireshark-dev () wireshark org>
Archives:    http://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
             mailto:wireshark-dev-request () wireshark org?subject=unsubscribe

Current thread: