OTP-21 support

[tsloughter]

Stacktraces are moving to a value matched in catch in OTP-21 and get_stacktrace() is deprecated.

It is a pretty simple fix in decode_msg to support 21, but supporting both will be kind of ugly.

Not sure if the best solution is generating different code for a 21 switch or generate 2 decode_msg functions with macros around them so the compilation of the erl files themselves can decide which to use.

[ericmj]

I would suggest making the decision of implementation at compile time instead of generation time. Otherwise you need to maintain multiple sets of generated files depending on which version you target.

[tsloughter]

Ah right, generated erl module are often committed to the repo.

[tomas-abrahamsson]

Is there a preprocessor-only way to check for for OTP version? I can't think of any, not without the help of the build system, but that would mean gpb would need to also generate rebar config scripts, Makefiles etc. I see the point about a compile-time check, but I fear it has wider implications.

I have locally a branch with generation time support for the OTP-21 stacktraces. Perhaps worth knowing is that there are already a few other OTP compatibility decisions at generation time in gpb, so the problem is not entirely new. For these decisions, it is possible to specify a target OTP version, and it defaults to the current at generation time.

[ferd]

Yeah unfortunately there are no macros for it yet. You can check for the default macros with a fake empty module and the following undocumented code:

4> {ok,Epp} = epp:open("mod.erl", []).
{ok,<0.87.0>}
5> epp:macro_defs(Epp).
[{{atom,'BASE_MODULE'},undefined},
 {{atom,'BASE_MODULE_STRING'},undefined},
 {{atom,'BEAM'},{none,[{atom,1,true}]}},
 {{atom,'FILE'},{none,[{string,1,"mod.erl"}]}},
 {{atom,'FUNCTION_ARITY'},undefined},
 {{atom,'FUNCTION_NAME'},undefined},
 {{atom,'LINE'},{none,[{integer,1,1}]}},
 {{atom,'MACHINE'},{none,[{atom,1,'BEAM'}]}},
 {{atom,'MODULE'},undefined},
 {{atom,'MODULE_STRING'},undefined}]

nothing shows up regarding the compiler version.

The only way I can think of to do it is to load a parse transform to check versions with system_info calls, but by that time, the macro branching will already have taken place I think.

[michalmuskala]

What about adding the compile attribute to not emit warnings for erlang:get_stacktrace()?

-compile({nowarn_deprecated_functions,[{erlang,get_stacktrace,0}]).

The problem is that this, in turn, will issue warnings in OTP < 20.1 because the function is not deprecated. It really seems there's no good way out of this.

[lrascao]

i'm curious as to the need of keeping generated protobuf marshaling stubs in the repo, is it to reduce build time?

[ferd]

In a specific use case we have, it's to talk with Hex APIs when bootstrapping rebar3, to go fetch the initial packages (such as a gpb compiler). We have to work from a dependency-free initial state.

[tomas-abrahamsson]

Thansk all for valuable input and insights! I'll push a version with generation time support, to start with.

For the use case of storing generated files, I was also thinking whether it would be possible to generate code that someone could more easily be integrated with a build system. For example if the code generated by gpb could use a ?GPB_OTP_VERSION macro or similar. Then the person storing the generated files could also add to his or her build system to define this macro. I haven't thought this through entirely, so don't yet know if this is possible or if the gpb support for it would be too complicated. Just thinking out loud.

[tomas-abrahamsson]

I pushed 4.1.5 which contains generation time support for the new stacktrace syntax.

[ferd]

One macro that got recommended to us by okeuday was either form of:

-define(STACKTRACE(Type, Reason, Stacktrace), Type:Reason:Stacktrace ->).
-define(STACKTRACE(Type, Reason, Stacktrace), Type:Reason -> Stacktrace = erlang:get_stacktrace(), ).

%% Usage

try
    ...
catch
    error:badarg ->
        whatever;
    ?STACKTRACE(E, R, Stack)
        {error, {E,R, Stack}}
end

Put them at the top of every file in a -ifdef based on the target version. If you're targetting OTP 20 or earlier at generation time do something like:

-ifdef(PATTERN_STACK).
-define(STACKTRACE(Type, Reason, Stacktrace), Type:Reason:Stacktrace ->).
-else.
-define(STACKTRACE(Type, Reason, Stacktrace), Type:Reason -> Stacktrace = erlang:get_stacktrace(), ).
-endif.

If targetting OTP21 or above do:

-ifdef(FUNCTION_STACK).
-define(STACKTRACE(Type, Reason, Stacktrace), Type:Reason -> Stacktrace = erlang:get_stacktrace(), ).
-else.
-define(STACKTRACE(Type, Reason, Stacktrace), Type:Reason:Stacktrace ->).
-endif.

That way the default is the correct targeted version, but build tools can override things.

The only downside is that it won't support guards unless you have an alternative macro forms (and even then you can't use , in there I guess), and multi-arity macros weren't supported in older OTP releases, so you may need a variant like:

