Normalize line endings

[lewisakura]

This will fix quirky issues with the shell scripts such as #96.

[TheOneric]

Thanks for your effort determining the root cause of the Windows issue!

However, I'm not sure this actually fully accomplishes what you want it to. To my knowledge the .gitattributes file does (by default) not affect submodules and a test on my machine with eol=crlf seems to confirm this. Perhaps there’s another option change needed for it to do? Otherwise it may be a difference between git versions or possibly you forgot to remove core.autocrlf = false when testing the patch?
Also, this isn't really specific to “shell scripts” but anything executed via its shebang, e.g. also build/license_lint.awk.

[lewisakura]

This would be correct, however this is primarily to target the shell scripts in this repository, less so the submodules. Upstream would have to add their own attributes.

I agree that the other file types do need it however, but I'm not sure which ones quite need it.

[TheOneric]

The error in #96 occurs when invoking a shell script from a submodule via shebang. Later on it’d face the same issue with files directly in this repo, yes, but unless the ones in the submodules also work this change is pointless.

Is there a way to also automatically stop the line ending mangling in submodules? If yes, that’s good and please test with a fresh clone and core.autocrlf = true. Otherwise there’s at least one other alternative approach that could be explored.

[lewisakura]

AFAIK there's no way to do so, because that's Git's design. What alternative are you looking at?

[TheOneric]

@LewisTehMinerz: Ensure core.autocrlf = true is set (globally), create a fresh clone from upstream master from MS Windows and apply the following diff, replacing the use of shebangs with explicit interpreter calls (though in theory, nothing is stopping autogen.sh from being a script other than POSIX shell or a binary executable):

--- a/Makefile_licence
+++ b/Makefile_licence
@@ -32,7 +32,7 @@ dist/license/all: dist/license/subtitlesoctopus $(addprefix dist/license/, $(LIB
        ) :

        mv dist/license/all dist/license/all.tmp
-       build/license_lint.awk dist/license/all.tmp build/license_fullnotice
+       awk -f build/license_lint.awk dist/license/all.tmp build/license_fullnotice
        cat dist/license/all.tmp build/license_fullnotice > dist/license/all

 dist/js/COPYRIGHT: dist/license/all
diff --git a/functions.mk b/functions.mk
index 6638a88..c0a83d2 100644
--- a/functions.mk
+++ b/functions.mk
@@ -25,7 +25,7 @@ define PREPARE_SRC_VPATH
 endef

 # All projects we build have autogen.sh, otherwise we could also fallback to `autoreconf -ivf .`
-RECONF_AUTO := NOCONFIGURE=1 ./autogen.sh
+RECONF_AUTO := NOCONFIGURE=1 sh ./autogen.sh

 # @arg1: path to source directory; defaults to current working directory
 define CONFIGURE_AUTO

Then attempt to build all binaries and accompanying files as described in README.md.

[lewisakura]

sh, or most shells for that matter, will not run the scripts without CRLF conversion. CRs are completely invalid characters. We won't even make it to that stage.

$ wsl ./run-docker-build.sh
zsh:1: ./run-docker-build.sh: bad interpreter: /bin/sh^M: no such file or directory
$ wsl sh ./run-docker-build.sh
./run-docker-build.sh: 2: set: Illegal option -

Also, for note, I already tried running autogen.sh with sh directly in the Docker container during my tests, and I got the same result.

Linux & co. has no special handling for CR at all, so it just treats it as yet another character. It only handles LFs.

[TheOneric]

Well, unless you’ve got another idea, I'm afraid nothing reasonable will make this automatically work with Docker on Windows. Since the shell inside the Linux Docker Container won't accept scripts wiht CRLF line endings, the configure and autogen.sh scripts in the submodules can't ever be used. Not even with sh < file.sh.

If you have no other suggestion, I guess we could at least put a note about autocrlf in the readme. Do you know if a Windows-native shell (e.g. the one from MSys2) treats CRLF line endings like LF? If so, at least a Container-less emscripten setup might work on Windows, even with core.autocrlf = true.

[lewisakura]

I can't think of any real alternative that would work except advising everyone to turn off autocrlf or run dos2unix on every file after cloning (e.g. dos2unix **/*).

re msys2: Pretty sure the bash included with that does the same thing too.

[TheOneric]

r run dos2unix on every file after cloning (e.g. dos2unix **/*).

For the record, that's a bad idea as some submodules contain files with intentional CRLF line-endings.

[lewisakura]

Thought it would be. 🤣

[TheOneric]

Can the autocrlf setting be set in the repo scope instead of global and still work with submdoules?
I.e. does this work?:

git clone https://github.com/libass/JavascriptSubtitlesOctopus.git jso_tmp
cd jso_tmp
git config core.autocrlf false
git add --renormalize .
git submodule update --init --recursive --force