Skip to content

Revert "nixos/desktop-managers/xterm: Disable by default"#70182

Closed
matthewbauer wants to merge 1 commit intoNixOS:masterfrom
matthewbauer:revert-67355
Closed

Revert "nixos/desktop-managers/xterm: Disable by default"#70182
matthewbauer wants to merge 1 commit intoNixOS:masterfrom
matthewbauer:revert-67355

Conversation

@matthewbauer
Copy link
Copy Markdown
Member

@matthewbauer matthewbauer commented Oct 1, 2019

This reverts commit f140dfb.
This reverts commit cf56cef.
This reverts commit 456c42c.

This has caused more issues than originally intended. It looks like many configurations rely on xterm being set to work correctly. This includes when you are starting a WM without going through a DE. We shouldn't break these setups (if stateVersion is unset). To make matters worse, most users don't actually know they are relying on xterm. LightDM seems to be the main one effected for now:

I expect more issues to come up as users of 19.03 upgrade to 19.09.

Motivation for this change
Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option 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 nix-review --run "nix-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.
Notify maintainers

cc @

This reverts commit f140dfb.
This reverts commit cf56cef.
This reverts commit 456c42c.
Copy link
Copy Markdown
Member

@Ericson2314 Ericson2314 left a comment

Choose a reason for hiding this comment

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

I don't like hacking around upstream bugs like this, despite having been impacted by this.

@Ericson2314
Copy link
Copy Markdown
Member

Plus, the state version is there to help with the migration.

@matthewbauer
Copy link
Copy Markdown
Member Author

I don't like hacking around upstream bugs like this, despite having been impacted by this.

I disagree that this is an upstream bug. The problem is we need some placeholder WM to be available when nothing else has been manually specified. For a long time on NixOS that has been xterm, so might as well keep it available for new users. And these are just defaults, so you can always disable xterm manually if you really don't like it for some reason.

@ofborg ofborg bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: changelog This PR adds or changes release notes 8.has: documentation This PR adds or changes documentation 8.has: module (update) This PR changes an existing module in `nixos/` 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux. labels Oct 1, 2019
@Ericson2314
Copy link
Copy Markdown
Member

It looks like both issues were with lightdm? I think it is an upstream bug in that lightdm should give a better error message when there is no selected session. It's a classic "forgot about base case" (0) bug.

@worldofpeace
Copy link
Copy Markdown
Contributor

Yeah, I honestly don't understand why anyone was ever abusing the xterm session for this purpose.

As for having a placeholder WM there's #68371
Which makes much more sense for a placeholder, as a placeholder shouldn't really be much of anything. Though I'm not sure what's being supported. It feels like supporting users not configuring their systems, why is a placeholder session needed? Why can't these things be coordinated with home-manager to have a placeholder WM so they can start their .xsession?

@worldofpeace
Copy link
Copy Markdown
Contributor

It looks like both issues were with lightdm? I think it is an upstream bug in that lightdm should give a better error message when there is no selected session. It's a classic "forgot about base case" (0) bug.

It seems that lightdm notoriously gives no feedback in multiple cases of a session not starting.
And in particular here, where no session exists at all. Not sure if that code is greeter specific though.

@timhae
Copy link
Copy Markdown
Contributor

timhae commented Oct 3, 2019

Having xterm set as default caused a lot of headache on my side when I tried to setup autologin with any display manager. I failed to set desktopManager.default = "none"; because I did not read the documentation of nix options properly.. So essentially I experienced the same issue but in reverse since my window manager (xmonad) is just a little different.

@worldofpeace
Copy link
Copy Markdown
Contributor

Hmm, it appears at least in lightdm this failure feedback is greeter side

I guess lightdm-gtk-greeter lacks code like that ^ @Ericson2314.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: changelog This PR adds or changes release notes 8.has: documentation This PR adds or changes documentation 8.has: module (update) This PR changes an existing module in `nixos/` 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants