intel / intel/ScalableVectorSearch

apply_patch_toml.sh: reverse-check probe prints 'patch does not apply' errors on every fresh configure

Open Beginner friendly
#356 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
C++
Stars
236
Forks
48
Avg merge
4d 22h
Merged PRs (30d)
10

Description

### Summary

`cmake/patches/apply_patch_toml.sh` prints four `error:` lines on every **fresh** configure, even though everything works correctly:

```
[ 44%] Performing patch step for 'tomlplusplus-populate'
error: patch failed: include/toml++/impl/make_node.h:134
error: include/toml++/impl/make_node.h: patch does not apply
error: patch failed: CMakeLists.txt:31
error: CMakeLists.txt: patch does not apply
```

### Cause

The script answers the question "was the patch already applied?" by attempting a reverse apply:

```sh
if ! git apply -R --ignore-whitespace ${TOML_PATCH} --check; then git apply --ignore-whitespace ${TOML_PATCH}; fi
```

`git apply -R --check` fails **loudly** when the patch is *not yet* applied — which is the normal case on every fresh FetchContent clone. The errors come from the probe, not from the actual application (which succeeds silently right after). Downstream consumers see scary `patch does not apply` errors in otherwise green build logs (we hit this building RediSearch → VectorSimilarity → SVS, and spent time confirming the patch was in fact applied).

### Suggested fix

Probe the state instead of reverse-applying — the `TOMLPLUSPLUS_INSTALL` option that the patch itself adds to `CMakeLists.txt` is proof of appliedness:

```sh
if ! grep -q TOMLPLUSPLUS_INSTALL CMakeLists.txt; then git apply --ignore-whitespace ${TOML_PATCH}; fi
```

- Same idempotence for CMake re-configures (the case the script guards against, per the comment in `cmake/toml.cmake`).
- Nothing is silenced: the real `git apply` keeps its stderr, so a genuinely failing patch still surfaces.
- Verified against a fresh `marzer/tomlplusplus` clone at `v3.3.0`: first run applies the patch with zero output, second run skips with zero output.

Happy to open a PR if you'd take one.

Contributor guide

Open the contributing guide

Research direction

Open cmake/patches/apply_patch_toml.sh and review how it checks whether the tomlplusplus patch is already applied; also read the related guard in cmake/toml.cmake. Run a fresh configure and a second configure to verify that the patch applies without probe errors, remains idempotent, and still reports genuine application failures.

Written by the indexing model from the issue text.

Assessment

Tech stack
cmake, git, shell
Domain
build-system
Issue type
Bug
Difficulty
1/5
Estimated time
Under an hour
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
88/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.