x11-plugins/gkrellm-leds: update EAPI 6 -> 8 #24729

laumann · 2022-03-23T19:06:30Z

Besides the EAPI bump, there's a bug for the referenced bug. The issue
seems to be simply a typo in src/Makefile.am resulting in a variable
@GK_LDFLAGS@ not being replaced appropriately.

Closes: https://bugs.gentoo.org/799176
Signed-off-by: Thomas Bracht Laumann Jespersen t@laumann.xyz

TODO

ebuild install -r1 and -r2 and compare the image folders

gentoo-bot · 2022-03-23T19:08:05Z

Pull Request assignment

Submitter: @laumann
Areas affected: ebuilds
Packages affected: x11-plugins/gkrellm-leds

x11-plugins/gkrellm-leds: @gentoo/proxy-maint (maintainer needed)

Linked bugs

Bugs linked: 799176

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

laumann · 2022-03-23T19:14:54Z

x11-plugins/gkrellm-leds/gkrellm-leds-0.8.2-r2.ebuild

+
+S="${WORKDIR}/${MY_P}"
+
+PATCHES=( "${FILESDIR}/${P}-r2-ldflags-typo.patch" )


I'm not entirely sure which variables I should use in reference to the patch. My understanding so far is that one should refrain from ${PF}, but the PMS lists several others (including ${P} here) that aren't "consistent".

I think it's "not consistent" in the sense that "it might be different in e.g. pkg_postrm", but I need to check. Today has been quite long and not been really at computer much.

It's safe to use it in PATCHES (but don't use ${PF} for the reasons @ionenwks mentioned in #gentoo-dev earlier; we generally anticipate we can revbump packages en-masse without affecting their installability. Using ${PF} harms that).

Typically:

Patch will likely keep across versions: ${PN}-0.0.0-name.patch (i.e. long standing issues, or Gentoo specific patches)

Patch that should be either reviewed or dropped next version: ${P}-name.patch (i.e may be fixed / upstreamed, if not, can change to ${PN} on bump -- Edit: I tend to prefer to use ${P} at first and change later like that, but depends)

And yeah, ${PF} is just different given it includes revision making it too volatile over nothing. It shouldn't be thrown around casually and revbumps with no changes preferably shouldn't be able to break things, normally only used for docs dir.

OK, seems like I was right to change ${PF} to ${P} here. I included -r2 in the patch name, because I wanted to reflect that it's been created for that revision. But it should probably also apply to any potential future revisions of this version.

Thanks for the input!

gentoo-repo-qa-bot · 2022-03-23T19:21:31Z

Pull request CI report

Report generated at: 2022-03-23 19:21 UTC
Newest commit scanned: 6bb4871
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/784ee7b755/output.html

laumann · 2022-03-27T21:34:55Z

@thesamesam ping on this one. I pushed some minor edits to the ebuild.

gentoo-repo-qa-bot · 2022-03-27T21:48:48Z

Pull request CI report

Report generated at: 2022-03-27 21:48 UTC
Newest commit scanned: ddd5b54
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/91c54e483c/output.html

gentoo-repo-qa-bot · 2022-04-05T07:42:21Z

Pull request CI report

Report generated at: 2022-04-05 07:42 UTC
Newest commit scanned: 1e999d7
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/d1921fe19f/output.html

gentoo-repo-qa-bot · 2022-04-05T07:57:07Z

Pull request CI report

Report generated at: 2022-04-05 07:57 UTC
Newest commit scanned: f42bc87
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/2488af4497/output.html

SoapGentoo · 2022-04-05T09:06:44Z

x11-plugins/gkrellm-leds/files/gkrellm-leds-0.8.2-r2-ldflags-typo.patch

+
+ ## linker flags, we are building a plugin so skip lib prefix and lib versions
+-gkleds_la_LDFLAGS = -module -avoid-version @GK_LDFLAGS@
+gkleds_la_LDFLAGS = -module -avoid-version @GKM_LDFLAGS@


in general, in variables you shouldn't reference @VAR@ but $(VAR) because AC_SUBST already creates a separate VAR = @VAR@ somewhere in Makefile.in.

OK, I wanted to go for the smallest change that would fix the problem. Do you want me make this change?

no, it's just something to be aware of if you're fixing build systems

Alright, thanks! 😄 I appreciate it.

SoapGentoo · 2022-04-05T09:07:27Z

x11-plugins/gkrellm-leds/gkrellm-leds-0.8.2-r2.ebuild

