[PATCH v8 1/6] libweston: set the seat automatically based on the XDG_SEAT environment variable

[Pekka Paalanen]

On Wed, 27 Jun 2018 20:44:17 -0400
nerdopolis <bluescreen_avenger at verizon.net> wrote:

> This will allow the seat to be set by the environment as pam_systemd typically
> sets the XDG_SEAT variable
> ---
>  compositor/main.c          |  2 +-
>  libweston/compositor-drm.c | 11 ++++++++---
>  libweston/compositor-drm.h |  3 ++-
>  man/weston-drm.man         |  7 +++++--
>  4 files changed, 16 insertions(+), 7 deletions(-)
> 
> diff --git a/compositor/main.c b/compositor/main.c
> index 2cb50c19..7bfe0a33 100644
> --- a/compositor/main.c
> +++ b/compositor/main.c
> @@ -482,7 +482,7 @@ usage(int error_code)
>  #if defined(BUILD_DRM_COMPOSITOR)
>  	fprintf(out,
>  		"Options for drm-backend.so:\n\n"
> -		"  --seat=SEAT\t\tThe seat that weston should run on\n"
> +		"  --seat=SEAT\t\tThe seat that weston should run on, instead of the seat defined in XDG_SEAT\n"
>  		"  --tty=TTY\t\tThe tty to use\n"
>  		"  --drm-device=CARD\tThe DRM device to use, e.g. \"card0\".\n"
>  		"  --use-pixman\t\tUse the pixman (CPU) renderer\n"
> diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
> index 8b1ea66d..4a352132 100644
> --- a/libweston/compositor-drm.c
> +++ b/libweston/compositor-drm.c
> @@ -6029,8 +6029,16 @@ drm_backend_create(struct weston_compositor *compositor,
>  	struct udev_device *drm_device;
>  	struct wl_event_loop *loop;
>  	const char *seat_id = default_seat;
> +	const char *session_seat;
>  	int ret;
>  
> +	session_seat = getenv("XDG_SEAT");
> +	if (session_seat)
> +		seat_id = session_seat;
> +
> +	if (config->seat_id)
> +		seat_id = config->seat_id;
> +
>  	weston_log("initializing drm backend\n");
>  
>  	b = zalloc(sizeof *b);
> @@ -6062,9 +6070,6 @@ drm_backend_create(struct weston_compositor *compositor,
>  	if (parse_gbm_format(config->gbm_format, GBM_FORMAT_XRGB8888, &b->gbm_format) < 0)
>  		goto err_compositor;
>  
> -	if (config->seat_id)
> -		seat_id = config->seat_id;
> -
>  	/* Check if we run drm-backend using weston-launch */
>  	compositor->launcher = weston_launcher_connect(compositor, config->tty,
>  						       seat_id, true);
> diff --git a/libweston/compositor-drm.h b/libweston/compositor-drm.h
> index 53222293..edf4b8e6 100644
> --- a/libweston/compositor-drm.h
> +++ b/libweston/compositor-drm.h
> @@ -106,7 +106,8 @@ struct weston_drm_backend_config {
>  
>  	/** The seat to be used for input and output.
>  	 *
> -	 * If NULL the default "seat0" will be used.  The backend will
> +	 * Set first by the XDG_SEAT variable, and overridable by the --seat option.

Hi,

there is no --seat at this level. There is only seat_id. The code here
(libweston) cannot know what the caller (the user of libweston) uses to
assign seat_id. The fix is simple:

"If seat_id is NULL, the seat is taken from XDG_SEAT environment
variable. If neither is set, "seat0" is used."

The same applies to compositor-fbdev.h in the other patch.

> +	 * If none are specified, the default "seat0" is assumed. The backend will
>  	 * take ownership of the seat_id pointer and will free it on
>  	 * backend destruction.
>  	 */
> diff --git a/man/weston-drm.man b/man/weston-drm.man
> index d4cb75a7..7ebfadf7 100644
> --- a/man/weston-drm.man
> +++ b/man/weston-drm.man
> @@ -105,8 +105,8 @@ status. For example, use
>  \fB\-\-seat\fR=\fIseatid\fR
>  Use graphics and input devices designated for seat
>  .I seatid
> -instead of the default seat
> -.BR seat0 .
> +instead of the seat defined in the environment variable
> +. BR XDG_SEAT ". If neither is specifed, seat0 will be assumed."

I missed the space between . and BR. It should be:

.BR XDG_SEAT ". If neither is specifed, seat0 will be assumed."

>  .TP
>  \fB\-\-tty\fR=\fIx\fR
>  Launch Weston on tty
> @@ -133,6 +133,9 @@ The file descriptor (integer) where
>  .B weston-launch
>  is listening. Automatically set by
>  .BR weston-launch .
> +.TP
> +.B XDG_SEAT
> +The seat Weston will start on, unless overridden on the command line.
>  .
>  .\" ***************************************************************
>  .SH "SEE ALSO"

With those two trivial fixes, this patch is:

Reviewed-by: Pekka Paalanen <pekka.paalanen at collabora.co.uk>

Mind, when you re-send patches, you should collect the Reviewed-by and
other tags already given if they (still) apply and add them below your
Signed-off-by line (imagine you have one). Otherwise they get lost,
reviewers miss their credit, and the patches might take additional
review effort as people don't see what has already been reviewed.


Thanks,
pq
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/wayland-devel/attachments/20180629/fd7857d9/attachment-0001.sig>