x11-misc/slim : Upstream project forked, updated ebuilds #29838

RobPearce · 2023-02-27T18:41:49Z

This pull request updates the Simple Login Manager to the latest release from the slim-fork project, which has resurrected what was a dead project and fixed lots of bugs.
Closes: https://bugs.gentoo.org/832562
Closes: https://bugs.gentoo.org/727544
Closes: https://bugs.gentoo.org/832303
Closes: https://bugs.gentoo.org/580458
Closes: https://bugs.gentoo.org/803476
Closes: https://bugs.gentoo.org/732430
Closes: https://bugs.gentoo.org/756181

gentoo-repo-qa-bot · 2023-02-27T19:03:33Z

Pull request CI report

Report generated at: 2023-02-27 19:03 UTC
Newest commit scanned: a989059
Status: ✅ good

There are existing issues already. Please look into the report to make sure none of them affect the packages in question:
https://qa-reports.gentoo.org/output/gentoo-ci/b329d22236/output.html

gentoo-bot · 2023-02-28T18:52:07Z

Pull Request assignment

Submitter: @RobPearce
Areas affected: ebuilds
Packages affected: x11-misc/slim

x11-misc/slim: @gentoo/proxy-maint (maintainer needed)

Linked bugs

Bugs linked: 580458, 732430, 832303, 832562, 803476, 756181, 727544
Cross-linking bugs disabled due to large number of bugs linked.

In order to force reassignment and/or bug reference scan, please append [please reassign] to the pull request title.

Docs: Code of Conduct ● Copyright policy (expl.) ● Devmanual ● GitHub PRs ● Proxy-maint guide

gentoo-repo-qa-bot · 2023-02-28T18:58:51Z

Pull request CI report

Report generated at: 2023-02-28 18:58 UTC
Newest commit scanned: ac84201
Status: ✅ good

There are existing issues already. Please look into the report to make sure none of them affect the packages in question:
https://qa-reports.gentoo.org/output/gentoo-ci/67c015b4bd/output.html

RobPearce · 2023-03-04T10:11:17Z

Would it help if I add myself as a proxy maintainer? I note that the Guide says to do so before committing the fixes but if I volunteer now, would that be OK?

ceamac

Thank you for your work on this, I see you closed a lot of bugs.

