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

pop-os-shell: init at 1.1.0 #104160

Closed
wants to merge 5 commits into from
Closed

pop-os-shell: init at 1.1.0 #104160

wants to merge 5 commits into from

Conversation

remunds
Copy link

@remunds remunds commented Nov 18, 2020

Motivation for this change

pop-os-shell version 1.0.0 was released 20 days ago. Now it's time to be a package :)
This is my first pr, so please tell me, if something is wrong :)

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 nixpkgs-review --run "nixpkgs-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.

@remunds remunds force-pushed the pop-os-shell branch 2 times, most recently from aeaa714 to 38f1bd9 Compare November 19, 2020 12:55

meta = with stdenv.lib; {
description = "Keyboard-driven layer for GNOME Shell";
license = licenses.gpl3;
Copy link
Contributor

Choose a reason for hiding this comment

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

gpl3 is unclear, use gpl3Plus or gpl3Only. See https://discourse.nixos.org/t/lib-licenses-gpl3-co-are-now-deprecated/8206 for more details.

Copy link
Author

Choose a reason for hiding this comment

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

I'm not entirely sure, which version to use. The license can be found here: https://github.com/pop-os/shell/blob/master/LICENSE. Also on the web i could not really identify what the difference is between gpl3-only and gpl3+. Can you tell me what we have here? @jtojnar

Copy link
Contributor

@jtojnar jtojnar Nov 19, 2020

Choose a reason for hiding this comment

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

Usually, the precise license is listed in README, but that is not the case here. Try asking on the upstream issue tracker. Example: rime/ibus-rime#107

Copy link

Choose a reason for hiding this comment

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

The license is here

Copy link
Contributor

Choose a reason for hiding this comment

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

That is just license terms, it needs to be explicitly specified whether the copyright owner allows using latter versions of the license, see the GNU links in the Discourse threads.

Copy link
Contributor

Choose a reason for hiding this comment

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

IIRC if not listed explicitly it should be GPL3 only, shouldn't it?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, that is all that can be inferred without any more info. Though people often intend -or-later so it does not hurt to ask.

@jtojnar
Copy link
Contributor

jtojnar commented Nov 19, 2020

@buckley310
Copy link
Contributor

This looks like it might be a duplicate of #102150

@jtojnar
Copy link
Contributor

jtojnar commented Nov 20, 2020

Oh, right. I was certain that we already had a PR open but could not find it with GitHub search 🤷‍♀️


nativeBuildInputs = [ glib nodePackages.typescript ];

makeFlags = [ "INSTALLBASE=$(out)/share/gnome-shell/extensions" ];
Copy link
Contributor

Choose a reason for hiding this comment

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

This one is nicer in that it does not abuse DESTDIR.

@amaxine
Copy link
Contributor

amaxine commented Nov 21, 2020

Oh, neat to see someone get around to packaging this! I've had a commit sitting around waiting for a 1.0 release that might help a bit - I see this doesn't install the glib schema or set the uuid like other gnome extensions.

Also, it might be nice to also package pop-shell-shortcuts (https://github.com/pop-os/shell-shortcuts) to go with it, I believe otherwise the extension will try to open a website with the shortcuts.

Here's my commit for pop-shell that does both things I mentioned: https://github.com/maxeaubrey/nixpkgs/commit/e46df8ddff5facaf57b86e5e7d5be189000e15a1
And here's pop-shell-shortcuts: https://github.com/maxeaubrey/nixpkgs/commit/5c14f15e6a2bfc9b6a11457ccb048a3668f10259

@jtojnar
Copy link
Contributor

jtojnar commented Nov 21, 2020

@remunds
Copy link
Author

remunds commented Nov 21, 2020

@maxeaubrey I had uuid set first, but i was advised to remove it if it is unnecessary.
Regarding pop-shell-shortcuts: I haven't had the problem with the website opening up and currently am using the package without shell-shortcuts. But maybe it's a good thing. Maybe putting it up as a second package is a good idea.

@amaxine
Copy link
Contributor

amaxine commented Nov 21, 2020

@remunds Oh, I guess uuid is obsolete now? 🤷‍♀️ Fair enough.

@jtojnar I had problems with the schema being missing and the extension not working correctly otherwise, but I cannot reproduce right now. Soz, what I said can be ignored on that.

@remunds remunds changed the title pop-os-shell: init at 1.0.0 pop-os-shell: init at 1.1.0 Dec 21, 2020
@mmstick
Copy link

mmstick commented Dec 21, 2020

I would recommend submitting patches upstream for locating the system path if the default GLib subprocess launcher can't find the binaries in your system path. You may add additional search paths with a subprocess launcher. There are a couple areas in Pop Shell that call out gjs, including some of the default plugins which are gjs scripts.

@jtojnar
Copy link
Contributor

jtojnar commented Dec 21, 2020

@mmstick The point of Nix is that there is not system path – every package is self-contained and hardcodes paths to everything it needs (in theory anyway). For some things like script shebangs, we have tooling that hardcodes the correct path but for exec calls in code, a more manual approach is required.

@mmstick
Copy link

mmstick commented Dec 21, 2020

The code that manually calls upon gjs can be changed to make use of the shebang line.

@jtojnar
Copy link
Contributor

jtojnar commented Dec 21, 2020

Right, that would be better for us too, we did the same with GSConnect. @remunds are you able to submit such patch upstream?

@ymarkus
Copy link
Contributor

ymarkus commented Dec 22, 2020

I've found another issue, although I'm not sure if it's maybe only on my end:
Deactivating "Show window titles" doesn't hide title bars.

@benneti
Copy link
Contributor

benneti commented Dec 22, 2020

This one is probably due to using Wayland where this is not supported upstream.

@remunds
Copy link
Author

remunds commented Dec 22, 2020

@jtojnar I will have a look at that later that day, but I am not sure whether I am capable enough. I'll let you know. Do I understand it right that the current approach is to create a patch that puts the shebang #!/usr/bin/env gjs in every file that needs it and then somehow modify the way those files are called?

@ymarkus that works for me.

@jtojnar
Copy link
Contributor

jtojnar commented Dec 22, 2020

@remunds Yes, that is correct. The files need to be invoked as scripts. Also, for that to work, you will need to make them executable if they are not already.

@remunds
Copy link
Author

remunds commented Dec 22, 2020

I'll give that a try, thanks :)

@remunds
Copy link
Author

remunds commented Dec 22, 2020

I was able to get the "floating window exception"-window with the other approach though.

@remunds
Copy link
Author

remunds commented Dec 22, 2020

Now its just the active hint color that doesn't work. Does anyone have an idea?

@hmenke
Copy link
Member

hmenke commented Apr 23, 2021

I've been using this for a few weeks now (updated to 1.2.0 though) on NixOS 20.09 under GNOME 3.36.5 Wayland and apart from the known upstream Wayland wonkiness, this actually works quite nicely.

@hpfr
Copy link

hpfr commented Apr 23, 2021

Tangentially related in case anyone is interested: https://blog.system76.com/post/648371526931038208/cosmic-to-arrive-in-june-release-of-popos-2104

https://github.com/pop-os/cosmic

It has pop-shell as a dependency.

{ stdenv, fetchFromGitHub, nodePackages, glib, substituteAll, gjs }:

stdenv.mkDerivation rec {
pname = "pop-os-shell";
Copy link
Contributor

Choose a reason for hiding this comment

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

🙋 Having looked inside pkgs/desktops/gnome-3/extensions, it appears all extensions are prefixed with gnome-shell-extension-, what are your thoughts on renaming?

Suggested change
pname = "pop-os-shell";
pname = "gnome-shell-extension-pop-os-shell";

@stale
Copy link

stale bot commented Nov 9, 2021

I marked this as stale due to inactivity. → More info

@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Nov 9, 2021
@genofire genofire mentioned this pull request Nov 26, 2021
13 tasks
@genofire
Copy link
Contributor

genofire commented Dec 2, 2021

Solved in #147542 - i believe we could close this PR now.
Maybe you add yourself to maintainers ;)

@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Dec 2, 2021
@Synthetica9 Synthetica9 closed this Dec 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet