openfrontier / docker-gerrit

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

Cannot remove settings once set #50

Open electrofelix opened 7 years ago

electrofelix commented 7 years ago

With the current variable testing before actions, it is not possible to unset or revert to default behaviour for any setting once previously set.

I wonder if it would be better to follow this pattern:

set_gerrit_config() {
  su-exec ${GERRIT_USER} git config -f "${GERRIT_SITE}/etc/gerrit.config" "$@"
}

unset_gerrit_config() {
  su-exec ${GERRIT_USER} git config -f "${GERRIT_SITE}/etc/gerrit.config" --unset "$@"
}

# test whether the variable has been set, even if blank
# if set and empty, remove the gerrit setting
# if set and non-empty, set the gerrit config option
[ "${TEST_VAR+x}" = "x" ] && if [ -z "${TEST_VAR}" ]
then
  unset_git_config some.gerrit_setting
else
  set_git_config some.gerrit_setting ${TEST_VAR}
fi

Thoughts?

This would certainly work from a docker-compose environment and I believe should avoid removing any settings unless the variable is explicitly exported while empty.

thinkernel commented 7 years ago

Hi there. I see the point that you need a way to unset some properties on startup. I agree with you that it can be a good feature. Unfortunately, the proposal that provided by you will make just one line logic into about six lines. That means gerrit-entrypoint.sh will be about 6 times longer and most of those lines are if/else. I think we might have to figure out another simpler way to accomplish this.

electrofelix commented 7 years ago

I'm hoping I can turn it into a function that is passed the name of the variable to test and corresponding config setting, but assuming that's possible, can you foresee and unexpected side effects from using the test for set but empty to remove settings?