Add support for building zstd binding against an external libzstd library

[dopuskh3]

This is heavily inspired by the following PR from @WGH-

Add the external_libzstd tag to be used when we want to build the binding against an externally provided
zstd library. If the tag is defined, the go build system will look for zstd pkg-config package to gather
build arguments.

Added CI jobs to run tests and benchmarks with latest go version.

[evanj]

I think this looks good, but we should get @Viq111 to take a look.

I also wonder if this package should be renaming all the zstd symbols by default, to allow it to be linked along side other versions of zstd? I think we would still want the option to use an external version, but this might reduce the number of places where people would need this option? (clearly that would be another thing to do in a future change, not here)

[WGH-]

When I was working on the PR, I got stuck because Debian Buster (10), on which golang Docker image was based on, had relatively old libzstd, and some of the .go files used new functions that weren't present. I couldn't find an easy way to make CI tests pass.

I suppose you circumvented this issue by waiting until golang images started to base on Debian Bullseye (11)?

If so, does it mean that external_libzstd won't work on older libzstd versions, even if user doesn't use any of those new functions?

[dopuskh3]

When I was working on the PR, I got stuck because Debian Buster (10), on which golang Docker image was based on, had relatively old libzstd, and some of the .go files used new functions that weren't present. I couldn't find an easy way to make CI tests pass.

I suppose you circumvented this issue by waiting until golang images started to base on Debian Bullseye (11)?

If so, does it mean that external_libzstd won't work on older libzstd versions, even if user doesn't use any of those new functions?

external_libzstd will work as soon as the symbols used in the go bindings are available in the lib zstd.h located in the path returned by pkg-config. Unfortunately, cgo doesn't provide the ability to ask for dependency prerequisites (it could for example use pkg-config --atleast/exact/max-version feature)

[dopuskh3]

I think this looks good, but we should get @Viq111 to take a look.

I also wonder if this package should be renaming all the zstd symbols by default, to allow it to be linked along side other versions of zstd?
I agree that bundling symbols that may be provided to the compiler by other projects linking with zstd can make the situation a little bit tricky to handle for larger projects. Specially, in this situation, forgetting to pass the external_libzstd tag to go build can end up silently use vendored source code for other dependencies using libzstd:

Here is what I observed in the following situation:

  [binary] <---- [DataDog/zstd] <---- [vendored zstd code]
     ^
     |
     +---------[confluent-kafka-go] <---- [librdkafka] <----[libzstd]

I observed that the calls to confluent-kafka-go using zstd actually ended up running the [vendored zstd code], which can be annoying in multiple ways.

In order to avoid this situation to silently happen, we could make passing a build tag mandatory by failing the compilation intentionally if none of the vendored_libztd nor external_libzstd is passed.

[Viq111]

Thanks! This looks good to me, I just added some small comments

[Viq111]

This is a bit hacky to define ZSTD_STATIC_LINKING_ONLY in the Go file and then unset in the C file. Is there a way to just have the switch in external_zstd.go ? (i.e: an else that sets it)
(I don't remember if we need to define & import zstd.h in each go file)

[dopuskh3]

Are you sure we actually need to define ZSTD_STATIC_LINKING_ONLY? It appears that library is building fine when that's not defined.

[dopuskh3]

When I was working on the PR, I got stuck because Debian Buster (10), on which golang Docker image was based on, had relatively old libzstd, and some of the .go files used new functions that weren't present. I couldn't find an easy way to make CI tests pass.
I suppose you circumvented this issue by waiting until golang images started to base on Debian Bullseye (11)?
If so, does it mean that external_libzstd won't work on older libzstd versions, even if user doesn't use any of those new functions?

external_libzstd will work as soon as the symbols used in the go bindings are available in the lib zstd.h located in the path returned by pkg-config. Unfortunately, cgo doesn't provide the ability to ask for dependency prerequisites (it could for example use pkg-config --atleast/exact/max-version feature)

@WGH- I added version constraint with preprocessor macros in the external_libzstd.go file here

[Viq111]

lgtm!

Maybe one last thing (that doesn't necessitate another review). Could you add the go build -tags external_libzstd as a section in the README ? That way it's discoverable for users without needing to read the code