Bug 246189

Summary: graphics/wayland: fix wl_client_get_credentials
Product: Ports & Packages Reporter: Greg V <greg>
Component: Individual Port(s)Assignee: freebsd-x11 (Nobody) <x11>
Status: New ---    
Severity: Affects Only Me CC: zeising
Priority: --- Keywords: buildisok
Version: LatestFlags: bugzilla: maintainer-feedback? (x11)
Hardware: Any   
OS: Any   
Attachments:
Description Flags
libwayland-creds.patch
none
libwayland-creds.patch v2 none

Description Greg V 2020-05-04 19:53:09 UTC
Created attachment 214123 [details]
libwayland-creds.patch

1) who wrote getsockopt(fd, SOL_SOCKET, LOCAL_PEERCRED)? that's wrong! with SOL_SOCKET, the value of LOCAL_PEERCRED (1) is interpreted as SO_DEBUG (1).
2) we do have the pid in xucred since r348419, let's use it.
Comment 1 Niclas Zeising freebsd_committer 2020-05-04 22:15:29 UTC
cr_pid is no longer a macro, it was changed (in head) in base r348847.  It does not exist at all in FreeBSD 11 or 12.
Comment 2 Greg V 2020-05-04 22:34:37 UTC
Created attachment 214128 [details]
libwayland-creds.patch v2

oh. __FreeBSD_version it is, then.
Comment 3 Jan Beich freebsd_committer 2020-05-05 05:27:27 UTC
Comment on attachment 214128 [details]
libwayland-creds.patch v2

> +#if __FreeBSD_version >= 1300030

Can be simplified to defined(__FreeBSD__) && __FreeBSD__ >= 13 to drop <sys/param.h>.

Checking __FreeBSD_version is only useful on /stable branches. On -CURRENT only the tip is supported. Some folks try to smooth the ride for week/months old snapshots but... base r348419 is almost 1 year old. Curiously, that commit was also tagged for MFC which didn't happen.
Comment 4 Jan Beich freebsd_committer 2020-05-05 06:34:16 UTC
FWIW, the patch fixes pid reporting for Wayland clients in x11-wm/sway e.g.,

  $ mpv /path/to/file.mp4
  $ MOZ_ENABLE_WAYLAND=1 firefox example.com

  # Before
  $ swaymsg -t get_tree | jq 'recurse(.nodes[]) | select(.shell == "xdg_shell") | .pid'
  0
  0

  # After
  $ swaymsg -t get_tree | jq 'recurse(.nodes[]) | select(.shell == "xdg_shell") | .pid'
  15582
  58058
Comment 5 Greg V 2020-05-05 12:20:30 UTC
(In reply to Jan Beich from comment #3)

> Can be simplified to defined(__FreeBSD__) && __FreeBSD__ >= 13

tbh, maybe the whole HAVE_SYS_UCRED_H thing should just be __FreeBSD__.

There's no guarantee that other systems don't have a sys/ucred.h that contains completely different stuff. We've had that with Linux capabilities vs Capsicum..
Comment 6 Niclas Zeising freebsd_committer 2020-05-05 13:24:17 UTC
Isn't it better to have a proper configure (or meson) check for ucred and for cr_pid?

I've also asked why the base changes weren't merged to 12, they look like they're useful (in this context).
Comment 7 Automation User 2020-05-20 00:13:44 UTC
Build info is available at https://gitlab.com/swills/freebsd-ports/pipelines/147746188
Comment 8 Jan Beich freebsd_committer 2020-06-18 01:25:10 UTC
(In reply to Niclas Zeising from comment #6)
> I've also asked why the base changes weren't merged to 12

So what was the response? Looking at bug 215202 no one else asked.
Comment 9 Jan Beich freebsd_committer 2020-09-10 11:11:19 UTC
Ping. See also sysutils/seatd/files/patch-pid and https://github.com/rust-lang/rust/pull/75148