Skip to content

Support file extension in the RABBITMQ_CONFIG_FILE env var values #711

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 12 commits into from
Apr 5, 2016

Conversation

hairyhum
Copy link
Contributor

Fixes #691

@michaelklishin michaelklishin self-assigned this Mar 29, 2016
@michaelklishin michaelklishin added this to the n/a milestone Mar 29, 2016
@michaelklishin michaelklishin changed the title Support file extension in RABBITMQ_CONFIG_FILE env Support file extension in the RABBITMQ_CONFIG_FILE env var values Mar 29, 2016
@michaelklishin
Copy link
Collaborator

This breaks support for paths without a filename extension. The problematic line seems to be
the one with "${RABBITMQ_CONFIG_FILE%.*}":

export RABBITMQ_CONFIG_FILE=./etc/rabbitmq/rabbitmq
echo "${RABBITMQ_CONFIG_FILE%.*}"

export RABBITMQ_CONFIG_FILE=./etc/rabbitmq/rabbitmq.config
echo "${RABBITMQ_CONFIG_FILE%.*}"
./etc/rabbitmq/rabbitmq
export RABBITMQ_CONFIG_FILE=./etc/rabbitmq/rabbitmq.sysctl
echo "${RABBITMQ_CONFIG_FILE%.*}"
./etc/rabbitmq/rabbitmq
export RABBITMQ_CONFIG_FILE=./etc/rabbitmq/rabbitmq.sysctl.conf
echo "${RABBITMQ_CONFIG_FILE%.*}"
./etc/rabbitmq/rabbitmq.sysctl

Otherwise if there is no dot in RABBITMQ_CONFIG_FILE,
RABBITMQ_CONFIG_FILE_NOEX ends up being empty.
@michaelklishin
Copy link
Collaborator

I've pushed something that fixes this particular problem by using dirname and basename. I'm hardly great at shell script, so @hairyhum and @dumbbell, please double check.

We also need to work on supporting both advanced and advanced.config.

@hairyhum
Copy link
Contributor Author

Or we can just change conditions, because extension is being checked there anyway. So we can use any extension trimming mechanism.

@michaelklishin
Copy link
Collaborator

@hairyhum this passes the acceptance criteria for rabbitmq.conf and rabbitmq.config but not advanced.config. I think we should support both options for the advanced.config as well.

@hairyhum
Copy link
Contributor Author

hairyhum commented Apr 1, 2016

Added support for advanced.config. Haven't checked windows scripts yet.

@michaelklishin
Copy link
Collaborator

This looks good on OS X (zsh) and Ubuntu (dash). Haven't had a chance to test on Windows yet.

) else (
if exist "!RABBITMQ_CONFIG_FILE!.config" (
set RABBITMQ_CONFIG_ARG=-config "!RABBITMQ_CONFIG_FILE!"
) else if exist "!RABBITMQ_CONFIG_FILE!.config" (
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this else if should be else if exist "!RABBITMQ_CONFIG_FILE!.conf" as rabbitmq-server.bat

@michaelklishin michaelklishin merged commit 9601bd6 into master Apr 5, 2016
@dumbbell dumbbell deleted the rabbitmq-server-691 branch January 2, 2018 15:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants