Skip to content

Commit

Permalink
Use def_ref_codec_lib instead of including def_ref_code.{cc|hpp} as s…
Browse files Browse the repository at this point in the history
…ources

def_ref_codec.cc is listed as a source of both object_codec_lib and
def_ref_codec_lib targets. It does not cause problems for binaries
that only depend on one of those targets, but when you start
depending on both you might hit some issues.

Specifically, in Envoy, we have an extension that depends on
//hessian2:codec_impl_lib and //hessian2/basic_codec:object_codec_lib (see
https://github.com/envoyproxy/envoy/blob/d82453231f89cb021eb716a23805073cff1f7179/source/extensions/filters/network/dubbo_proxy/BUILD#L12-L22).
As a result def_ref_codec.cc file is referenced as a source twice:

1. as one of  the sources of //hessian2/basic_codec:object_codec_lib
2. as one of the sources of //hessian2/basic_codec:def_ref_codec_lib that is a
   dependency of //hesian2:codec_impl_lib.

Using the same source file multiple times as a source in Bazel does not
seem to be common, but it also may cause issues.

For once, in C++ in principle having the same object file listed multiple
times when linking may result in multiple definitions error (depending on
the actual content of the object file), but in my case I hit a different
issue.

I'm working on a project in which I have to build Envoy, but, unfortunately,
I cannot use the Envoy CI image with all the tools preinstalled, as a result
environment in which I build Envoy is somewhat different from the one provided
by Envoy CI Docker image.

And when I try to build Envoy, at some point Bazel calls dwp tool to collect
debugging information from dwo files into a single dwp file. The command
Bazel uses to call dwp references def_ref_codec.dwo twice because
def_ref_codec.cc is referenced twice as a source. dwp tool is not equipped to
deal with that and complains about duplicate debug info and ultimately fails
build:

```
error: duplicate DWO ID (852C89B14FE85A8F) in 'external/com_github_alibaba_hessian2_codec/hessian2/basic_codec/def_ref_codec.cc' (from 'bazel-out/k8-opt/bin/external/com_github_alibaba_hessian2_codec/hessian2/basic_codec/_objs/def_ref_codec_lib/def_ref_codec.dwo') and 'external/com_github_alibaba_hessian2_codec/hessian2/basic_codec/def_ref_codec.cc' (from 'bazel-out/k8-opt/bin/external/com_github_alibaba_hessian2_codec/hessian2/basic_codec/_objs/object_codec_lib/def_ref_codec.dwo')
```

I know that when you build Envoy with Envoy CI docker image the same issue
does not happen, but after debugging for a few days, I still don't quite
understand why. I tried different versions of the dwp tool, but they all
return the same error, which makes me think that the issue is likely not
in the dwp tool itself.

Even though I still don't know why it works currently in Envoy, it seems
that this change makes sense (we should not list the same cc file multiple
times as a source) and it should be safe (the same code is still listed
as a dependency), so I figured that folks might be open to accept the change
even with some unknowns.

Signed-off-by: Mikhail Krinkin <krinkin.m.u@gmail.com>
  • Loading branch information
krinkinmu committed Dec 30, 2024
1 parent e528f1b commit 2a1733e
Showing 1 changed file with 1 addition and 2 deletions.
3 changes: 1 addition & 2 deletions hessian2/basic_codec/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -203,8 +203,6 @@ cc_library(
srcs = [
"class_instance_codec.cc",
"class_instance_codec.hpp",
"def_ref_codec.cc",
"def_ref_codec.hpp",
"list_codec.cc",
"list_codec.hpp",
"map_codec.cc",
Expand All @@ -217,6 +215,7 @@ cc_library(
":bool_codec_lib",
":byte_codec_lib",
":date_codec_lib",
":def_ref_codec_lib",
":number_codec_lib",
":ref_object_codec_lib",
":string_codec_lib",
Expand Down

0 comments on commit 2a1733e

Please sign in to comment.