On 29-08-2022 15:30, Thompson, David wrote:
On Mon, Aug 29, 2022 at 9:19 AM Maxime Devos
wrote:
On 29-08-2022 14:57, Thompson, David wrote:
> I disagree. I believe we shouldn't let perfect be the enemy of
the good.
I don't think your patch counts as "good" here -- while fixing the
bug
counts as "good", you are at the same time introducing a new bug (the
non-atomicity), which is bad. You would have to weigh the
goodness and
the badness to end up with an overall "good" (or maybe "bad",
depending
on the conclusion), but I'd think that the time required to do such a
weighing is better spent by doing a tiny bit of extra effort to
implement the new field (it should be very low effort, see other
response).
My patch has a very limited scope of only changing the gitolite
service. Your proposal has a much greater scope of modifying a core
structure used for system configuration.
It is a greater scope, but it's not really more effort.
The new bug you mention is only bad in a theoretical sense. In
practice, the permission bits are misconfigured for a blip of time
during system reconfiguration, which is a lot better than being
misconfigured all the time which is the status quo. It's the
difference between a gitolite that works nicely with cgit/gitweb and
one that doesn't. I agree that it's a good goal to improve atomicity
and I think making more general to allow for different
permission bits on the home directory is a good idea, but I see it as
one step removed from fixing this particular bug.
The time required to analyse it as "just theoretical" could have been
spent doing the tiny bit of extra effort.
Theoretical bugs like these are especially nasty, if you encounter them
there is often not a clue what the cause is unless you already know what
to look for.
My patch follows the recommended approach outlined in a comment in
(gnu build activation) written by Ludovic in 2019:
;; Always set ownership and permissions for home directories of
system
;; accounts. If a service needs looser permissions on its home
;; directories, it can always chmod it in an activation snippet.
I've refuted that recommendation (albeit without explicitly mentioning
that paragraph), that paragraph is a bug, see my previous comments on
non-atomicity. Please remove it in the v2 patch.
As there appears to be a lack of willingness to invest the tiniest bit
of extra effort to implement a proper patch, and given the length of
previous discussion, I think my time will be better spent continuing
fixing things in Guix rather than any failing attempts at convincing
you. As such, I'll stop responding until a v2 or questions on how to
implement a v2, but that cannot be interpreted as me agreeing with you.
Greetings,
Maxime
OpenPGP_0x49E3EE22191725EE.asc
Description: OpenPGP public key
OpenPGP_signature
Description: OpenPGP digital signature
