Skip to content

build: Set CMAKE_OSX_DEPLOYMENT_TARGET to 26.0#7785

Open
mathbunnyru wants to merge 2 commits into
XRPLF:developfrom
mathbunnyru:set_min_osx_target
Open

build: Set CMAKE_OSX_DEPLOYMENT_TARGET to 26.0#7785
mathbunnyru wants to merge 2 commits into
XRPLF:developfrom
mathbunnyru:set_min_osx_target

Conversation

@mathbunnyru

@mathbunnyru mathbunnyru commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

High Level Overview of Change

This is needed to use std::from/to_chars inside an macOS environment where auto-detection thinks we're targeting lower macOS version (inside Nix).

It would also allow us to explicitly see that using a new feature requires us upping the requirement for macOS version.
So, we won't accidentally bump the requirement for macOS version again.

Context of Change

API Impact

  • Public API: New feature (new methods and/or new fields)
  • Public API: Breaking change (in general, breaking changes should only impact the next api_version)
  • libxrpl change (any change that may affect libxrpl or dependents of libxrpl)
  • Peer protocol change (must be backward compatible or bump the peer protocol version)

@mathbunnyru mathbunnyru changed the title build: Set CMAKE_OSX_DEPLOYMENT_TARGET to 26.0 build: Set CMAKE_OSX_DEPLOYMENT_TARGET to 26.0 Jul 10, 2026
@mathbunnyru
mathbunnyru requested a review from vvysokikh1 July 10, 2026 12:52
@vvysokikh1
vvysokikh1 removed their request for review July 10, 2026 13:10
@codecov

codecov Bot commented Jul 10, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.3%. Comparing base (c7adb21) to head (348e716).
⚠️ Report is 3 commits behind head on develop.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff            @@
##           develop   #7785     +/-   ##
=========================================
- Coverage     82.3%   82.3%   -0.0%     
=========================================
  Files         1024    1024             
  Lines        78416   78416             
  Branches      8932    8932             
=========================================
- Hits         64550   64545      -5     
- Misses       13857   13862      +5     
  Partials         9       9             

see 4 files with indirect coverage changes

Impacted file tree graph

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@mathbunnyru
mathbunnyru requested review from godexsoft and ximinez July 10, 2026 13:34

@godexsoft godexsoft left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I can see how this may upset some people :rage1:

@ximinez ximinez left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Whether this is a better solution than just reverting std::from/to_chars is still up for debate, but I can at least say that this PR works as advertised.

@mathbunnyru mathbunnyru added Ready to merge *PR author* thinks it's ready to merge. Has passed code review. Perf sign-off may still be required. and removed Ready to merge *PR author* thinks it's ready to merge. Has passed code review. Perf sign-off may still be required. labels Jul 13, 2026
@bthomee
bthomee added this pull request to the merge queue Jul 13, 2026
@mathbunnyru
mathbunnyru removed this pull request from the merge queue due to a manual request Jul 13, 2026
@bthomee
bthomee requested a review from Copilot July 16, 2026 15:28

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Sets a default macOS deployment target in the top-level CMake configuration (and documents it) so builds don’t silently target an older macOS version—particularly in Nix-based macOS environments—when using newer standard library features.

Changes:

  • Default CMAKE_OSX_DEPLOYMENT_TARGET to 26.0 (only when not already provided) before project() runs.
  • Update BUILD.md to document the minimum supported macOS version and that the build defaults the deployment target.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
CMakeLists.txt Adds a default cached CMAKE_OSX_DEPLOYMENT_TARGET for macOS builds prior to project().
BUILD.md Documents the minimum supported macOS version and the new CMake default behavior.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread CMakeLists.txt
# configuring the compiler and SDK. A user-provided -DCMAKE_OSX_DEPLOYMENT_TARGET
# still takes precedence.
if(
CMAKE_HOST_SYSTEM_NAME STREQUAL "Darwin"
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants