Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use the uname syscall instead of calling the uname command #4274

Closed
wants to merge 2 commits into from

Conversation

kit-ty-kate
Copy link
Member

Opam uses the uname syscall to get hold of the os, arch and in some cases os-version variables in Unix environments.
On POSIX-compliant systems, the uname syscall is available which the uname command directly maps (for instance, in GNU coreutils: https://github.com/coreutils/coreutils/blob/master/src/uname.c)

The idea behind this PR is to get all these variables without having to call an external command and instead of the uname syscall.

This PR also fixes the value of os-version on FreeBSD, where it was previously gotten using uname -U (FreeBSD-only feature). However the value returned is rather odd for a version number:

$ uname -U
1201000

whereas the POSIX-compliant uname -r seems much more reasonable:

$ uname -r
12.1-RELEASE

@hannesm
Copy link
Member

hannesm commented Jul 16, 2020

fixes the value of os-version on FreeBSD

see discussion in #3058

If I understand correctly, the goal of os-version is to be able to express "in version X the feature Y was implemented, thus the dependency (or behaviour) should change after X". I'm not aware of any opam package using this for FreeBSD, but from my intuition I'd favour the uname -U (which is a number that can be compared, changes whenever the ABI changes). The output of uname -r is imprecise, especially when not on a release.

@dra27
Copy link
Member

dra27 commented Jul 16, 2020

On the implementation: opam-core already has C stubs for Windows and 79c9740 adds the possibility for these on Unix implementations (including Cygwin, which doesn't use the Windows stubs). At least for now, it remains important that opam-core can be built without needing C stubs, but it would be easy to generalise the src/stubs/libacl proposed in #4265 to be src/stubs/unix.

Out of interest, rather than automatic rejection, is there a specific motivation for converting this to a system call, or just hygiene?

@kit-ty-kate
Copy link
Member Author

Out of interest, rather than automatic rejection, is there a specific motivation for converting this to a system call, or just hygiene?

Mostly hygene. Closing as it is too much of a hassle for the very little gain on cleanliness and performance

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