erlang / rebar3

Erlang build tool that makes it easy to compile and test Erlang applications and releases.
http://www.rebar3.org
Apache License 2.0
1.71k stars 517 forks source link

Cannot compile Elixir dependencies that have an Erlang dependency #2419

Open aboroska opened 4 years ago

aboroska commented 4 years ago

Environment

Current rebar3 master and latest Elixir:

rebar 3.14.1+build.4820.ref5e223870 on Erlang/OTP 22 Erts 10.7.1 Elixir 1.11.1 (compiled with Erlang/OTP 22)

rebar3 report compile
Rebar3 report
 version 3.14.1+build.4820.ref5e223870
 generated at 2020-11-03T10:19:14+00:00
=================
Please submit this along with your issue at https://github.com/erlang/rebar3/issues (and feel free to edit out private information, if any)
-----------------
Task: compile
Entered as:
  compile
-----------------
Operating System: x86_64-apple-darwin19.6.0
ERTS: Erlang/OTP 22 [erts-10.7.1]
Root Directory: ~/otpinst/r22.3.2
Library directory: ~/otpinst/r22.3.2/lib
-----------------
Loaded Applications:
bbmustache: 1.10.0
certifi: 2.5.2
cf: 0.3.1
common_test: 1.18.2
compiler: 7.5.4
crypto: 4.6.5
cth_readable: 1.4.9
dialyzer: 4.1.1
edoc: 0.11
erlware_commons: 1.3.1
eunit: 2.4.1
eunit_formatters: 0.5.0
getopt: 1.0.1
hipe: 3.19.3
inets: 7.1.3
kernel: 6.5.2
providers: 1.8.1
public_key: 1.7.2
relx: 4.0.2
sasl: 3.4.2
snmp: 5.5
ssl_verify_fun: 1.1.6
stdlib: 3.12.1
syntax_tools: 2.2.1
tools: 3.3.1

-----------------
Escript path: ~/bin/rebar3
Providers:
  app_discovery as clean compile compile consolidate_protocols cover ct deps dialyzer do edoc escriptize eunit find_elixir_libs get-deps help install install_deps list lock new path pkgs release relup report repos shell state tar tree unlock update upgrade upgrade upgrade version xref

Current behaviour

Two example rebar.configs that fail to compile:

{plugins, [rebar_mix]}.
{deps, [logger_lager_backend]}.
{plugins, [rebar_mix]}.
{deps, [lager, logger_lager_backend]}.

The error is caused by compiling logger_lager_backend before lager.

rebar3 compile
===> Fetching rebar_mix v0.4.0
===> Analyzing applications...
===> Compiling rebar_mix
===> Verifying dependencies...
===> Fetching lager v3.8.1
===> Fetching logger_lager_backend v0.2.0
===> Fetching goldrush v0.1.9
===> Compiling logger_lager_backend
Compiling 1 file (.ex)
warning: :lager.dispatch_log/7 is undefined (module :lager is not available or is yet to be defined)
  lib/logger_lager_backend.ex:23: LoggerLagerBackend.handle_event/2

Compilation failed due to warnings while using the --warnings-as-errors option
===> Failed to compile application logger_lager_backend with mix

Expected behaviour

When an Elixir/Mix project depends on an Erlang project the Erlang dependency should be compiled first.

ferd commented 4 years ago

This might be an issue with the changes in 3.14, where apps with a custom compiler are always ran before the rest of regular rebar3 apps: https://github.com/erlang/rebar3/blob/5e2238707402fa6d149ca186494340ce7b9ee6a2/src/rebar_prv_compile.erl#L316-L325

A quickfix might be to swap the order and make the rebar3 regular apps build first, but we create the opposite scenario: if an Erlang app is trying to include an Elixir app and relies on specific headers or modules at build-time, it will fail.

This might be less frequent but isn't a long term fix. If you want to try swapping the lines and seeing if it builds we can always take it as a workaround fix to be addressed later fully.

