add support to configure application resource naming

[TheGeorge]

This adds the ability to configure the default naming of the .app.src and .app.src.script files for application discovery:

• application_resource_extensions can be set to define extension to the resource files
• the extensions will be tried to be resolved from left to right
• .app files are still handled like before (not configurable)
• mix.exs files are handled, but can be disabled (by removing from extension list), I'm not sure if this is the right thing todo here, or if they should be special cased like the .app files

Note: This still needs tests added.

[tsloughter]

Why is .app.src.script suitable for this?

Can it not read in any file and return it as the resulting .app file? So if you wanted to use myapp.test.app as the app file then have the script read in that file and return it.

Or am I missing the point of this change?

[ferd]

Why is .app.src.script suitable for this?

Can it not read in any file and return it as the resulting .app file? So if you wanted to use myapp.test.app as the app file then have the script read in that file and return it.

Or am I missing the point of this change?

@tsloughter The reason is that we don't pass in a profile to the .app.src.script files, they're just evaluated code. We do pass something like CONFIG to rebar.config.script, but not to the others, and there wouldn't necessarily be a thing to pass.

I had concerns about that because you can recall that the same underlying code is used for all of these, so we had people doing rebar.lock.script somehow, and I was deadly afraid of seeing profiles seep through all sorts of features of that kind. Additionally it would imply passing in a profile to rebar.config.script, but then you sort of need to know the profile's value to work with it when evaluating the profiles and it felt weird.

The clean design IMO if we needed profiles in app files would have been to move app file values into rebar.config (as you had wanted to do in the past), but that would now be a huge changeset.

So to keep magic profile vars out of .script files only for app files, the idea was to sort of allow optional app file selection based on profile values that would already be taking place within rebar.config; allowing to choose specific app files per profile was sort of the least disruptive thing config- and feature-wise, but the code underneath did not rely on config values, just conventions, and this makes it also a largeish changeset under the hood.

[tsloughter]

Eh, I think passing the profiles to the script file makes sense and is a simpler solution.

App file based on what profile is used would work ok I guess. Like if myapp.test.app.src exists and the test profile is in use, use that. But require exact match, so if the profiles test and dev are both being used then you only look for myapp.test+dev.app.src.

But, still think just having app.src.script able to know the environment it is running in makes the simplest change and most sense.

[ferd]

I still don't like it too much, but it might be less disruptive in practice. I'd like to at least make sure we don't get weird cases where lock files and rebar.config itself are passed profile values though.

[tsloughter]

We should make it so that rebar.lock.script can not exist :)

[TheGeorge]

There was a long discussion about why making the application resource files not aware of the profiles was desired (prior this PR). Having this configurable in the rebar.config file allows the downstream mechanisms to be completely unaware of the profile.

I personally like this solution more than adding the profile information into the .app.src.script since we can leave the application resource files are static config not a dynamic script.

But require exact match, so if the profiles test and dev are both being used then you only look for myapp.test+dev.app.src.

In this PR the naming is up to the discretion of the author of the rebar.config. If you configure in the test profile

{application_resource_extensions, [
    ".test.app.src",
    ".app.src"
]}

then this will also work for combined profiles as the configuration proliferates (unless overwritten by another profile). In this case you can either have an <app>.app.src and a <app>.test.app.src for default and test profile, or just <app>.app.src if there are no differences (e.g. with multiple apps in an umbrella project)

[TheGeorge]

I added some test for this functionality.

[TheGeorge]

@tsloughter @ferd Can you take a look?

[ferd]

This test is good, but it does raise a question; should there be any support for .app.src.script and .app.src at once? Was it supported prior to this patch and is it still supported (or not) in the same way after it?

[TheGeorge]

It was supported before the patch and is still supported in the same way. The user can choose to overwrite this however by removing either extension:

[".app.src.script", ".app.src"] %% yields previous behavior (and is the default value for this key)
[".app.src"] %% no script get ignored
[".app.src.script"] %% in this case the .app.src files are handled when resolving the script files, but an app needs an ".app.src.script" file to be discovered

[TheGeorge]

Are some of the tests flaky? They all pass locally and I only removed a single ct:pal("")

[TheGeorge]

@ferd any feedback to move this forward?

[ferd]

I'm going to be merging this by end of day unless @tsloughter feels strongly against it / like vetoing it.

I think there's interesting advantages to supporting more stuff in the future with it, such as being able to dynamically enable .app file content that wouldn't work on older OTP releases (e.g. adding breaking optional dep formats that could have been a possibility) outside of any profile mechanisms, or doing dynamic dispatch of things such as Wx support in files or libraries based on various measures.

This allows centralizing these config decisions within rebar.config mechanisms, at least until we decide to make the leap to configure app files within rebar.config itself. For example, it'd possibly be nice to get rid of our .app.src.script file doing weird dialyzer inclusions and possibly move to having a legacy and a current file based on these extentions at some point.

It's more complex as an implementation than profiles in the files, but it also de-couples profiles from app files, comes with tests, and we have the code here already.

[tsloughter]

Github refuses to open a revert PR for this, I was hoping I could just do that very quickly. On slack Grigory Starinkin (vellmir) verified that this commit broke rebar3_mix. Unless a fix can be added quickly so a release can be made we should revert it today and release.