[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



Dear G,

Thanks for your review!
I updated the package.

On Wed, Jun 22, 2016 at 1:28 AM, Gianfranco Costamagna
<locutusofborg@debian.org> wrote:
>>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)
>
>
> I was already ok with the patch, just adding some Description makes the
> review easier ;)

Thanks!
My additional description already added since previous update.

>>After reading your comment for three time, now I understand it.
>>I put it under dh_install as you recommend.
>>Now it's rebuildable.
>
> :)
>
>>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.
>
> mmm strange...
> this is what I have in the docs file
> cat debian/libcorkipset-doc.docs
> build-*/docs/html

Debian builds obj-<multi-arch> by default.
So it seems to specify "--builddirectory=build" safe for both debian and ubuntu.

>>Since the most authors quit redjack, and they're almost not active,
>
>>I consider it should be called libcorkipset.
>
> I see activity upstream, lets wait for the rename to be effective then!

Yes, upstream just don't like calling it libredjackipset.
They seems fine with libcorkipset, but proposed another option: libipaddrset

I don't like the latter one because the header will be installed to
/usr/include/libipaddrset/ipset.h
and user need to "#include <libipaddrset/ipset.h>"

I think "#include <libcorkipset/ipset.h>" makes more sense.

>>> "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.
>
>
> the missing LIB_SUFFIX makes it FTBFS on Ubuntu, where cmake has a little
> different implementation.
>
> (and a non-defined variable is just a no-op in Debian)

"LIB_SUFFIX" is added to patch.

Takes you time and thanks again!

Cheers,
-- 
Roger Shimizu, GMT +9 Tokyo
PGP/GPG: 4096R/6C6ACD6417B3ACB1


Reply to: