Skip to content

backport: bitcoin/bitcoin#32693: depends: fix cmake compatibility error for freetype#7372

Open
knst wants to merge 1 commit into
dashpay:developfrom
knst:bp-32693
Open

backport: bitcoin/bitcoin#32693: depends: fix cmake compatibility error for freetype#7372
knst wants to merge 1 commit into
dashpay:developfrom
knst:bp-32693

Conversation

@knst

@knst knst commented Jun 19, 2026

Copy link
Copy Markdown
Collaborator

Issue being fixed or feature implemented

It fixes freetype dependency build on Kubuntu 26.04 which provide cmake-4 by default.

What was done?

Backport bitcoin#32693

How Has This Been Tested?

Build succeed

Breaking Changes

N/A

Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have added or updated relevant unit/integration/functional/e2e tests
  • I have made corresponding changes to the documentation
  • I have assigned this pull request to a milestone (for repository code-owners and collaborators only)

@knst knst added this to the 23.1.5 milestone Jun 19, 2026
@knst knst requested review from PastaPastaPasta and UdjinM6 June 19, 2026 20:53
@thepastaclaw

thepastaclaw commented Jun 19, 2026 •
edited
Loading

Copy link
Copy Markdown

✅ Review complete (commit 4a8a5a3)

@github-actions

github-actions Bot commented Jun 19, 2026

Copy link
Copy Markdown

This pull request has conflicts, please rebase.

@github-actions

github-actions Bot commented Jun 19, 2026 •
edited
Loading

Copy link
Copy Markdown

✅ No Merge Conflicts Detected

This PR currently has no conflicts with other open PRs.

@knst knst changed the title Merge bitcoin/bitcoin#32693: depends: fix cmake compatibility error for freetype backport: bitcoin/bitcoin#32693: depends: fix cmake compatibility error for freetype Jun 19, 2026
@coderabbitai

coderabbitai Bot commented Jun 19, 2026 •
edited
Loading

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 11033a41-2f55-4a68-804b-1b95e8b60f47

📥 Commits

Reviewing files that changed from the base of the PR and between e0d2946 and 4a8a5a3.

📒 Files selected for processing (2)
  • depends/packages/freetype.mk
  • depends/patches/freetype/cmake_minimum.patch
🚧 Files skipped from review as they are similar to previous changes (2)
  • depends/packages/freetype.mk
  • depends/patches/freetype/cmake_minimum.patch

Walkthrough

The freetype dependency build definition is updated to apply a new patch (cmake_minimum.patch) during the preprocess stage. The patch file raises the cmake_minimum_required version in FreeType's CMakeLists.txt from 2.8.12 to 3.12. In freetype.mk, the patch is added to the $(package)_patches list and a new $(package)_preprocess_cmds macro is introduced to invoke patch -p1 with the patch file before the configure/build steps.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly describes the main change: a backport that fixes a CMake compatibility error for the freetype dependency, which is directly related to the changeset content.
Description check ✅ Passed The description is related to the changeset, explaining the issue being fixed (freetype build on Kubuntu 26.04 with CMake 4), the solution (backport of bitcoin#32693), and testing status.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@knst @fanquake
Merge bitcoin#32693: depends: fix cmake compatibility error for freetype
4a8a5a3 
d7c3790 build: patch cmake min version on freetype (josibake)

Pull request description:

  ## Problem

  While doing a depends build with CMake 4.0.1, I got the following error:

  ```
  Extracting freetype...
  /root/bitcoin/depends/sources/freetype-2.11.0.tar.xz: OK
  Preprocessing freetype...
  Configuring freetype...
  CMake Error at CMakeLists.txt:100 (cmake_minimum_required):
    Compatibility with CMake < 3.5 has been removed from CMake.

    Update the VERSION argument <min> value.  Or, use the <min>...<max> syntax
    to tell CMake that the project requires at least <min> but has been updated
    to work with policies introduced by <max> or earlier.

    Or, add -DCMAKE_POLICY_VERSION_MINIMUM=3.5 to try configuring anyway.

  -- Configuring incomplete, errors occurred!
  make: *** [funcs.mk:343: /root/bitcoin/depends/x86_64-pc-linux-gnu/.freetype_stamp_configured] Error 1
  make: Leaving directory '/root/bitcoin/depends'
  ```

  .. which led me to https://cmake.org/cmake/help/latest/release/4.0.html#deprecated-and-removed-features, which states compatibility with CMake versions less than 3.5 has been removed in 4.0.

  ## Fix
  Based on the suggestion from the error message (and from reading the CMake docs), I added `-DCMAKE_POLICY_VERSION_MINIMUM=3.22`. I picked `3.22` (as opposed to 3.5) since that is the minimum version of CMake we specify in `doc/dependencies.md`. Would be nice if there was a way to pipe the min version in as a variable (since presumably we'd want to update this to be in lock step with the minimum CMake version of the whole project), but I couldn't think of a simple way to do this. Open to suggestions on a more robust way to do this if this is deemed too brittle.

ACKs for top commit:
  fanquake:
    ACK d7c3790
  hebasto:
    ACK d7c3790. Tested on Ubuntu 25.04 with both cmake 4.0.2 and the default cmake 3.31.6.

Tree-SHA512: fb664ec73bfffc504f1dcc9076072307f443d056d14325de41c4a29f3ee4077f1922e79b5895b49e7354f45ad6a35be4973c153c2baf3376df6c0d209efc9c54

Co-authored-by: merge-script <fanquake@gmail.com>
thepastaclaw
thepastaclaw reviewed Jun 19, 2026
View reviewed changes

@thepastaclaw thepastaclaw left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

Faithful, minimal backport of bitcoin#32693 that fixes freetype's depends build under CMake 4 by patching freetype's CMakeLists.txt to require CMake >= 3.12. The freetype.mk wiring (patch registration + preprocess_cmds) matches established depends conventions, no Dash-specific interactions, and no prerequisite gaps.

Note: posted as a COMMENT review because GitHub rejected the clean APPROVE review for this stale, no-longer-head commit.

thepastaclaw
thepastaclaw approved these changes Jun 19, 2026
View reviewed changes

@thepastaclaw thepastaclaw left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

Faithful, minimal backport of bitcoin#32693 that fixes freetype's depends build under CMake 4.x by adding a patch raising freetype's cmake_minimum_required from 2.8.12 to 3.12 and wiring it through freetype.mk via the standard $(package)_patches / $(package)_preprocess_cmds pattern. The patch contents and depends glue match upstream exactly, the depends infrastructure used is already established in Dash's tree, and no prerequisite Bitcoin Core PRs are required. Commit history is clean (upstream-authored content commit + merge commit with full PR description and Tree-SHA512 trailer), and there are no prior findings to carry forward from the previous review at e0d2946 (delta is empty).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@UdjinM6 UdjinM6 Awaiting requested review from UdjinM6

@PastaPastaPasta PastaPastaPasta Awaiting requested review from PastaPastaPasta

1 more reviewer

@thepastaclaw thepastaclaw thepastaclaw approved these changes

Reviewers whose approvals may not affect merge requirements

At least 2 approving reviews are required to merge this pull request.

Assignees

No one assigned

Labels

backport-candidate-23.1.x

Projects

None yet

Milestone

23.1.5

Development

Successfully merging this pull request may close these issues.

2 participants

@knst @thepastaclaw