+}
+
+src_configure() {
+	PLUGIN_SO=( src/.libs/gkleds$(get_modname) )


is this really needed?

This should be the name of the plugin's .so file. The default is ${PN}$(get_modname), so I think it's needed, yes.

FWIW, referencing .libs/ tends to break with slibtool

What's the solution to that? Move the file after it's been built?

Do we actually need to set it at all in configure? Can't we just set it to the right name on install?

Besides the EAPI bump, there's a bug for the referenced bug. The issue seems to be simply a typo in src/Makefile.am resulting in a variable @GK_LDFLAGS@ not being replaced appropriately. Closes: https://bugs.gentoo.org/799176 Signed-off-by: Thomas Bracht Laumann Jespersen <t@laumann.xyz>

gentoo-repo-qa-bot · 2022-04-05T19:57:10Z

Pull request CI report

Report generated at: 2022-04-05 19:57 UTC
Newest commit scanned: e9b8c3f
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/4a84f30b2d/output.html

laumann force-pushed the eapi8/x11-plugins/gkrellm-leds branch from f114b27 to 6bb4871 Compare March 23, 2022 19:07

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). bug linked Bug/Closes found in footer, and cross-linked with the PR. labels Mar 23, 2022

laumann commented Mar 23, 2022

View reviewed changes

laumann force-pushed the eapi8/x11-plugins/gkrellm-leds branch from 6bb4871 to ddd5b54 Compare March 27, 2022 21:33

laumann force-pushed the eapi8/x11-plugins/gkrellm-leds branch 2 times, most recently from 1e999d7 to f42bc87 Compare April 5, 2022 07:30

SoapGentoo reviewed Apr 5, 2022

View reviewed changes

laumann force-pushed the eapi8/x11-plugins/gkrellm-leds branch from f42bc87 to e9b8c3f Compare April 5, 2022 19:09

gentoo-bot closed this in 1d557df Apr 7, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

x11-plugins/gkrellm-leds: update EAPI 6 -> 8 #24729

x11-plugins/gkrellm-leds: update EAPI 6 -> 8 #24729

laumann commented Mar 23, 2022 •

edited

gentoo-bot commented Mar 23, 2022

laumann Mar 23, 2022

thesamesam Mar 24, 2022 •

edited

ionenwks Mar 24, 2022 •

edited

laumann Mar 24, 2022

gentoo-repo-qa-bot commented Mar 23, 2022

laumann commented Mar 27, 2022

gentoo-repo-qa-bot commented Mar 27, 2022

gentoo-repo-qa-bot commented Apr 5, 2022

gentoo-repo-qa-bot commented Apr 5, 2022

SoapGentoo Apr 5, 2022

laumann Apr 5, 2022

SoapGentoo Apr 5, 2022

laumann Apr 5, 2022

SoapGentoo Apr 5, 2022

laumann Apr 5, 2022

thesamesam Apr 7, 2022

laumann Apr 7, 2022

thesamesam Apr 7, 2022 •

edited

gentoo-repo-qa-bot commented Apr 5, 2022


		S="${WORKDIR}/${MY_P}"

		PATCHES=( "${FILESDIR}/${P}-r2-ldflags-typo.patch" )

x11-plugins/gkrellm-leds: update EAPI 6 -> 8 #24729

x11-plugins/gkrellm-leds: update EAPI 6 -> 8 #24729

Conversation

laumann commented Mar 23, 2022 • edited

gentoo-bot commented Mar 23, 2022

Pull Request assignment

Linked bugs

Choose a reason for hiding this comment

thesamesam Mar 24, 2022 • edited

Choose a reason for hiding this comment

ionenwks Mar 24, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gentoo-repo-qa-bot commented Mar 23, 2022

Pull request CI report

laumann commented Mar 27, 2022

gentoo-repo-qa-bot commented Mar 27, 2022

Pull request CI report

gentoo-repo-qa-bot commented Apr 5, 2022

Pull request CI report

gentoo-repo-qa-bot commented Apr 5, 2022

Pull request CI report

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

thesamesam Apr 7, 2022 • edited

Choose a reason for hiding this comment

gentoo-repo-qa-bot commented Apr 5, 2022

Pull request CI report

laumann commented Mar 23, 2022 •

edited

thesamesam Mar 24, 2022 •

edited

ionenwks Mar 24, 2022 •

edited

thesamesam Apr 7, 2022 •

edited