Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

net-nntp/suck: Add version 4.3.3 #7185

Closed
wants to merge 2 commits into from
Closed

net-nntp/suck: Add version 4.3.3 #7185

wants to merge 2 commits into from

Conversation

jubalh
Copy link
Contributor

@jubalh jubalh commented Feb 14, 2018

Package-Manager: Portage-2.3.19, Repoman-2.3.6

@gentoo-repo-qa-bot gentoo-repo-qa-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 14, 2018
Copy link
Member

@a17r a17r left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your first contribution!

-e 's:/var/lib/news/history:/var/spool/news/db/history:' \
suck_config.h || die "path adaption sed failed"

default
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would recommend to run default as the first thing in src_prepare, so user patches applied in default can not be affected by the sed.

src_prepare() {
# Fix paths to the locations in Gentoo
sed -i \
-e 's:/usr/bin/rnews:/usr/lib/news/bin/rnews:' \
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this using rnews from net-nntp/inn? That package seems to install the binary to /usr/$(get_libdir)/news/$(get_libdir), so the sed should do the same (hardcoded lib is almost always wrong). net-nntp/inn is probably missing in RDEPEND as lpost seems useless without it. This would introduce a runtime-only dependency, usually if there are both DEPEND- and RDEPEND-only deps we use a helper var like COMMON_DEPEND that both DEPEND and RDEPEND can inherit from.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok! :) Will change this. However I suppose you had a typo in: /usr/$(get_libdir)/news/$(get_libdir) and you mean: /usr/$(get_libdir)/news/bin/rnews ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes that must have been a copy-paste error, I just looked at inn ebuild without having it installed


DEPEND="sys-libs/db
perl? ( dev-lang/perl )
ssl? ( dev-libs/openssl )"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At least openssl is linked to, so this needs to go into RDEPEND (and DEPEND can inherit RDEPEND not to duplicate the line). I also saw sys-libs/gdbm.5 be linked against, so same there. Both dependencies need to trigger rebuild of suck on subslot upgrade, in case of openssl append :0= (another slot exists there but we need 0), for gdbm append := to achieve that.

LICENSE="public-domain"
SLOT="0"
KEYWORDS="~amd64 ~ppc ~x86"
IUSE="ssl perl"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minor nitpick, but in general we keep things sorted, which becomes more important with growing lists of IUSE and DEPENDs.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, what do you mean by sorted here? Listing ssl after perl to have alphabetically ordered?

IUSE="ssl perl"

DEPEND="sys-libs/db
perl? ( dev-lang/perl )
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The perl flag does not seem to do much - part of the problem is that configure.ac is searching for libperl.a, which does not exist in Gentoo, but even if I change it to libperl.so there is no difference in the install image. It does not set a build switch either. Do you have an idea?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure how it was in suck 4.3.2, probably it had some effect there. In 4.3.3 maybe it can be omitted, I'm still learning about the package. I would let it like this for now and try to investigate later on more, would that be ok?

@jubalh jubalh changed the title net-nntp/suck: Add version 4.3.3 net-nntp/suck: Add version 4.3.3 [please reassign] Feb 15, 2018
@gentoo-repo-qa-bot gentoo-repo-qa-bot changed the title net-nntp/suck: Add version 4.3.3 [please reassign] net-nntp/suck: Add version 4.3.3 Feb 15, 2018
@gentoo-repo-qa-bot
Copy link
Collaborator

Pull Request assignment

Areas affected: ebuilds
Packages affected: net-nntp/suck

net-nntp/suck: @gentoo/proxy-maint (maintainer needed)

Bugs linked: 622880, 232581

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

@gentoo-repo-qa-bot gentoo-repo-qa-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. and removed assigned PR successfully assigned to the package maintainer(s). maintainer-needed There is at least one affected package with no maintainer. Review it if you can. no bug found No Bug/Closes found in the commits. labels Feb 15, 2018
@gentoo-repo-qa-bot
Copy link
Collaborator

Pull request CI report

Report generated at: 2018-02-15 13:17 UTC
Newest commit scanned: 7d24b6c
Status: ✅ good

Issues already there before the PR (double-check them):
https://qa-reports.gentoo.org/output/gentoo-ci/03b24d434/output.html#app-emulation/qemu

NeddySeagoon pushed a commit to NeddySeagoon/gentoo-arm64 that referenced this pull request Feb 17, 2018
Closes: https://bugs.gentoo.org/232581
Closes: https://bugs.gentoo.org/622880
Package-Manager: Portage-2.3.19, Repoman-2.3.6
Closes: gentoo#7185
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
assigned PR successfully assigned to the package maintainer(s). bug linked Bug/Closes found in footer, and cross-linked with the PR. maintainer-needed There is at least one affected package with no maintainer. Review it if you can.
Projects
None yet
3 participants