Drake: Some files in Drake have Windows-style newlines

Created on 2 Aug 2016  路  13Comments  路  Source: RobotLocomotion/drake

For instance, drake/systems/controllers/controlUtil.cpp

We should zap those.

backlog cleanup

All 13 comments

FYI:

$ git grep -lI $'\015' | wc -l
80

There are more files to scrub, but maybe after that is done, we should implement a commit hook to reject DOS-style endings, at least for certain file types?

I _think_ this can be done by providing a .gitattributes file at the drake-distro level. Github sez this.

The default git pre-commit hook will reject whitespace "errors". Besides that it's probably nice to use this anyway, I _think_ this will also catch wrong line endings. (A minor down-side is that it will also squawk if you make changes to a file already committed with wrong line endings, so someone doing so needs to first commit a change to fix the line endings before committing "real" work. This should be unusual though if we commit a mass-fix before a change turning on the extra check.)

The other subtlety to keep in mind is that I have left the third-party sources as-is. It should be the case that changes to those should be rare, but they may need to be whitelisted in any configuration or commit-hook approach.

Do the thirdparty upstreams have "correct" line endings in their repositories? If yes, I would fix the thirdparty stuff also. I don't think we should keep DOS line endings anywhere if they only exist because someone committed stuff from Windows, or from copying files from Windows.

Note: if I end up updating tinyxml (see #3065), I expect the updated files are going to have "correct" line endings...

I suppose I don't care if we fix line endings in thirdParty. I'll invite the opinion of resident license-cop @jwnimmer-tri.

By my count, the only remaining text files with CRLF are in thirdParty, and are mostly matlab sources:

$ git grep -l ^M |egrep -v '\.(pdf|png|jpg|STL|stl|mat|jar|mex.*[0-9])$'
drake/thirdParty/bsd/+toolbox_graph/read_gim.m
drake/thirdParty/bsd/+toolbox_graph/read_mesh.m
drake/thirdParty/bsd/+toolbox_graph/read_off.m
drake/thirdParty/bsd/+toolbox_graph/read_ply.m
drake/thirdParty/bsd/+toolbox_graph/read_smf.m
drake/thirdParty/bsd/+toolbox_graph/read_vtk.m
drake/thirdParty/bsd/+toolbox_graph/read_wrl.m
drake/thirdParty/bsd/arrow3d/arrow3d.m
drake/thirdParty/bsd/cprintf/cprintf.m
drake/thirdParty/bsd/plotregion/examples/example1.m
drake/thirdParty/bsd/plotregion/examples/example2.m
drake/thirdParty/bsd/plotregion/examples/example3.m
drake/thirdParty/bsd/plotregion/examples/example4.m
drake/thirdParty/bsd/plotregion/examples/example5.m
drake/thirdParty/bsd/plotregion/examples/example6.m
drake/thirdParty/bsd/plotregion/examples/example7.m
drake/thirdParty/bsd/plotregion/plotregion.m
drake/thirdParty/bsd/polytopes/lcon2vert.m
drake/thirdParty/bsd/polytopes/qlcon2vert.m
drake/thirdParty/bsd/polytopes/vert2lcon.m
drake/thirdParty/bsd/psm/CGL_Dmatrix.m
drake/thirdParty/bsd/psm/CGL_nodes.m
drake/thirdParty/bsd/psm/CGL_weights.m
drake/thirdParty/bsd/psm/LGL_Dmatrix.m
drake/thirdParty/bsd/psm/LGL_nodes.m
drake/thirdParty/bsd/psm/LGL_weights.m
drake/thirdParty/zlib/tinyxml2/tinyxml2.cpp
drake/thirdParty/zlib/tinyxml2/tinyxml2.h

egrep -v '\.(pdf|png|jpg|STL|stl|mat|jar|mex.*[0-9])$'

Assuming your intent is to avoid "binary data" files, the -I option to grep / git grep is much less typing :smile:.

I suppose I don't care if we fix line endings in thirdParty. I'll invite the opinion of resident license-cop @jwnimmer-tri.

The only question is of course the zlib license (not bsd), and marking modified works. If we are just getting new files from upstream and pushing them into our repo, there's of course no problem. If we are fixing newlines ourselves, that is beyond my lawyercat ability to make a ruling on.

I will happily assert that fixing line endings does not count as "modification".

Also: https://github.com/leethomason/tinyxml2/issues/462 :smile:

I think this issue is finished and can be closed now, @rpoyner-tri?

The only open question is some sort of automated flagging/rejection/fixing of DOS line endings via git hook or git configuration. I'd like to delegate that bit to kitware if I may; perhaps close this and open a new issue for automation?

Closing, automation bit forwarded to #3150.

Was this page helpful?
0 / 5 - 0 ratings