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.

Sign up for GitHub

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

Jump to bottom

lightdm-mini-greeter: init at 0.3.2 #39153

Merged
merged 1 commit into from Jun 18, 2018
Merged

lightdm-mini-greeter: init at 0.3.2 #39153

merged 1 commit into from Jun 18, 2018

Conversation

mnacamura
Copy link
Contributor

Motivation for this change

lightdm-mini-greeter is a neat, minimalistic LightDM greeter. It could be alternative to SLiM.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option build-use-sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Fits CONTRIBUTING.md.

@emmanuelrosa
Copy link
Contributor

Nice 😁

In the lightdm-mini-greeter documentation it says:

If you have multiple Desktop Environments or Window Mangers installed, you can specify the one to start by changing the user-session option as well(look in /usr/share/xsession for possible values).

In my case, although I only have i3 installed as the window manager lightdm-mini-greeter doesn't know to use it and instead simply runs xterm. How can/will the desktop environment or window manager be configured with this greeter on NixOS?

Other than that, I'm liking this! Here it is running in a VM built with nixos-rebuild build-vm:

lightdm-mini-greeter

@dotlambda
Copy link
Member

@emmanuelrosa For turning off xterm you might want to set the respective option to false: https://nixos.org/nixos/options.html#xterm.

@mnacamura
Copy link
Contributor Author

@emmanuelrosa You can choose your session in ~/.dmrc. Otherwise you can configure the default desktop session in /etc/nixos/configuration.nix by for example

services.xserver.desktopManager.default = "none";
services.xserver.windowManager.default = "bspwm";

in my case using bspwm.

@emmanuelrosa
Copy link
Contributor

This is what it took to make it work with i3:

services.xserver.windowManager.default = "i3";
services.xserver.desktopManager.xterm.enable = false;

When I set the Desktop Manager to "none" i3 started but so did xterm, and killing xterm also killed i3.

Given this, the only recommendation I have is to document what is needed to set the window/desktop manager (since this greeter doesn't provide a way to do so via the GUI). Better yet, an option which simply takes care of it. Something like this:

services.xserver.displayManager.lightdm.greeters.mini.userSession = "i3";

@mnacamura
Copy link
Contributor Author

Ahh I was wrong. ~/.dmrc is ignored. Mini greeter seems to start only the default session.

Given this, the only recommendation I have is to document what is needed to set the window/desktop manager (since this greeter doesn't provide a way to do so via the GUI). Better yet, an option which simply takes care of it. Something like this:

services.xserver.displayManager.lightdm.greeters.mini.userSession = "i3";

To document something like ``For using this greeter, you need to set the default session by ...'' would be sufficiently helpful IMHO.

This is what it took to make it work with i3:

services.xserver.windowManager.default = "i3";
services.xserver.desktopManager.xterm.enable = false;

I tested

services.xserver.desktopManager.default = "none";
services.xserver.windowManager.default = "i3";

and it worked fine. I did not need to set services.xserver.desktopManager.xterm.enable = false.

@mnacamura
Copy link
Contributor Author

All set, maybe. I'm waiting for review.

matthewbauer
matthewbauer requested changes May 9, 2018
View reviewed changes
nixos/modules/services/x11/display-managers/lightdm-greeters/mini.nix
'';
};

user = mkOption {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This add a few too many options. Can add an "extraConfig" like the lightdm-gtk-greeter has? Any options in common with lightdm-gtk-greeter are probably worth keeping.

Copy link
Contributor Author

@mnacamura mnacamura May 9, 2018 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree. However, mini-greeter requires that in the config file, all the options are explicitly specified; it does not provide default options when they are omitted in the config file. So, unlike GTK greeter, it could be unsafe to introduce extraConfig, with which users might forget to specify some options, causing failure, and after nixos-rebuild switch they cannot login.

EDIT: I noticed that glib's keyfile can have the same keys multiple times and the last entry wins. So it can be done by putting default options first and then overwriting them in extraConfig.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added extraConfig and most options can be set there.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@matthewbauer Could you review this again?

pkgs/applications/display-managers/lightdm-mini-greeter/default.nix Outdated
description = "A minimal, configurable, single-user GTK3 LightDM greeter";
homepage = https://github.com/prikhi/lightdm-mini-greeter;
license = licenses.gpl3;
maintainers = with maintainers; [ ];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you could add yourself as maintainer, this becomes a little easier to merge.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll be glad to.

@mnacamura mnacamura closed this May 9, 2018
@mnacamura mnacamura reopened this May 9, 2018
@matthewbauer matthewbauer merged commit 8180c32 into NixOS:master Jun 18, 2018
@mnacamura mnacamura deleted the lightdm-mini-greeter branch August 2, 2018 06:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@matthewbauer matthewbauer matthewbauer requested changes

Assignees
No one assigned
Labels
6.topic: nixos 8.has: module (update) 8.has: package (new) 10.rebuild-darwin: 0 10.rebuild-linux: 1-10
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@mnacamura @emmanuelrosa @dotlambda @matthewbauer @GrahamcOfBorg