dictBuilder sources not in single file library

[indygreg]

contrib/single_file_libs/zstd-in.c doesn't #include source files in dictBuilder/, meaning the various ZDICT_ symbols aren't included in the single file library.

Note that if you add the dictBuilder/ source files, you get various symbol collisions building the resulting file. Colliding symbols include prime4bytes, DEFAULT_SPLITPOINT, g_displayLevel, g_time, and others.

Should dictBuilder/ source files be included in the single file full library by default?

[Cyan4973]

My understanding is that this functionality, having the dictBuilder bundled inside the single-file library, is not supported yet. cc @cwoffenden .

[cwoffenden]

I could add that to the full library. I'll take a look.

[cwoffenden]

@Cyan4973 cover.c and fastcover.c contain a few sections of the same code. I could guard the macros and rename a few of the static vars. It would be more correct to share them in a common file though. I'll take a look when I can.

[terrelln]

@cwoffenden feel free to move them to a common file. If they don't need to be inlined then cover.c would work with a declaration in cover.h. If they do need to be inlined for speed then putting them in cover.h would be appropiate.

[cwoffenden]

This has now been merged as should work.

[Cyan4973]

Thanks @cwoffenden !

@indygreg , you might want to test latest development version in dev branch,
see if the single-file library can now fit your needs.

[yoshihitoh]

Thank you for this great feature!
I'm planning to use the amalgamated file on the Emscripten based project but failed to build it due to redefinitions of static vars.

Emscripten: 1.39.18
zstd branch: dev (7afd5d8)

$ ./build_library_test.sh
Amalgamating files... this can take a while
Processing: zstd-in.c
...
Combine script: PASSED
Single file library creation script: PASSED
Compiling roundtrip.c: PASSED
Compression: PASSED (size: 3127, uncompressed: 32768)
Decompression: PASSED
Byte comparison: PASSED
Running roundtrip.c: PASSED
zstd.c:30822:21: error: redefinition of 'g_ctx' with a different type: 'COVER_ctx_t *' vs 'POOL_ctx' (aka 'struct POOL_ctx_s')
static COVER_ctx_t *g_ctx = NULL;
                    ^
zstd.c:7413:17: note: previous definition is here
static POOL_ctx g_ctx;
                ^
zstd.c:30869:26: error: passing 'POOL_ctx' (aka 'struct POOL_ctx_s') to parameter of incompatible type 'COVER_ctx_t *'
  int result = COVER_cmp(g_ctx, lp, rp);
                         ^~~~~
zstd.c:30845:35: note: passing argument to parameter 'ctx' here
static int COVER_cmp(COVER_ctx_t *ctx, const void *lp, const void *rp) {
                                  ^
zstd.c:30879:27: error: passing 'POOL_ctx' (aka 'struct POOL_ctx_s') to parameter of incompatible type 'COVER_ctx_t *'
  int result = COVER_cmp8(g_ctx, lp, rp);
                          ^~~~~
zstd.c:30853:36: note: passing argument to parameter 'ctx' here
static int COVER_cmp8(COVER_ctx_t *ctx, const void *lp, const void *rp) {
                                   ^
zstd.c:31210:11: error: assigning to 'POOL_ctx' (aka 'struct POOL_ctx_s') from incompatible type 'COVER_ctx_t *'
    g_ctx = ctx;
          ^ ~~~
4 errors generated.
emcc: error: '/Users/yoshihitoh/workspace/tools/sdk/emsdk/upstream/bin/clang -target wasm32-unknown-emscripten -D__EMSCRIPTEN_major__=1 -D__EMSCRIPTEN_minor__=39 -D__EMSCRIPTEN_tiny__=18 -D_LIBCPP_ABI_VERSION=2 -Dunix -D__unix -D__unix__ -Werror=implicit-function-declaration -Xclang -nostdsysteminc -Xclang -isystem/Users/yoshihitoh/workspace/tools/sdk/emsdk/upstream/emscripten/system/include/compat -Xclang -isystem/Users/yoshihitoh/workspace/tools/sdk/emsdk/upstream/emscripten/system/include -Xclang -isystem/Users/yoshihitoh/workspace/tools/sdk/emsdk/upstream/emscripten/system/include/libc -Xclang -isystem/Users/yoshihitoh/workspace/tools/sdk/emsdk/upstream/emscripten/system/lib/libc/musl/arch/emscripten -Xclang -isystem/Users/yoshihitoh/workspace/tools/sdk/emsdk/upstream/emscripten/system/local/include -Xclang -isystem/Users/yoshihitoh/workspace/tools/sdk/emsdk/upstream/emscripten/system/include/SSE -Xclang -isystem/Users/yoshihitoh/workspace/tools/sdk/emsdk/upstream/emscripten/cache/wasm-lto/include -DEMSCRIPTEN -fignore-exceptions -fno-inline-functions -Wall -Wextra -Werror -Os -flto -I. zstd.c -Xclang -isystem/Users/yoshihitoh/workspace/tools/sdk/emsdk/upstream/emscripten/system/include/SDL -c -o /var/folders/x7/xy9p7bj978sgz5b15_l26_4c0000gq/T/emscripten_temp_g31g5lty/zstd_0.o -flto=full' failed (1)
Compiling emscripten.c: FAILED

The symbol g_ctx is used for different types COVER_ctx_t* and POOL_ctx.
I confirmed renaming those static variables fixes this compilation error.

https://github.com/yoshihitoh/zstd/commit/c6548eac8ea6c9c6faf464d7dda9b6ec81123e5d

$ ./build_library_test.sh
Amalgamating files... this can take a while
Processing: zstd-in.c
...
Combine script: PASSED
Single file library creation script: PASSED
Compiling roundtrip.c: PASSED
Compression: PASSED (size: 3127, uncompressed: 32768)
Decompression: PASSED
Byte comparison: PASSED
Running roundtrip.c: PASSED
Compiling emscripten.c: PASSED

If it is allowed to rename those static variables, I want to make a PR. :)

[cwoffenden]

Hmm, odd that it hadn't failed for me (it does, and on the original branch I where I made the changes).

Checking, it only fails when the Emscripten compiler is installed (and the Emscripten test runs). I should really extend the API tests for the regular cc.

[yoshihitoh]

How about using docker to test Emscripten build if emcc is not available?
I think it is very difficult to test all flags combination, so to test builds for specific platforms is a good idea.

I confirmed this command works well on my branch https://github.com/yoshihitoh/zstd/commit/0c2c9efcbda04a45cfec7d867508a69153251e8f. (Running on contrib/single_file_libs)

docker container run --rm \
    --volume $PWD:/code \
    --workdir /code \
    trzeci/emscripten \
    emcc $CC_FLAGS -s WASM=1 -I. -o $OUT_WASM zstd.c examples/roundtrip.c

[cwoffenden]

I think that's a good idea, yes.