Fix alpine incompatibility

[CDFN]

Fixes #1480

[shotah]

Should this be a global variable?

[CDFN]

Definitely no, I'm not very into bash scripting. Thanks for looking, I'll fix in a bit :)

[CDFN]

It looks like there's already global variable called distro so we can just use it. I removed mine completely.

[itzg]

No, there's not a global variable. Instead there's a helper function used as:

  distro=$(getDistro)

however, mc-send-to-console doesn't currently import start-utils, which would need this at the top

. "/start-utils"

[CDFN]

That's weird. I rebuilt the container and the command seems to work just fine wihout declaring own distro

root@7b2c0b045c7c:/data# cat /usr/local/bin/mc-send-to-console 
#!/bin/bash

: "${CONSOLE_IN_NAMED_PIPE:=/tmp/minecraft-console-in}"

if [ $# = 0 ]; then
  echo "ERROR: pass console commands as arguments"
  exit 1
fi

if [ ! -p "${CONSOLE_IN_NAMED_PIPE}" ]; then
  echo "ERROR: named pipe ${CONSOLE_IN_NAMED_PIPE} is missing"
  exit 1
fi

if [ "$(id -u)" = 0 ]; then
  if [[ $distro == alpine ]]; then
    exec su-exec minecraft bash -c "echo $* > '${CONSOLE_IN_NAMED_PIPE:-/tmp/minecraft-console-in}'"
  else
    exec gosu minecraft bash -c "echo $* > '${CONSOLE_IN_NAMED_PIPE:-/tmp/minecraft-console-in}'"
  fi
else
  echo "$@" >"${CONSOLE_IN_NAMED_PIPE:-/tmp/minecraft-console-in}"
fi
root@7b2c0b045c7c:/data# mc-send-to-console op CDFN
root@7b2c0b045c7c:/data# 

[itzg]

Since distro is unset, it'll be an empty string. So it'll be going in the else block, using gosu without any errors on a debian based image.

[CDFN]

Oh... I forgot that when you won't set --build-args it uses debian by default so that would be why 🤦 I'll re-test with alpine based image, sorry for that 😅

[itzg]

...fyi if you're doing a docker build with the default, then it's building a debian image. Add the build args BASE_IMAGE=openjdk:8-jdk-alpine3.8; DISTRO=alpine to build a java8 alpine image.

[CDFN]

Should be sorted now :)

[shotah]

I don't know enough here, but is there more than just this one command that needs to get flipped? I can do some searching too. :)

[CDFN]

I haven't looked at other scripts, I just came accross this one when using on own server.

[CDFN]

cdfn@cdfn-ryzen dev/docker-minecraft-server (fix/mc-send-to-console-alpine %) » grep -r "gosu"
scripts/start:    exec gosu ${runAsUser}:${runAsGroup} "${SCRIPTS:-/}start-configuration" "$@"
build/ubuntu/install-packages.sh:  gosu \
bin/mc-send-to-console:    exec gosu minecraft bash -c "echo $* > '${CONSOLE_IN_NAMED_PIPE:-/tmp/minecraft-console-in}'"

It looks like there are only 3 usages, first one is covered with check already, second one is installation script for debian based distros and third is corrected via this PR.

[itzg]

The only other use of gosu is in start, which is already wrapped in a conditional:

docker-minecraft-server/scripts/start

Lines 48 to 53 in 901b889

	distro=$(getDistro)
	if [[ $distro == alpine ]]; then
	exec su-exec ${runAsUser}:${runAsGroup} "${SCRIPTS:-/}start-configuration" "$@"
	else
	exec gosu ${runAsUser}:${runAsGroup} "${SCRIPTS:-/}start-configuration" "$@"
	fi

[itzg]

I think this will need to be the following to actually run the function and not be a bare string "getDistro", which will never equal "alpine":

Suggested change

	if [[ getDistro == alpine ]]; then
	if [[ $(getDistro) == alpine ]]; then

[CDFN]

Yes, again I have tested with wrong command zsh suggested me and ran build without args 🤦 Commit fixing it is there.

[itzg]

Totally understand. I am grateful that IntelliJ makes it easy for me to switch between build types:

[itzg]

Tested (with correct image 😉) and ready to merge?

[CDFN]

Yes 😇