On 17/08/15 21:53, Oxan van Leeuwen wrote:
> Hi,
>
> On 15-08-15 12:21, Tomasz Buchert wrote:
> >I've tested the AppArmor profile too, it looks fine, although I'm not
> >sure if 'm' is needed in the profile for '/usr/sbin/postsrsd', since
> >it seems to work just fine without it. I've a rather basic knowledge
> >about AppArmor, so if you could explain it to me, I'd be grateful.
> I'm not sure about it either, I just copied the AppArmor profile from
> upstream. Judging from the AppArmor docs it's indeed not needed, I'll do
> some further research tomorrow and remove it if possible.
Hi,
great! Just nit-picking here, really. And trying to understand
AppArmor :).
>
> >And the last thing: in the systemd unit, you do:
> >
> > -d$${SRS_DOMAIN:-$$(postconf -h mydomain 2>/dev/null)}
> >
> >Well, first if SRS_DOMAIN is set to something that it's fine, if it's not,
> >then postconf is used to get the mail server domain. But postconf may not
> >be present! You probably need to depend on postfix, unless postsrsd can be used
> >with other mail software.
> Technically postsrsd can be used with other mail servers too, though I doubt
> it will happen in practice. Is it really a problem if postconf is called
> when it's not present, though?
Yes, I tried without postconf present and the unit failed.
> Its output is silenced, and postsrsd fails
> when the -d argument is empty anyway.
I don't think you want this systemd unit to fail *by default*.
>
> >I'd also recommend using EnvironmentFile in your systemd unit [1].
> The systemd unit is already loading /etc/default/postsrsd as
> EnvironmentFile, it's just the defaults that are in the systemd unit. Not
> sure why I did that though (I believe we usually don't support removing
> files from /etc/default), so they can be safely removed. I'll push a fix.
Sorry, I didn't notice that. I agree, though, that it would make sense
to put all configuation in one place.
>
> >I also pushed some minor fixes.
> Are you sure? I don't see any new commits in the collab-maint repository.
>
No, I'm not sure ;). It's pushed right now (sometimes I miss svn, where
when you commit you also push ;) ).
> >When this is done, I'll be happy to push your package,
> >I already use it myself for some time :).
> That's great!
>
> Cheers,
> Oxan
>
Cheers,
Tomasz
Attachment:
signature.asc
Description: Digital signature