[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



[CC upstream]
[Douglas & John are original author of this library, and Greg
currently has write permission to git repo on github]

Dear G,

Thanks for your time to review again!

On Thu, Jun 23, 2016 at 1:00 AM, Gianfranco Costamagna
<locutusofborg@debian.org> wrote:
> Hi Roger
>
>>> 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.
>
>
> yes, but useless...
> maybe you didn't get completely the hint, but my guess was:
>
> remove the --builddirectory stuff
> and change
>
> build/docs/html
>
> to
> build-*/docs/html
>
> probably this sounds stupid/nitpick to you, but allows
> people to easily cross-compile stuff, or to compile the library for
> both amd64 and i386 without having to choose one or the other.
> that triplet is trivial, and makes less confusion to porters
> or even developer, who might want to build it on their own laptop
> and use on different architectures.
>
>
> (note: this isn't a blocker)

I think you build on ubuntu, so without "--builddirectory stuff" the
build directory is
build-*/
When I tested in Debian, the build directory is obj-*/
That's why I insist keeping "--builddirectory stuff" to make same
install file for both Debian and Ubuntu.

>>Yes, upstream just don't like calling it libredjackipset.
>>They seems fine with libcorkipset, but proposed another option: libipaddrset
>
> I'll wait for them make a decision then

Already posted to https://github.com/redjack/ipset/issues/41
But I don't think there will be a clear reply.

> so, please consider applying my above fixes, and ask me to sponsor&upload&review
> when upstream releases a new fixed renamed tarball
> (this should even avoid your dh_install override if I'm correctly understanding it)

The original author (in CC list) left redjack, so now they don't have
write access to that repo, and cannot release a new version with new
name.

I think the upstream (the original author and redjack) know what we're
doing here, and if they aren't against here, we can just upload.


>>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.
>
>
> just explain that upstream :)
>
> I honestly don't care too much, but I want to use the name that upstream choose
> to avoid overcomplicate things in the long run

Explained before you asked me. No reply yet.

>>"LIB_SUFFIX" is added to patch.
>
>
> bad me, it didn't work :(
> the patch wasn't correct, because the variable gets overridden anyway on the following line.
>
> I tweaked the patch, to do something like
> +if(NOT CMAKE_INSTALL_LIBDIR)
> +    set(CMAKE_INSTALL_LIBDIR lib CACHE STRING
> +         "The base name of the installation directory for libraries")
> +endif(NOT CMAKE_INSTALL_LIBDIR)
>
>
> otherwise no matter who sets it, it gets defaulted to what upstream thinks it is better for everybody.

Thanks for your patch, applied well.


> BTW I can't run dpkg-buildpackage twice, because a "RELEASE-VERSION" file is added to the source
> tree
>
> echo RELEASE-VERSION > debian/clean might just fix that

Applied.
Thanks for your suggestion!

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


Reply to: