Bug#672394: RFS: ipset/6.12-1 -- administration tool for kernel IP sets

[Neutron Soutmun]

Package: sponsorship-requests
Version: RFS: ipset/6.12-1 -- administration tool for kernel IP sets
Severity: normal

Dear mentors,

I am looking for a sponsor for my package "ipset"

* Package name : ipset
Version : 6.12-1
Upstream Author : Jozsef Kadlecsik <kad...@blackhole.kfki.hu>
* URL : http://ipset.netfilter.org/
* License : GPL
Section : net

It builds those binary packages:

ipset - administration tool for kernel IP sets
libipset-dev - Development files for IP sets
libipset2 - IP sets library

To access further information about this package, please visit the
following URL:

http://mentors.debian.net/package/ipset

Alternatively, one can download the package with dget using this command:

dget -x http://mentors.debian.net/debian/pool/main/i/ipset/ipset_6.12-1.dsc

Changes since the last upload:

ipset (6.12-1) unstable; urgency=low

* Imported Upstream version 6.12
* Add build failed patch
* debian/patches/90-build-fix-build-failed-on-disable-dependency-trackin.patch:
- Fix the build failed on --disable-dependency-tracking is being parse.
- Fix the ipset could not build out of the source tree.
* debian/patches/99-ipset-shared-libs.patch:
- Drop as the upstream provides the configuration parameters to force
the ipset binary to link with shared lib.
* Enable settype dynamic modules support
* debian/rules:
- Add --enable-settype-modules to configure.
All settype modules still build as static which included in libipset
as usual but the ipset binary could runtime load the additional modules
installed at /usr/lib/[arch triplet]/ipset.
* debian/control:
- Add libltdl-dev to build-dep.
* Add DM-Upload-Allowed field

-- System Information:
Debian Release: wheezy/sid
APT prefers unstable
APT policy: (500, 'unstable'), (1, 'experimental')
Architecture: amd64 (x86_64)

Kernel: Linux 3.3.0-trunk-amd64 (SMP w/2 CPU cores)
Locale: LANG=en_US.UTF-8, LC_CTYPE=en_US.UTF-8 (charmap=UTF-8)
Shell: /bin/sh linked to /bin/dash

[Neutron Soutmun]

Package: sponsorship-requests
Followup-For: Bug #672394

Dear mentors,

As the upstream releases the new version 6.12.1 of ipset for the
build system bug fix that including my proposed patch.
Therefore, I'm working on this and will upload the new updated package
soon.

Best regards,
Neutron Soutmun

[Arno Töll]

tags 672394 + moreinfo
owner 672394 !
thanks

Hi Neutrom,

this is a review of your package ipset.

* Do not set "DM-Upload-Allowed: yes" on your own. It's your sponsor's
domain to do so. It's hard enough to find sponsors as is, no need to
scare off even more potential sponsors by adding DMUA for packages which
show up on debian-mentors without prior agreement.

* You declare the debhelper compat[ibility] to be 9, but you build
depend on "debhelper (>= 9)". Please use a version which actually
supports the finalized level 9. That is 9.20120115.

* Do not start with uppercase characters in libipset-dev's description.
You did it correctly for the other binary package, though.

* Why do you use dh_autoreconf? You do not patch automake stuff and a
brief test seems to confirm it is not needed.

* Are you sure about the location of the binary in the file system?
iptables is in /sbin, why do you install ipset to /usr/sbin?


I am willing to sponsor your package if you fix the fist two concerns at
very least.

--
with kind regards,
Arno Töll
IRC: daemonkeeper on Freenode/OFTC
GnuPG Key-ID: 0x9D80F36D

[Neutron Soutmun]

On Tue, May 15, 2012 at 12:51:40AM +0200, Arno Töll wrote:
Hi,

>
> this is a review of your package ipset.
>
> * Do not set "DM-Upload-Allowed: yes" on your own. It's your sponsor's
> domain to do so. It's hard enough to find sponsors as is, no need to
> scare off even more potential sponsors by adding DMUA for packages which
> show up on debian-mentors without prior agreement.

Sorry, drop in the latest upload.

> * You declare the debhelper compat[ibility] to be 9, but you build
> depend on "debhelper (>= 9)". Please use a version which actually
> supports the finalized level 9. That is 9.20120115.

I'm following the debhelper(7) manpage which it said that add
debhelper (>= 9) for compat level 9.
However, I adjust it to debhelper (>= 9.20120115~) as your suggestion.

>
> * Do not start with uppercase characters in libipset-dev's description.
> You did it correctly for the other binary package, though.

Adjusted and for libipset2 description also.

> * Why do you use dh_autoreconf? You do not patch automake stuff and a
> brief test seems to confirm it is not needed.

Drop, it's unnecessary as your suggestion.

> * Are you sure about the location of the binary in the file system?
> iptables is in /sbin, why do you install ipset to /usr/sbin?

The ipset depends on libmnl which it is installed in /usr/lib or
/usr/lib/[arch triplet] if it willing support multiarch.

I think, it's not appropriate to install ipset in /sbin when it's still
depends on libmnl that installed in /usr.

> I am willing to sponsor your package if you fix the fist two concerns at
> very least.

Thanks in advance and please review again.

To access further information about this package, please visit the
following URL:

http://mentors.debian.net/package/ipset

