[gentoo-dev] Re: [gentoo-commits] repo/gentoo:master commit in: net-libs/libssh2/

[gentoo-dev] Re: [gentoo-commits] repo/gentoo:master commit in: net-libs/libssh2/

[Jeroen Roovers]

On Mon, 01 Oct 2018 17:00:24 +0200
Michał Górny <mgorny@gentoo.org> wrote:

> So your src_compile() is now broken for Ninja users, and src_test()
> is broken altogether.  How about cmake-multilib eclass?

DROP

> IUSE=test is not used at all.

THAT

> By the way, MULTILIB_USEDEP certainly missing here.

BLOODY

> You sure this autoconf patch is needed for CMake build?

AWFUL

> They must have hacked that CMake hard to get it to generate .la files.

ATTITUTE

Re: [gentoo-dev] Re: [gentoo-commits] repo/gentoo:master commit in: net-libs/libssh2/

[Michał Górny]

[-- Attachment #1: Type: text/plain, Size: 993 bytes --]

On Mon, 2018-10-01 at 21:30 +0200, Jeroen Roovers wrote:
> On Mon, 01 Oct 2018 17:00:24 +0200
> Michał Górny <mgorny@gentoo.org> wrote:
> 
> > So your src_compile() is now broken for Ninja users, and src_test()
> > is broken altogether.  How about cmake-multilib eclass?
> 
> DROP
> 
> > IUSE=test is not used at all.
> 
> THAT
> 
> > By the way, MULTILIB_USEDEP certainly missing here.
> 
> BLOODY
> 
> > You sure this autoconf patch is needed for CMake build?
> 
> AWFUL
> 
> > They must have hacked that CMake hard to get it to generate .la files.
> 
> ATTITUTE

Then please kindly improve the quality of your Gentoo work.  Ebuild
writing is more than copying files and doing the minimal change
necessary for it not to explode in your face.

Furthermore, I don't really understand why you're CC-ing netmon@ here. 
Given you've cut off all the context, my suggestions would seem rather
meaningless to the other member.

-- 
Best regards,
Michał Górny

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 963 bytes --]

Re: [gentoo-dev] Re: [gentoo-commits] repo/gentoo:master commit in: net-libs/libssh2/

[Michał Górny]

[-- Attachment #1: Type: text/plain, Size: 964 bytes --]

On Mon, 2018-10-01 at 21:30 +0200, Jeroen Roovers wrote:
> On Mon, 01 Oct 2018 17:00:24 +0200
> Michał Górny <mgorny@gentoo.org> wrote:
> 
> > So your src_compile() is now broken for Ninja users, and src_test()
> > is broken altogether.  How about cmake-multilib eclass?
> 
> DROP
> 
> > IUSE=test is not used at all.
> 
> THAT
> 
> > By the way, MULTILIB_USEDEP certainly missing here.
> 
> BLOODY
> 
> > You sure this autoconf patch is needed for CMake build?
> 
> AWFUL
> 
> > They must have hacked that CMake hard to get it to generate .la files.
> 
> ATTITUTE

Since I've been approached by developers asking me about the original
mail missing in the archives, I would like to note that I didn't send it
to this mailing list.  My purpose was to ask him to improve the ebuild
and not publicly point out his mistakes.

The choice of posting a reply to gentoo-dev was entirely his decision.

-- 
Best regards,
Michał Górny

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 963 bytes --]

[gentoo-dev] Re: [gentoo-commits] repo/gentoo:master commit in: net-libs/libssh2/

[Jeroen Roovers]

OK, let's do a full review of your review.


On Mon, 01 Oct 2018 17:00:24 +0200
Michał Górny <mgorny@gentoo.org> wrote:

