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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

nixos/slim: remove #73251

Merged
merged 3 commits into from Nov 15, 2019
Merged

nixos/slim: remove #73251

merged 3 commits into from Nov 15, 2019

Conversation

@worldofpeace
Copy link
Member

worldofpeace commented Nov 11, 2019

Motivation for this change

Let's 馃憢 goodbye to SLIM.
We've already changed the default display manager to LightDM for the exact reasons why it's being removed here #30890. The next logical step is to remove it entirely.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • 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 @

@worldofpeace worldofpeace requested a review from Lassulus Nov 11, 2019
@worldofpeace worldofpeace requested a review from nbp as a code owner Nov 11, 2019
@worldofpeace

This comment has been minimized.

Copy link
Member Author

worldofpeace commented Nov 11, 2019

I've already made the graphical iso use sddm #71622, which makes the most sense for a Plasma 5 desktop.

@jtojnar

This comment has been minimized.

Copy link
Contributor

jtojnar commented Nov 11, 2019

Needs release notes.

@worldofpeace

This comment has been minimized.

Copy link
Member Author

worldofpeace commented Nov 11, 2019

Needs release notes.

Did that 馃憤.

@jtojnar

This comment has been minimized.

Copy link
Contributor

jtojnar commented Nov 11, 2019

# SLiM). The display manager allows the user to select a *session

option is supported is used; notably, SLiM is not supported.

displayManager.slim = {
enable = true;

The SLIM project is abandoned and their last release was in 2013.
Because of this it poses a security risk to systems, no one is working
on it or picked up maintenance. It also lacks compatibility with systemd
and logind sessions. For users, there liikely isn't anything like slim
that's as lightweight in terms of dependencies.
@worldofpeace worldofpeace force-pushed the worldofpeace:remove-slim branch from b90985c to 9c7db09 Nov 11, 2019
@worldofpeace

This comment has been minimized.

Copy link
Member Author

worldofpeace commented Nov 11, 2019

@jtojnar

displayManager.slim = {
enable = true;
was removed in c9601a6, my branch needed to be rebased.

The others were corrected in the force push.

@avnik

This comment has been minimized.

Copy link
Contributor

avnik commented Nov 12, 2019

;(
Any alternatives to slim, not pulling massive toolkits, like gnome or kde?
It would be sad to revert to xdm, as well as need tons of stuff need for DM only.

@Lassulus

This comment has been minimized.

Copy link
Contributor

Lassulus commented Nov 12, 2019

hmm, personally I would not remove stuff which still works and isn't maintained anymore. But I can see the point to not ship unmaintained stuff anymore and higher the burden for people to migrate to maintained grounds.

@avnik maybe something like services.xserver.displayManager.startx.enable is something for you?

@worldofpeace

This comment has been minimized.

Copy link
Member Author

worldofpeace commented Nov 13, 2019

It doesn't work probably, in that it doesn't fully support systemd and especially logind sessions.
With NixOS being tied to systemd this is going to be an issue. All other display managers in nixpkgs support systemd properly.

@rnhmjoj

This comment has been minimized.

Copy link
Contributor

rnhmjoj commented Nov 15, 2019

馃憥 for me.

LightDM, despite the name, is not lightwight and I don't know of any other comparable software, besides no display manager at all (startx). Slim has problems and that's fine for not being the default DM but I don't see how removal should be a logical consequence of that decision.

I don't think keeping slim around requires any extra work and we have plenty of old unmaintained software that still works and is useful to some.

@worldofpeace

This comment has been minimized.

Copy link
Member Author

worldofpeace commented Nov 15, 2019

Here's mostly what I had to say about this

most of the discussion happened in nixos-chat.

Bottom line is unmaintained software just doesn't belong in nixpkgs, and if people rely on software that's unmaintained in nixpkgs we already have an issue. In this kind of situation we usually have to do one of two things:

  1. Search for a maintainer or someone willing to step up to maintain its integration in NixOS
    IMHO, we're way past that point and many years.

  2. Remove SLIM
    We'd be kidding ourselves presenting like we can keep up integration with it its current state.

No one anywhere has stepped up to actually maintain it, though perhaps @cleverca22 has something in mind. But at that point you might as well start from scratch and make a display-manager.

Now someone could easily add this module and package to NUR and keep using it, but it doesn't change the fact that we can't support it.

@worldofpeace worldofpeace merged commit 4583e29 into NixOS:master Nov 15, 2019
12 checks passed
12 checks passed
Evaluation Performance Report Evaluator Performance Report
Details
grahamcofborg-eval ^.^!
Details
grahamcofborg-eval-check-meta config.nix: checkMeta = true
Details
grahamcofborg-eval-darwin nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A darwin-tested
Details
grahamcofborg-eval-nixos nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./nixos/release-combined.nix -A tested
Details
grahamcofborg-eval-nixos-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./nixos/release.nix -A manual
Details
grahamcofborg-eval-nixos-options nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./nixos/release.nix -A options
Details
grahamcofborg-eval-nixpkgs-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A manual
Details
grahamcofborg-eval-nixpkgs-tarball nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A tarball
Details
grahamcofborg-eval-nixpkgs-unstable-jobset nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A unstable
Details
grahamcofborg-eval-package-list nix-env -qa --json --file .
Details
grahamcofborg-eval-package-list-no-aliases nix-env -qa --json --file . --arg config { allowAliases = false; }
Details
@worldofpeace worldofpeace deleted the worldofpeace:remove-slim branch Nov 15, 2019
dtzWill added a commit to dtzWill/nixpkgs that referenced this pull request Nov 16, 2019
nixos/slim: remove

(cherry picked from commit 4583e29)
aszlig added a commit to openlab-aux/vuizvui that referenced this pull request Dec 8, 2019
This fixes the following evaluation error:

  The option `services.xserver.displayManager.slim' can no longer be
  used since it's been removed. The SLIM project is abandoned and their
  last release was in 2013.

  Because of this it poses a security risk to your system.
  Other issues include it not fully supporting systemd and logind
  sessions.
  Please use a different display manager such as LightDM, SDDM, or GDM.
  You can also use the startx module which uses Xinitrc.

Here is the nixpkgs upstream pull request removing SLiM:

  NixOS/nixpkgs#73251

Since I was using a custom theme for SLiM and actually liked the
minimalism, it's probably time to start patching LightDM soon. For now
however, I'll stay with a default LightDM configuration and wait until
I'm getting annoyed :-)

Signed-off-by: aszlig <aszlig@nix.build>
@oxij

This comment has been minimized.

Copy link
Contributor

oxij commented Dec 9, 2019

@worldofpeace

This comment has been minimized.

Copy link
Member Author

worldofpeace commented Dec 9, 2019

that it is not actually unmaintaned since Gentoo guys maintain it in https://gitweb.gentoo.org/repo/gentoo.git/tree/x11-misc/slim/files.

I mentioned that Gentoo had patches from certain places, we haven't identify their sources or authorship.
My initial assumption is that they're patches left behind from various efforts to fix certain things about slim. A distro carrying patches doesn't connotate the intrinsically different type of maintenance slim needs, a developer and proper systemd support. I would mention that it's more reasonable to remove something when alternatives exist, but it stands you weren't pleased with that reasoning #30890.


Also, a display-manager is a much different kind of software than the examples you've mentioned.
You can say certain software requires different types of effort in upkeep. I'm easily convinced it's much more difficult than an emacs mirror mode or a Firefox plugin. I'm thinking of security too.

@oxij

This comment has been minimized.

Copy link
Contributor

oxij commented Dec 9, 2019

@oxij oxij mentioned this pull request Dec 9, 2019
3 of 3 tasks complete
@oxij

This comment has been minimized.

Copy link
Contributor

oxij commented Dec 9, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can鈥檛 perform that action at this time.