Re: [RFC PATCH v4 0/3] fix `sudo make install` regression in maint

[Carlo Arenas <carenas@gmail.com>]

On Tue, May 10, 2022 at 7:17 AM Phillip Wood <phillip.wood123@gmail.com> wrote:
> On 07/05/2022 17:35, Carlo Marcelo Arenas Belón wrote:
> > A reroll for cb/path-owner-check-with-sudo with most of the suggestions
> > included but planned originally as an RFC, because I am frankly not sure
> > that I read and addressed all of it, but also because after seeing how
> > my only chance to get an official Reviewed-by: Junio vanished I also
> > realized that I wasn't clear enough and careful enough from the
> > beginning to explain the code correctly and therefore had maybe wasted
> > more of the review quota this change should require.
> >
> > Important changes (eventhough most are not affecting the logic)
> > * Document hopefully better which environments are supported and what
> >    to do if you want to test it in one that is not (thanks to dscho)
> > * Removed the arguably premature optimization to try to keep the sudo
> >    cache warm which was actually buggy, as it was also not needed.
> >    The CI does passwordless sudo unconditionally and even when running
> >    it locally it will go so fast that is shouldn't be an issue (thanks
> >    to Phillip)
> > * No longer using the ugly and controversial variable name so now it
> >    will need GIT_TEST_ENABLE_SUDO to be used to enable it on CI (which
> >    is not done in this series, but a run with it enabled on top of
> >    seen is available[1])
> > * Stop the arguing about what is or not a regression worth fixing and
> >    instead document it as one through a test, which would be easy to
> >    fix in a follow up since the code was already provided by Junio
>
> I've had a read of the patches and I agree with Junio's comments on the
> second patch.

Not sure which comments you are referring to, but
I'd implemented Junio's suggestion and removed the "too clever" (uid_t)-1 in v4.

> I'd really like us to avoid sudo in the tests if we can as
> it causes a lot of problems.

Even if it might not seem like it, I agree with you both (and dscho)
too, and if I could come out with a way to do so that would be still
secure, I would do it in a pinch, but I can't (at least until now) and
I don't want for that to hold up this fix so will be publishing soon a
v4 that still uses sudo in the tests, I am afraid.

> Just to let you know I'm going to be off
> the list for the next couple of weeks, so I wont be looking at these
> patches in that time.

Thanks for all your help reviewing them and more importantly improving
them, enjoy your time off.

Carlo