> On Mon, 2018-10-01 at 11:46 +0000, Jeroen Roovers wrote:
> > commit:     d866d4705e1e4a092579a31df2815e3407950a19
> > Author:     Jeroen Roovers <jer <AT> gentoo <DOT> org>
> > AuthorDate: Mon Oct  1 11:45:43 2018 +0000
> > Commit:     Jeroen Roovers <jer <AT> gentoo <DOT> org>
> > CommitDate: Mon Oct  1 11:46:10 2018 +0000
> > URL:
> > https://gitweb.gentoo.org/repo/gentoo.git/commit/?id=d866d470
> > 
> > net-libs/libssh2: Add USE=mbedtls, switch to cmake for building
> > 
> > * Add support for net-libs/mbedtls
> > * Switch to cmake as the autotools build is even more broken
> > * Remove USE=static-libs as that inhibits building shared libs
> > * Use REQUIRED_USE to force choosing a crypto backend

You completely skipped over the improvements. In effect, you show
yourself to be completely unresponsive to what were considered positive
changes to the author of the work.

Then you begin to pick apart what you think is wrong. It's not obvious
why you are doing it this way, and with regard to practically all
of your earlier e-mails addressed to me, I can only assume malice.

> So your src_compile() is now broken for Ninja users, and src_test()
> is broken altogether.  How about cmake-multilib eclass?

ninja is a thing? Can you explain what it is? Can you show me how it's
broken, and how I reproduce the ninja problem, pro forma by filing a bug
report at https://bugs.gentoo.org/

> > +DESCRIPTION="Library implementing the SSH2 protocol"
> > +HOMEPAGE="https://www.libssh2.org"
> > +SRC_URI="https://www.${PN}.org/download/${P}.tar.gz"
> > +
> > +LICENSE="BSD"
> > +SLOT="0"
> > +KEYWORDS="~alpha ~amd64 ~arm ~arm64 ~hppa ~ia64 ~mips ~ppc ~ppc64
> > ~s390 ~sh ~sparc ~x86 ~amd64-fbsd ~x86-fbsd ~amd64-linux ~x86-linux
> > ~ppc-macos ~x64-macos ~x86-solaris" +IUSE="gcrypt libressl mbedtls
> > test zlib"  
> 
> IUSE=test is not used at all.

So that's a minor problem we could fix even without a revision bump.
You could have pointed it out in that missing bug report.

> > +REQUIRED_USE="
> > +	?? ( gcrypt libressl mbedtls )
> > +"
> > +
> > +RDEPEND="
> > +	!libressl?
> > ( >=dev-libs/openssl-1.0.1h-r2:0=[${MULTILIB_USEDEP}] )
> > +	gcrypt?
> > ( >=dev-libs/libgcrypt-1.5.3:0[${MULTILIB_USEDEP}] )
> > +	libressl? ( dev-libs/libressl:0=[${MULTILIB_USEDEP}] )
> > +	mbedtls? ( net-libs/mbedtls )  
> 
> By the way, MULTILIB_USEDEP certainly missing here.

This wants another explanation. How is this different from the ebuild
that went before, -r1? You don't even explain what you think the
problem is. You're not trying to be nice. I have to make an effort
to turn your comment into intelligible English.

MULTILIB_USEDEP *is* missing here? By here you presumably mean
net-libs/mbedtls needs it because that's the line your comment follows?
Right?

So you could have said:

That should be
mbedtls? ( net-libs/mbedtls[MULTILIB_USEDEP] )