Alternatively, one can download the package with dget using this command:

dget -x http://mentors.debian.net/debian/pool/main/i/ipset/ipset_6.12.1-1.dsc

Changes since the last upload:

* Imported Upstream version 6.12.1

* debian/patches/99-ipset-shared-libs.patch:
- Drop as the upstream provides the configuration parameters to force
the ipset binary to link with shared lib.

* debian/patches/90-fix-typo.patch: Add typo fix

* Enable settype dynamic modules support
* debian/rules:
- Add --enable-settype-modules to configure.
All settype modules still build as static which included in libipset
as usual but the ipset binary could runtime load the additional modules
installed at /usr/lib/[arch triplet]/ipset.
* debian/control:
- Add libltdl-dev to build-dep.

* debian/libipset2.symbols: Add symbols file
* debian/control:
- Set debhelper (>= 9.20120115~). (thanks, Arno Töll)
- Drop dh-autoreconf from build-deps as it's unnecessary.
(thanks, Arno Töll)
- Adjust the packages description which should not start with capital
letter. (thanks, Arno Töll)
* debian/rules: Drop autoreconf addon from debhelper configuration.

Best regards,
Neutron Soutmun

[Debian Bug Tracking System]

Your message dated Tue, 15 May 2012 14:08:46 +0200
with message-id <4FB2474...@debian.org>
and subject line Re: Bug#672394: RFS: ipset/6.12-1 -- administration tool for kernel IP sets
has caused the Debian Bug report #672394,
regarding RFS: ipset/6.12.1-1 -- administration tool for kernel IP sets
to be marked as done.

This means that you claim that the problem has been dealt with.
If this is not the case it is now your responsibility to reopen the
Bug report if necessary, and/or fix the problem forthwith.

(NB: If you are a system administrator and have no idea what this
message is talking about, this may indicate a serious mail system
misconfiguration somewhere. Please contact ow...@bugs.debian.org
immediately.)


--
672394: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=672394
Debian Bug Tracking System
Contact ow...@bugs.debian.org with problems

[Henrique de Moraes Holschuh]

On Tue, 15 May 2012, Neutron Soutmun wrote:
> > * Are you sure about the location of the binary in the file system?
> > iptables is in /sbin, why do you install ipset to /usr/sbin?
>
> The ipset depends on libmnl which it is installed in /usr/lib or
> /usr/lib/[arch triplet] if it willing support multiarch.

Please file a bug requesting that libmnl be moved to /

IMHO ipset can live in /usr/sbin until libmnl gets moved to /, but you
could also ship it already in /sbin from day one (and document in
readme.debian that it requires libmnl, which is currently in /usr/lib).

> I think, it's not appropriate to install ipset in /sbin when it's still
> depends on libmnl that installed in /usr.

That depends. If it *is* going to get moved to /sbin, it is best to
already ship it in /sbin so that people don't start using
"/usr/sbin/ipset" or something...

--
"One disk to rule them all, One disk to find them. One disk to bring
them all and in the darkness grind them. In the Land of Redmond
where the shadows lie." -- The Silicon Valley Tarot
Henrique Holschuh



--
To UNSUBSCRIBE, email to debian-bugs-...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listm...@lists.debian.org

[Neutron Soutmun]

Hi,

On Wed, May 16, 2012 at 12:13 AM, Henrique de Moraes Holschuh
<h...@debian.org> wrote:
> On Tue, 15 May 2012, Neutron Soutmun wrote:
>> > * Are you sure about the location of the binary in the file system?
>> > iptables is in /sbin, why do you install ipset to /usr/sbin?
>>
>> The ipset depends on libmnl which it is installed in /usr/lib or
>> /usr/lib/[arch triplet] if it willing support multiarch.
>
> Please file a bug requesting that libmnl be moved to /
>
> IMHO ipset can live in /usr/sbin until libmnl gets moved to /, but you
> could also ship it already in /sbin from day one (and document in
> readme.debian that it requires libmnl, which is currently in /usr/lib).

Sure, I'll file a bug to libmnl. Thanks, that you remind me again.

>> I think, it's not appropriate to install ipset in /sbin when it's still
>> depends on libmnl that installed in /usr.
>
> That depends.  If it *is* going to get moved to /sbin, it is best to
> already ship it in /sbin so that people don't start using
> "/usr/sbin/ipset" or something...

Ok, I'll move it from /usr to / as your suggestion in the next release
(6.12.1-2).

Best regards,
Neutron Soutmun

[Dmitry Smirnov]

Hi Arno,

On 15 May 2012 08:51, Arno Töll <ar...@debian.org> wrote:
> * You declare the debhelper compat[ibility] to be 9, but you build
> depend on "debhelper (>= 9)". Please use a version which actually
> supports the finalized level 9. That is 9.20120115.

No need to impose this requirement because non-finalized versions are
not present in the archive:
http://packages.debian.org/search?keywords=debhelper&searchon=names&suite=all&section=all

IMHO "debhelper (>= 9)" would be perfectly fine.
Moreover this is a common practice.

Strict dependency would be justified only if there were any particular
bugs in prior 9.20120115 but even then we could safely use (>= 9)
simply because no "broken" versions are present in the archive.

Please share your concerns if I'm missing anything.

Thank you.

Regards,
Dmitry.