getredis: apply obsolete ccflags_lto Makefile patch best-effort (non-fatal)#151
Open
gabsow wants to merge 1 commit into
Open
getredis: apply obsolete ccflags_lto Makefile patch best-effort (non-fatal)#151gabsow wants to merge 1 commit into
gabsow wants to merge 1 commit into
Conversation
gabsow
force-pushed
the
tom.gabsow/fix-obsolete-ccflags-lto-patch
branch
from
b108f5d to
9ed0455
Compare
…applies redis upstream incorporated the clang-aware LTO ccflags change and refactored src/Makefile, so ccflags_lto.patch no longer applies cleanly on current redis. Applying it unconditionally either aborted the whole redis build (fatal) or, worse, applied partially (one hunk) and corrupted the Makefile, breaking the build. Gate it behind a --dry-run: apply only if the whole patch still applies, otherwise skip it (modern redis already selects the correct LTO flags, so the tweak is redundant and a no-op skip is safe). Never leave the Makefile half-patched. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
gabsow
force-pushed
the
tom.gabsow/fix-obsolete-ccflags-lto-patch
branch
from
9ed0455 to
8266cf9
Compare
gabsow
added a commit
to RedisBloom/RedisBloom
that referenced
this pull request
Jun 4, 2026
…er/valgrind) Bumps deps/readies to the getredis fix (apply the obsolete ccflags_lto Makefile patch only if it fully applies; never partially) to verify it unblocks the redis build and the linux-sanitizer / linux-valgrind jobs. readies PR: RedisLabsModules/readies#151 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
gabsow
added a commit
to RedisBloom/RedisBloom
that referenced
this pull request
Jun 4, 2026
…r/valgrind redis Two changes to green the linux-sanitizer / linux-valgrind jobs on this branch: 1. deps/readies bumped to the getredis fix (apply obsolete ccflags_lto patch only if it fully applies; never partially). 2. tests/flow/tests.sh: use the sanitizer/valgrind redis-server that .install/install_redis.sh already builds in the CI image, instead of building an older redis (6.2) from source via getredis (mirrors master, which dropped the getredis-asan/vg build). readies PR: RedisLabsModules/readies#151 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
getredispatches redis'ssrc/Makefilewithccflags_lto.patch(a clang-aware LTO ccflags tweak). That change has since been upstreamed into redis and the Makefile was refactored (LTO flags moved ontoOPTIMIZATION, and theCLANG := $(findstring clang,...)detection added upstream). The patch's context no longer exists on current redis, so:Because the call was fatal, this aborted the entire
getredisredis build → noredis-server→ every downstream test failed withConnection refused.Fix
Apply the patch with
_try=Trueso a non-applying (now redundant) LTO tweak doesn't abort the redis build. Modern redis already selects the correct LTO flags for clang/gcc, so skipping it is harmless. One-line change + explanatory comment.How it surfaced
RedisBloom release-branch CI (
build/linux-sanitizer/linux-valgrindjobs) install redis viadeps/readies/bin/getredis -v <N>; all failed at theccflags_lto.patchstep. (Modulemasterbranches that install redis via a plainmakeare unaffected.)Note
Low Risk
Build-script-only change; skips a redundant Makefile patch when context is missing, with no runtime or security impact.
Overview
getredisno longer treats a failedccflags_lto.patchapply as fatal when building Redis from source.ccflags_lto_patch()now runspatch -p1 --dry-runfirst (via_try=True). The real patch runs only if the dry-run succeeds; if hunks no longer match (e.g. LTO/clang logic already upstream), the step is skipped and the Redis build continues.This unblocks CI that installs Redis through
getredison current Redis trees where the patch is redundant.Reviewed by Cursor Bugbot for commit 8266cf9. Bugbot is set up for automated code reviews on this repo. Configure here.