zstd CLI can now be linked to libzstd dynamic library

[Cyan4973]

supersedes #2456
and possibly #2450

[cemeyer]

I'm not sure this is needed. When I looked, it appeared that our default -fvisibility=hidden was not applied to xxhash symbols in libzstd.so at all. (Maybe that is a bug.)

[Cyan4973]

It is not the intended visibility anyway

[cemeyer]

$ nm -D lib/libzstd.so.1.4.8 | grep XXH
000000000000f7c0 T ZSTD_XXH32
0000000000010360 T ZSTD_XXH32_canonicalFromHash
000000000000f760 T ZSTD_XXH32_copyState
000000000000fc20 T ZSTD_XXH32_createState
000000000000feb0 T ZSTD_XXH32_digest
000000000000fc30 T ZSTD_XXH32_freeState
0000000000010380 T ZSTD_XXH32_hashFromCanonical
000000000000fc60 T ZSTD_XXH32_reset
000000000000fd00 T ZSTD_XXH32_update
000000000000f930 T ZSTD_XXH64
0000000000010370 T ZSTD_XXH64_canonicalFromHash
000000000000f780 T ZSTD_XXH64_copyState
000000000000fc40 T ZSTD_XXH64_createState
0000000000010140 T ZSTD_XXH64_digest
000000000000fc50 T ZSTD_XXH64_freeState
0000000000010390 T ZSTD_XXH64_hashFromCanonical
000000000000fca0 T ZSTD_XXH64_reset
000000000000ff80 T ZSTD_XXH64_update
000000000000f750 T ZSTD_XXH_versionNumber

Oops :)

[Cyan4973]

I tested 1.4.8 on a recent Ubuntu station,
and observed the same issue : all symbols, including internal ones, seem to be visible
(note: I assume that a symbol name prefixed by T means "visible", while t means "not visible").

What's strange is that I also checked the compilation line, and the link stage does include the -fvisibility=hidden directive. So visibility was supposed to be more selective.

However, we have recently changed the build system to separate compilation and link stage, in order to offer incremental build for faster development iteration. -fvisibility=hidden is not part of compilation (into *.o), only part of link stage.

I tested making it part of compilation too, and it seems to fix the visibility issue.

Which seems strange to me, because it seems to contradict all documentation I could read about this feature (and re-checked today), which was labelled "link stage only". So I'm no longer sure what's the "ideal" solution here.

That's probably an issue to fix in another PR.

[terrelln]

The changes look good to me!

I do have a question about the objective. Do we want to officially support linking the zstd CLI against the dynamic library? If so, do we want to only support CLI version == lib version? Should we add a version check the the CLI that checks ZSTD_VERSION_NUMBER == ZSTD_versionNumber()?

[cemeyer]

Do we want to officially support linking the zstd CLI against the dynamic library?

I would like that, if it isn't too much additional burden.

If so, do we want to only support CLI version == lib version?

I would be happy with this restriction. I don't know to what extent we think about ABI compatibility and symbol versioning in the libzstd shared library today (for non-CLI consumers).

The proposed implementation of CLI version == lib version sounds reasonable to me.

Thanks, folks!

[terrelln]

I would be happy with this restriction. I don't know to what extent we think about ABI compatibility and symbol versioning in the libzstd shared library today (for non-CLI consumers).

We keep ABI compatibility for the "stable" portion of our API (everything that isn't hidden behind ZSTD_STATIC_LINKING_ONLY). But the CLI uses "unstable" symbols.

[Cyan4973]

With these changes, the zstd CLI no longer depends on "internal" symbols,
but it still depends on experimental capabilities and symbols exposed by the library.
We need this capability, as the zstd CLI is one of these "controlled environment" where we can introduce new stuff, observe and refine, up to the point that it makes sense for it to become part of the "stable" section of the library's API.

Because experimental symbols and capabilities can change from one version to another, it's indeed safer to lock the library version.

One thing which I'm not completely sure of is :
what happens in case of a version mismatch, specifically if the CLI needs a symbol which is not present in the library (following a rename, or a deprecation) ?
Does it even have the time to check the version number and exit with a clean message ?
Or will it die immediately due to a missing symbols ?

I presume it may depend on the way library symbols are linked in the program : lazy evaluation should defer the discovery of the missing symbol up to the moment it's needed. Therefore, there is time for a runtime check.
But maybe this outcome depends on the specificity of the link model, hence may depend on the Operating System.

[Cyan4973]

There is a remaining test error, but it's unrelated :

latest: Pulling from emscripten/emsdk
anyrequests: You have reached your pull rate limit. You may increase the limit by authenticating and upgrading: https://www.docker.com/increase-rate-limit.
See 'docker run --help'.
Compiling examples/emscripten.c (using docker): FAILED
Makefile:121: recipe for target 'contrib' failed
make: *** [contrib] Error 1