Skip to content
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

Add support for relx config file #2435

Merged
merged 4 commits into from
Nov 30, 2020

Conversation

plux-kivra
Copy link
Contributor

@plux-kivra plux-kivra commented Nov 23, 2020

This PR adds support for reading the relx configuration from a file when generating a release.
Rebar will continue to use the rebar.config relx section as the preferred source of truth, but if there is no such section it will use the configuration from default file relx.config.
However if the --config flag specifies a configuration file it will be preferred over of the configuration from rebar.config or relx.config.

I've added a test that should cover these different scenarios.

Copy link
Collaborator

@ferd ferd left a comment

Choose a reason for hiding this comment

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

Looks good to me, just have some minor comments about a path handling issue and some debug message formatting

ConfigFile = proplists:get_value(config, Options, []),
case ConfigFile of
"" ->
case {rebar_state:get(State, relx, []), file:consult("relx.config")} of
Copy link
Collaborator

Choose a reason for hiding this comment

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

We may want to use filename:join([rebar_dir:root_dir(State), "relx.config"]) since we allow changing the base context of where things run.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should this also affect --config flag? I didn't implement that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 7884a4b

?DEBUG("Using relx.config", []),
Config;
{Config, {error, enoent}} ->
?DEBUG("Using relx config from rebar.config", []),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Configuring releases with the {relx, ...} entry from rebar.config

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 96d61cf

"" ->
case {rebar_state:get(State, relx, []), file:consult("relx.config")} of
{[], {ok, Config}} ->
?DEBUG("Using relx.config", []),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Configuring releases with relx.config

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 96d61cf

ConfigFile ->
case file:consult(ConfigFile) of
{ok, Config} ->
?DEBUG("Using relx config file: ~p", [ConfigFile]),
Copy link
Collaborator

Choose a reason for hiding this comment

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

same formatting here. Might want to use ~ts for the output of the file though, so we show the string as-is and in unicode-aware text

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 96d61cf

@ferd ferd merged commit ef6476d into erlang:master Nov 30, 2020
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