Bug#827082: RFS: libredjackipset/1.1.1+20150311-1 [ITP] -- C library to store sets/maps of IP address
Dear G,
Thanks for your review!
On Mon, Jun 20, 2016 at 10:03 PM, Gianfranco Costamagna
<locutusofborg@debian.org> wrote:
> control: owner -1 !
> control: tags -1 moreinfo
>
> Hi, lets see
>
> 1) control: why the -dev package is not multiarch?
> sounds like it is multiarch ready to me
> (I didn't check but include/* seems to have the same md5sum on different archs
Yes, it's multi-arch, just forgot to mark it in d/control.
Fixed.
> 2) the dev package should suggest the doc one
Fixed.
> 3) d/p/0002-fix-build-with-static-libcheck-library.patch
>
> I can't complain here, I seem to know the author of that patch :p
Thanks for your great help!
This convinced me that this library can be packaged, too.
> 3) d/p/0004-remove-test-of-python-cram.patch
>
> why? python-cram is available on Debian
Yes, I used python3-cram in libcork, but this time it just doesn't work.
So I removed it.
> 4) d/p/0007-*
> why? please be more verbose on patches, otherwise I find difficult to understand why you are using sphinx instead
> of pandoc to build documentation
upstream's docs/CMakeLists.txt just output nothing.
You can see "set(MAN_PAGES)", the target is empty!
So I used the same way in libcork to generate docs.
Actually the upstream was during the transition from sphinx to pandoc.
They removed sphinx setting, added pandoc setting, but didn't remove
sphinx docs.
So I patched back sphinx setting, just works.
(docs from upstream just poor, even I wrote a patch to add index page)
> 5) rules:
> I don't like hacks
> and your sed makes the package non-rebuildable.
>
> maybe overriding the
>
> dh_install target, and acting under debian/libfoo-dev/usr/include
>
> is a "better" way (also, a mv instead of ln -s since this old directory should go away)
gbp-buildpackage and git-pbuilder just behave differently.
the former does: build => install => build test + test
the latter does: build => build test + test => install
Because the library is renamed from libipset to libredjackipset,
current hack is the simplest way to make both gbp-buildpackage and
git-pbuilder happy.
> --buildsystem=cmake --builddirectory=build should be useless
I removed "--buildsystem=cmake"
Thanks for your suggestion!
Without "--builddirectory=build", the docs will be built under
<Multi-Arch>/docs/
This make install file hard to write.
> also
> DPKG_EXPORT_BUILDFLAGS = 1
> include /usr/share/dpkg/buildflags.mk
Removed.
Thanks for your suggestion!
> 6) did you forward the rename to upstream?
https://github.com/redjack/ipset/issues/41
> 7) libredjackipset-docs.docs
> is it useful? there is no package with that name, so probably the file can be removed
Fixed.
> 8) I'm not a fan of dh-exec, and your patch about multiarch seems incomplete
>
> +include(GNUInstallDirs)
>
> this has to be done *before*
>
> "set(CMAKE_INSTALL_LIBDIR lib${LIB_SUFFIX} CACHE STRING"
> note the LIB_SUFFIX
>
> and then the install files can change from
> usr/lib/lib*.so usr/lib/${DEB_HOST_MULTIARCH}
> to
> usr/lib/*/*.so
> and similar for others
Yes, moving "include(GNUInstallDirs)" makes dh-exec useless.
My mistake forced me to introduced this complex. Sorry about that.
> 9)
>
> did you try compat level 10?
> it should speed up the build by running dh --parallel :)
> (you can also experiment with --parallel on the default dh call in rules file)
I'd love to try new stuff after first release :-D
Thanks again!
Cheers,
--
Roger Shimizu, GMT +9 Tokyo
PGP/GPG: 17B3ACB1
Reply to: