Add x11 service for unclutter-xfixes #18398

erikryb · 2016-09-07T17:52:42Z

Motivation for this change

This is a service similar to the unclutter service, but for the new unclutter-xfixes. There is some effort to make unclutter-xfixes compatible with unclutter, but for now it is probably best to keep them as two different modules.

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
- OS X
- Linux
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.

mention-bot · 2016-09-07T17:52:42Z

@erikryb, thanks for your PR! By analyzing the annotation information on this pull request, we identified @edolstra, @bjornfor and @offlinehacker to be potential reviewers

bjornfor · 2016-09-07T17:57:39Z

nixos/modules/services/x11/unclutter-xfixes.nix

+      default = 1;
+    };
+
+    threeshold = mkOption {


Typo (one 'e' too many).

groxxda · 2016-09-07T18:20:40Z

nixos/modules/services/x11/unclutter-xfixes.nix

+  config = mkIf cfg.enable {
+    systemd.user.services.unclutter-xfixes = {
+      description = "unclutter-xfixes";
+      wantedBy = [ "default.target" ];


Minor nitpick: Imo this should be graphical.target.

Yeah, that makes sense. Better to be explicit.

groxxda · 2016-09-07T18:45:14Z

We also might want some ordering After=display-manager.service and possibly Requisite=display-manager.service (?).

erikryb · 2016-09-08T07:52:51Z

Why "Requisite" instead of "Requires"?

groxxda · 2016-09-08T12:34:31Z

Well, in my opinion if display-manager is not pulled in on it's own, unclutter should respect that and just refuse to start.. PartOf or BindsTo could also be valid choices, that's why I added a ❓ 😉

erikryb · 2016-09-08T12:52:33Z

I see. It seems that BindsTo is a stronger version of Requires, so
that should also not be used for the same reason. Am I correct in that
PartOf is a weaker version of Requisite, that is, does Requisite
also restart or stop unclutter when display-manager restarts or
stops? If so, I believe Requisite is the best option, as you first
stated.

On Thu, Sep 08, 2016 at 05:34:35AM -0700, Alexander Ried wrote:

Well, in my opinion if display-manager is not pulled in on it's own, unclutter should respect that and just refuse to start.. PartOf or BindsTo could also be valid choices, that's why I added a ❓ 😉

You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub:
#18398 (comment)

groxxda · 2016-09-08T15:46:09Z

I did not try the exact behavior, but Requisite sounded appropriate to me. I expect clutter to fail anyway when display-manager stops / fails / restarts, so it's not really that important whether it's propagated by systemd. But feel free to try and report what happens 👍

Apart from this, if you look at the redshift service that's quite similar, it will get merged even if it's not perfect.. 😞

By the way: doesn't this need the DISPLAY variable set?

pdfjam is included in texlive fixes NixOS#19826

In category `tools`, subcategory `text`, add a package definition for the program [`agrep`] [1] — "Approximate `grep` for fast fuzzy string searching". I have tested this patch per nixpkgs manual section 11.1 ("Making patches"). [1]: <https://www.tgries.de/agrep/>

Fixes build against linux >=4.8 Full changelog at https://git.lttng.org/?p=lttng-tools.git;a=blob_plain;f=ChangeLog;hb=13dc409a5ea439b96b805c3c71886a3fcfad18e8 Tested with nix-build -A linuxPackages.lttng-modules -A linuxPackages_latest.lttng-modules

pdfjam: remove obsolete version

agrep: init at 3.41.5

lumina: init at 1.1.0-p1

lttng-modules: 2.8.0 -> 2.8.3

bjornfor · 2016-11-06T17:04:21Z

Why did you close?

erikryb · 2016-11-06T17:35:38Z

It was a mistake. I rebased my master branch and the pull request got closed. I guess that's why I should have used a dedicated branch for the pull request. How do I reopen it? Or should I just start a new pull request?

EDIT: Alright, I reopened the pull request, but I don't know why the other commits other than mine got included.

joachifm · 2016-11-29T13:30:44Z

@erikryb I rebased & merged this at 2f0cc0d. Thank you

erikryb · 2016-11-29T13:35:06Z

Thanks!

bjornfor reviewed Sep 7, 2016
View reviewed changes

nixos/modules/services/x11/unclutter-xfixes.nix

default = 1;

};

threeshold = mkOption {

Copy link

Contributor

bjornfor Sep 7, 2016

Choose a reason for hiding this comment

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

Typo (one 'e' too many).

erikryb force-pushed the master branch from 8cc691f to 6128c22 Compare September 7, 2016 18:06

groxxda reviewed Sep 7, 2016
View reviewed changes

erikryb force-pushed the master branch from 6128c22 to 02fbc37 Compare September 7, 2016 18:30

fpletz added 6.topic: nixos 8.has: module (new) labels Sep 13, 2016

romildo and others added 4 commits November 1, 2016 17:06

lumina: init at 1.1.0-p1

a25e6bc

pdfjam: remove obsolete version

f00e281

pdfjam is included in texlive fixes NixOS#19826

lttng-modules: 2.8.0 -> 2.8.3

ae5d404

Fixes build against linux >=4.8 Full changelog at https://git.lttng.org/?p=lttng-tools.git;a=blob_plain;f=ChangeLog;hb=13dc409a5ea439b96b805c3c71886a3fcfad18e8 Tested with nix-build -A linuxPackages.lttng-modules -A linuxPackages_latest.lttng-modules

erikryb closed this Nov 6, 2016

erikryb force-pushed the master branch from 02fbc37 to cbec6a3 Compare November 6, 2016 13:35

joachifm added 4 commits November 6, 2016 15:16

Merge pull request NixOS#20077 from FlorentBecker/remove_pdfjam

5bfbf6a

pdfjam: remove obsolete version

Merge pull request NixOS#20178 from 8573/8573/pkg/add/agrep/1

be59aa0

agrep: init at 3.41.5

Merge pull request NixOS#20063 from romildo/new.lumina

858f088

lumina: init at 1.1.0-p1

Merge pull request NixOS#20216 from joachifm/lttng-modules

aa094fe

lttng-modules: 2.8.0 -> 2.8.3

Add x11 service for unclutter-xfixes

bd7521b

erikryb reopened this Nov 6, 2016

joachifm closed this in 2f0cc0d Nov 29, 2016

infinisil mentioned this pull request Nov 16, 2018

NixOS services: When to have a package option #50476

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add x11 service for unclutter-xfixes #18398

Add x11 service for unclutter-xfixes #18398

erikryb commented Sep 7, 2016

mention-bot commented Sep 7, 2016

bjornfor Sep 7, 2016

groxxda Sep 7, 2016

erikryb Sep 7, 2016

groxxda commented Sep 7, 2016

erikryb commented Sep 8, 2016

groxxda commented Sep 8, 2016

erikryb commented Sep 8, 2016

groxxda commented Sep 8, 2016 •

edited

Loading

bjornfor commented Nov 6, 2016

erikryb commented Nov 6, 2016 •

edited

Loading

joachifm commented Nov 29, 2016

erikryb commented Nov 29, 2016

Add x11 service for unclutter-xfixes #18398

Add x11 service for unclutter-xfixes #18398

Conversation

erikryb commented Sep 7, 2016

Motivation for this change

Things done

mention-bot commented Sep 7, 2016

bjornfor Sep 7, 2016

Choose a reason for hiding this comment

groxxda Sep 7, 2016

Choose a reason for hiding this comment

erikryb Sep 7, 2016

Choose a reason for hiding this comment

groxxda commented Sep 7, 2016

erikryb commented Sep 8, 2016

groxxda commented Sep 8, 2016

erikryb commented Sep 8, 2016

groxxda commented Sep 8, 2016 • edited Loading

bjornfor commented Nov 6, 2016

erikryb commented Nov 6, 2016 • edited Loading

joachifm commented Nov 29, 2016

erikryb commented Nov 29, 2016

groxxda commented Sep 8, 2016 •

edited

Loading

erikryb commented Nov 6, 2016 •

edited

Loading