elixir-lang / elixir

Elixir is a dynamic, functional language for building scalable and maintainable applications
https://elixir-lang.org/
Apache License 2.0
24.3k stars 3.35k forks source link

Mix compile of erlang repository breaks on preprocessor use #7736

Closed okeuday closed 6 years ago

okeuday commented 6 years ago

Environment

Current behavior

Building works fine with rebar2 and rebar3 but fails with mix

$ git clone https://github.com/CloudI/cloudi_core/
$ cd cloudi_core
$ mix deps.get
$ mix compile
Compiling 48 files (.erl)
src/cloudi_core_i_logger.erl:1002: illegal pattern
src/cloudi_core_i_logger.erl:1005: variable 'Error' is unbound
src/cloudi_core_i_logger.erl:1006: variable 'ErrorStackTrace' is unbound
...

% Due to using an Erlang macro to handle the `erlang:get_stacktrace/0` deprecation:
-ifdef(ERLANG_OTP_VERSION_19).
-else.
-ifdef(ERLANG_OTP_VERSION_20).
-else.
-define(ERLANG_OTP_VERSION_21_FEATURES, true).
-endif.
-endif.
% Get the stacktrace in a way that is backwards compatible
-ifdef(ERLANG_OTP_VERSION_21_FEATURES).
-define(STACKTRACE(ErrorType, Error, ErrorStackTrace),
        ErrorType:Error:ErrorStackTrace ->).
-else.
-define(STACKTRACE(ErrorType, Error, ErrorStackTrace),
        ErrorType:Error ->
            ErrorStackTrace = erlang:get_stacktrace(),).
-endif.
% Which is used like:
try do_stuff()
catch
    ?STACKTRACE(ErrorType, Error, ErrorStackTrace)
        ErrorMessage = cloudi_string:
                       format_to_binary("formatter(~p) ~p ~p~n~p",
                                        [Formatter, ErrorType, Error,
                                         ErrorStackTrace]),
        % ...
end

Expected behavior

No illegal pattern errors should occur.

fishcakez commented 6 years ago

Mix's behavior is correct here because ERLANG_OTP_VERSION_19 and ERLANG_OTP_VERSION_20 are not defined in the mix.exs file (only in rebar.config files) so ERLANG_OTP_VERSION_21_FEATURES will be defined in the metaprogramming. This will lead to the OTP 21 style stacktrace handling on every version of OTP. However this is being run on OTP 20 so will fail to compile.

okeuday commented 6 years ago

@fishcakez There is no way to define ERLANG_OTP_VERSION_19 and ERLANG_OTP_VERSION_20 based on the version of Erlang/OTP in the mix.exs file, right? Anyway, the compilation should be using rebar2 because the lock file doesn't exist, right?

fishcakez commented 6 years ago

There is no way to define ERLANG_OTP_VERSION_19 and ERLANG_OTP_VERSION_20 based on the version of Erlang/OTP in the mix.exs file, right?

Its possible to set any Erlang compiler option with erlc_options in the mix project via mix.exs. Platform define isn't directly supported because its a rebar feature but could be emulated.

e.g. erlc_options: [{:d, :ERLANG_OTP_VERSION_19}]

Anyway, the compilation should be using rebar2 because the lock file doesn't exist, right?

No, mix will be used in the project itself if mix is invoked. The failure is at the top level application/ project here which is always going to use mix to compile.

okeuday commented 6 years ago

@fishcakez Thank you for pointing out erlc_options, going to use that with:

{:d, :erlang.list_to_atom('ERLANG_OTP_VERSION_' ++ :erlang.system_info(:otp_release))}
michalmuskala commented 6 years ago

@okeuday can I ask why have both mix and rebar in the project? Having just rebar should be perfectly enough, shouldn't it? Mix can handle rebar-only dependencies just fine.

okeuday commented 6 years ago

@michalmuskala mix use was mainly for hex support before it existed in rebar3. rebar3 isn't being used completely due to its lack of filesystem-based dependency support and lack of backwards-compatibility with rebar2 in the main CloudI repository (https://github.com/CloudI/CloudI/). rebar3 did become usable in this repository (https://github.com/CloudI/cloudi_core/) in 2016 or 2017, after it had some improvements, though it hasn't been a high priority to pursue.