Skip to content

Conversation

@vyasgun
Copy link
Contributor

@vyasgun vyasgun commented Oct 15, 2025

Fixes: #378

Summary by CodeRabbit

  • Chores
    • Raised the minimum macOS version targeted by CGO-built binaries to macOS 13.0.
  • Documentation
    • Added a Supported Host Platforms guide.
    • Clarifies macOS-only support and recommends macOS 14.0+.
    • Notes macOS 13 may work but isn’t officially supported; CI includes newer macOS versions.
    • Lists supported architectures: Intel x86_64 and Apple Silicon.
    • Mentions limitation: raw block devices require macOS 14+.

@openshift-ci openshift-ci bot requested review from anjannath and lstocchi October 15, 2025 09:13
@openshift-ci
Copy link

openshift-ci bot commented Oct 15, 2025

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign lstocchi for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@coderabbitai
Copy link

coderabbitai bot commented Oct 15, 2025

Walkthrough

Updated CGO macOS deployment target to 13.0 in the Makefile and added a new documentation page describing supported host macOS versions, architectures, CI coverage, and a note that raw block devices require macOS 14+.

Changes

Cohort / File(s) Summary
Build target update
Makefile
Change CGO_CFLAGS minimum macOS version from -mmacosx-version-min=11.0 to -mmacosx-version-min=13.0 for CGO builds.
Supported platforms documentation
doc/supported_platforms.md
Add new document describing supported host platforms: official support policy (recommend macOS 14.0+; CI on macOS 14/15/26), note that macOS 13 may work but is not officially supported, list Intel x86_64 and Apple Silicon, and state raw block devices require macOS 14+.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested labels

lgtm, approved

Suggested reviewers

  • lstocchi
  • nirs

Poem

A hop and a bop through versioned hills,
We nudge our flags to newer thrills.
Docs sprout fresh like clover green,
14 and up—crisp, serene.
13 may linger, soft twilight hue—
Thump-thump! The build now hops anew. 🐇✨

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The PR title "Doc: Add document for supported macos versions" directly refers to the primary objective of this pull request—adding documentation for supported macOS versions. The title is clear, concise, and accurately describes the main change presented in the diff. While the PR also includes a Makefile update to CGO_CFLAGS, the documentation addition is the primary focus as indicated by the linked issue and PR description, making the title appropriately scoped to the main change.
Linked Issues Check ✅ Passed The PR satisfies all the key requirements from issue #378. The documentation file (doc/supported_platforms.md) has been added, specifying support for the latest three macOS releases (14, 15, and 26) with macOS 14.0+ officially recommended and supported. The PR also addresses the CGO_CFLAGS update from -mmacosx-version-min=11.0 to -mmacosx-version-min=13.0 as suggested in the issue. Additionally, macOS 13 is appropriately documented as a "grey area" (not officially supported but tested via integration tests), aligning with the issue's proposal to maintain it temporarily without full support.
Out of Scope Changes Check ✅ Passed All changes in the PR are directly addressed in issue #378. The Makefile modification to update CGO_CFLAGS from 11.0 to 13.0 is explicitly proposed in the issue, and the addition of doc/supported_platforms.md is the core deliverable requested. No extraneous changes have been introduced beyond what was outlined in the linked issue, and both modifications contribute to the stated objectives of establishing a clear macOS support policy.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
doc/supported_platforms.md (1)

11-11: Wrap the bare URL in Markdown link syntax.

We run markdownlint (MD034), so please enclose the runner changelog URL in link syntax to avoid the lint warning.

-**macOS 13.0** may work but is not officially supported. Integration tests currently run on macOS 13, but this GitHub Actions runner will be deprecated soon. See: https://github.blog/changelog/2025-07-11-upcoming-changes-to-macos-hosted-runners-macos-latest-migration-and-xcode-support-policy-updates/#macos-13-is-closing-down
+**macOS 13.0** may work but is not officially supported. Integration tests currently run on macOS 13, but this GitHub Actions runner will be deprecated soon. See [GitHub’s announcement](https://github.blog/changelog/2025-07-11-upcoming-changes-to-macos-hosted-runners-macos-latest-migration-and-xcode-support-policy-updates/#macos-13-is-closing-down).
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3530298 and 72e2c7c.

📒 Files selected for processing (2)
  • Makefile (1 hunks)
  • doc/supported_platforms.md (1 hunks)
🧰 Additional context used
🪛 LanguageTool
doc/supported_platforms.md

[grammar] ~7-~7: There might be a mistake here.
Context: ...ended and fully supported. CI tested on: - macOS 14 (Sonoma) - CI tested on Apple S...

(QB_NEW_EN)


[grammar] ~8-~8: There might be a mistake here.
Context: ...14 (Sonoma) - CI tested on Apple Silicon - macOS 15 (Sequoia) - CI tested on Apple ...

(QB_NEW_EN)


[grammar] ~13-~13: There might be a mistake here.
Context: ...cos-13-is-closing-down Architectures: - Intel x86_64 - Apple Silicon ## Limitat...

(QB_NEW_EN)


[grammar] ~14-~14: There might be a mistake here.
Context: ...-down Architectures: - Intel x86_64 - Apple Silicon ## Limitations - **Raw b...

(QB_NEW_EN)

🪛 markdownlint-cli2 (0.18.1)
doc/supported_platforms.md

11-11: Bare URL used

(MD034, no-bare-urls)

@vyasgun vyasgun force-pushed the pr/docs/supported-macos branch from 72e2c7c to 3f5c582 Compare October 15, 2025 10:18
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 72e2c7c and 3f5c582.

📒 Files selected for processing (1)
  • doc/supported_platforms.md (1 hunks)
🧰 Additional context used
🪛 LanguageTool
doc/supported_platforms.md

[grammar] ~7-~7: There might be a mistake here.
Context: ...ended and fully supported. CI tested on: - macOS 14 (Sonoma) - CI tested on Apple S...

(QB_NEW_EN)


[grammar] ~8-~8: There might be a mistake here.
Context: ...14 (Sonoma) - CI tested on Apple Silicon - macOS 15 (Sequoia) - CI tested on Apple ...

(QB_NEW_EN)


[grammar] ~13-~13: There might be a mistake here.
Context: ...s-13-is-closing-down). Architectures: - Intel x86_64 - Apple Silicon ## Limitat...

