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

[nerdopolis]

On Tuesday, June 12, 2018 7:23:19 AM EDT Pekka Paalanen wrote:
> On Tue, 23 Jan 2018 22:15:43 -0500
> 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 | 5 +++++
> >  man/weston-drm.man         | 7 +++++--
> >  3 files changed, 11 insertions(+), 3 deletions(-)
> > 
> 
> Hi,
> 
> this looks like a good addition. Mostly cosmetic nitpicks below.
> 
> > diff --git a/compositor/main.c b/compositor/main.c
> > index 7feb4cb0..72ae14b9 100644
> > --- a/compositor/main.c
> > +++ b/compositor/main.c
> > @@ -563,7 +563,7 @@ usage(int error_code)
> >  #if defined(BUILD_DRM_COMPOSITOR)
> >  	fprintf(stderr,
> >  		"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 3eda70f3..5585944e 100644
> > --- a/libweston/compositor-drm.c
> > +++ b/libweston/compositor-drm.c
> > @@ -4036,8 +4036,13 @@ 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;
> > +
> 
> It would be good to move the config->seat_id handling here as well so
> they are all in the same place.
> 
> In compositor-drm.h the seat_id member's comments need updating too.
> 
> >  	weston_log("initializing drm backend\n");
> >  
> >  	b = zalloc(sizeof *b);
> > diff --git a/man/weston-drm.man b/man/weston-drm.man
> > index 75d79021..883395f2 100644
> > --- a/man/weston-drm.man
> > +++ b/man/weston-drm.man
> > @@ -101,8 +101,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 " , and If neither is specifed, seat0 will be assumed."
> 
> I'd format that like this:
> . BR XDG_SEAT ". If neither is specifed, seat0 will be assumed."
> 
> >  .TP
> >  \fB\-\-tty\fR=\fIx\fR
> >  Launch Weston on tty
> > @@ -124,6 +124,9 @@ The file descriptor (integer) where
> >  .B weston-launch
> >  is listening. Automatically set by
> >  .BR weston-launch .
> > +.TP
> > +.B XDG_SEAT
> > +The seat that Weston will start on.
> 
> The default seat, since we have the command line option to override it.
I think I was sure on your other requested changes, and made them. I guess
I will have to figure out the new way to submit this... ...however this is 
the one thing I have a remaining question on Should it say 
"The default seat Weston will start on"?
or should it say
"The default seat if the --seat command line option is not specified"

Thanks
> 
> >  .
> >  .\" ***************************************************************
> >  .SH "SEE ALSO"
> 
> 
> Thanks,
> pq