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

Bug#848993: RFS: llmnrd/0.2-1 [ITP]



Hi there,

sorry for the formatting, writing this on my phone.


Am 23. Dezember 2016 10:18:52 MEZ, schrieb Andreas Henriksson <andreas@fatal.se>:
>On Fri, Dec 23, 2016 at 12:12:17AM +0100, Christian Seiler wrote:
>>  - init.d: this file name works with dh_installinit, but is not
>>    documented, so I'd recommend using llmnrd.init as the file name
>
>I see you're already credited by upstream so I assume you have
>already established a good relationship with your upstream.
>That's very good and very useful. Keep your upstream happy.
>Upstreams like contributions. You have a golden opportunity 
>on upstream issue #2.

I'm not sure that'll work. In contrast to systemd services init scripts are necessarily very distro-dependent. You can hack together something that's cross-distro, but that's really ugly.

Also, Debian (+ derivatives) is just about the only major distro that still supports traditional init scripts, except for maybe Slackware: Gentoo always had their own thing that wasn't compatible.

RH had /etc/sysconfig instead of /etc/default and had different includes for helper functions, just to give an idea what differences there are. SuSE hat yet another include library. RH didn't support LSB headers but had similar headers based on chkconfig to express dependencies.

>>  - init.d: any particular reason you don't use init-d-script? (See
>>    current /etc/init.d/skeleton for how this works; it will
>>    automatically source /etc/default/$scriptname and interpret the
>>    DAEMON_ARGS variable, so your init script could probably be just
>>    a couple of lines that set the name of the executable)
>
>I'd recommend *against* "init-d-script". It has several outstanding
>issues, is unmaintained/orphaned/unproven and AIUI that also means the
>init script becomes debian-only.

IMHO init scripts are distro-dependent anyway (see above). I didn't know about the issues in init-d-script and since I use that in my own packages, I'll look into that. Any pointers?


>>  - any reason you don't install the systemd service provided by
>>    upstream in addition to the init script?
>
>Please do. Please also consider improving the systemd service
>shipped by upstream. (Another golden opportunity for upstream
>contributions.)
>Most importantly have a look at the User= directive as it seems
>like running unprivilegied is preferred (see upstream issue #4).
>See also the Restrict*= directives provided by systemd which
>would also be nice to limit the potential attack surface.

Ack.

>>  - you should probably add a line "export Q =" to debian/rules to
>>    disable silent builds. While these look nicer, automated build
>>    log scanners such as blhc aren't able to catch problems.
>
>debhelper today automatically disables silent rules when building
>on buildds. Using Q environment variables isn't the normal thing
>though.
>Even better than to explicitly disable silent build would be to
>hook up Q to the automatic debhelper version (V=1?).


Yeah, probably do something like

ifneq($(V),1)
Q?=@
endif

instead of just

Q?=@

in upstream's Makefile.

That said: I concur that these are all minor issues that can be fixed later and that d/copyright is the only blocker for an upload. And if this is to go into Stretch, the upload needs to happen today.

Since Andreas is willing to sponsor I'd recommend fixing that issue immediately and after Jan. 5th when it is in Stretch to fix the rest.

Regards,
Christian


Reply to: