Hi,

Le 22/03/2023 à 17:58, Marc Dequènes (duck) a écrit :
Quack,

Honestly when I read the title I really wondered how phog could have ended-up shipping this file. I forgot it initially, was asked about it and added it quickly, so it's not like I would have rejected the idea.

Well yes, it was only supposed to be transitional waiting for https://lists.sr.ht/~kennylevinsen/greetd/patches/36264 to land upstream, but I went a bit too optimistic on that one, my bad...

Anyway, back to the patch itself. First I wonder if it's useful to ship the second PAM config since in the code (greetd/src/server.rs#211) it simply use the base greetd PAM configuration as a fallback; this is not a blocker though.

The greeter PAM config drops the gnome-keyring/kwallet bits in order to be a bit lighter at runtime (those lines cause at least "gnome-keyring-daemon" to be started for user "_greetd", which is basically useless as it's a system user with no actual use of a keyring). Therefore I feel it's best to keep both config separate, but I'd be fine with a single config if you prefer it that way.

Then I would prefer if the changelog entries were shipped with the corresponding changes and not in a lump afterwards. Also the "debian:" and "d/*:" prefixes are not the style I use. Maybe I'm missing why some people still use it but with the VCS taking care of remembering which files have been changed I don't feel the need to add this anymore and it's not very non-DD friendly. I like your comments to clearly explain the rationale.

Thanks for the comments, I'm attaching the updated patches.

Cheers,
Arnaud


Regards.
\_o<

From 3494b22e9819cf6e1fd8c20623060272c1ea2c51 Mon Sep 17 00:00:00 2001
From: Arnaud Ferraris <arnaud.ferra...@collabora.com>
Date: Wed, 15 Mar 2023 13:51:11 +0100
Subject: [PATCH 1/2] Update PAM configuration(s)

Except for the gnome-keyring bits, all items currently set in the
`greetd` PAM config are already part of the `login` config. Including
the latter makes the `greetd` config simpler.

This commit also calls the PAM modules needed for unlocking the KDE
wallet as well, and adds the `greetd-greeter` config (simply including
`login` as the greeter itself doesn't need to deal with keyrings).

Finally, switch to using debhelper for installing the configs instead of
handling those manually.
---
 debian/changelog                 | 14 ++++++++++++++
 debian/greetd.greetd-greeter.pam |  2 ++
 debian/greetd.greetd.pam         |  8 ++++++++
 debian/greetd.install            |  1 -
 debian/pam.d/greetd              | 22 ----------------------
 debian/rules                     |  5 ++++-
 6 files changed, 28 insertions(+), 24 deletions(-)
 create mode 100644 debian/greetd.greetd-greeter.pam
 create mode 100644 debian/greetd.greetd.pam
 delete mode 100644 debian/pam.d/greetd

diff --git a/debian/changelog b/debian/changelog
index 707afdf..ae9095e 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,17 @@
+greetd (0.9.0-3) UNRELEASED; urgency=medium
+
+  * Update PAM configuration(s)
+    Except for the gnome-keyring bits, all items currently set in the
+    `greetd` PAM config are already part of the `login` config. Including
+    the latter makes the `greetd` config simpler.
+    This commit also calls the PAM modules needed for unlocking the KDE
+    wallet as well, and adds the `greetd-greeter` config (simply including
+    `login` as the greeter itself doesn't need to deal with keyrings).
+    Finally, switch to using debhelper for installing the configs instead of
+    handling those manually.
+
+ -- Arnaud Ferraris <aferra...@debian.org>  Fri, 24 Mar 2023 10:20:21 +0100
+
 greetd (0.9.0-2) unstable; urgency=medium
 
   * Provide PAM configuration (Closes: #1032786).
diff --git a/debian/greetd.greetd-greeter.pam b/debian/greetd.greetd-greeter.pam
new file mode 100644
index 0000000..1ed7162
--- /dev/null
+++ b/debian/greetd.greetd-greeter.pam
@@ -0,0 +1,2 @@
+#%PAM-1.0
+@include login
diff --git a/debian/greetd.greetd.pam b/debian/greetd.greetd.pam
new file mode 100644
index 0000000..7f2a906
--- /dev/null
+++ b/debian/greetd.greetd.pam
@@ -0,0 +1,8 @@
+#%PAM-1.0
+@include login
+
+-auth        optional        pam_gnome_keyring.so
+-auth        optional        pam_kwallet5.so
+
+-session     optional        pam_gnome_keyring.so auto_start
+-session     optional        pam_kwallet5.so auto_start
diff --git a/debian/greetd.install b/debian/greetd.install
index 0a3fbfb..83f2dad 100644
--- a/debian/greetd.install
+++ b/debian/greetd.install
@@ -1,4 +1,3 @@
 greetd.service /lib/systemd/system/
 config.toml /etc/greetd/
 debian/needrestart/50_greetd.conf /etc/needrestart/conf.d/
-debian/pam.d/greetd /etc/pam.d/
diff --git a/debian/pam.d/greetd b/debian/pam.d/greetd
deleted file mode 100644
index 062217b..0000000
--- a/debian/pam.d/greetd
+++ /dev/null
@@ -1,22 +0,0 @@
-#%PAM-1.0
-
-# Block login if they are globally disabled
-auth      requisite pam_nologin.so
-
-# Load environment from /etc/environment and ~/.pam_environment
-session      required pam_env.so readenv=1
-session      required pam_env.so readenv=1 envfile=/etc/default/locale
-
-@include common-auth
-
--auth  optional pam_gnome_keyring.so
-
-@include common-account
-
-session  required        pam_limits.so
-session  required        pam_loginuid.so
-@include common-session
-
--session optional        pam_gnome_keyring.so auto_start
-
-@include common-password
diff --git a/debian/rules b/debian/rules
index 523b72b..1c1d9c5 100755
--- a/debian/rules
+++ b/debian/rules
@@ -30,10 +30,13 @@ execute_after_dh_install:
 	# bad perms
 	chmod a-x debian/greetd/lib/systemd/system/greetd.service
 
+override_dh_installpam:
+	dh_installpam --name=greetd
+	dh_installpam --name=greetd-greeter
+
 override_dh_installsystemd:
 	dh_installsystemd --no-stop-on-upgrade --no-start
 
 execute_after_dh_auto_clean:
 	make -C man clean
 	rm -f debian/cargo-checksum.json
-
-- 
2.39.1

From 058e8d82ef7ed744d92ab3e25cdec89ae1ce743c Mon Sep 17 00:00:00 2001
From: Arnaud Ferraris <arnaud.ferra...@collabora.com>
Date: Wed, 15 Mar 2023 13:52:39 +0100
Subject: [PATCH 2/2] Add Breaks/Replaces relationships on older `phog`

`phog` used to ship the `greetd` and `greetd-greeter` PAM configs,
leading to conflicts with the latest version of the `greetd` package.
This commit ensures we avoid this conflict and maintain a clean
upgrade path for both those packages.
---
 debian/changelog | 7 ++++++-
 debian/control   | 3 ++-
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/debian/changelog b/debian/changelog
index ae9095e..2d5959d 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -9,8 +9,13 @@ greetd (0.9.0-3) UNRELEASED; urgency=medium
     `login` as the greeter itself doesn't need to deal with keyrings).
     Finally, switch to using debhelper for installing the configs instead of
     handling those manually.
+  * Add Breaks/Replaces relationships on older `phog`
+    `phog` used to ship the `greetd` and `greetd-greeter` PAM configs,
+    leading to conflicts with the latest version of the `greetd` package.
+    This commit ensures we avoid this conflict and maintain a clean
+    upgrade path for both those packages.
 
- -- Arnaud Ferraris <aferra...@debian.org>  Fri, 24 Mar 2023 10:20:21 +0100
+ -- Arnaud Ferraris <aferra...@debian.org>  Fri, 24 Mar 2023 10:21:55 +0100
 
 greetd (0.9.0-2) unstable; urgency=medium
 
diff --git a/debian/control b/debian/control
index 98466f4..c5d8cb6 100644
--- a/debian/control
+++ b/debian/control
@@ -35,7 +35,8 @@ Depends:
  adduser
 Provides: x-display-manager
 Suggests: wlgreet
+Breaks: phog (<< 0.1.3-2)
+Replaces: phog (<< 0.1.3-2)
 Description: minimal Wayland login manager
  greetd is a minimal and flexible login manager daemon that makes no
  assumptions about what you want to launch.
-
-- 
2.39.1

Reply via email to