> > +	zlib? ( >=sys-libs/zlib-1.2.8-r1[${MULTILIB_USEDEP}] )
> > +"
> > +DEPEND="${RDEPEND}"
> > +
> > +PATCHES=(
> > +	"${FILESDIR}"/${PN}-1.8.0-libgcrypt-prefix.patch  
> 
> You sure this autoconf patch is needed for CMake build?

It's harmless. See above. But you also know that it is not needed, but
you're not kindly pointing that out, you're just taking any opportunity
to make a snide comment.

> > +	"${FILESDIR}"/${PN}-1.8.0-mansyntax_sh.patch
> > +	"${FILESDIR}"/${PN}-1.8.0-openssl11-memleak.patch
> > +	"${FILESDIR}"/${PN}-1.8.0-openssl11.patch
> > +)
> > +
> > +multilib_src_configure() {
> > +	local crypto_backend=OpenSSL
> > +	if use gcrypt; then
> > +		crypto_backend=Libgcrypt
> > +	elif use mbedtls; then
> > +		crypto_backend=mbedTLS
> > +	fi
> > +
> > +	local mycmakeargs=(
> > +		-DBUILD_SHARED_LIBS=ON
> > +		-DCRYPTO_BACKEND=${crypto_backend}
> > +		-DENABLE_ZLIB_COMPRESSION=$(usex zlib)
> > +	)
> > +	cmake-utils_src_configure
> > +}
> > +
> > +multilib_src_install_all() {
> > +	einstalldocs
> > +	find "${D}" -name '*.la' -delete || die  
> 
> They must have hacked that CMake hard to get it to generate .la files.

So again it's harmless? But you're in snide mode now.

> > +	mv "${D}"/usr/share/doc/${PN}/*
> > "${D}"/usr/share/doc/${PF}/ || die
> > +	rm -r "${D}"/usr/share/doc/${PN}/ || die
> > +}
> > 
> > diff --git a/net-libs/libssh2/metadata.xml
> > b/net-libs/libssh2/metadata.xml index e9e734ab02f..2149f2ed0c6
> > 100644 --- a/net-libs/libssh2/metadata.xml
> > +++ b/net-libs/libssh2/metadata.xml
> > @@ -10,5 +10,7 @@
> >  </maintainer>
> >  <use>
> >  	<flag name="gcrypt">Use <pkg>dev-libs/libgcrypt</pkg>
> > instead of <pkg>dev-libs/openssl</pkg></flag>
> > +	<flag name="libressl">Use <pkg>dev-libs/libressl</pkg>
> > instead of <pkg>dev-libs/openssl</pkg></flag>
> > +	<flag name="mbedtls">Use <pkg>net-libs/mbedtls</pkg>
> > instead of <pkg>dev-libs/openssl</pkg></flag> </use>
> >  </pkgmetadata>

You're quoting the rest because you agree? Or what? OK, let's go back
to the beginning:

> > net-libs/libssh2: Add USE=mbedtls, switch to cmake for building
> > 
> > * Add support for net-libs/mbedtls
> > * Switch to cmake as the autotools build is even more broken
> > * Remove USE=static-libs as that inhibits building shared libs
> > * Use REQUIRED_USE to force choosing a crypto backend

I spent a lot of time trying to figure out why and how the autotools
build system was broken and altogether too much time figuring out why it
couldn't be fixed and that upstream isn't very much interested in why
it's broken and how it can be fixed. When the autotools route failed
hard, I recovered at least some of the work by adding mbedtls support
and switching to cmake, which is probably the way forward in so far as
upstream intentions can be read. The only thing you came up with in
response is a couple of minor issues mostly wrapped in snide
oneliner remarks. In the same time you could have fixed the issues
yourself and congratulated yourself and the rest of the team on getting
ahead with libssh2 despite impossible odds.

But of course that is not what happened. This is what happened (in the
natural order of events, reversed from how git likes it):

commit 4cb94ede1a2a8d2b57e866d1fb2b909b857806d0
Author: Michał Górny <mgorny@gentoo.org>
Date:   Thu Aug 24 09:01:24 2017 +0200

    net-libs/libssh2: Add myself as backup maint (dep of libgit2)

[A stabilisation effort]
[Pacho Ramos fixing a security bug]
[Another stabilisation effort]

commit 8698aae507b6cd43aa3accec8cd5af4fe1ecdea5
Author: Michał Górny <mgorny@gentoo.org>
Date:   Thu Sep 6 22:46:57 2018 +0200

    net-libs/libssh2: Clean old up

commit d866d4705e1e4a092579a31df2815e3407950a19
Author: Jeroen Roovers <jer@gentoo.org>
Date:   Mon Oct 1 13:45:43 2018 +0200

    net-libs/libssh2: Add USE=mbedtls, switch to cmake for building

    * Add support for net-libs/mbedtls
    * Switch to cmake as the autotools build is even more broken
    * Remove USE=static-libs as that inhibits building shared libs
    * Use REQUIRED_USE to force choosing a crypto backend

    Package-Manager: Portage-2.3.50, Repoman-2.3.11
    Signed-off-by: Jeroen Roovers <jer@gentoo.org>

Someone suggested in an e-mail that "he is just annoyed that you broke
an ebuild that he has spent some time maintaining", but `git shortlog
-- .` would tell you quite a different story. What I think is happening
here is that you think I am "touching your stuff". You have to nitpick
at it instead of just fixing it together and then sending me an update
about the extra work or by pointing out the problems in a more humane
way and leaving me to fix them.

The way you're "working" on the problem means I am not really inclined
to fix any problems for you. So drop the attitude. Nobody likes it.



     jer

Re: [gentoo-dev] Re: [gentoo-commits] repo/gentoo:master commit in: net-libs/libssh2/

[Michał Górny]

[-- Attachment #1: Type: text/plain, Size: 2829 bytes --]

On Mon, 2018-10-01 at 23:13 +0200, Jeroen Roovers wrote:
> On Mon, 01 Oct 2018 17:00:24 +0200
> Michał Górny <mgorny@gentoo.org> wrote:
> 
> > On Mon, 2018-10-01 at 11:46 +0000, Jeroen Roovers wrote:
> > > commit:     d866d4705e1e4a092579a31df2815e3407950a19
> > > Author:     Jeroen Roovers <jer <AT> gentoo <DOT> org>
> > > AuthorDate: Mon Oct  1 11:45:43 2018 +0000
> > > Commit:     Jeroen Roovers <jer <AT> gentoo <DOT> org>
> > > CommitDate: Mon Oct  1 11:46:10 2018 +0000
> > > URL:
> > > https://gitweb.gentoo.org/repo/gentoo.git/commit/?id=d866d470
> > > 
> > > net-libs/libssh2: Add USE=mbedtls, switch to cmake for building
> > > 
> > > * Add support for net-libs/mbedtls
> > > * Switch to cmake as the autotools build is even more broken
> > > * Remove USE=static-libs as that inhibits building shared libs
> > > * Use REQUIRED_USE to force choosing a crypto backend
> 
> You completely skipped over the improvements. In effect, you show
> yourself to be completely unresponsive to what were considered positive
> changes to the author of the work.
> 
> Then you begin to pick apart what you think is wrong. It's not obvious
> why you are doing it this way, and with regard to practically all
> of your earlier e-mails addressed to me, I can only assume malice.

I'm sorry but I'm not going to help you if you keep attacking me like
this.  I have merely pointed out mistakes I've noticed.  I did it
privately specifically with respect to you.  You turn this into some
kind of public slandering effort, apparently for no other reason than
'assuming malice'.

I'm sorry if you can't see anything good in people.  When someone points
out an issue with my ebuild, I assume he just wants to have the ebuild
improved.  But now I learn people do that out of sheer malice.

I'm sorry that my comments were not detailed enough.  I have wrongly
assumed they'd point you in the right direction, and you'd be able to
solve those issues with help of eclass documentation.

> Someone suggested in an e-mail that "he is just annoyed that you broke
> an ebuild that he has spent some time maintaining", but `git shortlog
> -- .` would tell you quite a different story. What I think is happening
> here is that you think I am "touching your stuff". You have to nitpick
> at it instead of just fixing it together and then sending me an update
> about the extra work or by pointing out the problems in a more humane
> way and leaving me to fix them.

That's one way to look at it.  The other way is that you're so horribly
abrasive that I'm *scared* to touch this ebuild beyond what needs to be
absolutely done.  That's why I didn't switch it to CMake earlier --
because I presumed you'd attack me again.  It seems I wasn't wrong after
all.

-- 
Best regards,
Michał Górny

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 963 bytes --]