Remove dependency on hut

[saleyn]

Removed logging dependency on hut.

[cw789]

Thank you so much. 💛 This is / was the last thing that does depend on rebar in my deps.

[saleyn]

The OTP log macros expect either a text string or a map with an error report. Implementation was adjusted accordingly.

[seriyps]

Oh, but why do you need a new header file? I think it might be better to just use OTP ?LOG_* macros directly? Also, it might be a good idea to add the domain meta-key https://erlang.org/doc/man/logger_filters.html (say, [gen_smtp, client], [gen_smtp, server]) or maybe just [gen_smtp])

[saleyn]

The kernel's ?LOG_* macros prefer log reports as opposed to formatted log messages, so, to use them directly would require to convert all calls to pass log reports (i.e. a hash table of key/value pairs). This is doable but not a backward-compatible change for logging in this project.

[saleyn]

Regarding the failing CI run - in OTP 24 the erlang:get_stacktrace/0 was removed, and the rebar3 included in the project is failing when run on a system with the latest Erlang/OTP. Probably it's best not to include the rebar3 in the project, but to use the externally installed rebar3?

[seriyps]

?LOG_INFO("hello ~s", ["world"]) works just as you expect...

[saleyn]

?LOG_INFO("hello ~s", ["world"]) works just as you expect...

For some reason when running in OTP 24 this was causing some crash errors in the logger. Maybe it was something related to my environment. I'll double check.

[saleyn]

The issue was related to my local environment. It seems to be good now.

[saleyn]

Is this really necessary to include rebar3 binary in the repos? This is generally not considered a good practice. The CI environment can get a binary of rebar3 independently of this project. Note that running the build with the old rebar3 in the master on the system with OTP 24 installed crashes with:

$ make
...
escript: exception error: undefined function erlang:get_stacktrace/0
  in function  rebar3:main/1 (/opt/rebar3-3.11.0/src/rebar3.erl, line 72)
  in call from escript:run/2 (escript.erl, line 750)
  in call from escript:start/1 (escript.erl, line 277)
  in call from init:start_em/1 
  in call from init:do_boot/3 
make: *** [Makefile:4: compile] Error 127

[seriyps]

I honestly don't have an opinion on whether include it or not. But I think it might be nice to at least have a fixed version of rebar3 in CI (by, eg, downloading it not from https://s3.amazonaws.com/rebar3/rebar3 but from, say, github releases: https://github.com/erlang/rebar3/releases/download/3.16.1/rebar3; not sure which exact version would work here though)

[mworrell]

The newest rebar3 only works with the last three OTP versions (it doesn't run anymore on OTP20).

With Zotonic we tried to download a fixed version from GH, but we ran into build issues where our downloads were failing. We reverted back to using the newest rebar3 version.

[saleyn]

The newest rebar3 only works with the last three OTP versions (it doesn't run anymore on OTP20).

With Zotonic we tried to download a fixed version from GH, but we ran into build issues where our downloads were failing. We reverted back to using the newest rebar3 version.

@mworrell, wouldn't it make sense though to make the version-specific rebar3 download to be part of the CI build for a given environment rather than including it in the project? Github provides a template example for setting this up.

[cw789]

Is there anything I (as noob) can help to get this pull request ready?

[seriyps]

I think we should merge #273 and then rebase this PR on top of it. Not sure if any help is needed besides maybe reviewing #273.

[mworrell]

273 has been merged, so this can now be rebased on top of master

[seriyps]

Guess this line should be removed as well: https://github.com/gen-smtp/gen_smtp/blob/d0128401dd7821178b6bfe8168281fd87cb4a0a4/rebar.config#L47

[seriyps]

Also please consider adding the domain to the log metadata. This way it would be much easier to filter the logs created by gen_smtp https://github.com/gen-smtp/gen_smtp/pull/267#issuecomment-869244220

smth like

?LOG_INFO("smth smth ~p", [Param], #{domain => [gen_smtp]}) or ?LOG_INFO("smth smth ~p", [Param], #{domain => [gen_smtp, client]}) / ?LOG_INFO("smth smth ~p", [Param], #{domain => [gen_smtp, server]}) etc.

so later some special treatment can be done for the logs originating from from the gen_smtp, say:

 {handler, default, logger_std_h,
   #{
         level => debug,
         filters => [{stop_gen_smtp, {fun logger_filters:domain/2, {stop, sub, [gen_smtp]}}}],
...

but I won't insist if you have no time for that.

[cw789]

May I kindly ask if there is a chance to get this into the next release.

[seriyps]

Created a new PR #309 that does more or less the same

[mworrell]

Merged #309

[mworrell]

Closing, as #309 has the same effect of removing hut.