0004212: git core.autocrlf=true option causes erroneously modified source files and test failures - MantisBT

ID	Project	Category	View Status	Date Submitted	Last Update

0004212	Slicer4	Core: Building (CMake, Superbuild)	public	2016-06-17 15:35	2017-11-01 08:57

Reporter	msmolens	Assigned To	jcfr
Priority	normal	Severity	minor	Reproducibility	always
Status	assigned	Resolution	open
Platform		OS	Windows	OS Version
Product Version	Slicer 4.6.0
Target Version		Fixed in Version

Summary	0004212: git core.autocrlf=true option causes erroneously modified source files and test failures
Description	Using Git for Windows (http://git-for-windows.github.io) $ git config --list --show-origin ... file:"C:\ProgramData/Git/config" core.autocrlf=true ... When the core.autocrlf option is true, git coerces Utilities/Scripts/SlicerWizard/version.py to have CRLF line endings. Building Slicer runs CMake code that writes that same file with LF line endings [1]. Now git sees the file as being modified, because the line endings have changed. One solution could be to add a .gitattributes file that specifies to always checkout Utilities/Scripts/SlicerWizard/version.py as 'text eol=lf'. See https://help.github.com/articles/dealing-with-line-endings/. Several similar problems occur when running tests: Testing/Data/Input/ExecutionModelTourTest.mrml is shown as modified Some tests fail because the tests generate output files with LF line endings, but the checked out baseline files have been coerced to have LF line endings. See test MergeModelsTestCompare and file Modules/CLI/MergeModels/Data/Baseline/sphereCube.vtp. [1] https://github.com/Slicer/Slicer/commit/765eb8155f7d02e2958aeb343bb76d2afbe5c5e9 (BUG: Fixed constantly conflicting version.py)
Steps To Reproduce	On Windows, install Git for Windows from http://git-for-windows.github.io. I believe the default install options configure core.autocrlf=true. Verify that using 'git config --list'. Checkout Slicer, build, and test. Results: $ git status modified: Testing/Data/Input/ExecutionModelTourTest.mrml modified: Utilities/Scripts/SlicerWizard/version.py $ ctest -C Release -R MergeModelsTestCompare -V ... 470: CMake Error at C:/path/to/Slicer/Modules/CLI/MergeModels/Testing/run_MergeModelsTest.cmake:34 (message): 470: C:/path/to/SR/Slicer-build/Testing/Temporary/sphereCube.vtp does not match 470: C:/path/to/Slicer/Modules/CLI/MergeModels/Testing/../Data/Baseline/sphereCube.vtp! ... $ file /path/to/SR/Slicer-build/Testing/Temporary/sphereCube.vtp /path/to/Slicer/Slicer-build/Testing/Temporary/sphereCube.vtp: XML 1.0 document, ASCII text, with very long lines $ file /path/to/Slicer/Modules/CLI/MergeModels/Data/Baseline/sphereCube.vtp /path/to/Slicer/Modules/CLI/MergeModels/Data/Baseline/sphereCube.vtp: XML 1.0 document, ASCII text, with very long lines, with CRLF line terminators
Tags	No tags attached.

inorton 2017-11-01 08:46 developer ~0015372 Last edited: 2017-11-01 08:57 View 2 revisions	I was also seeing this with CMake 3.9.4 for the extension description test, until I changed to core.autocrlf=input and reset the index. That's not ideal, but I don't think there's any better option until this CMake issue is resolved and we bump to that version (so not for several years at least): https://gitlab.kitware.com/cmake/cmake/issues/13007 I assume the dashboards must configured to use core.autocrlf=input (or false?) otherwise these text comparison tests would fail... Should we add a .gitattributes to make everyone use `autocrlf=input` by default and for the whole project? That's a big hammer and will probably create some confusion for visual studio users.

Date Modified	Username	Field	Change
2016-06-17 15:35	msmolens	New Issue
2016-06-17 15:35	msmolens	Status	new => assigned
2016-06-17 15:35	msmolens	Assigned To	=> jcfr
2016-06-17 15:36	msmolens	Steps to Reproduce Updated	View Revisions
2016-06-17 15:37	msmolens	Steps to Reproduce Updated	View Revisions
2016-06-17 15:55	msmolens	Steps to Reproduce Updated	View Revisions
2016-06-17 15:56	msmolens	Steps to Reproduce Updated	View Revisions
2017-11-01 08:46	inorton	Note Added: 0015372
2017-11-01 08:57	inorton	Note Edited: 0015372	View Revisions