arbor-sim / arbor

The Arbor multi-compartment neural network simulation library.
https://arbor-sim.org
BSD 3-Clause "New" or "Revised" License
105 stars 59 forks source link

Avoid `bash\r` with gitattributes eol #2252

Open Helveg opened 5 months ago

Helveg commented 5 months ago

Building arbor on WSL from a clone checked out with Windows line endings causes the following error:

[  0%] Generating _always_rebuild
[  2%] Generating version.hpp-test
/usr/bin/env: ‘bash\r’: No such file or directory
make[2]: *** [arbor/include/CMakeFiles/generate_version_hpp.dir/build.make:77: arbor/include/version.hpp-test] Error 127
make[2]: *** Deleting file 'arbor/include/version.hpp-test'
make[1]: *** [CMakeFiles/Makefile2:1613: arbor/include/CMakeFiles/generate_version_hpp.dir/all] Error 2
make[1]: *** Waiting for unfinished jobs....
[  2%] Building CXX object ext/units/units/CMakeFiles/units.dir/units.cpp.o
[  2%] Building CXX object ext/units/units/CMakeFiles/units.dir/commodities.cpp.o
[  4%] Building CXX object ext/units/units/CMakeFiles/units.dir/x12_conv.cpp.o
[  4%] Building CXX object ext/units/units/CMakeFiles/units.dir/r20_conv.cpp.o
[  4%] Linking CXX static library ../../../lib/libunits.a
[  4%] Built target units
make: *** [Makefile:136: all] Error 2

The shebang line in arbor/include/git-source-id of the following custom command is misinterpreted:

add_custom_command(
    OUTPUT version.hpp-test
    DEPENDS _always_rebuild
    COMMAND ${CMAKE_CURRENT_SOURCE_DIR}/git-source-id ${FULL_VERSION_STRING} ${ARB_ARCH} ${arb_config_str} ${arb_features} > version.hpp-test
)

Prepending bash directly to the custom command seems to fix the problem. I don't know how to confirm whether the version.hpp-test mechanism still works as intended, so someone should probably review that.

halfflat commented 5 months ago

An alternative would be to have git enforce a LF-only ending on scripts such as git-source-id — would this avoid the issue? Using GitHub's line ending config docs as a guide, we should be able to add lines to .gitattributes such as:

arbor/include/git-source-id text eol=lf
Helveg commented 5 months ago

That seems to work too!

thorstenhater commented 5 months ago

Lint takes issue with the stub files ... @Helveg could you fix and we'll merge?

Helveg commented 4 months ago

What needs to happen here? I didn't change those files.