jonls / redshift

Redshift adjusts the color temperature of your screen according to your surroundings. This may help your eyes hurt less if you are working in front of the screen at night.
http://jonls.dk/redshift
GNU General Public License v3.0
5.92k stars 429 forks source link

Hooks not running #850

Closed yannickperrenet closed 2 years ago

yannickperrenet commented 3 years ago

Describe the bug Redshift does not trigger any hooks.

According to #335 hooks are now also running on startup, but when I run redshift my hooks do not run (from what I can tell).

To Reproduce Create an executable file ~/.config/redshift/hooks/test.sh with content:

#!/bin/sh
case $1 in
    period-changed)
        exec touch /home/ubuntu/myredshifthook
esac

Next I start redshift using redshift and I don't see the file being created.

Expected behavior The hook should run and create the given file.

Software versions (please complete the following information):

yannickperrenet commented 3 years ago

My guess is that it is related to https://github.com/jonls/redshift/issues/672 but I don't how how to check that.

yannickperrenet commented 2 years ago

It was indeed related to #672.

I inspected the files /etc/apparmor.d/usr.bin.redshift and /etc/apparmor.d/local/usr.bin.redshift and found that $XDG_CONFIG_HOME/redshift/* was not allowed.

To solve my issue I first created the correct setup for the local file and reloaded the apparmor service (i.e. sudo systemctl reload apparmor.service) but wasn't quite happy with the result and ended up removing both aforementioned files from /etc/apparmor.d.

Now everything runs as expected.


Writing this down in case anyone else runs into the issue. First I checked whether apparmor was indeed preventing redshift from working how I envisioned. So I searched for apparmor="DENIED" lines that also contain redshift in the output of sudo dmesg | less after running redshift -c /path/to/config. And indeed apparmor was the cause.

Konfekt commented 1 year ago

Did adding

owner @{HOME}/.config/redshift/** r,

to /etc/apparmor.d/local/usr.bin.redshift and sudo systemctl reload apparmor.service not fix the execution of hooks?

yannickperrenet commented 1 year ago

@Konfekt That is indeed what I did. Should've written something similar to your explanation instead of my cumbersome explanation :+1:

NeatNit commented 1 year ago

For anyone else seeing this in the future, the above addition was still not enough - at least, not to /etc/apparmor.d/usr.bin.redshift. I had to add owner @{HOME}/.config/redshift/hooks/** ux, as well. I've never heard of AppArmor before now so I hope I did it right.

The full contents of my /etc/apparmor.d/usr.bin.redshift are now:

# ------------------------------------------------------------------
#
#    Copyright (C) 2015 Cameron Norman <camerontnorman@gmail.com>
#
#    This program is free software: you can redistribute it and/or modify
#    it under the terms of the GNU General Public License as published by
#    the Free Software Foundation, either version 3 of the License, or
#    (at your option) any later version.
#
#    This program is distributed in the hope that it will be useful,
#    but WITHOUT ANY WARRANTY; without even the implied warranty of
#    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
#    GNU General Public License for more details.
#
#    You should have received a copy of the GNU General Public License
#    along with this program.  If not, see <http://www.gnu.org/licenses/>.
#
# ------------------------------------------------------------------

#include <tunables/global>
/usr/bin/redshift {
  #include <abstractions/base>
  #include <abstractions/nameservice>
  #include <abstractions/dbus-strict>
  #include <abstractions/wayland>
  #include <abstractions/X>

  dbus send
       bus=system
       path=/org/freedesktop/GeoClue2/Client/[0-9]*,

  dbus receive
       bus=system
       path=/org/freedesktop/GeoClue2/Manager,

  # Allow but log any other dbus activity
  audit dbus bus=system,

  owner @{HOME}/.config/redshift.conf r,
  owner @{HOME}/.config/redshift/** r,
  owner @{HOME}/.config/redshift/hooks/** ux,
  owner /run/user/*/redshift-shared-* rw,

  # Site-specific additions and overrides. See local/README for details.
  #include <local/usr.bin.redshift>
}
Konfekt commented 1 year ago

Thank you @NeatNit. Wouldn't it be for the better to promote this to a pull request?

NeatNit commented 1 year ago

Thank you @NeatNit. Wouldn't it be for the better to promote this to a pull request?

From my POV, you are more than welcome to do that (though maybe you shouldn't, see below). As I said, I never heard of AppArmor before today so I don't feel comfortable making a PR like that without fully understanding the repercussions. Specifically, there are several different 'execute' permissions available in AppArmor and I didn't fully understand the pros and cons of each.

However, see PR #864 and @CameronNemo's comment:

I will remind everyone at Debian that Gammastep is a fork of Redshift that is maintained and accepting pull requests. It still supports all of the FOSS backends that Redshift supports (only Windows and macOS support were removed, not X or DRM; Wayland support was added).

You may also find this fix, that has been included in gammastep for over 2 years, helpful:

https://gitlab.com/chinstrap/gammastep/-/commit/9db19f5ccfad20ab7b501daedcfd5ef137860dd2

Gammastep in Debian could use an update, from 2.0.2 to 2.0.9. There have been some relevant fixes since the last version was uploaded to Sid. https://gitlab.com/chinstrap/gammastep/-/blob/master/NEWS.md

I might migrate to it myself. Only reason I'm using Redshift is that (I think) it came pre-installed with Linux Mint.

CameronNemo commented 1 year ago

Yes unfortunately no Debian Developer has stepped up to update gammastep since the initial few uploads. So looks like another stable Debian will be missing some important fixes that I released upstream.

NeatNit commented 1 year ago

Yes unfortunately no Debian Developer has stepped up to update gammastep since the initial few uploads. So looks like another stable Debian will be missing some important fixes that I released upstream.

Is there a procedure for getting them to update it? Or help them out with it somehow? I don't think making comments on PRs will get any Debian maintainer's attention.

CameronNemo commented 1 year ago

@NeatNit I dunno I don't use or contribute to Debian. Just thought I would mention it since you are on Mint and thus will be getting the 2.0.2 version that is a couple years out of date.

edit: this appears to be relevant https://www.debian.org/doc/manuals/developers-reference/pkgs.html#adopting-a-package

Konfekt commented 1 year ago

@NeatNit : Thank you, I wasn't aware of https://github.com/jonls/redshift/pull/864 that has been opened in the meanwhile. @CameronNemo : Thank you for solving this nagging bug in your fork already two years ago. May I ask how Gammastep differs from Redshift on X beyond this ?

NeatNit commented 1 year ago

Looks like one of you grabbed a Debian maintainer's attention: https://metadata.ftp-master.debian.org/changelogs//main/g/gammastep/gammastep_2.0.9-1_changelog

gammastep (2.0.9-1) unstable; urgency=medium

  • QA upload.
  • New upstream release. (LP: #1995497)
  • debian/: Apply "wrap-and-sort -abst".
  • debian/control:
    • Bump Standards-Version to 4.6.2.
    • Bump debhelper compat to v13.
    • Do not use branch in Vcs-Git field.
    • Do not explicitly build-depends on autopoint, not needed.
  • debian/rules: Use dh13 syntax.
  • debian/patches/: Dropped, all merged upstream.

    -- Boyuan Yang byang@debian.org Wed, 15 Feb 2023 00:06:45 -0500

Note that this is in the unstable repository. The testing and stable repositories are still on 2.0.2-6 and 2.0.2-4 respectively. I've got no idea what the typical timeline is for packages to move along to testing and stable - hopefully it'll happen soon - but I think we can install this package from unstable through apt somehow. A bit of DuckDuckGo will surely get us there :)

The package's page is here: https://packages.debian.org/sid/gammastep

The top right section has navigation between the different repositories - bullseye is stable, bookworm is testing, and sid is unstable. I'm not familiar with Debian's versioning scheme, package update policies, and whatnot.

Edit: this seems to be a good introduction for me, and perhaps for you: https://www.debian.org/distrib/packages

CameronNemo commented 1 year ago

Yeah unfortunately I think we are post-freeze for the next stable, but at least it will trickle through eventually.

LinuxOnTheDesktop commented 1 year ago

I am on Mint and I got the workaround for the hooks problem to work. However, I am unsure that I understand the discussion, above, about versions. Are matters as follows?

1) Recent versions of 'gammastep' - gammastep being a fork of redshift lack the hooks (/appArmor) problem.

2) In Debian, the 'unstable' flavour has a version of gammastep that is recent, but the other flavours of Debian do not.

3) Mint 21.x has redshift itself - not gammastep - and thus lacks the hooks fix.

4) redshift's mainainter would find it hard to implement the hooks fix within redshift itself.

Presuming that all or at least most of 1-4 are true, here are some questions.

Q1) Despite 4, could at least man redshift give the workaround for the hook problem? That manual will reach Mint eventually.

Q2) Does Ubuntu have redshift or does it have gammastep (or both)?

Q3) Should we try to get Mint, which ships redshift, to switch to gammastep?

CameronNemo commented 1 year ago

Ubuntu has had gammastep since 22.04 (https://packages.ubuntu.com/jammy/gammastep), but you want the updated version that is in lunar (23.04) or later.

Mint should probably switch to gammastep for future releases, yeah.

Edit: also appears that the new gammastep made it into bookworm (current testing, planned to be stable next month AFAIK).

NeatNit commented 1 year ago

Yup, just to reiterate your edit, Debian Bookworm will soon™ be stable, and it already includes gammastep v2.0.9 (the most recent version). After that, it will eventually trickle down to Ubuntu (and Mint) in future releases.

If they put v2.0.9 on Bullseye as well - that is, the Debian release which is currently stable - then it would trickle down to current releases of Ubuntu as well. I have no idea if or when this would happen.