openfrontier / docker-gerrit

Build a Docker image with the Gerrit code review system
Apache License 2.0
197 stars 116 forks source link

Strange behavior with bash variables expansion #71

Closed avoidik closed 7 years ago

avoidik commented 7 years ago

In entrypoint here we have a generic template like:

#!/usr/bin/env sh
...
su-exec ${GERRIT_USER} java ${JAVA_OPTIONS} ${JAVA_MEM_OPTIONS} -jar ${GERRIT_WAR} init --batch --no-auto-start -d ${GERRIT_SITE} ${GERRIT_INIT_ARGS}

And then during execution this expands to:

su-exec gerrit2 java -jar /var/gerrit/gerrit.war init --batch --no-auto-start -d /var/gerrit/review_site --install-plugin=download-commands --install-plugin=replication

So no problem here.

If we change template above to template below (switch to bash, and properly enclose variables into a quotes):

#!/usr/bin/env bash
...
su-exec "${GERRIT_USER}" java "${JAVA_OPTIONS}" "${JAVA_MEM_OPTIONS}" -jar "${GERRIT_WAR}" init --batch --no-auto-start -d "${GERRIT_SITE}" "${GERRIT_INIT_ARGS}"

Then this one will be expanded to:

su-exec gerrit2 java '' '' -jar /var/gerrit/gerrit.war init --batch --no-auto-start -d /var/gerrit/review_site '--install-plugin=download-commands --install-plugin=replication'

As you can see some variables has been expanded with the single quotes and this issue causes a lot of problems with proper Gerrit startup.

Could not find or load main class

Could someone suggest something about this problem?

avoidik commented 7 years ago

My fault underlying alpine image doesn't have bash at all

thinkernel commented 7 years ago

If you are talking about this project, bash is included at this line.

avoidik commented 7 years ago

how to reproduce this behavior

  1. docker build -t gerrit-test .
  2. docker run -it gerrit-test bash
  3. apk add nano
  4. nano /tmp/test.sh
    #!/usr/bin/env bash
    set -x
    su-exec "${GERRIT_USER}" java "${JAVA_OPTIONS}" "${JAVA_MEM_OPTIONS}" -jar "${GERRIT_WAR}" init --batch --no-auto-start -d "${GERRIT_SITE}" "${GERRIT_INIT_ARGS}"
  5. chmod +x /tmp/test.sh
  6. /tmp/test.sh

is that su-exec issue or bash?

ncopa commented 7 years ago

This has nothing to do with su-exec and you have same problem without it. When you use quotes you will always expand it as a single argument, with spaces preserved. So JAVA_OPTIONS="--option1 --option2"; ... java "${JAVA_OPTIONS}" will pass one single option with a space to java. (--option1 --option2 instead of --option1 + --option2)

The behavior is equal in busybox ash and bash (but bash's set -x does a better job in displaying what goes on by displaying the '')

My guess is that this is what you want:

... java ${JAVA_OPTIONS} ${JAVA_MEM_OPTIONS} -jar "${GERRIT_WAR}" init --batch --no-auto-start -d "${GERRIT_SITE}" ${GERRIT_INIT_ARGS}

That way you can have multiple JAVA_OPTIONS, JAVA_MEM_OPTIONS and GERRIT_INIT_ARGS, but you cannot have spaces in those, as space will be separator. At the same time you can have "${GERRIT_WAR}" in a path with spaces (eg GERRIT_WAR="/path/to/my folder with wars/foo.war") and have spaces in the GERRIT_SITE.

thinkernel commented 7 years ago

Yes, I agree with @ncopa . "${VAR}" is a good practice when you want to quote a path which may has spaces between it. But it doesn't mean you must use this format everywhere.