Elixir: Mix.Config needs a function for importing configs reliably in umbrella applications

[lukaszsamson]

Rationale:
Given the umbrella application structure:

config/
  config.exs
apps/
  app_common/
    config/
      config.exs
  app_myapp/
    config/
      config.exs

where app_myapp depends on app_common (has {:app_common, in_umbrella: true} in deps). apps/app_common/config/config.exs provides some config setting
config :app_common, :setting, "default"
while app_myapp overrides it in apps/app_myapp/config/config.exs
config :app_common, :setting, "not default"

The pattern encouraged by mix new and mix phx.new tasks for importing configs in umbrella applications is to use:
import_config "../apps/*/config/config.exs"
in config/config.exs. With this pattern the config overriding works as expected.

However, it can lead to subtle errors. Wildcard import means that configs are processed alphabetically. Suppose you do a refactoring and rename app_common to common. app_myapp still correctly depends on common but now common config overrides app_myapp config.

Currently, the only reliable solution is to do all imports explicitly. It would be much better if there was a function importing configs in the same order that applications are started by mix.

[josevalim]

Currently, the only reliable solution is to do all imports explicitly. It would be much better if there was a function importing configs in the same order that applications are started by mix.

Please do all imports manually then. That's almost always the best way to go.

[lukaszsamson]

Please do all imports manually then. That's almost always the best way to go.

Thats what I ended up doing. But what about mix new --umbrella and mix phx.new --umbrella? Shouldn't those tasks be about sane defaults and generate code adhering to the best way? Or at least introduce a warning in generated configs. This issue report is the result of a real production problem.

[josevalim]

The problem is that if those tools generate something that is manual from the beginning, folks will generate new apps, forget to add a new config entry, and then be puzzled about why it doesn't work. So if we don't want compromise the development experience nor the production experience, you are right, we need to have something that address it. Thanks.

[wojtekmach]

Is it useful to configure the same app+key in _multiple_ child apps? If not (or there's a good workaround) maybe we could warn instead?

Edit: just to clarify, I mean detecting that we're overwriting the same key from a different child app. We'd still want to allow overriding _within_ the same app without a warning, e.g.: default value in apps/a/config/config.exs and override in apps/a/config/prod.exs

[josevalim]

It is also worth noticing that changing the umbrella root does not cause children configs to recompile or vice-versa. We should solve it when tackling this.

[josevalim]

I think we need to do three changes here:

1. The first one is to properly track configuration files, so changing an umbrella child properly recompiles the parent

2. The second is to introduce an explicit function for importing wildcards, instead of relying on import_config detecting the presence of a wildcard or not. Maybe import_configs_from_pattern? Or import_wildcard_configs? Thoughts?

3. However, I think for the original issue in this PR, the real solution is to actually encourage all all shared configuration to be in the umbrella and not in each child application. To be more precise, I would change Phoenix so that each umbrella child can only configure itself in its config/* files and the application of any other dependency goes to the parent one. Changing this in the generators should be enough to push people to the proper direction

[michalmuskala]

I think there's another possibility to make the configs less confusing in umbrellas and also make it future-proof for generating multiple applications: don't do per-app configs. It's confusing because it looks like you can configure each app separately while you can't do that unless you generate separate releases. I propose keeping only the top-level files in umbrella root and ditching config folders in sub-apps entirely. This is something I do in my umbrella projects and I've heard of many other people doing exactly the same. The case of separating umbrellas into multiple releases is an edge case - we shouldn't optimise defaults towards it.

[josevalim]

@michalmuskala my goal was not to separate umbrellas per multiple releases but mostly to avoid really large config files. In any case, what you suggest is the next thing we are going to try in case what I propose doesn't work or it is deemed too confusing.

/cc @chrismccord