app_vars_file applies env vars to every app rebar3 compiles

[JesseStimpson]

Pre-Check

• I wasn't able to find any existing issues or docs describing this behavior
• I will submit a Pull Request for a suggested change directly after submitting this issue.

Environment

For my output below, I had done a rebar3 local upgrade first, to try to be on the latest release.

$ rebar3 --version
rebar 3.13.1 on Erlang/OTP 23 Erts 11.2

The app I'm trying to build is closed-source but I'll provide a minimal set of steps to reproduce the behavior below.

Current behaviour

When the config option app_vars_file is provided in the rebar.config, the vars contained within are applied to every application's .app file that rebar3 is compiling. Since in my experience app vars are usually defined to be specific per app, this is unexpected behavior.

In the foo app, I'm using the wonderful 'recon' as a dep just to illustrate the behavior. Any dependency project will show the same result.

jstimpson:[~/dev/erlang]: rebar3 new app foo
===> Writing foo/src/foo_app.erl
===> Writing foo/src/foo_sup.erl
===> Writing foo/src/foo.app.src
===> Writing foo/rebar.config
===> Writing foo/.gitignore
===> Writing foo/LICENSE
===> Writing foo/README.md
jstimpson:[~/dev/erlang]: cd foo
jstimpson:[~/dev/erlang/foo]: echo '{deps, [{recon, {git, "https://github.com/ferd/recon.git", {branch, "master"}}}]}.' > rebar.config
jstimpson:[~/dev/erlang/foo]: echo '{app_vars_file, "myvars.app.config"}.' >> rebar.config
jstimpson:[~/dev/erlang/foo]: echo '{env, [{bar, baz}]}.' > myvars.app.config
jstimpson:[~/dev/erlang/foo]: rebar3 compile
===> Verifying dependencies...
===> Fetching recon (from {git,"https://github.com/ferd/recon.git",{branch,"master"}})
===> Compiling recon
===> Loading app vars from "myvars.app.config"
===> Compiling foo
===> Loading app vars from "myvars.app.config"
jstimpson:[~/dev/erlang/foo]: cat _build/default/lib/recon/ebin/recon.app
{application,recon,
             [{description,"Diagnostic tools for production use"},
              {vsn,"2.5.2"},
              {modules,[recon,recon_alloc,recon_lib,recon_map,recon_rec,
                        recon_trace]},
              {registered,[]},
              {applications,[kernel,stdlib]},
              {licenses,["BSD"]},
              {links,[{"Github","https://github.com/ferd/recon/"},
                      {"Documentation","http://ferd.github.io/recon/"}]},
              {build_tools,["mix","rebar3"]},
              {files,["src/","script/","rebar.lock","mix.exs","README.md",
                      "LICENSE"]},
              {env,[{bar,baz}]}]}.

Please notice:

1. The presence of bar=baz in recon.app's env section
2. The "Loading app vars" info state from rebar3 printed twice -- for each app.

Expected behaviour

I would expect to be able to target specific apps with the app_vars_file config option. The current behavior is not a bug, but the behavior is surprising enough that I thought it warranted an issue to go along with the Pull Request.

Thanks for reading, and thanks for the great tooling, as always. I hope you'll find my PR useful.

[ferd]

First of all, this isn't the newest release (3.17 is out), but second of all, wow I had no idea this even was a feature. This appears to be code that was put in place in 2010, and imported straight from legacy rebar 2.x into rebar3 a few years later.

This is probably a bug. though I'd have to figure out how the old rebar2 dealt with this in multi-app cases. The weird things I can imagine here are:

1. if it's intended to be env vars put in a single app, why not put it in the .app.src file?
2. if it's intended to be env vars put into multiple apps, I would just expect breaking because of default values already there

Option 1 is probably the less problematic one, but given that I'd be more tempted to just take it away if we're to break behaviour anyway.

[JesseStimpson]

Admittedly, I didn't spend enough time ensuring I was on the latest release since I skipped ahead and read the code on master. Just now, I bootstrapped the master branch of rebar3 and got the same result as in the description.

1. The app I'm porting to rebar3 uses this feature in a unique way. For better or worse, the pre-rebar build process is generating a different vars file depending on build options. Then we are expecting rebar to include those build-specific options in the .app file. There are surely other (and better!) ways to do this, but since the feature exists in rebar3 I thought it an interesting enough use case to submit an issue.

2. Yes, I agree with that conclusion.

We don't rely heavily on this feature -- and I would not object to taking the feature away. As-is, due to your point #2, use of the feature could cause some nasty unintended bugs in dependencies.

In any case, I'll submit my PR to aide your decision.

[ferd]

Yeah, the advantage of this could be to generate profile-specific env values (be aware that 'prod' is always used as an env on top of default for deps), so having it be per-app and work nicely with profiles would probably be the best way to keep the feature.