[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#827082: RFS: libredjackipset/1.1.1+20150311-1 [ITP] -- C library to store sets/maps of IP address



Control: retitle -1 RFS: libcorkipset/1.1.1+20150311-1 [ITP] -- C
library to store sets/maps of IP address

Dear G,

I think you didn't review since you did last time, and I did some
changes based on some changes in previous post, so here I *rewrite*
the previous post.

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.
Without my patch, python-cram just report "no test" error.
I guess upstream didn't finish the test case of cram.
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)

After reading your comment for three time, now I understand it.
I put it under dh_install as you recommend.
Now it's rebuildable.

> --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.
So here I keep as it was.

> also
> DPKG_EXPORT_BUILDFLAGS = 1
> include /usr/share/dpkg/buildflags.mk

Removed.
Thanks for your suggestion!

> 6) did you forward the rename to upstream?

Since the most authors quit redjack, and they're almost not active,
I consider it should be called libcorkipset.

> 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)

Tried, just added a lintian warning.
Should be fixed when debhelper 10 released.
So I keep as it is.

Thanks again!

Cheers,
-- 
Roger Shimizu, GMT +9 Tokyo
PGP/GPG: 17B3ACB1


Reply to: