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 an option to select the application to build an escript for #2527

Merged
merged 1 commit into from
Apr 12, 2021

Conversation

galdor
Copy link
Contributor

@galdor galdor commented Apr 12, 2021

In the current state, in a multi-application project, the escriptize plugin
will only allow building a single application as an escript, and will force
developers to provide a "main" application with the escript_main_app
configuration variable.

This commit add a command line option for the escriptize plugin which let the
user select the application to build the escript for. The escript_main_app
configuration variable is still used if it is set for backward compatibility.

@galdor
Copy link
Contributor Author

galdor commented Apr 12, 2021

See https://github.com/galdor/rebar3-test/tree/multi-escripts for an example. rebar3 escriptize -n escript1 and rebar3 escriptize -n escript2 build both escripts separately.

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.

First of all thanks for the contribution. There are a few things to change to get it in line with the way most rebar3 options work, and that is:

  • respecting the priority between command-line and config options
  • using command-line options that map to those in config files when possible

As such, we should use --main-app (short -a or -m at your leisure) to fit with escript_main_app and/or --name (short -n) to match the escript_name parameters from config.

Right now --name is used for both indirectly, and the command line options should take precedence over the config written in rebar.config, such that if I have a config file specifying {escript_main_app, app1} but call it as rebar3 escriptize --main-app=app2, then I get app2 as the main app we build for.

As the code currently stands, the command line option is a default value for the config, and the naming isn't aligned with the stuff in the config file.

In the current state, in a multi-application project, the escriptize plugin
will only allow building a single application as an escript, and will force
developers to provide a "main" application with the escript_main_app
configuration variable.

This commit adds a command line option for the escriptize plugin which lets
the user select the application to build the escript for. The command line
option overrides escript_main_app if it is set.
@galdor galdor force-pushed the escript-selection branch from 68a850b to 3cfc720 Compare April 12, 2021 13:15
@galdor
Copy link
Contributor Author

galdor commented Apr 12, 2021

It makes sense, I updated the patch to use --main-app/-a and to change the selection order so that the command line option overrides escript_main_app.

@ferd ferd changed the title add an option to select the application to build an script for add an option to select the application to build an escript for Apr 12, 2021
@ferd ferd merged commit 5002700 into erlang:master Apr 12, 2021
@galdor
Copy link
Contributor Author

galdor commented Apr 12, 2021

Thank you so much for merging it so quickly !

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