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-libs/libsrsirc: New package #4355

Closed
wants to merge 4 commits into from
Closed

Conversation

Learath2
Copy link
Contributor

@Learath2 Learath2 commented Apr 4, 2017

A lightweight, cross-platform IRC library
Written in portable standard C (C99)

Gentoo-bug: 602354

Package-Manager: Portage-2.3.3, Repoman-2.3.1

A lightweight, cross-platform IRC library
Written in portable standard C (C99)

Gentoo-bug: 602354

Package-Manager: Portage-2.3.3, Repoman-2.3.1
@gentoo-repo-qa-bot gentoo-repo-qa-bot added new package The PR is adding a new package. assigned PR successfully assigned to the package maintainer(s). labels Apr 4, 2017
@gentoo-repo-qa-bot
Copy link
Collaborator

Pull Request assignment

Areas affected: ebuilds
Packages affected: net-libs/libsrsirc

net-libs/libsrsirc: @gentoo/proxy-maint (new package)

@gktrk gktrk self-requested a review April 4, 2017 17:48
Copy link
Member

@gktrk gktrk left a comment

Choose a reason for hiding this comment

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

Can you also add a static-libs USE flag, pass --disable-static to econf unless that USE flag is enabled, remove the .la file in src_install using the command find "${D}" -name '*.la' -delete || die as explained here: https://wiki.gentoo.org/wiki/Project:Quality_Assurance/Handling_Libtool_Archives

If this is just a library, why is it installing icat and iwat in /usr/bin?

@@ -0,0 +1,24 @@
# Copyright 1999-2017 Gentoo Foundation
# Distributed under the terms of the GNU General Public License v2
# $Id$
Copy link
Member

Choose a reason for hiding this comment

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

The Id line should be removed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This looks like something repoman could check.


EAPI=6

inherit eutils
Copy link
Member

Choose a reason for hiding this comment

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

No functionality from eutils is used, so it should be removed as well

@Learath2
Copy link
Contributor Author

Learath2 commented Apr 4, 2017

iwat is an example application, icat is netcat for IRC, they both come with the library. I initially wanted to make them a use flag but there is no configure option to disable building them and I didn't know what the correct way to remove them would be.

Package-Manager: Portage-2.3.3, Repoman-2.3.1
Package-Manager: Portage-2.3.3, Repoman-2.3.1
@gktrk
Copy link
Member

gktrk commented Apr 4, 2017

You don't have to remove them, I just wondered.


src_configure() {
use static-libs && econf $(use_with ssl)
! use static-libs && econf --disable-static $(use_with ssl)
Copy link
Member

Choose a reason for hiding this comment

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

You have a typo here

Copy link
Member

Choose a reason for hiding this comment

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

use_enable should just work here, no?

Copy link
Member

Choose a reason for hiding this comment

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

Sorry, let me clarify. You can do (not tested):

src_configure() {
    econf $(use_with ssl) $(use_enable static-libs static)
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That would do --enable-static in case static-libs is set. Which is slightly different to what you asked for.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Now that I think about it that's probably the correct way.

Package-Manager: Portage-2.3.3, Repoman-2.3.1
@gktrk gktrk self-assigned this Apr 5, 2017
@gktrk
Copy link
Member

gktrk commented Apr 5, 2017

Merged in 564c213...1d9a135. Thanks!

I've squashed your commits prior to merging and made some minor visual changes.

@gktrk gktrk closed this Apr 5, 2017
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). new package The PR is adding a new package.
Projects
None yet
3 participants