I guess part of the problem there is that we don't use the deps chart to actually define build-time dependency; we focus on code analysis. Injecting builder apps into that workflow is potentially doable, but not very simple on the surface.

starbelly commented 4 years ago

A quickfix might be to swap the order and make the rebar3 regular apps build first, but we create the opposite scenario: if an Erlang app is trying to include an Elixir app and relies on specific headers or modules at build-time, it will fail.

I found that this creates another problem, specifically with this case :

===> Compiling logger_lager_backend
Compiling 1 file (.ex)
warning: :lager.dispatch_log/7 defined in application :lager is used by the current application but the current application does not directly depend on :lager. To fix this, you must do one of:

  1. If :lager is part of Erlang/Elixir, you must include it under :extra_applications inside "def application" in your mix.exs

  2. If :lager is a dependency, make sure it is listed under "def deps" in your mix.exs

  3. In case you don't want to add a requirement to :lager, you may optionally skip this warning by adding [xref: [exclude: :lager] to your "def project" in mix.exs

  lib/logger_lager_backend.ex:23: LoggerLagerBackend.handle_event/2

Compilation failed due to warnings while using the --warnings-as-errors option
ferd commented 4 years ago

uh so that lib both needs to be compiled before and after lager at the same time to work? Or it's just that it badly declared its dependency?

starbelly commented 4 years ago

uh so that lib both needs to be compiled before and after lager at the same time to work? Or it's just that it badly declared its dependency?

Nah, the mix file for the lib is fine. It may that elixir simply isn't aware that it's compiling in a rebar3 project and doesn't know it should probably skip application tracing in that case. Regardless, I'm not terribly sure swapping the order is a stable solution for this problem. As you alluded to, this could break things devoid of elixir deps.

ericmj commented 4 years ago

It looks like logger_lager_backend is being compiled without lager being in the code path. Disabling tracing will not work because if it's a compile time dependency it would fail to compile if it's not in the code path.

starbelly commented 4 years ago

It looks like logger_lager_backend is being compiled without lager being in the code path. Disabling tracing will not work because if it's a compile time dependency it would fail to compile if it's not in the code path.

That makes sense, but rebar_mix does prepend a path... "elixir -pa \"../*/ebin\" -S mix compile --no-load-deps " ... removing this definitely triggers warning: :lager.dispatch_log/7 is undefined (module :lager is not available or is yet to be defined). That seems to suggest that the code path is prepended and used, no?

ericmj commented 4 years ago

I think I am missing something. If removing the code path triggers warning: :lager.dispatch_log/7 is undefined then don't remove it.

starbelly commented 4 years ago

@ericmj Yeah, I was just demonstrating how the code path is in place. Maybe you're thinking it can't find the source for lagger?

ericmj commented 4 years ago

[...] removing this definitely triggers warning: :lager.dispatch_log/7 is undefined (module :lager is not available or is yet to be defined)

I am saying that if removing the code path causes the warning to be triggered then don't remove the code path. It seems like that would fix the problem.

Another suggestion would be to disable --warnings-as-errors. I don't think that's possible to configure without changing mix.exs so please open an issue on the elixir project.

starbelly commented 4 years ago

As to why we get warning: :lager.dispatch_log/7 defined in application :lager is used by the current application but the current application does not directly depend on :lager. To fix this, you must do one of: when swapping the order, it's because lager.app isn't in lagger's ebin dir yet. Everything but the .app file is in there.

So, if swapping the order is indeed the fix then we have to overcome that hurdle. However, if the order in which things happened has always been this way, then once again I'd be reluctant to say let's just swap the order.

I don't think there's a simple solution to this. I think it's going to involve rebar3 sussing out that a custom dep needs dep X, but that defeats the purpose of having a custom builder, and really you'd just be merging rebar_mix into rebar3 at that point.

An alternative is to put the onus entirely on rebar_mix to figure that out and to ask rebar3 to go ahead and compile it, then return back to rebar_mix. My gut tells me there's a catch with that though.

josevalim commented 3 years ago

If the issue is the application tracer, we can have a flag that disables it too.