net-p2p/retroshare: Version bump #21725

G10h4ck · 2021-07-20T12:10:28Z

Fixes a bunch of bugs fixed upstream in newer version and disabling
components that are been obsoleted upstream in older versions

Bug: https://bugs.gentoo.org/785964
Bug: https://bugs.gentoo.org/779838
Bug: https://bugs.gentoo.org/798048
Bug: https://bugs.gentoo.org/798099
Package-Manager: Portage-3.0.20, Repoman-3.0.2

Signed-off-by: Gioacchino Mazzurco gio@altermundi.net

gentoo-bot · 2021-07-20T12:12:08Z

Pull Request assignment

Submitter: @G10h4ck
Areas affected: ebuilds
Packages affected: net-p2p/retroshare

net-p2p/retroshare: @G10h4ck, @gentoo/proxy-maint

Linked bugs

Bugs linked: 798048, 798099, 785964, 779838

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 · 2021-07-20T12:29:38Z

Pull request CI report

Report generated at: 2021-07-20 12:29 UTC
Newest commit scanned: 7ecd9de
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/bba661908e/output.html

ionenwks

Be nice if the ebuild could receive some modernizing.

Small review for that if interested in a few suggestions (haven't really looked into building/runtime/bugs side of things).

ionenwks · 2021-07-21T22:27:59Z

net-p2p/retroshare/metadata.xml

+		<flag name="autologin">Enables potentially insecure autologin capability via libsecret</flag>
+		<flag name="cli">Enables terminal login support for retroshare-service</flag>
 		<flag name="control-socket">Enables API via Unix socket support</flag>
 		<flag name="gnome-keyring">Enables potentially insecure autologin capability via Gnome Keyring</flag>
 		<flag name="jsonapi">Enables the new RetroShare JSON API</flag>
+		<flag name="libupnp">Enables UPnP port forwarding via libupnp</flag>
+		<flag name="miniupnp">Enables UPnP port forwarding via miniupnpc</flag>


If going to mention other packages, usually should use, e.g. <pkg>app-crypt/libsecret</pkg>

ionenwks · 2021-07-21T23:16:12Z

net-p2p/retroshare/retroshare-0.6.6.ebuild

+HOMEPAGE="https://retroshare.cc"
+SRC_URI="http://download.opensuse.org/repositories/network:/retroshare/Debian_Testing/retroshare-common_${PV}.orig.tar.gz -> ${P}.tar.gz"
+
+# pegmarkdown can also be used with MIT


What is this about? If something allows picking one of two license, you can use || ( license1 license2 )

ionenwks · 2021-07-21T23:17:28Z

net-p2p/retroshare/retroshare-0.6.6.ebuild

+	libupnp? ( net-libs/libupnp )
+	miniupnp? ( net-libs/miniupnpc )


Both of these define a subslot, unsure how often it changes but usually enough reason to add :=

ionenwks · 2021-07-21T23:18:42Z

net-p2p/retroshare/retroshare-0.6.6.ebuild

+	virtual/pkgconfig
+	jsonapi? ( app-doc/doxygen )"
+
+S="${WORKDIR}"/RetroShare


Just style but be nice to move this right under SRC_URI with no blank line to match skel.ebuild ordering, and for S= usually S="${var}/full quoting" is used (not that I'm saying should do that everywhere, just more of a convention thing in global scope except for EAPI variable).

ionenwks · 2021-07-21T23:19:48Z

net-p2p/retroshare/retroshare-0.6.6.ebuild

+	# CRLF endings break patch...
+	edos2unix retroshare-gui/src/gui/elastic/elnode.h


It's okay to leave like that not to complicate things with two patch versions, but just to say patch could've been converted to contain the right line terminators instead.

ionenwks · 2021-07-21T23:35:57Z

net-p2p/retroshare/retroshare-0.6.6.ebuild

+	done
+}
+
+pkg_pretend() {


Phases should be sorted in execution order, pretend is the first thing executed.
https://devmanual.gentoo.org/ebuild-writing/functions/index.html

Although see next comment.

ionenwks · 2021-07-21T23:37:12Z

net-p2p/retroshare/retroshare-0.6.6.ebuild

+pkg_pretend() {
+	if ! use sqlcipher; then
+		ewarn "You have disabled GXS database encryption, ${PN} will use SQLite"


Given this doesn't abort, there may not be much sense in having this in pretend.

If you wish, you could display this message only when flag is actually toggled. Can be done by checking in pkg_preinst what is currently set on the system, e.g.
if ! use sqlcipher && ! has_version "net-p2p/retroshare[-sqlcipher]"
not to nag every single time if it was disabled.

ionenwks · 2021-07-21T23:38:21Z

net-p2p/retroshare/retroshare-0.6.6.ebuild

+pkg_preinst() {
+	local ver
+	for ver in ${REPLACING_VERSIONS}; do
+		if ver_test ${ver} -lt 0.5.9999; then
+			ewarn "You are upgrading from Retroshare 0.5.* to ${PV}"
+			ewarn "Version 0.6.* is backward-incompatible with 0.5 branch"
+			ewarn "and clients with 0.6.* can not connect to clients that have 0.5.*"
+			ewarn "It's recommended to drop all your configuration and either"
+			ewarn "generate a new certificate or import existing from a backup"
+			break
+		fi
+		if ver_test ${ver} -ge 0.6.0 && ver_test ${ver} -lt 0.6.4; then
+			elog "Main executable has been renamed upstream from RetroShare06 to retroshare"
+			break
+		fi
+	done
+}


Messages normally go in pkg_postinst rather than preinst but it's okay if you find it convenient (notably useful if need to use has_version checks, see sqlcipher comment).

Also, loop isn't wrong per-se but if this package is never going to be slotted then${REPLACING_VERSIONS} will only ever be either an empty string or exactly 1 version. This allows to safely do something like:

if [[ ${REPLACING_VERSIONS} ]]; then if ver_test ${REPLACING_VERSIONS} -lt ...; then

ionenwks · 2021-07-21T23:39:15Z

net-p2p/retroshare/retroshare-0.6.6.ebuild

+pkg_postinst() {
+	xdg_icon_cache_update
+}
+
+pkg_postrm() {
+	xdg_icon_cache_update
+}


Maybe migrate from xdg-utils to xdg.eclass, just need to inherit it, remove this stuff, and ensure exported functions are called if overriding them (except xdg_src_prepare, that one is not important and export considered for removal in eapi-8).
https://devmanual.gentoo.org/eclass-reference/xdg.eclass/index.html

ionenwks · 2021-07-22T00:02:50Z

net-p2p/retroshare/retroshare-0.6.5-r3.ebuild

@@ -0,0 +1,143 @@
+# Copyright 1999-2021 Gentoo Authors


I see updating the old ebuild too, is there a reason to? i.e. are you planning to stabilize -r3 over 0.6.6 or something? Or is there a problem with 0.6.6 that will make users want to keep using 0.6.5?

I don't use this, so I wouldn't know.

I plan to stabilize 0.6.6 but there might be problems with new version so I also wanted to do a minimal maintenance of 0.6.5 in case someone need to stick to that retroshare version

G10h4ck · 2021-08-04T16:00:08Z

Thanks for suggestions @ionenwks I have updated 0.6.6 ebuild according to them, can this be merged now?

gentoo-repo-qa-bot · 2021-08-04T16:09:50Z

Pull request CI report

Report generated at: 2021-08-04 16:09 UTC
Newest commit scanned: ca7505c
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/4e938b8069/output.html

ionenwks

Thanks for suggestions @ionenwks I have updated 0.6.6 ebuild according to them, can this be merged now?

Thanks, changes look good but still a few things.

Keeping 0.6.5-r3 is fine but I forgot to mention that it should be added in a separate commit. Version bumps are normally done in their own commit to allow easy reversal and could have a message for changes to specific to 0.6.5.

May also want to change Bug: tags to Closes:, if it's fixed it can be closed (unless you prefer to wait for stabilization to do that, up to you how to handle your bugs -- personally tend to close when fixed in ~arch unless it's affecting many users that may need to find the bug).

But more importantly, this time I tried to build and seem to get getting build issues with gcc11 for 0.6.6 with any USE combinations:

util/rsdir.cc:280:38: error: 'create_directories' is not a member of 'std::filesystem'
  280 |                 if(!std::filesystem::create_directories(dest_dir))

Builds fine with append-cxxflags -std=c++14 (that's because gcc11 defaults to c++17), but that's more of a temporary workaround than a fix, preferable would be to patch rsdir.cc to #include <filesystem> and send that upstream (if haven't yet, do check the clean patch howto).

Also, noticed a few other minor things.

ionenwks · 2021-08-05T18:12:55Z

net-p2p/retroshare/retroshare-0.6.6.ebuild

+	fi
+}
+
+pkg_preinst() {


No longer need to define the postinst/postrm with xdg.eclass, but it has a preinst and being overwritten here.

Suggested change

pkg_preinst() {

pkg_preinst() {

xdg_pkg_preinst

Or else it leads to:

* QA Notice: new icons were found installed but icon cache * has not been updated: * /usr/share/icons/hicolor/64x64/apps/retroshare.png

ionenwks · 2021-08-05T18:14:14Z

net-p2p/retroshare/retroshare-0.6.6.ebuild

+	fi
+
+	if [[ ${REPLACING_VERSIONS} ]]; then
+		if ver_test ${REPLACING_VERSIONS} -lt 0.5.9999; then


Could make this -lt 0.6

Fixes a bunch of bugs fixed upstream in newer version and disabling components that are been obsoleted upstream in older versions Modernize 0.6.6 ebuild after ionenwks suggestions Bug: https://bugs.gentoo.org/785964 Bug: https://bugs.gentoo.org/779838 Bug: https://bugs.gentoo.org/798048 Bug: https://bugs.gentoo.org/798099 Package-Manager: Portage-3.0.20, Repoman-3.0.2 Signed-off-by: Gioacchino Mazzurco <gio@altermundi.net>

Fixes a bunch of bugs disabling components that are been obsoleted upstream in older versions Bug: https://bugs.gentoo.org/779838 Bug: https://bugs.gentoo.org/798048 Bug: https://bugs.gentoo.org/798099 Package-Manager: Portage-3.0.20, Repoman-3.0.2 Signed-off-by: Gioacchino Mazzurco <gio@altermundi.net>

gentoo-repo-qa-bot · 2021-08-06T15:44:43Z

Pull request CI report

Report generated at: 2021-08-06 15:44 UTC
Newest commit scanned: 87bd077
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/3fd20b99a2/output.html

G10h4ck · 2021-08-06T15:46:06Z

Thanks for the careful review @ionenwks all your remarks should be addressed ;)
Is it ok to merge this now?

ionenwks

Thanks for the careful review @ionenwks all your remarks should be addressed ;)
Is it ok to merge this now?

Indeed, looks good. I see cleaned the edos2unix too, nice -- and hadn't realized you were the upstream, guess I didn't need to say anything about upstreaming patches :) Thanks for your work.

May also want to change Bug: tags to Closes:, if it's fixed it can be closed (unless you prefer to wait for stabilization to do that, [...]

Haven't heard back of your preference on this point, assume it was overlooked considering there's no reason not to close the version bump one at least. I'll go ahead and switch to Closes: myself where relevant so this doesn't have to wait longer.

G10h4ck · 2021-08-07T07:29:04Z

Thanks!

gentoo-bot added self-maintained The PR changes only packages that are maintained by the submitter (i.e. no need to ask anybody else) assigned PR successfully assigned to the package maintainer(s). bug linked Bug/Closes found in footer, and cross-linked with the PR. labels Jul 20, 2021

ionenwks reviewed Jul 22, 2021

View reviewed changes

G10h4ck force-pushed the net-p2p/retroshare branch from 7ecd9de to ca7505c Compare August 4, 2021 15:58

ionenwks reviewed Aug 5, 2021

View reviewed changes

G10h4ck added 2 commits August 6, 2021 17:03

G10h4ck force-pushed the net-p2p/retroshare branch from ca7505c to 87bd077 Compare August 6, 2021 15:32

ionenwks approved these changes Aug 7, 2021

View reviewed changes

gentoo-bot closed this in ecb9cc6 Aug 7, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

net-p2p/retroshare: Version bump #21725

net-p2p/retroshare: Version bump #21725

G10h4ck commented Jul 20, 2021

gentoo-bot commented Jul 20, 2021

gentoo-repo-qa-bot commented Jul 20, 2021

ionenwks left a comment

ionenwks Jul 21, 2021

ionenwks Jul 21, 2021

ionenwks Jul 21, 2021

ionenwks Jul 21, 2021

ionenwks Jul 21, 2021

ionenwks Jul 21, 2021

ionenwks Jul 21, 2021

ionenwks Jul 21, 2021

ionenwks Jul 21, 2021

ionenwks Jul 22, 2021

G10h4ck Aug 4, 2021

G10h4ck commented Aug 4, 2021

gentoo-repo-qa-bot commented Aug 4, 2021

ionenwks left a comment

ionenwks Aug 5, 2021

ionenwks Aug 5, 2021

gentoo-repo-qa-bot commented Aug 6, 2021

G10h4ck commented Aug 6, 2021

ionenwks left a comment

G10h4ck commented Aug 7, 2021

		libupnp? ( net-libs/libupnp )
		miniupnp? ( net-libs/miniupnpc )

		# CRLF endings break patch...
		edos2unix retroshare-gui/src/gui/elastic/elnode.h

net-p2p/retroshare: Version bump #21725

net-p2p/retroshare: Version bump #21725

Conversation

G10h4ck commented Jul 20, 2021

gentoo-bot commented Jul 20, 2021

Pull Request assignment

Linked bugs

gentoo-repo-qa-bot commented Jul 20, 2021

Pull request CI report

ionenwks 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

G10h4ck commented Aug 4, 2021

gentoo-repo-qa-bot commented Aug 4, 2021

Pull request CI report

ionenwks left a 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 Aug 6, 2021

Pull request CI report

G10h4ck commented Aug 6, 2021

ionenwks left a comment

Choose a reason for hiding this comment

G10h4ck commented Aug 7, 2021