Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

lightdm: allow dm-tool lock to work properly #11686

Closed
wants to merge 1 commit into from
Closed

lightdm: allow dm-tool lock to work properly #11686

wants to merge 1 commit into from

Conversation

philandstuff
Copy link
Contributor

Fixes #5038.

The dm-tool lock command fails to work for reasons described in #5038:
it tries passing arguments to the xserver command, but the wrapper
throws the arguments away and replaces them with different arguments.

I fixed this by specifying the xserver args in the lightdm.conf file and
having the wrapper pass them through. This works for lightdm on
startup, and also works for dm-tool lock when it later passes a
different set of arguments to start a separate X server on another
display.

Ping @ocharles @wkennington, who are listed as maintainers of the lightdm package.

@mention-bot
Copy link

By analyzing the blame information on this pull request, we identified @ocharles, @viric and @wkennington to be potential reviewers

@philandstuff
Copy link
Contributor Author

It's probably worth saying that dm-tool lock doesn't, by itself, lock the screen so there's still more work to be done after this PR.

@ocharles
Copy link
Contributor

This looks OK from me, @viric, @wkennington - any thoughts?

@cillianderoiste cillianderoiste added the 0.kind: enhancement Add something new label Jan 10, 2016
@zimbatm
Copy link
Member

zimbatm commented Feb 26, 2016

@philandstuff do you mind rebasing on top of master ?

There is just one thing that needs to be fixed: $@ needs to be quoted for shell expansion to work properly. So "$@" instead of just $@

Fixes #5038.

The `dm-tool lock` command fails to work for reasons described in #5038:
it tries passing arguments to the xserver command, but the wrapper
throws the arguments away and replaces them with different arguments.

I fixed this by specifying the xserver args in the lightdm.conf file
instead of in the wrapper.  This works for lightdm on startup, and also
works for `dm-tool lock` when it later passes a different set of
arguments to start a separate X server on another display.
@philandstuff
Copy link
Contributor Author

hmm, it looks like 9be012f by @obadz (merged in #11338) has already added the "$@" pass-through. It certainly looks like it was solving a related problem.

The only thing remaining in this PR is whether ${dmcfg.xserverArgs} should remain hardcoded in the xserver-wrapper script, or if it should be put in to lightdm.conf. I kind of feel that putting the xserverArgs in lightdm.conf makes some of the logic in the xserver-wrapper unnecessary, but since I opened this PR I've moved to gdm instead of lightdm and so I don't have the setup to test this change any more. #11338 changed the semantics of xserverArgs so I don't have confidence in this.

I've rebased the commit and I'll leave this open for a few days if someone wants to take it up, but if there's no further activity within a week I suggest closing this PR.

As an aside, I wonder if #5038 was fixed by #11338?

@obadz
Copy link
Contributor

obadz commented Feb 28, 2016

@philandstuff I wonder that too

@obadz
Copy link
Contributor

obadz commented Feb 28, 2016

@philandstuff actually I'm pretty sure it's fixed

@philandstuff
Copy link
Contributor Author

Closing (re #11686 (comment) )

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
0.kind: enhancement Add something new
Projects
None yet
Development

Successfully merging this pull request may close these issues.

lightdm fails to lock screen due to ignored arguments to X
6 participants