Config files check #1671
Config files check #1671
Conversation
…ormat config is used
The ${FOO%.*} expression removes everything after the last dot,
which returns wrong results for paths like /foo/bar.baz/bar.
Make it use basename and dirname intead.
If advanced config file has a wrong extension and is to be used as a -config argument - the script should fail with an error.
…ors to the .config check
|
Can we add some bats/shell script tests for this? |
| @@ -48,9 +48,97 @@ if not exist "!ERLANG_HOME!\bin\erl.exe" ( | |||
|
|
|||
| set RABBITMQ_EBIN_ROOT=!RABBITMQ_HOME!\ebin | |||
|
|
|||
| if not exist "!RABBITMQ_SCHEMA_DIR!" ( | |||
There was a problem hiding this comment.
What do you think about combining the configuration setup code into its own .bat file to be shared between rabbitmq-server.bat and rabbitmq-service.bat?
There was a problem hiding this comment.
We do have rabbitmq-env.bat. Do you think it can be moved there?
There was a problem hiding this comment.
Let's wait for future powershell work
scripts/rabbitmq-env
Outdated
| # Config file has an extension, but it's neither .conf or .config | ||
| echo "ERROR: Wrong extension for RABBITMQ_CONFIG_FILE: $RABBITMQ_CONFIG_FILE" 2>&1 | ||
| echo "ERROR: extension should be either .conf or .config" 2>&1 | ||
| return 1 |
There was a problem hiding this comment.
This exit should stop the rabbitmq-server sctipt, but it doesn't.
Is there a set -e missing anywhere?
I wonder if this check should go to rabbitmq-server to keep this file clean from logic.
There was a problem hiding this comment.
That is surprising. There is a set -e in the _rmq_env_initialize function but only if RABBITMQ_SCRIPTS_DIR is unset. Let me change the code to check the result of rabbitmq_config
There was a problem hiding this comment.
Looks fine after 55a6953
|
@lukebakken I keep discovering single closing curly braces like |
lukebakken
force-pushed
the
config_files_check
branch
from
00637fe
to
34bba9b
Compare
|
The integration tests are in rabbitmq-ci branch |
There was a problem hiding this comment.
👍 bats tests pass on Linux and Windows 2012. Nice job @hairyhum
Changes:
Bugfixes:
Getting filename without extension to check extensions fixed for unix:
before it was getting everything before the last dot as a name without extension,
now it uses basename and dirname to only strip extension from the file name.
Example:
For
/foo/bar.baz/filefilename without extension was/foo/barand checkfor
/foo/bar==/foo/bar.baz/file(that the file has no extension) returned false.Behaviour changes (script):
If RABBITMQ_ADVANCED_CONFIG_FILE exists, but RABBITMQ_CONFIG_FILE does not,
advanced config will be used as
-config. This removes confusion if people wantto configure using the erlang terms format.
The script will print a warning to the stdout about that.
If both old and new config files exist and RABBITMQ_CONFIG_FILE points
to both (no extension) - there will be a warinig in the stdout
and the old format config file will be used as
-config.New configs will be ignored.
If RABBITMQ_CONFIG_FILE is configured with a wrong extension - there will be an
error and the script will fail.
This might be a breaking change if people are using files like
rabbit.foo.configand configure RABBITMQ_CONFIG_FILE to
rabbit.foo. Should we support that?If RABBITMQ_ADVANCED_CONFIG_FILE is configured with a wrong extension and is to
be used as
-config- there will be an error and the script will fail.Behaviour changes (erlang):
Prelaunch checks that RABBITMQ_ADVANCED_CONFIG_FILE is a valid config file
using
file:consultand that RABBITMQ_CONFIG_FILE is either a config fileusing
file:consultif it has the.configextension or not a config fileif it has the
.confextension.The same thing is checked on server start.
If '.config' file does not exist - it will not be shown in logs.
-conf_advancedargument now HAS TO always have extension.The
.configextension is always set in scripts.Refactorings:
Extension check,
-configargument and generated config arguments (-conf,-conf_advanced)and file existense checks now separated.
"Filename without extension" (e.g. RABBITMQ_CONFIG_FILE_NOEX) variables
are set for all config files and used in more places.
More comments in code to explain what's going on.
Defaults:
advanced file now has the .config extension by default, because it can only
have one extension. Configuring it without extension is still supported.