Conversation
|
@lorentey so I'm interested in landing these sketches. The API design question here is how best to surface global calls. Here we have a |
lorentey
left a comment
There was a problem hiding this comment.
I like this, but I have many comments! It would be great if we could unify all these under a single API umbrella, but I have some doubts if that's the right direction.
| /// | ||
| /// The corresponding C function is `sysconf`. | ||
| @_alwaysEmitIntoClient | ||
| public static func get(_ name: Name) throws -> Int { |
There was a problem hiding this comment.
Nit: naming suggestions to fit the API guidelines about non-mutating functions:
public static func value(for name: Name) throws -> Int
public static func systemConfiguration(named name: Name) throws -> Int
This is not a strong opinion -- I know I've flip-flopped about this, and I'll probably keep doing that. :-P
I think there would be value in using different base names for the various underlying calls (sysconf/pathconf/sysctl/what have you).
We could instead define a separate property for each supported option, but that won't work for sysctl or even pathconf. (We should do that anyway for the most high-profile options -- page size, core count etc.)
There was a problem hiding this comment.
(How) does sysctl fit into this naming scheme?
| */ | ||
|
|
||
| /// A namespace to access system variables | ||
| public enum SystemConfig {} |
There was a problem hiding this comment.
Config feels a bit unusual -- do we have a precedent for using it?
How about SystemConfiguration or just Configuration? (A quick search through the SDKs implies we prefer to spell this out.)
SystemInformation might be another option.
| /// | ||
| /// The corresponding C constant is `_SC_STREAM_MAX` | ||
| @_alwaysEmitIntoClient | ||
| public static var maxStreams: Self { Self(_SC_STREAM_MAX) } |
There was a problem hiding this comment.
Swift doesn't use FILE *, so I think this may be okay to exclude.
| @_alwaysEmitIntoClient | ||
| public static var pageSize: Self { Self(_SC_PAGESIZE) } | ||
|
|
||
| /// The minimum maximum number of streams that a process may have open at |
There was a problem hiding this comment.
Typo: minimum maximum (here and below)
| /// | ||
| /// The corresponding C constant is `_SC_TZNAME_MAX` | ||
| @_alwaysEmitIntoClient | ||
| public static var maxTimezones: Self { Self(_SC_TZNAME_MAX) } |
There was a problem hiding this comment.
The man page is gibberish 🙈 -- this is the maximum length of a time zone name.
I don't think we need to expose this until/unless we expose C time zones, which seems unlikely right now.
| /// | ||
| /// The corresponding C constant is _SC_ARG_MAX) `` | ||
| @_alwaysEmitIntoClient | ||
| public static var maxArgumentBytes: Self { Self(_SC_ARG_MAX) } |
There was a problem hiding this comment.
It would make sense to delay introducing this until we start wrapping execve -- otherwise we risk defining this with inconsistent terminology. (E.g., I'm pretty sure we'd want to clarify that this is for command line arguments passed to a process, not e.g. function arguments.)
| /// | ||
| /// The corresponding C constant is _SC_IOV_MAX) `` | ||
| @_alwaysEmitIntoClient | ||
| public static var maxIOV: Self { Self(_SC_IOV_MAX) } |
There was a problem hiding this comment.
I think we should delay adding this until we start wrapping readv and friends.
| /// | ||
| /// The corresponding C constant is _SC_CLK_TCK) `` | ||
| @_alwaysEmitIntoClient | ||
| public static var clockTicks: Self { Self(_SC_CLK_TCK) } |
There was a problem hiding this comment.
I think we should delay adding this until we start wrapping the relevant stats syscalls.
| /// | ||
| /// The corresponding C function is `pathconf`. | ||
| @_alwaysEmitIntoClient | ||
| public static func get(_ name: PathName, for path: FilePath) throws -> Int { |
There was a problem hiding this comment.
Same naming nit as above. The function name ought to be a noun, and I think there would be value in differentiating pathconf from sysconf from sysctl.
| /// | ||
| /// The corresponding C constant is `Name` | ||
| @_alwaysEmitIntoClient | ||
| public static var processorsOnline: Self { Self(_SC_NPROCESSORS_ONLN) } |
There was a problem hiding this comment.
Does the configured vs online distinction make sense these days?
Just a sketchup of support for POSIX system variable configuration. Does not include the more complete, and platform-specific,
sysctl.NOTE: This is based on top of the cleanup and fixes in #32.