View Issue Details

IDProjectCategoryView StatusLast Update
0004212Slicer4Core: Building (CMake, Superbuild)public2017-11-01 08:57
ReportermsmolensAssigned Tojcfr 
PrioritynormalSeverityminorReproducibilityalways
Status assignedResolutionopen 
PlatformOSWindowsOS Version
Product VersionSlicer 4.6.0 
Target VersionFixed in Version 
Summary0004212: 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

TagsNo tags attached.

Activities

inorton

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.

Issue History

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