(QB_NEW_EN)


[grammar] ~14-~14: There might be a mistake here.
Context: ...own). Architectures: - Intel x86_64 - Apple Silicon ## Limitations - **Raw b...

(QB_NEW_EN)

@vyasgun vyasgun force-pushed the pr/docs/supported-macos branch 3 times, most recently from bad7ab4 to 869f730 Compare October 15, 2025 10:44

## Limitations

- **Raw block devices**: Using raw block devices like `/dev/disk1` requires macOS 14+. See Apple's [VZDiskBlockDeviceStorageDeviceAttachment](https://developer.apple.com/documentation/virtualization/vzdiskblockdevicestoragedeviceattachment) documentation.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don’t think we should list these limitations here, gpu support is also macOS 14+, nested virt is macOS 15+, …
We can remove this section and list the limitation in the cmd line options documentation for example.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ah okay, makes sense. I'll remove this section.

- **macOS 13.0** may work but is not officially supported. Integration tests currently run on macOS 13, but this GitHub Actions runner will be deprecated soon. See [GitHub’s announcement](https://github.blog/changelog/2025-07-11-upcoming-changes-to-macos-hosted-runners-macos-latest-migration-and-xcode-support-policy-updates/#macos-13-is-closing-down).

**Architectures:**
- Intel x86_64
Copy link
Collaborator

Choose a reason for hiding this comment

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

Need to decide if we want to start deprecating this, or if we wait for apple to officialy drop support.
Just food for thought, no changes needed in this PR.

@vyasgun vyasgun force-pushed the pr/docs/supported-macos branch from 869f730 to c7ba078 Compare October 16, 2025 07:43
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 869f730 and c7ba078.

📒 Files selected for processing (1)
  • doc/supported_platforms.md (1 hunks)
🧰 Additional context used
🪛 LanguageTool
doc/supported_platforms.md

[grammar] ~9-~9: There might be a mistake here.
Context: ...ended and fully supported. CI tested on: - macOS 14 (Sonoma) - CI tested on Apple S...

(QB_NEW_EN)


[grammar] ~10-~10: There might be a mistake here.
Context: ...14 (Sonoma) - CI tested on Apple Silicon - macOS 15 (Sequoia) - CI tested on Apple ...

(QB_NEW_EN)


[grammar] ~11-~11: There might be a mistake here.
Context: ...5 (Sequoia) - CI tested on Apple Silicon - macOS 26 (Tahoe) - CI tested on Apple Si...

(QB_NEW_EN)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: build (macos-15)
  • GitHub Check: build (macos-14)
  • GitHub Check: lint

Comment on lines +10 to +12
- macOS 14 (Sonoma) - CI tested on Apple Silicon
- macOS 15 (Sequoia) - CI tested on Apple Silicon
- macOS 26 (Tahoe) - CI tested on Apple Silicon
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Fix incorrect macOS version in CI coverage list.

There’s no macOS 26 release; this looks like a typo. Update the entry to the actual major version you test (likely macOS 16) so we don’t publish misleading support information.

🧰 Tools
🪛 LanguageTool

[grammar] ~10-~10: There might be a mistake here.
Context: ...14 (Sonoma) - CI tested on Apple Silicon - macOS 15 (Sequoia) - CI tested on Apple ...

(QB_NEW_EN)


[grammar] ~11-~11: There might be a mistake here.
Context: ...5 (Sequoia) - CI tested on Apple Silicon - macOS 26 (Tahoe) - CI tested on Apple Si...

(QB_NEW_EN)

🤖 Prompt for AI Agents
In doc/supported_platforms.md around lines 10 to 12, the third entry incorrectly
lists "macOS 26 (Tahoe)"; change that line to the correct major release you test
(replace with "macOS 16 (Tahoe)" or the actual release name you validate in CI)
so the CI coverage list is accurate, and ensure the line formatting matches the
other entries.

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.

Add doc about supported macos versions

2 participants