Re: [PATCH v4 1/3] t: regression git needs safe.directory when using sudo

[Carlo Arenas <carenas@gmail.com>]

On Tue, May 10, 2022 at 4:44 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> Carlo Arenas <carenas@gmail.com> writes:
> >>
> >> Mental note.  We do not need root to be owned by 'root' with the
> >> tests we see here.  Perhaps we would add some that requires it in
> >> later patches.  We'll see.
> >
> > I am not good with subtle messages in a foreign language, but is this
> > a way to imply that I shouldn't need to chown and instead use the
> > GIT_TEST_PRETEND feature more?
>
> No.  I just said I made a mental note of the fact that the ownership
> of root/ directory (not root/r which is the other directory this
> test script creates in this step) does not matter at least in patch
> 1/3, but I cannot say, by that observation alone, that chown we saw
> earlier should be removed, because patches 2/3 and 3/3 might start
> requiring root/ to be owned by 'root'.  Hence "I made a mental note
> here.  We do not have anything that requires the above chown in this
> patch, but we might see something that makes it a good idea to keep
> the chown in later patches".

got it; we actually DO (or at least I intended to, but didn't because
of a bug and complicating the tests unnecessarily).but then as usual I
didn't document it well, apologize for that.

`root` is called that way because it was meant to be a ceiling of
sorts and used as a safetynet (like GIT_CEILING in the test suite) to
block the tests in this file for going up one more level and using the
default git repository from the suite by mistake.  That is also why it
is owned by root.

of course later I find out that for it to really stop the walking it
needed a .git on its own and to create one I needed to do it as root
which I didn't even attempt to do until patch 3 (at that time too, I
was thinking maybe I could get patch 1 and 2 ready first, so my name
wouldn't be on every git update and one of the reasons why nobody can
get anything merged into next)

one option would be to consolidate its use with the root repository
that gets created in patch 3, which I could have done originally
instead of just using the ones you provided almost AS-IS, or as you
pointed out we could remove the safety net since it is not needed and
it is buggy anyway.

will remove the chown in v5 otherwise but I think the plan above
shouldn't be that intrusive and might be better.

Carlo