proxy-wasm / proxy-wasm-cpp-sdk

WebAssembly for Proxies (C++ SDK)
Apache License 2.0
139 stars 67 forks source link

LOG macro conflicts #154

Open kyessenov opened 1 year ago

kyessenov commented 1 year ago

Abseil has defined LOG as well: https://github.com/abseil/abseil-cpp/blob/master/absl/log/log.h

proxy-wasm-api.h defining the macro unconditionally causes issues importing abseil into wasm.

mpwarres commented 1 year ago

That's an unfortunate collision, though I guess it isn't surprising. We can't simply switch to something like PROXY_WASM_LOG since that would break existing code. But maybe we could have a conditional define similar to PROXY_WASM_PROTOBUF, which (if defined) would cause proxy_wasm_api.h to define the LOG macros as PROXY_WASM_LOG instead of LOG.

@kyessenov supposing we added such a USE_PROXY_WASM_LOG macro, would that address your issue?

kyessenov commented 1 year ago

Yeah, that would work. The problem is mainly "null VM" code since it imports both parts of Envoy with absl and proxy-wasm header. I think it's fine to disable the log macros in those modules since they are not standalone and coupled with Envoy.

PiotrSikora commented 1 year ago

Does anybody use LOG directly? I think it's an internal implementation detail and end users use LOG_TRACE, etc. If that's the case, then we should be able to rename it without any extra defines.

kyessenov commented 1 year ago

@PiotrSikora Well, it's public so we can't be sure. I'm not sure about the desire for backwards compatibility for the SDK - it might be OK to break it since it doesn't seem to be versioned in any way.