erlavro icon indicating copy to clipboard operation
erlavro copied to clipboard

snappyer introduces a c++ build requirement

Open mrjoelkemp opened this issue 2 years ago • 2 comments

Hey! Just wanted to flag that the use of snappyer in recent versions seems to now require a C++ toolchain in order to build erlavro.

This is fixable at the end-user level but breaking in docker environments. For example, with FROM hexpm/elixir:1.13.3-erlang-24.2.2-ubuntu-bionic-20210930 as builder, I'm now seeing a build error like:

10:33:49.099Z #13 40.78 ===> Compiling /app/deps/snappyer/c_src/snappy-sinksource.cc
10:33:49.099Z #13 40.78 ===> sh: exec: line 1: c++: not found
10:33:49.099Z #13 40.78 
10:33:49.099Z #13 40.80 ** (Mix) Could not compile dependency :snappyer, "/root/.mix/rebar3 bare compile --paths /app/_build/test/lib/*/ebin" command failed. Errors may have been logged above. You can recompile this dependency with "mix deps.compile snappyer", update it with "mix deps.update snappyer" or clean it with "mix deps.clean snappyer"
10:33:49.099Z #13 ERROR: executor failed running [/bin/sh -c mix do deps.get --only $MIX_ENV, deps.compile]: exit code: 1
10:33:49.099Z ------
10:33:49.099Z  > [ 9/15] RUN mix do deps.get --only test, deps.compile:
10:33:49.099Z ------
10:33:49.099Z executor failed running [/bin/sh -c mix do deps.get --only $MIX_ENV, deps.compile]: exit code: 1

I only flag this because I wasn't sure of the intentions and wanted to provide the datapoint. I went from 2.9.3 to 2.9.8 and saw this issue, which is unexpected for patch upgrades.

mrjoelkemp avatar Jul 18 '22 15:07 mrjoelkemp

I think that ship has sailed. Also lots of people, ourselves included, do have a C++ compiler in their builders, so won't notice this regression. I think the cleanest way out of this would be to make any compressor/decompressor a user-supplied module. That would put the burden of building and enabling such on the users and not the library itself. I'm not sure that's worth it just to eliminate a C++ requirement. Note most of our team is on vacation right now so responses here will be slow.

mikpe avatar Jul 19 '22 08:07 mikpe

We've just updated erlavro in a nerves setup and also ran into issues with building snappyer, though more specifically with the cross compilation setup nerves provides. We're not using snappy compression, so making it an optional dependency does sound like a nice option to us. I also saw that snappy is an optional codec listed in the avro spec, so I don't think it would be unreasonable to handle it as optional from the erlavro perspective.

LostKobrakai avatar Feb 09 '24 13:02 LostKobrakai