On Tue, May 10, 2022 at 4:44 PM Junio C Hamano <gitster@xxxxxxxxx> wrote: > > Carlo Arenas <carenas@xxxxxxxxx> 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
