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/libsrtp: Bump to 1.5.4 and 2.0.0 #2689

Closed
wants to merge 1 commit into from

Conversation

lluixhi
Copy link
Contributor

@lluixhi lluixhi commented Oct 28, 2016

Add LibreSSL Support
Switch to EAPI 6
Cleanup of src_prepare

Gentoo-Bug: 596398

Question:
What should I do with openssl-kdf? It requires OpenSSL-1.1.0 and doesn't work with LibreSSL

@lluixhi lluixhi force-pushed the master branch 3 times, most recently from 808f63d to 25f109d Compare October 28, 2016 21:16
@lluixhi
Copy link
Contributor Author

lluixhi commented Oct 28, 2016

This'll necessitate changing some slots on packages.
As far as I can tell, qtwebengine needs libsrtp:0= while the newer versions of chromium (55+?) need libsrtp:2=

I'm not sure about anything else.

# using test/rtpw.c guaratees the file exists in any case
sed -i -e "s:/usr/share/dict/words:rtpw.c:" test/rtpw.c || die

default
Copy link
Member

Choose a reason for hiding this comment

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

call default first thing in src_prepare

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Alright.

KEYWORDS="~alpha ~amd64 ~arm ~hppa ~ia64 ~ppc ~ppc64 -sparc ~x86 ~x86-fbsd ~ppc-macos ~x64-macos ~x86-macos"
IUSE="aesicm console debug doc libressl openssl static-libs syslog test"

REQUIRED_USE="libressl? ( openssl )"
Copy link
Member

Choose a reason for hiding this comment

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

don't need this

KEYWORDS="~alpha ~amd64 ~arm ~hppa ~ia64 ~ppc ~ppc64 -sparc ~x86 ~x86-fbsd ~ppc-macos ~x64-macos ~x86-macos"
IUSE="aesicm console debug doc libressl openssl static-libs syslog test"

REQUIRED_USE="libressl? ( openssl )"
Copy link
Member

Choose a reason for hiding this comment

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

get rid of this. USE="libressl" = "use libressl, if and only if, building with SSL support"

Copy link
Contributor Author

@lluixhi lluixhi Oct 30, 2016

Choose a reason for hiding this comment

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

This ebuild depends on libcrypto -- which is provided by both OpenSSL and LibreSSL -- what would an alternative be?

I don't think we should restrict the benefits of using Open/LibreSSL crypto functions to only OpenSSL users.

Copy link
Member

@SoapGentoo SoapGentoo Nov 2, 2016

Choose a reason for hiding this comment

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

if the ebuild unconditionally needs openssl, drop the openssl flag. I have yet to see an ebuild that requires REQUIRED_USE="libressl? ( openssl )", because the libressl and openssl USE flags are orthogonal to each other. USE="libressl" does NOT imply "build with libressl unconditionally", rather it means, if you are building with SSL, then use libressl.

Copy link
Contributor Author

@lluixhi lluixhi Nov 2, 2016

Choose a reason for hiding this comment

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

media-video/ffmpeg has a similar problem to what I'm trying to avoid in this ebuild. It should actually have a REQUIRED_USE="libressl? ( openssl )", because if you don't enable both, the libressl USE flag does nothing.

This ebuild does not unconditionally depend on OpenSSL, it has an optional dependency on OpenSSL to accelerate their crypto functions.

Alternatively, I can modify the line $(use_enable openssl) to pass '--enable-openssl' to configure regardless of whether we are using OpenSSL or LibreSSL.

I think I'll do that.

# using test/rtpw.c guaratees the file exists in any case
sed -i -e "s:/usr/share/dict/words:rtpw.c:" test/rtpw.c || die

default
Copy link
Member

Choose a reason for hiding this comment

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

same thing here

@lluixhi
Copy link
Contributor Author

lluixhi commented Oct 30, 2016

Re openssl-kdf:
We can depend on Openssl-1.1.0+ after upstream's next release.
It doesn't work yet:
cisco/libsrtp@0b45423

@gktrk
Copy link
Member

gktrk commented Nov 1, 2016

@gentoochainsaw

@gktrk gktrk added bugfix assigned PR successfully assigned to the package maintainer(s). labels Nov 1, 2016
@lluixhi lluixhi force-pushed the master branch 2 times, most recently from fd4352e to b4db83a Compare November 2, 2016 20:36
DEPEND="
openssl? ( dev-libs/openssl:0= )
libressl? ( dev-libs/libressl:0= )
"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay this still seems wrong.
@blueness do you have any advice?

Copy link
Member

Choose a reason for hiding this comment

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

@lluixhi I will repeat it one last time. USE="libressl" does NOT mean "use libressl". What USE="libressl" means, is "use libressl, if and only if, building against an openssl backend". If you do not follow this line of reasoning, see the project page https://wiki.gentoo.org/wiki/Project:LibreSSL which just reiterates all of this. The correct solution is:

openssl? (
    !libressl? ( dev-libs/openssl:0= )
    libressl? ( dev-libs/libressl:0= )
)

i.e. USE="openssl libressl" -> libressl, USE="openssl -libressl" -> openssl, USE="-openssl" -> no SSL. If you won't change it to this, I will close the PR for not being in line with the libressl project guidelines.

Copy link
Contributor Author

@lluixhi lluixhi Nov 2, 2016

Choose a reason for hiding this comment

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

@SoapGentoo
That page only shows the

ssl? (
    !libressl? ( dev-libs/openssl:0= )
    libressl? ( dev-libs/libressl:0= )
)

construct

Anyway, my understanding from that page and what @hasufell said in the past was that if a user had USE="libressl ssl" set globally, it should enable working libressl support on all packages that support an openssl-like backend without also including USE="openssl".

From the page:
"For most packages, migration is as simple as just replacing the openssl atom with a choice between openssl and libressl"
"Eventually the process should be as easy as just adding USE=libressl to your make.conf file and then doing an emerge -uvNDq world"

It is unclear to me that USE="openssl libressl" is necessary to use an openssl-like backend with libressl, but I'm changing it to what you have.

libressl? ( dev-libs/libressl:0= )
)
"
RDEPEND="${DEPEND}"
Copy link
Member

@SoapGentoo SoapGentoo Nov 3, 2016

Choose a reason for hiding this comment

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

while this is formally correct, could you still swap RDEPEND and DEPEND? It's nicer reading deps with := in RDEPEND, as subslot operators have no effect in DEPEND.

Add LibreSSL Support
Switch to EAPI 6
Cleanup of src_prepare

Gentoo-Bug: 596398
@lluixhi
Copy link
Contributor Author

lluixhi commented Nov 14, 2016

As of 5842294
we do have a precedent for putting crypto implementations into REQUIRED_USE

@SoapGentoo
Copy link
Member

@lluixhi thanks for pointing out, that || ( gcrypt libressl nettle openssl ) is wrong. The openssl USE flag doesn't actually control anything. Stay with how I said it and how the LibreSSL Project page documents it.

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).
Projects
None yet
3 participants