intel / intel/ScalableVectorSearch
apply_patch_toml.sh: reverse-check probe prints 'patch does not apply' errors on every fresh configure
- 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
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