opam icon indicating copy to clipboard operation
opam copied to clipboard

Use a C stub to call uname(2) instead of calling the uname(1) command

Open kit-ty-kate opened this issue 1 year ago • 4 comments

~Queued on top of #6216~ ~F.i.x.e.s #6215~

~For the backport to 2.3, do we want to backport the entire PR or just the OpenBSD fix?~ ~Partially backported to 2.3 in https://github.com/ocaml/opam/pull/6228~

Queued on #6230

kit-ty-kate avatar Sep 30 '24 19:09 kit-ty-kate

Works great on my OpenBSD desktop:

$ uname -a
OpenBSD 7.5 GENERIC.MP#82 amd64
<><> Global opam variables ><><><><><><><><><><><><><><><><><><><><><><><><><><>
arch              x86_64            # Inferred from system
exe                                 # Suffix needed for executable filenames (Windows)
jobs              1                 # The number of parallel jobs set up in opam configuration
make              gmake             # The 'make' command to use
opam-version      2.4.0~alpha1~dev  # The currently running opam version
os                openbsd           # Inferred from system
os-distribution   openbsd           # Inferred from system
os-family         bsd               # Inferred from system
os-version        7.5               # Inferred from system
root              /home/avsm/.opam2 # The current opam root directory
switch            default           # The identifier of the current switch
sys-ocaml-arch    x86_64            # Target architecture of the OCaml compiler present on your system
sys-ocaml-cc      cc                # Host C Compiler type of the OCaml compiler present on your system
sys-ocaml-libc    libc              # Host C Runtime Library type of the OCaml compiler present on your system
sys-ocaml-system  openbsd           # Target system of the OCaml compiler present on your system
sys-ocaml-version 4.14.1            # OCaml version present on your system independently of opam, if any

One test fails:

File "tests/reftests/dune.inc", line 2082, characters 0-243:
2082 | (rule
2083 |   (targets opam-repo-143dd2a2f59f5befbf3cb90bb2667f911737fbf8)
2084 |   (action
2085 |    (progn
2086 |     (run mkdir -p %{targets})
2087 |     (run tar -C %{targets} -xzf %{dep:opam-archive-143dd2a2f59f5befbf3cb90bb2667f911737fbf8.tar.gz} --strip-components=1))))

but looks unrelated (that tar needs to be gtar due to the --strip-components)

avsm avatar Sep 30 '24 19:09 avsm

Thanks. This PR fixes #6215 for me. My opinion may not matter on the patch, but that looked good to me too. I like the switch to uname via C stubs.

mndrix avatar Oct 01 '24 13:10 mndrix

turns out we still need to call the getconf process. sysconf(3) depends on the libc architecture and doesn't have the necessary query with musl (_SC_LONG_BIT is glibc only).

We could rely on Sys.word_size as an alternative but it would depend on the architecture of the opam binary, not the userspace architecture (e.g. you can call an arm64 binary just fine on Raspberry Pi OS 32bit, which has a 32bit userspace but 64bit kernel)

I've also removed the C stub for FreeBSD as __FreeBSD_version is defined at compile-time and does not show the actual runtime version.

kit-ty-kate avatar Oct 01 '24 19:10 kit-ty-kate

I've split off the fix for OpenBSD to #6230 for simplification. Leaving this PR just for the uname improvement

kit-ty-kate avatar Oct 08 '24 19:10 kit-ty-kate

Except the naming, LGTM! We can discuss that monday.

rjbou avatar Nov 22 '24 14:11 rjbou

As discussed on dev meeting, we should use uname function instead of uname(2) and uname binary or tool instead of uname(1).

rjbou avatar Nov 26 '24 15:11 rjbou