1 |
OK, let's do a full review of your review. |
2 |
|
3 |
|
4 |
On Mon, 01 Oct 2018 17:00:24 +0200 |
5 |
Michał Górny <mgorny@g.o> wrote: |
6 |
|
7 |
> On Mon, 2018-10-01 at 11:46 +0000, Jeroen Roovers wrote: |
8 |
> > commit: d866d4705e1e4a092579a31df2815e3407950a19 |
9 |
> > Author: Jeroen Roovers <jer <AT> gentoo <DOT> org> |
10 |
> > AuthorDate: Mon Oct 1 11:45:43 2018 +0000 |
11 |
> > Commit: Jeroen Roovers <jer <AT> gentoo <DOT> org> |
12 |
> > CommitDate: Mon Oct 1 11:46:10 2018 +0000 |
13 |
> > URL: |
14 |
> > https://gitweb.gentoo.org/repo/gentoo.git/commit/?id=d866d470 |
15 |
> > |
16 |
> > net-libs/libssh2: Add USE=mbedtls, switch to cmake for building |
17 |
> > |
18 |
> > * Add support for net-libs/mbedtls |
19 |
> > * Switch to cmake as the autotools build is even more broken |
20 |
> > * Remove USE=static-libs as that inhibits building shared libs |
21 |
> > * Use REQUIRED_USE to force choosing a crypto backend |
22 |
|
23 |
You completely skipped over the improvements. In effect, you show |
24 |
yourself to be completely unresponsive to what were considered positive |
25 |
changes to the author of the work. |
26 |
|
27 |
Then you begin to pick apart what you think is wrong. It's not obvious |
28 |
why you are doing it this way, and with regard to practically all |
29 |
of your earlier e-mails addressed to me, I can only assume malice. |
30 |
|
31 |
> So your src_compile() is now broken for Ninja users, and src_test() |
32 |
> is broken altogether. How about cmake-multilib eclass? |
33 |
|
34 |
ninja is a thing? Can you explain what it is? Can you show me how it's |
35 |
broken, and how I reproduce the ninja problem, pro forma by filing a bug |
36 |
report at https://bugs.gentoo.org/ |
37 |
|
38 |
> > +DESCRIPTION="Library implementing the SSH2 protocol" |
39 |
> > +HOMEPAGE="https://www.libssh2.org" |
40 |
> > +SRC_URI="https://www.${PN}.org/download/${P}.tar.gz" |
41 |
> > + |
42 |
> > +LICENSE="BSD" |
43 |
> > +SLOT="0" |
44 |
> > +KEYWORDS="~alpha ~amd64 ~arm ~arm64 ~hppa ~ia64 ~mips ~ppc ~ppc64 |
45 |
> > ~s390 ~sh ~sparc ~x86 ~amd64-fbsd ~x86-fbsd ~amd64-linux ~x86-linux |
46 |
> > ~ppc-macos ~x64-macos ~x86-solaris" +IUSE="gcrypt libressl mbedtls |
47 |
> > test zlib" |
48 |
> |
49 |
> IUSE=test is not used at all. |
50 |
|
51 |
So that's a minor problem we could fix even without a revision bump. |
52 |
You could have pointed it out in that missing bug report. |
53 |
|
54 |
> > +REQUIRED_USE=" |
55 |
> > + ?? ( gcrypt libressl mbedtls ) |
56 |
> > +" |
57 |
> > + |
58 |
> > +RDEPEND=" |
59 |
> > + !libressl? |
60 |
> > ( >=dev-libs/openssl-1.0.1h-r2:0=[${MULTILIB_USEDEP}] ) |
61 |
> > + gcrypt? |
62 |
> > ( >=dev-libs/libgcrypt-1.5.3:0[${MULTILIB_USEDEP}] ) |
63 |
> > + libressl? ( dev-libs/libressl:0=[${MULTILIB_USEDEP}] ) |
64 |
> > + mbedtls? ( net-libs/mbedtls ) |
65 |
> |
66 |
> By the way, MULTILIB_USEDEP certainly missing here. |
67 |
|
68 |
This wants another explanation. How is this different from the ebuild |
69 |
that went before, -r1? You don't even explain what you think the |
70 |
problem is. You're not trying to be nice. I have to make an effort |
71 |
to turn your comment into intelligible English. |
72 |
|
73 |
MULTILIB_USEDEP *is* missing here? By here you presumably mean |
74 |
net-libs/mbedtls needs it because that's the line your comment follows? |
75 |
Right? |
76 |
|
77 |
So you could have said: |
78 |
|
79 |
That should be |
80 |
mbedtls? ( net-libs/mbedtls[MULTILIB_USEDEP] ) |
81 |
|
82 |
> > + zlib? ( >=sys-libs/zlib-1.2.8-r1[${MULTILIB_USEDEP}] ) |
83 |
> > +" |
84 |
> > +DEPEND="${RDEPEND}" |
85 |
> > + |
86 |
> > +PATCHES=( |
87 |
> > + "${FILESDIR}"/${PN}-1.8.0-libgcrypt-prefix.patch |
88 |
> |
89 |
> You sure this autoconf patch is needed for CMake build? |
90 |
|
91 |
It's harmless. See above. But you also know that it is not needed, but |
92 |
you're not kindly pointing that out, you're just taking any opportunity |
93 |
to make a snide comment. |
94 |
|
95 |
> > + "${FILESDIR}"/${PN}-1.8.0-mansyntax_sh.patch |
96 |
> > + "${FILESDIR}"/${PN}-1.8.0-openssl11-memleak.patch |
97 |
> > + "${FILESDIR}"/${PN}-1.8.0-openssl11.patch |
98 |
> > +) |
99 |
> > + |
100 |
> > +multilib_src_configure() { |
101 |
> > + local crypto_backend=OpenSSL |
102 |
> > + if use gcrypt; then |
103 |
> > + crypto_backend=Libgcrypt |
104 |
> > + elif use mbedtls; then |
105 |
> > + crypto_backend=mbedTLS |
106 |
> > + fi |
107 |
> > + |
108 |
> > + local mycmakeargs=( |
109 |
> > + -DBUILD_SHARED_LIBS=ON |
110 |
> > + -DCRYPTO_BACKEND=${crypto_backend} |
111 |
> > + -DENABLE_ZLIB_COMPRESSION=$(usex zlib) |
112 |
> > + ) |
113 |
> > + cmake-utils_src_configure |
114 |
> > +} |
115 |
> > + |
116 |
> > +multilib_src_install_all() { |
117 |
> > + einstalldocs |
118 |
> > + find "${D}" -name '*.la' -delete || die |
119 |
> |
120 |
> They must have hacked that CMake hard to get it to generate .la files. |
121 |
|
122 |
So again it's harmless? But you're in snide mode now. |
123 |
|
124 |
> > + mv "${D}"/usr/share/doc/${PN}/* |
125 |
> > "${D}"/usr/share/doc/${PF}/ || die |
126 |
> > + rm -r "${D}"/usr/share/doc/${PN}/ || die |
127 |
> > +} |
128 |
> > |
129 |
> > diff --git a/net-libs/libssh2/metadata.xml |
130 |
> > b/net-libs/libssh2/metadata.xml index e9e734ab02f..2149f2ed0c6 |
131 |
> > 100644 --- a/net-libs/libssh2/metadata.xml |
132 |
> > +++ b/net-libs/libssh2/metadata.xml |
133 |
> > @@ -10,5 +10,7 @@ |
134 |
> > </maintainer> |
135 |
> > <use> |
136 |
> > <flag name="gcrypt">Use <pkg>dev-libs/libgcrypt</pkg> |
137 |
> > instead of <pkg>dev-libs/openssl</pkg></flag> |
138 |
> > + <flag name="libressl">Use <pkg>dev-libs/libressl</pkg> |
139 |
> > instead of <pkg>dev-libs/openssl</pkg></flag> |
140 |
> > + <flag name="mbedtls">Use <pkg>net-libs/mbedtls</pkg> |
141 |
> > instead of <pkg>dev-libs/openssl</pkg></flag> </use> |
142 |
> > </pkgmetadata> |
143 |
|
144 |
You're quoting the rest because you agree? Or what? OK, let's go back |
145 |
to the beginning: |
146 |
|
147 |
> > net-libs/libssh2: Add USE=mbedtls, switch to cmake for building |
148 |
> > |
149 |
> > * Add support for net-libs/mbedtls |
150 |
> > * Switch to cmake as the autotools build is even more broken |
151 |
> > * Remove USE=static-libs as that inhibits building shared libs |
152 |
> > * Use REQUIRED_USE to force choosing a crypto backend |
153 |
|
154 |
I spent a lot of time trying to figure out why and how the autotools |
155 |
build system was broken and altogether too much time figuring out why it |
156 |
couldn't be fixed and that upstream isn't very much interested in why |
157 |
it's broken and how it can be fixed. When the autotools route failed |
158 |
hard, I recovered at least some of the work by adding mbedtls support |
159 |
and switching to cmake, which is probably the way forward in so far as |
160 |
upstream intentions can be read. The only thing you came up with in |
161 |
response is a couple of minor issues mostly wrapped in snide |
162 |
oneliner remarks. In the same time you could have fixed the issues |
163 |
yourself and congratulated yourself and the rest of the team on getting |
164 |
ahead with libssh2 despite impossible odds. |
165 |
|
166 |
But of course that is not what happened. This is what happened (in the |
167 |
natural order of events, reversed from how git likes it): |
168 |
|
169 |
commit 4cb94ede1a2a8d2b57e866d1fb2b909b857806d0 |
170 |
Author: Michał Górny <mgorny@g.o> |
171 |
Date: Thu Aug 24 09:01:24 2017 +0200 |
172 |
|
173 |
net-libs/libssh2: Add myself as backup maint (dep of libgit2) |
174 |
|
175 |
[A stabilisation effort] |
176 |
[Pacho Ramos fixing a security bug] |
177 |
[Another stabilisation effort] |
178 |
|
179 |
commit 8698aae507b6cd43aa3accec8cd5af4fe1ecdea5 |
180 |
Author: Michał Górny <mgorny@g.o> |
181 |
Date: Thu Sep 6 22:46:57 2018 +0200 |
182 |
|
183 |
net-libs/libssh2: Clean old up |
184 |
|
185 |
commit d866d4705e1e4a092579a31df2815e3407950a19 |
186 |
Author: Jeroen Roovers <jer@g.o> |
187 |
Date: Mon Oct 1 13:45:43 2018 +0200 |
188 |
|
189 |
net-libs/libssh2: Add USE=mbedtls, switch to cmake for building |
190 |
|
191 |
* Add support for net-libs/mbedtls |
192 |
* Switch to cmake as the autotools build is even more broken |
193 |
* Remove USE=static-libs as that inhibits building shared libs |
194 |
* Use REQUIRED_USE to force choosing a crypto backend |
195 |
|
196 |
Package-Manager: Portage-2.3.50, Repoman-2.3.11 |
197 |
Signed-off-by: Jeroen Roovers <jer@g.o> |
198 |
|
199 |
Someone suggested in an e-mail that "he is just annoyed that you broke |
200 |
an ebuild that he has spent some time maintaining", but `git shortlog |
201 |
-- .` would tell you quite a different story. What I think is happening |
202 |
here is that you think I am "touching your stuff". You have to nitpick |
203 |
at it instead of just fixing it together and then sending me an update |
204 |
about the extra work or by pointing out the problems in a more humane |
205 |
way and leaving me to fix them. |
206 |
|
207 |
The way you're "working" on the problem means I am not really inclined |
208 |
to fix any problems for you. So drop the attitude. Nobody likes it. |
209 |
|
210 |
|
211 |
|
212 |
jer |