Wireshark · Wireshark-dev: Re: [Wireshark-dev] Git and line endings

[Bálint Réczey <balint@xxxxxxxxxxxxxxx>]

Hi,

2014-02-04 Pascal Quantin <pascal.quantin@xxxxxxxxx>:
> 2014-02-04 Bálint Réczey <balint@xxxxxxxxxxxxxxx>:
>
>> Hi,
>>
>>
>> 2014-02-04 Pascal Quantin <pascal.quantin@xxxxxxxxx>:
>> >
>> >
>> >
>> > 2014-02-04 Graham Bloice <graham.bloice@xxxxxxxxxxxxx>:
>> >>
>> >> On 4 February 2014 11:18, Pascal Quantin <pascal.quantin@xxxxxxxxx>
>> >> wrote:
>> >>>
>> >>> 2014-02-04 Graham Bloice <graham.bloice@xxxxxxxxxxxxx>:
>> >>>
>> >>>>
>> >>>> On 3 February 2014 22:50, Pascal Quantin <pascal.quantin@xxxxxxxxx>
>> >>>> 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@xxxxxxxxxxxxxxx>
>> 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