-ifdef(FUNCTION_STACK).
-define(STACKTRACE(Type, Reason, Stacktrace), Type:Reason -> Stacktrace = erlang:get_stacktrace(), ).
-define(G_STACKTRACE(Type, Reason, Stacktrace, Guards), Type:Reason Guards -> Stacktrace = erlang:get_stacktrace(), ).
-else.
-define(STACKTRACE(Type, Reason, Stacktrace), Type:Reason:Stacktrace ->).
-define(G_STACKTRACE(Type, Reason, Stacktrace, Guards), Type:Reason:Stacktrace Guards ->).
-endif.

%% usage

try
    ...
catch
    ?G_STACKTRACE(error, R, Stack, when R == error orelse R == badarg)
        {handled, Stack};
    ?STACKTRACE(E, R, Stack)
        {error, {E,R, Stack}}
end

I haven't tried the guarded variant, but it should work fine.

[tomas-abrahamsson]

Yes, that ought to do it. I don't need guards in the catch, but thanks for the thoughts about it, should the need arise in the future. I suppose the FUNCTION_STACK and PATTERN_STACK definitions should be GPB_ prefixed so they won't clash for files generated in whatever other project.

[tomas-abrahamsson]

I've just pushed 4.1.6, which addresses this, as far as is currently possible. It adapts the generated code to the target otp release like in the previous 4.1.5, and now it is also possible to override when compiling the generated code.

When targeting Erlang 21 or later, gpb generates code that uses (by default) the new stacktrace syntax, but if the user defines GPB_FUNCTION_STACK when compiling the generated code, it will instead call erlang:get_stacktrace/0

Similarly, when targeting Erlang 20 or earlier, gpb generates code that calls erlang:get_stacktrace/0, but the user can define GPB_PATTERN_STACK to override when compiling the generated code.

[tomas-abrahamsson]

I'm closing this ticket since 4.1.6 is out containing the proposed stacktrace support. Please feel free to just reopen, if there's anything more to take care of!

[tomas-abrahamsson]

I'm reopening this. Now that erlang/otp#1810 was merged, it is possible to do better using the ?OTP_RELEASE macro.

[tomas-abrahamsson]

I've pushed 4.1.8, which generates code that checks whether OTP_RELEASE is defined.

Since current latest official release or rc, Erlang 21.0-rc1, has deprecated erlang:get_stacktrace/0 but no OTP_RELEASE, I left the code in place that also checks for the GPB_FUNCTION_STACK and GPB_PATTERN_STACK. But when Erlang 21.0 is released, I plan to remove that.

So to be clear, in 4.1.8, it generates code like when on Erlang 21 (also when on Erlang 21.0-rc1)

-ifdef('OTP_RELEASE').
    try ...
    catch Class:Reason:StackTrace -> ...
    end
-else.
-ifdef('GPB_FUNCTION_STACK').
    try ...
    catch Class:Reason ->
        StackTrace = erlang:get_stacktrace(),
        ...
    end.
-else.
    try ...
    catch Class:Reason:StackTrace -> ...
    end.
-endif.
-endif.

When on Erlang 20 or earlier the ifdef(GPB_FUNCTION_STACK) is instead ifdef(GPB_PATTERN_STACK) with content of inner 'then' and 'else' branches swapped, just as agreed earlier and in gpb 4.1.6.

And when Erlang 21 gets released, I plan to change generation so that the inner ifdef(GPB_FUNCTION_STACK)...else...endif part to just an old-style try ... catch Class:Reason -> with a call to erlang:get_stacktrace() regardless of Erlang version.

So I'm closing this again. Just re-open if there's anything more.

[tsloughter]

@tomas-abrahamsson hm, not sure if I'm just missing something or do we need to currently add the erl_opts d to rebar.config for it to work?

[tomas-abrahamsson]

@tsloughter What versions of things? I just tried with Erlang 21-rc2 from the git repo (cabbb94eab) and also with 20.3.7+dfsg-1 (and gpb 597c91b) and then compiled a simple proto file, which compiles cleanly with erlc with no particular options, with both Erlang setups.

[tsloughter]

My bad... after cleaning everything out I'm now getting a different result from the gpb plugin, one that works :). No idea what I was doing last night, I must not have had it actually recompiling the proto files.

Sorry for the noise, it is working great :)

[tomas-abrahamsson]

No problems, glad things sorted themselves out!