[pkg] CurveDNS - review

[Stéphane Neveu]

Hi Lucas,

Again, many thanks for your help.

>
> On Wed, 21 Jun 2017 13:43:43 +0200
> Stéphane Neveu <stefneveu at gmail.com> wrote:
>> > I noticed the source includes (and compiles+uses) the nacl library.
>> > While that library is already packaged in Debian, it's unfortunately
>> > not available as shared object.
>> > (...)
>>
>> Yes, I don't know how to deal with that really. Should I ask on
>> mentors mailing list ?
>> I'm not sure to be able to convince the maintainer.
>
> I did some digging and found libsodium. From the description:
>
>   "Sodium is a portable, cross-compatible, installable, packageable
>    fork of NaCl, with a compatible API"
>
> It's packaged in Debian and is available as shared library.  Do you
> think it's feasible to link curvedns against that?  A quick search
> suggests that others have done that already successfully…
>


I had a look at how it' done for FreeBSD. Here is my try to do the
same, so two patches added :
- libsodium.patch
- makefile.patch


>
>> > * debian/curvedns.config: don't you need to db_get in order to be
>> > able to use $RET (it's commented out)?
>>
>> Suprisingly, it does not work with db_get whereas it does when
>> commented out... did I missed something ? yes probably.
>
> What do you mean with "does not work".  What is happening and what are
> you expecting to happen.  Is the question being asked?  What is stored
> in debconf?
>
> Without the `db_get` your $RET will probably contain "ok" from
> `db_go`.  Did you check what $RET actually contains?
>
>
> Apart from that problem, I the logic of you config script still
> needs some work:
> * How important is it to correctly get the fqdn?
> * What happens if the user does not enter an fqdn and `curvedns-keygen`
>   is not run?  Does the software still function as normal?
>   - if the user needs to edit /etc/default/curvedns anyways, could we
>     configure the FQDN there?
> * What happens if the user enters something illegal as fqdn (e.g.
>   something containing spaces)?  Does curvedns-keygen exit uncleanly?
>   Can we handle that case and re-prompt if need be?
>   - You should quote "$RET" in the curvedns-keygen command
>

Yes, I agree with you. I'm still working on it, I'm trying to add some
more controls but for now the db_input high curvedns/ask_again is now
showing up... Still need to work on this like you said :)
Note : I also added db_purge in postrm.

>
>> > * debian/curveudns.{postrm,prerm}: you should not do something like
>> >
>> >       if not_my_phase; then
>> >           exit 0
>> >       fi
>> >       #commands here
>> >
>> >   because that will prevent #DEBHELPER# later to execute what it may
>> >   need to in other phases.  Instead do something like:
>> >
>> >       if my_phase; then
>> >           #commands here
>> >       fi
>> >
>> > * Suggestions for "beautification":
>> >   - debian/control: make whitespace consistent, don't mix tabs and
>> >     spaces
>>
>> Thank you, it's done.
>
> It's not yet done in curvedns.postinst.
>
> In curvedns.{postrm,prerm} your logic is now wrong.  E.g. in
> curvedns.postrm you have
>
>   if [ x"$1" != "xpurge" ]; then
>     # purge commands here
>   fi
>
> but now you actually want
>
>   if [ x"$1" = "xpurge" ]; then
>     # purge commands here
>   fi
>
> You also need to negate your first if in curvedns.prerm accordingly
> (and it shouldn't be indented).
>
>> >   - debian/copyright: link to format could be https :)
>>
>> Well the tls certificate doesn't seem to be valid :/
>> https://curvedns.on2it.net/
>
> Yes, I've noticed, but the same does not apply for the "format" link:
> https://www.debian.org/doc/packaging-manuals/copyright-format/1.0/
> works fine.
>

Is it a bit better ?


> Regards
> Lukas

Best regards,
Stephane