Elixir: Missing 'function undefined' warning when compiling umbrella applications

[michallepicki]

Environment

• Elixir & Erlang/OTP versions (elixir --version):

Erlang/OTP 22 [erts-10.4.4] [source] [64-bit] [smp:6:6] [ds:6:6:10] [async-threads:1] [hipe]
Elixir 1.9.1 (compiled with Erlang/OTP 22)

• Operating system: Ubuntu 18.04

Current behavior

• Given an umbrella application with app_a and app_b where app_a depends on app_b
• After compiling the application successfully (run mix compile --warnings-as-errors from umbrella root)

$ mix compile --warnings as errors
==> app_b
Compiling 1 file (.ex)
Generated app_b app
==> app_a
Compiling 1 file (.ex)
Generated app_a app

• Adding a call to AppA.hello() in app_b and recompiling does not produce a warning (checkout naughty-function-call in above repo and run mix compile --warnings-as-errors)

$ git checkout naughty-function-call 
Switched to branch 'naughty-function-call'
Your branch is up to date with 'origin/naughty-function-call'.
$ mix compile --warnings-as-errors
==> app_b
Compiling 1 file (.ex)
$ echo $?
0

Expected behavior

Recompiling an app that has a call to a different app it doesn't depend on produces the same error as we get when compiling from clean _build:

$ rm -r _build/
$ mix compile --warnings-as-errors
==> app_b
Compiling 1 file (.ex)
warning: function AppA.hello/0 is undefined (module AppA is not available)
  lib/app_b.ex:16

$ echo $?
1

[michallepicki]

I dug a bit deeper and have another (more general?) example where I'd love to see this warning: fresh compile, bigger umbrella app, a cross-application function call could produce a warning but doesn't:

$ rm -r _build/
$ mix compile --warnings-as-errors
==> app_b
Compiling 1 file (.ex)
Generated app_b app
==> app_c
Compiling 1 file (.ex)
Generated app_c app
==> app_a
Compiling 1 file (.ex)
Generated app_a app
$ echo $?
0
$ rm -r _build/
$ cd apps/app_c/
apps/app_c$ mix compile --warnings-as-errors
Compiling 1 file (.ex)
warning: function AppB.hello/0 is undefined (module AppB is not available)
  lib/app_c.ex:16

$ echo $?
1

I think having this warning showing up (no matter the compilation order or cache presence) would be a huge improvement to code quality feedback we're getting from the compiler. Currently I don't see a way to get it to show up reliably other than compiling each app separately to an empty _build dir

[josevalim]

This is actually quite hard to guarantee because we first compile app A, then we compile app B. So by the time we compile B, A was already loaded. We can improve the situation by removing A from the codepath after compilation and adding it back after everything is compiled, but any module that has been compiled by A will still be seen by B. So it will work more consistently but not when both apps are compiled at the same time.

[michallepicki]

Not sure how well would that work on different OSes but maybe during umbrella compilation, each application can have a temporary directory analogous to _build where each subdir is a link to a dir in _build?

[josevalim]

The problem is not having things in _build. The problem is state being carried over from a previous compilation that cannot be fully addressed when compiling multiple apps.

[michallepicki]

Oh. I see, this lays deeper than I thought. I'm not familiar with guts of mix/compiler but can this state be stored in a tree-like structure? Or can we add metadata that would allow us to resolve this relationship information? Or would that make the compilation too slow?

[josevalim]

@michallepicki not really, the issue are the modules that are loaded into the VM, as well as any process or state created and not reclaimed while compiling the previous app.

[josevalim]

So the issue can't really be fixed because when A is compiled, all modules from A are loaded into memory, which means they will be available. :/

One solution is to compile using mix cmd: mix cmd mix compile --warnings-as-errors. This guarantees each app is compiled in isolation, which avoid leaking state. It is (a bit) slower but probably a price worth paying in CIs.

[michallepicki]

@josevalim I tried doing that and got stuck on dependency issues (Unchecked dependencies for environment test:). We have multiple override: true in the top-level mix.exs file. I double-checked and we do specify the lockfile for each app but the resolved and downloaded dependency version from the lockfile doesn't get prioritized over version specified in each app's mix.exs files - I'm sure there are important reasons for that...

We may decide on maintaining a list of common overrides and include them for each umbrella app but we'll see.

[josevalim]

@michallepicki if you want different dependencies or versions per app, then you don't want umbrella applications. The whole point of umbrella is that applications are loaded into the same VM, with the same configs and deps. And if that is not working out, then it is best to break them into separate applications. The docs for umbrellas and the guides on the website both cover it. :)

[michallepicki]

@josevalim We do want to use the same versions for each app. Our umbrella overrides are not used to streamline app dependencies differences, but to get different versions of packages that our app's dependencies depend on further down in the deps tree. Because we have a monorepo and each app isn't used in isolation (right now), the umbrella approach of having overrides in one place in the top-level directory worked great so far, but does not work after switching to using mix cmd. (I think my previous comment may make more sense now)

BTW thank you for your help, I'm just leaving notes for whoever else wants do deal with this and finds this issue.

[josevalim]

Your approach however means you lose one of the big benefits of umbrellas: which is being able to run the tests or compile each app in isolation. So you effectively have a monolithic app disguised as an umbrella, since everything is so tied anyway.

In any case, I'm glad to help!

[michallepicki]

I tested the new warnings introduced in https://github.com/elixir-lang/elixir/commit/9d56a019ee0fe9475a61583f26f815e4bdf5717b .

In scenario from the issue (partially recompiling umbrella app) I saw the warning but --warnings-as-errors didn't fail the compilation:

$ echo $?
0

I removed the linked repo since then but I think this is easy to reproduce.

Added a third app to see if a warning now always shows up in the second scenario (apps compilation order determining warning presence), after adding a third app and rm -rf _build and re-running the compilation, I got the old warning (A.hello/0 is undefined (module A is not available or is yet to be defined)). Similarily, despite --warnings-as-errors present I saw 0 status code.

After moving the cross-apps call from B to C I got the new warning, but also 0 status code.

[josevalim]

@michallepicki please open up a new issue about warnings as errors no working in those cases. :)

[josevalim]

Done here #9861.