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

use file:script if a .config.script file present #210

Closed
wants to merge 5 commits into from

Conversation

uwiger
Copy link
Contributor

@uwiger uwiger commented Apr 5, 2012

This patch makes rebar look for a file named rebar.config.script before reading rebar.config. The main use of this would be to be able to "patch" the rebar config, based on the current environment.

If the .script file is present, rebar will process it using file:script(File, [{'SCRIPT', File}]). The .script file may in its turn process rebar.config and use it as a basis for customizations. Note that the variable SCRIPT is pre-bound to the name of the .script file.

The rebar_config module looks for the rebar.config.script file in the same location as the rebar.config. To be precise, it appends ".script" to the name of the config file that would otherwise be read. Thus, if rebar is called with rebar --config /tmp/foo ..., then rebar will try to read /tmp/foo.script first.

@ghost
Copy link

ghost commented Apr 7, 2012

Looks good to me.

@jlouis
Copy link

jlouis commented Apr 7, 2012

I am wondering what the use case of this feature is. It looks awfully powerful, but in some way the idea of live-patching a rebar.config sounds to me like a dirty hack.

@hyperthunk
Copy link
Contributor

There are many use cases for dynamic reconfiguration. Sharing/extending common configuration, switching options depending on the environment (os, word size, etc) are just two examples. There have been suggestions about how to do these before (such as my patch for config merging - #156) but those require supporting a full feature in rebar itself, whereas this option puts the onus on the user.

@ghost
Copy link

ghost commented Apr 7, 2012

Dynamic or templated config. To be used instead of adding support
for special-purpose expandable variables. Similar to this in principle.
Ulf's original mail is here.

@hyperthunk
Copy link
Contributor

This is basically an alternative to what you'd do with ./configure in a traditional build system. You want to introspect some aspects of the environment (or user input?) in order to dynamically configure what the build will do.

The issue with expanding variables is who decides what the set of environment variables is in order to support everyone's needs? That issue would plague the templated config option as well, as you'd still need to build up the available parameters for the templates to consume.

@hyperthunk
Copy link
Contributor

I like this, but I have some questions.

Why not just allow expressions in config files when parsing?

As in https://github.com/hyperthunk/libconf/blob/master/src/config.erl#L8 or https://github.com/hyperthunk/rebar_dist_plugin/wiki/Configuring-Assemblies.

{target_dir, filename:join(erlang:tuple_to_list(
                       rebar_rel_utils:get_rel_release_info(?FILE))}.

Why not allow config files to contain template variables?

Initialised with parent + global rebar config and perhaps the contents of os:getenv/0:

{target_dir, "${app_name}/${app_version}"}.
%% etc ....

@uwiger
Copy link
Contributor Author

uwiger commented Apr 8, 2012

Well, this is more or less what file:script/2 does:

https://github.com/erlang/otp/blob/master/lib/kernel/src/file.erl#L1034

One could imagine raising the bar further by expanding special patterns in
the file, but then one departs from the semantics of file:consult() - and
file:script()...

I have done this myself in e.g. the 'setup' application:

https://github.com/esl/setup/blob/master/src/setup.erl#L119

...allowing environment variables like:

{kvdb_databases, [{rpc_queues,
[{file, "$DATA_DIR/$APP/rpc_queues.db"},

where the $DATA_DIR and $APP variables are automatically expanded by
the find_env_vars() function.

I believe Tuncer is fairly reluctant to go this route with rebar.

BR,
Ulf W

…ript

rebar_rel_utils:is_rel_dir/1 will now return true if Dir contains a
reltool.config.script file or a reltool.config file. The return value is
{true, F}, where F is the found file (reltool.config.script if both were
present).

rebar_config:consult_file(F) now consults the config file and passes its
contents to the .script (if present), bound to the variable CONFIG.
@ghost
Copy link

ghost commented Apr 8, 2012

I'm reluctant to introduce more var-expansion syntax because it doesn't look
like it will suffice. Inevitably someone is going to want to eval an erlang
expression. Out of all options file:script looks like the most elegant to
integrate and maintain.

@hyperthunk
Copy link
Contributor

Hi Ulf, yes I've had a crack at this a couple of times too. The main difference is that the approach I'm suggesting here behaves like file:consult (in that it returns a list of all the contained expressions so the file is more like a config file) but like file:script, it allows expressions such as function calls, rather than just supporting terms as file:consult does.

Tuncer: In answer to 1, take a look at this bit of in-place eval:

{erts_vsn, erlang:system_info(version)}.
{config_file_path, 
    filename:join(element(2, file:get_cwd()), "blah.config")}.

That one, for example, works with https://github.com/hyperthunk/libconf/blob/master/src/config.erl#L8. If you want to get fancy and do other kinds of expansion, such as adding the parsed terms to the bindings as you're evaluating the file and supporting simple string interpolation, you can also do that:

{erts_vsn, erlang:system_info(version)}.
{erts_string, "erts-${erts_vsn}"}.
{config_file_suffix, ".config"}.
{config_file_name, "${erts_string}.config"}.
{config_file_path, 
    filename:join(element(2, file:get_cwd()), resolve(config_file_name))}.

Taken from https://github.com/hyperthunk/econfig/blob/master/itest/econfig_SUITE_data/terms.config. Now econfig was an attempt to unify the ideas I had from parsing configuration files containing internal/external function calls and expressions in both the rebar_dist_plugin (producing results such as this for example) and in https://github.com/hyperthunk/libconf/blob/master/src/config.erl#L8. I didn't really complete it as I had more pressing things and the other ideas of it were fulfilled by gproc so I parked it, although this config parsing bit works fine as a proof of concept.

Point I'm raising is that in my little brain I prefer the look of this kind of config file, rather than a straight script. It is more complex to implement though, with the most complete example (last one) taking up the latter half of https://github.com/hyperthunk/econfig/blob/master/src/econfig.erl#L90.

@ghost
Copy link

ghost commented Apr 9, 2012

I'm not sure, but the implementation overhead and that you
create bindings which can be reused in subsequent variables
seem undesirable on a first review.

How would this expression

Fun = fun(C1) ->
              S = do_something(),
              L = case foo(S) of
                      Bar -> modify_foo(Bar);
                      Baz -> something_else(Baz)
                  end,
              list:foreach(fun(C2) -> do_final(IE) end, L)
      end,
IsX = fun(X) -> ... end,
IsY = fun(Y) -> ... end,
list:filter(fun(Bar) -> IsX(Bar);
               (Foo) -> IsY(Foo);
               (_)   -> false
            end, Fun(CONFIG)).

taken from a .script file look with in-place expression support?

@hyperthunk
Copy link
Contributor

First of all, it would look pretty much the same, with begin/end in place to support the multiple steps required:

{xyz, begin
        Fun = fun(C1) ->
                      S = do_something(),
                      L = case foo(S) of
                              Bar -> modify_foo(Bar);
                              Baz -> something_else(Baz)
                          end,
                      list:foreach(fun(C2) -> do_final(IE) end, L)
              end,
        IsX = fun(X) -> X == 10 end,
        IsY = fun(Y) -> Y == 20 end,
        list:filter(fun(Bar) -> IsX(Bar);
                       (Foo) -> IsY(Foo);
                       (_)   -> false
                    end, Fun([]))
      end}.

Secondly, I can't imagine you'd put so much logic into an embedded expression. This kind of complicated logic is exactly what plugins are good at, and plugins are loaded onto the code path when they're referenced, so any functions they export are made available to running code, including this code embedded in a config file.

@ghost
Copy link

ghost commented Apr 9, 2012

OK, it looks how I imagined it would.

@uwiger
Copy link
Contributor Author

uwiger commented Apr 9, 2012

(edited and subsequent comments deleted)
I think config files created with file:script/2 can be made more readable,
through the use of funs and successive building of complex expressions, like
Tuncer's example illustrates.

While this can be done with the "enhanced file:consult" style as well, I think the
scope handling is less clear. Variables (e.g. funs) declared in the "body" of a
previous expression can be reused in subsequent expressions (or conflict with
variables further down). In essence, the representation still has "script" semantics,
even though it looks like a set of orthogonal config items.

The file:script-style file, on the other hand, behaves the same way as if the commands
had been entered into a shell, and - with minor cleanup editing - could even be such a
sequence copy-pasted from a shell.

BR,
Ulf

@hyperthunk
Copy link
Contributor

Ulf - I must concede that you're right about the clarity of scope handling here - a very complex expression is better not embedded, but placed either in a script or plugin.

As you can see in my prior example, I got around the scope 'problem' by re-using previously evaluated elements, but I do realise that such an approach is a bit opaque at best.

@ghost
Copy link

ghost commented Apr 16, 2012

Thanks, merged.

@ghost ghost closed this Apr 16, 2012
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants