Improve COSMIC support on NixOS by thefossguy · Pull Request #369113 · NixOS/nixpkgs

thefossguy · 2024-12-29T13:07:38Z

I noticed that COSMIC from nixpkgs was not working well on my rock-5b and in x86 and aarch64 VMs. The screen would stay grey. I switched from COSMIC using Lily Foster's flake and it worked perfectly. This is my effort in upstreaming Lily's out of tree code into nixpkgs.

cc: @nyabinary @a-kenji @ahoneybun @alyssais

Things done

Add a 👍 reaction to pull requests you find important.

- Switch from `makeBinaryWrapper` to `libcosmicAppHook` - Bump version of all cosmic packages to 'epoch-1.0.0-alpha.4' - Add 'thefossguy' to the maintainer for every updated package - Initialize `cosmic-idle`, `cosmic-player` and `cosmic-reader`

SigmaSquadron · 2024-12-29T13:13:15Z

I'm pretty sure that the COSMIC upstreaming effort is being coordinated by the COSMIC team. This is a massive PR and it'll be really hard to review this, especially since it seems to be a spur of the moment thing.

thefossguy · 2024-12-29T13:19:11Z

Not exactly a spur of the moment thing. I have been working this since the 4th alpha dropped. I couldn't work before because I didn't have a spare machine to test my changes on. There have been some refactors too.

On your second point, yes this is a big PR. I understand that this is quite some work but I would really appreciate some time for a good review in hopes of merging it into nixpkgs.

SigmaSquadron · 2024-12-29T13:23:00Z

By spur of the moment I meant that you worked on this without the COSMIC people. (Notably, Lily, since this is based on her work primarily.)

I'm sure that the COSMIC team had a very good reason to not upstream the flake yet.

emilazy · 2024-12-29T13:25:39Z

Neither the files nor the commits contain any attribution to the original authors of this code, which is a copyright issue; you’ll at least need Co-authored-by: trailers on the commits. The central mega‐commit also doesn’t follow our commit message guidelines; it would need to be split up into many more commits.

I would strongly suggest this kind of work proceed incrementally rather than in one huge PR; there’s little chance it will be reviewed and merged in its current state.

cc @lilyinstarlight

thefossguy · 2024-12-29T13:26:30Z

That I admit, this was very much done without Lily. The reason why this isn't merged in nixpkgs by Lily is because she is no longer interacting with nixpkgs after the nix drama. She is maintaining her out-of-tree flake for that reason.

SigmaSquadron · 2024-12-29T13:28:12Z

Related: lilyinstarlight/nixos-cosmic#245 (comment)

thefossguy · 2024-12-29T13:29:21Z

@emilazy Noted. I'll talk to Lily about getting her approval on a Co-authored-by in the commits. As for splitting the commits, that's what I wanted to do but thought a single commit with the same description would be a better way forward. I'll split them.

Marking current PR as a draft for now.

SigmaSquadron

This diff is really hard to read. (Did you reorder attributes for some reason?)

You should really split this in separate PRs to facilitate merging. (Broken stuff won't block working stuff)

SigmaSquadron · 2024-12-29T13:29:09Z

nixos/doc/manual/release-notes/rl-2505.section.md

+- [COSMIC DE](https://system76.com/cosmic/), the COSMIC desktop environment by Pop!_OS.
+


Feel free to bump this to highlights. A new desktop environment is very notable.

SigmaSquadron · 2024-12-29T13:31:21Z

nixos/modules/services/desktop-managers/cosmic.nix

+
+  options = {
+    services.desktopManager.cosmic = {
+      enable = lib.mkEnableOption "COSMIC desktop environment";


Suggested change

enable = lib.mkEnableOption "COSMIC desktop environment";

enable = lib.mkEnableOption "the COSMIC desktop environment";

mkEnableOption doesn't add the article.

SigmaSquadron · 2024-12-29T13:31:41Z

nixos/modules/services/desktop-managers/cosmic.nix

+    services.desktopManager.cosmic = {
+      enable = lib.mkEnableOption "COSMIC desktop environment";
+
+      unstable.protocols.enable = lib.mkEnableOption "Enable the [currently unstable] cosmic-protocols" // {


Suggested change

unstable.protocols.enable = lib.mkEnableOption "Enable the [currently unstable] cosmic-protocols" // {

unstable.protocols.enable = lib.mkEnableOption "Enable the currently unstable `cosmic-protocols`" // {

SigmaSquadron · 2024-12-29T13:32:11Z

nixos/modules/services/desktop-managers/cosmic.nix

+      enable = lib.mkEnableOption "COSMIC desktop environment";
+
+      unstable.protocols.enable = lib.mkEnableOption "Enable the [currently unstable] cosmic-protocols" // {
+        default = false;


Suggested change

default = false;

mkEnableOption options are false by default.

SigmaSquadron · 2024-12-29T13:32:28Z

nixos/modules/services/desktop-managers/cosmic.nix

+        default = false;
+      };
+
+      xwayland.enable = lib.mkEnableOption "Xwayland support for cosmic-comp" // {


Suggested change

xwayland.enable = lib.mkEnableOption "Xwayland support for cosmic-comp" // {

xwayland.enable = lib.mkEnableOption "Xwayland support for `cosmic-comp`" // {

SigmaSquadron · 2024-12-29T13:33:05Z

nixos/modules/services/desktop-managers/cosmic.nix

+    };
+
+    environment.cosmic.excludePackages = lib.mkOption {
+      description = "List of COSMIC packages to exclude from the default environment";


Suggested change

description = "List of COSMIC packages to exclude from the default environment";

description = "List of COSMIC packages to exclude from the default environment.";

SigmaSquadron · 2024-12-29T13:38:46Z

nixos/modules/services/desktop-managers/cosmic.nix

+    assertions = [
+      {
+        assertion = lib.elem "libcosmic-app-hook" (
+          lib.map (
+            drv: lib.optionalString (lib.isDerivation drv) (lib.getName drv)
+          ) pkgs.cosmic-comp.nativeBuildInputs
+        );
+        message = ''
+          It looks like the provided `pkgs` to the NixOS COSMIC module is not usable for a working COSMIC
+          desktop environment.
+
+          If you are erroneously passing in `pkgs` to `specialArgs` somewhere in your system configuration,
+          this is is often unnecessary and has unintended consequences for all NixOS modules. Please either
+          remove that in favor of configuring the NixOS `pkgs` instance via `nixpkgs.config` and
+          `nixpkgs.overlays`.
+
+          If you must instantiate your own `pkgs`, then please include the overlay from the NixOS COSMIC flake
+          when instantiating `pkgs` and be aware that the `nixpkgs.config` and `nixpkgs.overlays` options will
+          not function for any NixOS modules.
+
+          Note that the COSMIC packages in Nixpkgs are still largely broken as of 2024-10-16 and will not be
+          usable for having a fully functional COSMIC desktop environment. The overlay is therefore necessary.
+        '';


Suggested change

assertions = [

{

assertion = lib.elem "libcosmic-app-hook" (

lib.map (

drv: lib.optionalString (lib.isDerivation drv) (lib.getName drv)

) pkgs.cosmic-comp.nativeBuildInputs

);

message = ''

It looks like the provided `pkgs` to the NixOS COSMIC module is not usable for a working COSMIC

desktop environment.

If you are erroneously passing in `pkgs` to `specialArgs` somewhere in your system configuration,

this is is often unnecessary and has unintended consequences for all NixOS modules. Please either

remove that in favor of configuring the NixOS `pkgs` instance via `nixpkgs.config` and

`nixpkgs.overlays`.

If you must instantiate your own `pkgs`, then please include the overlay from the NixOS COSMIC flake

when instantiating `pkgs` and be aware that the `nixpkgs.config` and `nixpkgs.overlays` options will

not function for any NixOS modules.

Note that the COSMIC packages in Nixpkgs are still largely broken as of 2024-10-16 and will not be

usable for having a fully functional COSMIC desktop environment. The overlay is therefore necessary.

'';

This isn't applicable in Nixpkgs. It's also up to you to fix the bugs that this assertion tries to warn users about.

SigmaSquadron · 2024-12-29T13:40:23Z

pkgs/build-support/setup-hooks/libcosmicAppHook/default.nix

+    );
+  };
+
+  meta = with lib; {


Suggested change

meta = with lib; {

meta = {

For every other package as well.

SigmaSquadron · 2024-12-29T13:40:48Z

pkgs/build-support/setup-hooks/libcosmicAppHook/default.nix

This should go to by-name.

SigmaSquadron · 2024-12-29T13:42:28Z

pkgs/by-name/co/cosmic-icons/package.nix

    owner = "pop-os";
    repo = "cosmic-icons";
-    rev = "epoch-1.0.0-alpha.3";
+    rev = "refs/tags/epoch-1.0.0-alpha.4";


Suggested change

rev = "refs/tags/epoch-1.0.0-alpha.4";

tag = "epoch-1.0.0-alpha.4";

Ditto for every other package.

lilyinstarlight

This needs to follow terms of MIT license if you're going to be copy-pasting this code

It would help to have much more assurance of anyone here's intent and ability to maintain the copy-pasted code and work with COSMIC upstream on stuff, because as far as I know mostly me and @alyssais and @ahoneybun (who works for sys76) have bothered with that

lilyinstarlight · 2024-12-29T17:06:07Z

nixos/modules/services/desktop-managers/cosmic.nix

+        xdg-user-dirs
+      ]
+      ++ lib.optionals cfg.unstable.protocols.enable [
+        cosmic-protocols


This does not do anything useful as far as I know...?

lilyinstarlight · 2024-12-29T17:06:27Z

nixos/modules/services/desktop-managers/cosmic.nix

+
+    xdg.portal = {
+      enable = true;
+      extraPortals = lib.mkBefore [ pkgs.xdg-desktop-portal-cosmic ];


What is the mkBefore fixing here?

lilyinstarlight · 2024-12-29T17:07:01Z

nixos/modules/services/desktop-managers/cosmic.nix

+    xdg.portal = {
+      enable = true;
+      extraPortals = lib.mkBefore [ pkgs.xdg-desktop-portal-cosmic ];
+      configPackages = lib.mkDefault ([ pkgs.xdg-desktop-portal-cosmic ]);


What is mkDefault solving here? It should be matching the prio level of other DEs

ahoneybun · 2024-12-29T21:09:07Z

This needs to follow terms of MIT license if you're going to be copy-pasting this code

It would help to have much more assurance of anyone here's intent and ability to maintain the copy-pasted code and work with COSMIC upstream on stuff, because as far as I know mostly me and @alyssais and @ahoneybun (who works for sys76) have bothered with that.

Thank you for helping with this review!

Pandapip1 · 2024-12-29T21:19:42Z

pkgs/by-name/co/cosmic-wallpapers/package.nix

Please revert the changes to this file, except for the maintainer list update if you want to be added. The current derivation functions as expected and has some improvements over the one in the flake. If you'd like to make specific changes, we can discuss them, but don't blindly paste the code from lily's flake, especially since she explicitly asked for me to rewrite it myself.

nyabinary · 2024-12-30T12:41:45Z

Supersedes #339287 #292601

nyabinary · 2025-02-07T23:19:45Z

@thefossguy Are you still working on this?

thefossguy · 2025-02-08T06:10:34Z

@nyabinary Given that the cosmic greeter from nixpkgs doesn't work, I intend to. I'm not sure if I should rebase this branch with requested changes or individual PRs for each commit.

Pandapip1 · 2025-02-08T07:58:17Z

Individual PRs for each package is probably the best way to go. This will also encourage making your own changes as you go (e.g. changing ref to tag, using the new cargo fetcher, etc...) and will help avoid you making incorrect changes like removing my patch.

wrenix · 2025-03-17T19:17:48Z

What is left over (after merge of e.g. #380312 ) of this MR? Could you rebase?

thefossguy added 4 commits December 29, 2024 18:26

libcosmicAppHook: init

360f951

initialize and update COSMIC packages

4f94299

- Switch from `makeBinaryWrapper` to `libcosmicAppHook` - Bump version of all cosmic packages to 'epoch-1.0.0-alpha.4' - Add 'thefossguy' to the maintainer for every updated package - Initialize `cosmic-idle`, `cosmic-player` and `cosmic-reader`

nixos/modules: cosmic-greeter: init

7e6351f

nixos/modules: cosmic: init

c81e338

github-actions bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` labels Dec 29, 2024

nix-owners bot requested a review from Ericson2314 December 29, 2024 13:09

nixos/modules: cosmic: add 25.05 release notes entry

6470bba

github-actions bot added 8.has: documentation This PR adds or changes documentation 8.has: changelog This PR adds or changes release notes labels Dec 29, 2024

github-actions bot added 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 11-100 This PR causes between 11 and 100 packages to rebuild on Linux. labels Dec 29, 2024

thefossguy marked this pull request as draft December 29, 2024 13:29

SigmaSquadron reviewed Dec 29, 2024

View reviewed changes

SigmaSquadron added 9.needs: clean-up Somebody please clean up this mess! 9.needs: community feedback This needs feedback from more community members. and removed 9.needs: community feedback This needs feedback from more community members. labels Dec 29, 2024

lilyinstarlight reviewed Dec 29, 2024

View reviewed changes

ofborg bot added the 8.has: package (new) This PR adds a new package label Dec 29, 2024

ofborg bot requested review from Pandapip1, a-kenji, ahoneybun and alyssais December 29, 2024 18:49

ofborg bot requested a review from nyabinary December 29, 2024 18:49

ofborg bot added the 11.by: package-maintainer This PR was created by a maintainer of all the package it changes. label Dec 29, 2024

Pandapip1 reviewed Dec 29, 2024

View reviewed changes

a-kenji removed their request for review December 31, 2024 00:43

Lord-Valen mentioned this pull request Jan 1, 2025

nixos/cosmic-greeter: init; nixos/cosmic: init #292601

Closed

13 tasks

nyabinary mentioned this pull request Jan 1, 2025

[DRAFT] libcosmicAppHook: init #339287

Closed

13 tasks

thefossguy mentioned this pull request Feb 8, 2025

libcosmicAppHook: init #380312

Merged

13 tasks

wegank added the 2.status: merge conflict This PR has merge conflicts with the target branch label Feb 15, 2025

thefossguy mentioned this pull request Mar 24, 2025

nixos/cosmic-greeter: init; nixos/cosmic: init #392837

Merged

13 tasks

JohnRTitor closed this in #392837 Mar 31, 2025

		- [COSMIC DE](https://system76.com/cosmic/), the COSMIC desktop environment by Pop!_OS.

	enable = lib.mkEnableOption "COSMIC desktop environment";
	enable = lib.mkEnableOption "the COSMIC desktop environment";

	unstable.protocols.enable = lib.mkEnableOption "Enable the [currently unstable] cosmic-protocols" // {
	unstable.protocols.enable = lib.mkEnableOption "Enable the currently unstable `cosmic-protocols`" // {

	xwayland.enable = lib.mkEnableOption "Xwayland support for cosmic-comp" // {
	xwayland.enable = lib.mkEnableOption "Xwayland support for `cosmic-comp`" // {

	description = "List of COSMIC packages to exclude from the default environment";
	description = "List of COSMIC packages to exclude from the default environment.";

	rev = "refs/tags/epoch-1.0.0-alpha.4";
	tag = "epoch-1.0.0-alpha.4";

Uh oh!

Conversation

thefossguy commented Dec 29, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Things done

Uh oh!

SigmaSquadron commented Dec 29, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

thefossguy commented Dec 29, 2024

Uh oh!

SigmaSquadron commented Dec 29, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

emilazy commented Dec 29, 2024

Uh oh!

thefossguy commented Dec 29, 2024

Uh oh!

SigmaSquadron commented Dec 29, 2024

Uh oh!

thefossguy commented Dec 29, 2024

Uh oh!

SigmaSquadron left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lilyinstarlight left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ahoneybun commented Dec 29, 2024

Uh oh!

Pandapip1 Dec 29, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nyabinary commented Dec 30, 2024

Uh oh!

nyabinary commented Feb 7, 2025

Uh oh!

thefossguy commented Feb 8, 2025

Uh oh!

Pandapip1 commented Feb 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wrenix commented Mar 17, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

thefossguy commented Dec 29, 2024 •

edited

Loading

SigmaSquadron commented Dec 29, 2024 •

edited

Loading

SigmaSquadron commented Dec 29, 2024 •

edited

Loading

Pandapip1 Dec 29, 2024 •

edited

Loading

Pandapip1 commented Feb 8, 2025 •

edited

Loading