Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid bash\r with gitattributes eol #2252

Merged
merged 2 commits into from
Oct 2, 2024
Merged

Conversation

Helveg
Copy link
Collaborator

@Helveg Helveg commented Jan 29, 2024

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
Copy link
Contributor

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
Copy link
Collaborator Author

Helveg commented Jan 30, 2024

That seems to work too!

Copy link
Contributor

@halfflat halfflat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is the easiest fix!

@Helveg Helveg changed the title Avoid bash\r from shebang by calling bash directly on custom command Avoid bash\r with gitattributes eol Feb 19, 2024
@thorstenhater
Copy link
Contributor

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

@Helveg
Copy link
Collaborator Author

Helveg commented Mar 6, 2024

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

@thorstenhater thorstenhater merged commit 6524ce7 into master Oct 2, 2024
20 of 21 checks passed
@thorstenhater thorstenhater deleted the fix/windows-cmake branch October 2, 2024 06:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants