Skip to content

Enhance evaluation of environment variables#426

Merged
tsloughter merged 2 commits intoerlware:masterfrom
saleyn:vars
Dec 22, 2015
Merged

Enhance evaluation of environment variables#426
tsloughter merged 2 commits intoerlware:masterfrom
saleyn:vars

Conversation

@saleyn
Copy link
Copy Markdown
Contributor

@saleyn saleyn commented Dec 22, 2015

This patch addresses the following issues:

  • When RELX_REPLACE_OS_VARS is set, evaluation of environment vars
    is done by the shell rather than awk, this allows to use more
    powerful notation of environment variables in sys.config and vm.args
    (e.g. -sname abc@${HOSTNAME,,} or {myapp, [{user, ${USER:-unknown}}]}
  • Using shell vars rather than unnecessarily forking awk/grep/etc

This patch addresses the following issues:

* When RELX_REPLACE_OS_VARS is set, evaluation of environment vars
  is done by the shell rather than awk, this allows to use more
  powerful notation of environment variables in sys.config and vm.args
  (e.g. `-sname abc@${HOSTNAME,,}` or `{myapp, [{user, ${USER:-unknown}}]}`
* Using shell vars rather than unnecessarily forking awk/grep/etc
@saleyn
Copy link
Copy Markdown
Contributor Author

saleyn commented Dec 22, 2015

No, because there's a separate template script for windows: extended_bin_windows, which I haven't modified. The functionality of the two is not identical anyway.

@saleyn saleyn force-pushed the vars branch 2 times, most recently from 0621344 to e1d3141 Compare December 22, 2015 15:00
@saleyn saleyn changed the title Optimize evaluation of environment variables Enhance evaluation of environment variables Dec 22, 2015
Add ability to also run shell commands contained in the sys.config.
E.g.:

# In this example the node name defaults to name of the release
# and can be overriden at run-time, appended with current year
$ head -1 vm.args
-sname ${NODE_NAME:-$REL_NAME}$(date +%Y)

# If the $NAME is 'abc2015', and hostname is 'MyHost', below the 'node'
# parameter gets set to 'Abc2015@myhost'
$ grep node sys.config
   {node, $(echo ${NAME^})@${HOSTNAME,,}}
@tsloughter
Copy link
Copy Markdown
Member

Sweet, I've wanted this but didn't know how to do it :)

tsloughter added a commit that referenced this pull request Dec 22, 2015
Enhance evaluation of environment variables
@tsloughter tsloughter merged commit 818a2c5 into erlware:master Dec 22, 2015
@tsloughter
Copy link
Copy Markdown
Member

@saleyn crap, i'm only now testing this so I can cut a new version and it fails for me. For example:

NAME_ARG=( $(egrep '^-s?name' "$VMARGS_PATH" || true) )

I get:

Syntax error: "(" unexpected

Any idea?

@tsloughter
Copy link
Copy Markdown
Member

Aaah, on ubuntu sh is dash not bash. I guess we need to explicitly use bash.

@tsloughter
Copy link
Copy Markdown
Member

I reverted this PR.

@saleyn
Copy link
Copy Markdown
Contributor Author

saleyn commented Dec 31, 2015

Actualy I believe I addressed this in this the #427 PR. In that PR the #!/bin/sh was changed to #!/bin/bash. I believe that in combination with this PR that should do it. Though I don't have a ubuntu handy to test this.

@tsloughter
Copy link
Copy Markdown
Member

We can't rely on bash.

@saleyn
Copy link
Copy Markdown
Contributor Author

saleyn commented Dec 31, 2015

Isn't it present in all UNIX's (event though it may not be the default shell)? The extended variable expansion is only meaningful in bash's context.

@tsloughter
Copy link
Copy Markdown
Member

Not BSDs i believe.

@saleyn
Copy link
Copy Markdown
Contributor Author

saleyn commented Dec 31, 2015

Maybe then we could have an option in relx which shell to use when generating the script?

@saleyn
Copy link
Copy Markdown
Contributor Author

saleyn commented Dec 31, 2015

So far this works great for me in Arch and Fedora.

@tsloughter
Copy link
Copy Markdown
Member

May be. I do want to have the option for a cuttlefish dependent start script, so this would be similar.

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.

2 participants