Skip to content

Add watchOS support to sysctl crate#74

Open
setoelkahfi wants to merge 4 commits into
johalun:masterfrom
setoelkahfi:feature/watchos
Open

Add watchOS support to sysctl crate#74
setoelkahfi wants to merge 4 commits into
johalun:masterfrom
setoelkahfi:feature/watchos

Conversation

@setoelkahfi

@setoelkahfi setoelkahfi commented Apr 9, 2026

Copy link
Copy Markdown

Transitive dependency when working on adding watchOS support here.

@johalun

johalun commented Apr 16, 2026

Copy link
Copy Markdown
Owner

Please rebase and eliminate whitespace changes

@setoelkahfi

setoelkahfi commented Apr 16, 2026

Copy link
Copy Markdown
Author

Please rebase and eliminate whitespace changes

@johalun Done

Comment thread Cargo.toml
[package]
name = "sysctl"
version = "0.7.1"
version = "0.6.0"

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Did you inspect the diff at all? You're changing crate version.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

My bad

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

@johalun I need to build my sdk now and my gemm-common transitive dep needs the 0.6 version. Feel free to revert it before merging this, unless you have other concerns.

@johalun

johalun commented Apr 16, 2026

Copy link
Copy Markdown
Owner

Since you're working with Apple devices maybe you know, is it safe to assume that all Apple devices have the same sysctl interface and functionality? I'm thinking maybe I'll group them all into a single feature flag is_apple or similar.

@asomers

asomers commented Apr 16, 2026

Copy link
Copy Markdown
Contributor

Since you're working with Apple devices maybe you know, is it safe to assume that all Apple devices have the same sysctl interface and functionality? I'm thinking maybe I'll group them all into a single feature flag is_apple or similar.

Might I suggest that rather than add a feature flag, you use a cfg target alias? For example: https://github.com/nix-rust/nix/blob/69c05059fb38a85facd63af80456705ad91e9491/build.rs#L21 .

@setoelkahfi

Copy link
Copy Markdown
Author

Since you're working with Apple devices maybe you know, is it safe to assume that all Apple devices have the same sysctl interface and functionality? I'm thinking maybe I'll group them all into a single feature flag is_apple or similar.

Yeah, I believe so. It's Darwin anyway.

The cfg target alias looks neater I would say.

@setoelkahfi setoelkahfi requested a review from johalun April 17, 2026 18:12

@setoelkahfi setoelkahfi left a comment

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Hey @johalun, I've rebased and cleaned up the whitespace as requested. I also fixed the version to stay at 0.6.0 since this is a patch on the current release, not a new minor.

Regarding the is_apple cfg alias, I agree with @asomers that a build.rs target alias is the cleanest approach. Happy to implement that in a follow-up PR so we can keep this one minimal (just adding target_os = "watchos" to the existing gates). That way:

  1. This PR is small, reviewable, and unblocks watchOS users now (maybe just me)
  2. A follow-up PR introduces the cfg(apple) alias and replaces all the any(macos, ios, tvos, visionos, watchos, ...) lists across the codebase, a much cleaner refactor on its own

Let me know if you'd prefer both in one PR or if splitting them works for you.

Comment thread Cargo.toml
[package]
name = "sysctl"
version = "0.7.1"
version = "0.6.0"

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

@johalun I need to build my sdk now and my gemm-common transitive dep needs the 0.6 version. Feel free to revert it before merging this, unless you have other concerns.

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.

3 participants