Closed

It was stopped because the rust coverage unit tests require many (more than 400) symbols to be defined. It’s not clear why the linker behavior changes. A speculation is that it is related to changes due to command specifics:

cargo +nightly llvm-cov test --doctests --codecov --workspace ...

Initial Goal

Link RSValue and dependencies, which is a variant data type that contains:

• Constructor Functions for each type
  • Four different pathways for allocations (malloc, mempool_t, sds and strdup)
• Set Functions for each type
• Helper Functions (ToString, RSValue_StringPtrLen, compare_arrays, convert..., …)
• RSValue_SendReply

Problem

There was a dependency on mempool_t, which required parts of the RedisModule API.

We started by mocking those RedisModule API parts, but more unresolved symbols appeared.

The C Code is not very modular, which means that we need many symbols quickly. There are two possible solutions:

1. Modularize the C Code, which includes many headers if not needed.
2. Do the work and mock all the open symbols.

Solved Challenges

• Linked C-Code usable in Rust Unit tests
• Rust allocator shims were not C standard conformant. MOD-10248 - Less restrictive Rust allocator shim #6367
• Workaround for mocking a C function with variadic arguments.

  RediSearch/src/c2rust/variadic_helper.c

  Line 16 in 52d9810

  // part of workaround for <https://github.com/rust-lang/rust/issues/44930>

Open Challenges

The CI job that generates the test coverage report uses different linker settings. However, the output is not easily reproducible.

The command ./build.sh FORCE COV=1 RUN_UNIT_TESTS RUN_RUST_TESTS acts like the CI job. The problem occurs in Rust, so running ./build.sh FORCE DEBUG COV=1 RUN_RUST_TESTS should be enough.

• macOS: Not reproducible on macOS
• The Ubuntu-based development container generates unrelated errors when running coverage. The container does not have the right tools or the correct version of the tools installed.
• CI and Pieters system: Fail to link redis_mock

When we get rid of the error-limit of the linker, we still receive over 400 symbols that need to be defined for the PR. The following code shows how to get rid of the linker error:

if target_os != "macos" {
        //println!("cargo:rustc-link-arg=-Wl,--unresolved-symbols=ignore-all");
        println!("cargo:rustc-link-arg=-Wl,--error-limit=0");
    }

The line linker argument --unresolved-symbols=ignore-all shifts the problem from build time to runtime and breaks on the first symbol that cannot be resolved.

Describe the changes in the pull request

We need these changes to run Rust unit tests that use RSValue internally; more precisely, RSValue's reference-counted methods require linking to the Rust test binaries. The tests with coverage require part of the RedisModule API mocked out.

1. Generate Bindings for value.c module (RSValue)
2. Generate Bindings for mempool.c module (mempool_t) used by RSValue for allocations
3. Generate Bindings for global variables setup in module initialization (RSGlobalConfig and RSDummyContext)
4. Be less restrictive in Rust's allocator shims (no panics if size=0 is given as parameter anymore) --> mempool_t relies on allocations with size zero. And it's safe as long as the pointers don't get dereferenced.
5. Mock/Impl more methods for allocation, e.g. RedisModule_TryAlloc, the reply functions, the Command API.

Extracted from: #6311

Mark if applicable

• This PR introduces API changes
• This PR introduces serialization changes