opam icon indicating copy to clipboard operation
opam copied to clipboard

Fix opam unable to find executables on systems where users belong to more than 32 groups when opam is built using musl libc

Open kit-ty-kate opened this issue 2 years ago • 3 comments

Details of the issue in musl described in https://www.openwall.com/lists/musl/2021/07/03/1 Fixes https://github.com/ocaml/opam/issues/5373

kit-ty-kate avatar Dec 06 '22 19:12 kit-ty-kate

I think there's a couple of bits in configure.ac from https://github.com/ocaml/opam/pull/4265/commits/5377b60d1df9f6dd2c42b5a386630fcc745271b4#diff-49473dca262eeab3b4a43002adb08b4db31020d190caaad1594b47f1d5daa810 which also need reverting.

We should double-check that Unix.access doesn't regress either https://github.com/ocaml/opam/pull/4265/commits/d6faa8d45a3b06270863fb1c8d0848d7b45c6f36 or https://github.com/ocaml/opam/pull/4265/commits/6f756efc48da31e917a4a36f4c557b3e155d076b (this is probably just a case of checking the Open Group POSIX docs... I'll be amazed if it regresses the first and quite surprised if it regresses the second!). It would be nice to check that the original bug being fixed in #4265, but that may be too tricky to set-up again.

dra27 avatar Dec 06 '22 20:12 dra27

Annoying, Unix.access checks ruid and rgid rather than euid and egid (which is what exec will do). Ideally we therefore want to use faccessat with AT_EACCESS instead, which should be exactly equivalent to the present check, but without needing the libacl hackery for Cygwin (and, in fairness, any other Unix system which chooses to grant permissions with ACLs instead). However, that would cause opam to require C stubs for all builds, and I'm not so sure about that :shrug:

dra27 avatar Dec 09 '22 16:12 dra27

Discussed today: we decided we're not worried about requiring stubs in opam-core for all platforms, so wrapping faccessat should be fine, which we can then use instead of Unix.access. Happy for either of us to get to this - either way, we get rid of the libacl bindings!

dra27 avatar Dec 14 '22 15:12 dra27

squashed

kit-ty-kate avatar Sep 17 '24 10:09 kit-ty-kate

Does doc/index.html need to be updated by the last commit

done. Feel free to squash if you're fine with the two last fixup commits

kit-ty-kate avatar Sep 17 '24 14:09 kit-ty-kate