search

abarichello / godot-ci

Docker image to export Godot Engine games. Templates for Gitlab CI and GitHub Actions to deploy to GitLab Pages/GitHub Pages/Itch.io.
https://hub.docker.com/r/barichello/godot-ci
MIT License
759 stars 133 forks source link

Fix Dockerfile editor settings writing #158

Closed Calinou closed 1 month ago

Calinou commented 1 month ago

The script relies on Bash-specific features, so Bash must be used instead of POSIX sh or dash.

NetSysFire commented 1 month ago

Why do this hack? This could be solved with much less effort by simply changing to bash for the time being. Apparently Dockerfiles support SHELL: https://github.com/moby/moby/issues/7281#issuecomment-389440503

I'd revert all those changes and simply slap a SHELL statement that points to bash onto the Dockerfile. This way the variables and substitutions can stay as they are. Support should be fine as bash is pretty much omnipresent everywhere.

NetSysFire commented 1 month ago

I'd revert RUN <<EOF, too because that renders you unable to place proper comments, which you even had to remove for this. I think you did that in the first place to accomodate the first attempt of trying to make this work. I am unfamiliar with docker but I would not be surprised that if anything in that giant run block fails, it will not point out what specific line but just the giant run block as a whole, making debugging slightly harder.

So the commit should ideally just be adding that single SHELL line.

Also, to get back to the bashisms part because I think it may be good to elaborate: Yes, it does make sense to avoid them for compatibility. Yet as you can see, pure POSIX does lack a lot of features, making things generally more complex to achieve. However, this is a container with a controlled environment. This is not going to run on weird embedded systems with only busybox. The compatibility loss is thus very low to potentially nonexistent. Bash is present by default on virtually any Linux distribution.

I'd amend the PR description as you are embracing bashisms now for the sake of simplicity.

Calinou commented 1 month ago

The standard image build succeeds now: https://github.com/abarichello/godot-ci/actions/runs/11469069732

The Mono image needs further fixes still.