I didn't install and use this new slim yet, so I may come with further feedback after I do. It builds succesfully. I noticed 1.4.0 is missing a few files that still exist in 1.3.9 -
usr/share/slim/themes/original/{background.jpg,panel.png,slim.theme. Is this intentional? Maybe upstream removed them?

Also - please squash your commits and then force push the changes. Ideally we should have 1 commit for 1.3.9 and one for 1.4.0, but having only one for both is also acceptable.

Thank you!

ceamac · 2023-03-12T11:35:18Z

x11-misc/slim/files/slim-1.3.9-config.diff

@@ -0,0 +1,47 @@
+--- a/slim.conf.orig	2010-08-25 11:52:23.000000000 -0400
+++ b/slim.conf	2010-08-25 11:58:58.000000000 -0400


Please fix the filename, it should be the same (probably a/slim.conf and b/slim.conf). Afterwards you can run scrub-patch files/slim-1.3.9-config.diff to remove the timestamps. It will also output a few warnings, see if anything should be fixed or they can be ignored.
scrub-patch comes with app-portage/iwdevtools.

Also please add a header to the patch file, i.e., where does it come from? What is it trying to fix? Maybe some relevant links.

ceamac · 2023-03-12T11:38:29Z

x11-misc/slim/files/slim-1.4.0-config.diff

+Index: slim-fork-code/slim.conf
+===================================================================
+--- slim-fork-code/slim.conf	(revision 54)
+++ slim-fork-code/slim.conf	(working copy)


It is customary to prefix the files in the patch with a/ and b/. For example a/slim.conf. The patch is applied with -p1 by default, so it also works as it is, it's just a matter of style.

ceamac · 2023-03-12T11:39:30Z

x11-misc/slim/metadata.xml

@@ -3,6 +3,6 @@
 <pkgmetadata>
 	<!-- maintainer-needed -->


Yes, please add yourself as a maintainer here if you are interested in maintaining this.

ceamac · 2023-03-12T11:40:43Z

x11-misc/slim/slim-1.3.6-r5.ebuild

@@ -20,7 +20,7 @@ RDEPEND="x11-libs/libXmu
 	x11-libs/libXft
 	x11-libs/libXrandr
 	media-libs/libpng:0=
-	virtual/jpeg:=
+	media-libs/libjpeg-turbo


This is the stable ebuild. You cannot change RDEPEND without a revision bump, and bumping it would drop to unstable, so please don't change this ebuild, better wait until 1.3.9 (or 1.4.0) reaches stable and remove this old revision then.

OK, fair point. I got carried away fixing pkgdev warnings

ceamac · 2023-03-12T11:41:11Z

x11-misc/slim/slim-1.3.9-r1.ebuild

+# Copyright 1999-2023 Gentoo Authors
+# Distributed under the terms of the GNU General Public License v2
+
+EAPI=7


We try to bump all new ebuilds to EAPI 8.

ceamac · 2023-03-12T11:46:22Z

x11-misc/slim/slim-1.3.9-r1.ebuild

+}
+
+pkg_postinst() {
+	# massage ${REPLACING_VERSIONS} to come up with whether or not it's a


1.3.2 was removed from the tree in 2013, 10 years ago. Let's remove this check, it's long obsolete.

Is it worth retaining (and, I note, fixing) the elog for a first install, or do you think that's superfluous? Ah, I see you mention this below. OK

There is also a pretty comprehensive (and slightly outdated) wiki page for SLIM: https://wiki.gentoo.org/wiki/SLiM so you can display a link instead or add it to that text.

ceamac · 2023-03-12T11:48:00Z

x11-misc/slim/slim-1.3.9-r1.ebuild

@@ -0,0 +1,122 @@
+# Copyright 1999-2023 Gentoo Authors


Short question - why is this ebuild named slim-1.3.9-r1.ebuild? There was no -r0, you can start with slim-1.3.9.ebuild.

I had a 1.3.9 locally and posted it on the bugzilla (#832562) before spotting a fix that I felt was worth patching. As that ebuild had been made public and offered for inclusion in the tree, I wanted to avoid confusion.

bugzilla attachments are irrelevant for portage ebuild revisions.

ceamac · 2023-03-12T12:06:17Z

x11-misc/slim/slim-1.3.9-r1.ebuild

+		fi
+	done
+
+	if [[ ${rv} == none ]]; then


You can use the readme.gentoo-r1 eclass to display a message only on first installation, to skip this none check logic altogether.

(this is optional - we can leave it for another time)

ceamac · 2023-03-12T12:07:50Z

x11-misc/slim/slim-1.3.9-r1.ebuild

+	x11-libs/libXft
+	x11-libs/libXrandr
+	media-libs/libpng:0=
+	media-libs/libjpeg-turbo


The dependencies should be sorted alphabetically. So please move media-libs/ deps upwards.

ceamac · 2023-03-12T12:12:24Z

x11-misc/slim/slim-1.4.0.ebuild

+
+inherit cmake pam systemd
+
+if [[ ${PV} == "9999" ]] ; then


There is no slim-9999.ebuild, did you add one and forgot to push it?

I use one locally for testing - I'm not sure whether it would be desirable to add it. Which way would you prefer I fix this?

Add it and keep it in sync with the last version, no point in keeping it private since you already use it :)

ceamac · 2023-03-12T15:14:18Z

x11-misc/slim/slim-1.3.9-r1.ebuild

+	x11-libs/libXft
+	x11-libs/libXrandr
+	media-libs/libpng:0=
+	media-libs/libjpeg-turbo


Suggested change

media-libs/libjpeg-turbo

media-libs/libjpeg-turbo:=

is it necessary though? (I realise libjpeg-turbo has a subslot, but not every revdep links against specific soversion)

This is linked against libjpeg.so.62. So I guess if the 62 changes it will need rebuilding.

ceamac · 2023-03-12T15:14:49Z

x11-misc/slim/slim-1.3.9-r1.ebuild

+IUSE="branding pam"
+
+RDEPEND="x11-libs/libXmu
+	x11-libs/libX11


x11-libs/libXext

I think that's only required for the "slimlock" tool, which is only built if PAM is enabled, so should this be
pam? (x11-libs/libXext)
?

Sure. If it's used only with pam, add it conditionally.

ceamac · 2023-03-12T16:04:17Z

I tried them both, they work. I like version 1.4 better, it has a nice green theme.

RobPearce · 2023-03-12T17:14:40Z

I noticed 1.4.0 is missing a few files that still exist in 1.3.9 -
usr/share/slim/themes/original/{background.jpg,panel.png,slim.theme.

Is it not the other way round? Upstream has a new default theme for 1.4.0 and those files are the old theme from 1.3.x retained for people who prefer pink to green.

ceamac · 2023-03-12T18:34:41Z

I noticed 1.4.0 is missing a few files that still exist in 1.3.9 -
usr/share/slim/themes/original/{background.jpg,panel.png,slim.theme.

Is it not the other way round? Upstream has a new default theme for 1.4.0 and those files are the old theme from 1.3.x retained for people who prefer pink to green.

Right, I diffed them in reverse and got confused. Thanks for clarifying.

RobPearce · 2023-03-13T20:19:15Z

Hopefully I've addressed all the issues and managed to squash the commits as requested.

gentoo-repo-qa-bot · 2023-03-13T21:14:44Z

Pull request CI report

Report generated at: 2023-03-13 21:14 UTC
Newest commit scanned: 366370d
Status: ✅ good

There are existing issues already. Please look into the report to make sure none of them affect the packages in question:
https://qa-reports.gentoo.org/output/gentoo-ci/d60598921b/output.html

ceamac

Only a few minor issues remain.

The commit message should start with x11-misc/slim: , the prefix got lost while squashing. Also you lost all the Closes: tags, please add them back.

Thank you!

ceamac · 2023-03-14T07:14:29Z

x11-misc/slim/slim-1.3.9.ebuild

+
+EAPI=8
+
+inherit cmake pam systemd readme.gentoo-r1


Please sort inherits alphabetically (except when one eclass needs to be last, not the case here)

ceamac · 2023-03-14T07:15:24Z

x11-misc/slim/slim-1.3.9.ebuild

+	x11-libs/libXmu
+	x11-libs/libX11
+	x11-libs/libXpm
+	pam? ( x11-libs/libXext )


Conditional dependencies go last. There is already a pam? ( ... ) below, please move x11-libs/libXext there.

The rest of the dependencies need to be sorted too, for example x11-libs/libXft comes before x11-libs/libXpm, x11-apps/ before x11-libs/ and so on.

OK, though it raises a separate question, which is a matter of preference, I think:
The pam USE flag currently controls whether slimlock is built. This follows the logic in the upstream CMakeLists from when Nobuhiro-san added it (at V1.3.6). That was, I think, an error - the slimlock code needs PAM and so should not be built without USE_PAM, but it's not required to build it because of PAM. This logic has changed upstream from V1.4.0 but I've retained the old behaviour through the ebuild. It might be considered better to introduce a different (package-specific) USE flag to control it, then encode that this USE flag needs USE=pam first. Of course, that would also be a change of behaviour, so might need a message, too... and I'm not sure it's really necessary.

Do you see any value in building with pam but without slimlock? Does slimlock require any extra dependencies, or does it take a long time to build? If not, no need to add an extra USE flag, tying slimlock to the same knob as pam sounds reasonable.

I don't think there's any real benefit other than "removal of cruft" (since I suspect very few people actually use slimlock). It adds only one dependency (libXext) but from the looks of things that's actually already an indirect dependency (via libXft?) and the build step for slimlock is pretty quick (one C++ file and a link).
So yes, the current approach feels pragmatically sensible.

ceamac · 2023-03-14T07:17:54Z

x11-misc/slim/slim-1.3.9.ebuild

+}
+
+DISABLE_AUTOFORMATTING=1
+DOC_CONTENTS="


Global variables go before functions. Please move them above src_prepare.

Oh, OK. Once again I copied this from an existing ebuild after searching for examples of the readme.gentoo-r1 eclass :(

gentoo-repo-qa-bot · 2023-03-14T08:44:45Z

Pull request CI report

Report generated at: 2023-03-14 08:44 UTC
Newest commit scanned: f3aabf9
Status: ✅ good

There are existing issues already. Please look into the report to make sure none of them affect the packages in question:
https://qa-reports.gentoo.org/output/gentoo-ci/9f4edb754c/output.html

ceamac · 2023-03-14T16:14:54Z

MissingSignOff: commit be9ed1856466, missing sign-off: Robert Pearce <gentoo@flitspace.org.uk>

It appears pkgcheck is unhappy your name in the GCO is different from the one in Author.

Also the ~riscv keyword is missing, it was added to the tree on Feb 28 and your PR is a bit older than that. I can add it while merging, but I wouldn't want to touch your Signed-Off-By, so can you fix that please?

RobPearce · 2023-03-14T18:13:58Z

Also the ~riscv keyword is missing, it was added to the tree on Feb 28 and your PR is a bit older than that. I can add it while merging, but I wouldn't want to touch your Signed-Off-By, so can you fix that please?

It looks like pkgcheck doesn't like ~riscv - should I revert that?

ceamac · 2023-03-14T18:22:09Z

No, it's ok. Your branch is older than the commit where x11-themes/slim-themes was keyworded ~riscv. If you rebase against master, the error goes away.

But the name in the Signed-off-by line is still different from the one in Author.

Fixed issues from pkgcheck List myself as a proxy maintainer Correct several issues pointed out in review by ceamac and added a -9999 ebuild in light of discussion Closes: https://bugs.gentoo.org/832562 Closes: https://bugs.gentoo.org/727544 Closes: https://bugs.gentoo.org/832303 Closes: https://bugs.gentoo.org/580458 Closes: https://bugs.gentoo.org/803476 Closes: https://bugs.gentoo.org/732430 Closes: https://bugs.gentoo.org/756181 Signed-off-by: Robert Pearce <gentoo@flitspace.org.uk>

RobPearce · 2023-03-14T18:25:04Z

But the name in the Signed-off-by line is still different from the one in Author.

Really? I was sure I'd changed it. Try now...

gentoo-repo-qa-bot · 2023-03-14T18:44:37Z

Pull request CI report

Report generated at: 2023-03-14 18:44 UTC
Newest commit scanned: 0387e24
Status: ❌ broken

New issues caused by PR:
https://qa-reports.gentoo.org/output/gentoo-ci/c2fa40de16/output.html#x11-misc/slim

There are existing issues already. Please look into the report to make sure none of them affect the packages in question:
https://qa-reports.gentoo.org/output/gentoo-ci/c2fa40de16/output.html

ceamac

Thank you!

ceamac · 2023-03-14T19:37:53Z

While trying to assign you the remaining bugs in bugzilla, it appears your bugzilla email is different from the one in Author and GCO. Please change it to be the same, so the automated tools work properly.

gentoo@flitspace.org.uk vs bugs.gentoo@flitspace.org.uk

RobPearce · 2023-03-14T20:09:02Z

While trying to assign you the remaining bugs in bugzilla, it appears your bugzilla email is different from the one in Author and GCO. Please change it to be the same, so the automated tools work properly.

OK, will do. I see you found some old bugs I hadn't seen, so I'll look see if there's anything to be done. Although the one raised against a 2017 vintage -9999 ebuild may be tricky to follow up.

ceamac · 2023-03-14T20:30:22Z

Sorry, I meant the email in metadata.xml. Do you want me to change it to your current bugzilla email, or will you change the bugzilla email instead?

RobPearce · 2023-03-14T21:14:44Z

Sorry, I meant the email in metadata.xml

I thought I'd set that to match the bugzilla one. Having "bugs" in the commit author email wouldn't make sense but the docs said the metadata one needs to match the bugzilla login.

ceamac · 2023-03-15T05:22:12Z

Then we're good. Thank you!

gentoo-bot added maintainer-needed There is at least one affected package with no maintainer. Review it if you can. assigned PR successfully assigned to the package maintainer(s). no bug found No Bug/Closes found in the commits. labels Feb 27, 2023

RobPearce force-pushed the slim-fork branch from a989059 to ac84201 Compare February 28, 2023 18:46

RobPearce changed the title ~~x11-misc/slim : Upstream project forked, updated ebuilds~~ x11-misc/slim : Upstream project forked, updated ebuilds [please reassign] Feb 28, 2023

gentoo-bot changed the title ~~x11-misc/slim : Upstream project forked, updated ebuilds [please reassign]~~ x11-misc/slim : Upstream project forked, updated ebuilds Feb 28, 2023

ceamac suggested changes Mar 12, 2023

View reviewed changes

ceamac reviewed Mar 12, 2023

View reviewed changes

RobPearce force-pushed the slim-fork branch from ac84201 to 366370d Compare March 13, 2023 20:17

ceamac suggested changes Mar 14, 2023

View reviewed changes

RobPearce force-pushed the slim-fork branch from 366370d to f3aabf9 Compare March 14, 2023 08:31

RobPearce force-pushed the slim-fork branch from f3aabf9 to 2512871 Compare March 14, 2023 17:51

RobPearce force-pushed the slim-fork branch from 2512871 to 0387e24 Compare March 14, 2023 18:24

gentoo-bot closed this in 33127b6 Mar 14, 2023

ceamac approved these changes Mar 14, 2023

View reviewed changes

RobPearce deleted the slim-fork branch March 15, 2023 20:18

		@@ -0,0 +1,47 @@
		--- a/slim.conf.orig 2010-08-25 11:52:23.000000000 -0400
		+++ b/slim.conf 2010-08-25 11:58:58.000000000 -0400

x11-misc/slim : Upstream project forked, updated ebuilds #29838

x11-misc/slim : Upstream project forked, updated ebuilds #29838

Conversation

RobPearce commented Feb 27, 2023

gentoo-repo-qa-bot commented Feb 27, 2023

Pull request CI report

gentoo-bot commented Feb 28, 2023

Pull Request assignment

Linked bugs

gentoo-repo-qa-bot commented Feb 28, 2023

Pull request CI report

RobPearce commented Mar 4, 2023

ceamac left a comment

Choose a reason for hiding this comment

ceamac Mar 12, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RobPearce Mar 12, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RobPearce Mar 12, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ceamac Mar 12, 2023 • edited

Choose a reason for hiding this comment

RobPearce Mar 12, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ceamac commented Mar 12, 2023

RobPearce commented Mar 12, 2023 • edited

ceamac commented Mar 12, 2023

RobPearce commented Mar 13, 2023

gentoo-repo-qa-bot commented Mar 13, 2023

Pull request CI report

ceamac left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gentoo-repo-qa-bot commented Mar 14, 2023

Pull request CI report

ceamac commented Mar 14, 2023

RobPearce commented Mar 14, 2023

ceamac commented Mar 14, 2023

RobPearce commented Mar 14, 2023

gentoo-repo-qa-bot commented Mar 14, 2023

Pull request CI report

ceamac left a comment

Choose a reason for hiding this comment

ceamac commented Mar 14, 2023

RobPearce commented Mar 14, 2023

ceamac commented Mar 14, 2023

RobPearce commented Mar 14, 2023

ceamac commented Mar 15, 2023

ceamac Mar 12, 2023 •

edited

RobPearce Mar 12, 2023 •

edited

RobPearce Mar 12, 2023 •

edited

ceamac Mar 12, 2023 •

edited

RobPearce Mar 12, 2023 •

edited

RobPearce commented Mar 12, 2023 •

edited