Re: [PATCH v5 00/11] Avoid removing the current working directory, even if it becomes empty

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Dec 8, 2021, 10:23 UTC
Message-ID: <211208.86a6hbmlvj.gmgdl@evledraar.gmail.com>
In-Reply-To: <31bef4f9-2313-83a2-14e9-4ed1cc73a59c@gmail.com>

On Tue, Dec 07 2021, Derrick Stolee wrote:

Show 35 quoted lines

> On 12/7/2021 1:30 PM, Ævar Arnfjörð Bjarmason wrote:
>> 
>> On Tue, Dec 07 2021, Derrick Stolee wrote:
>>> test_expect_success 'remove cwd' '
>>> 	init_repos &&
>>>
>>> 	test_sparse_match git sparse-checkout set deep/deeper1/deepest &&
>>> 	for repo in sparse-checkout sparse-index
>>> 	do
>>> 		(
>>> 			cd $repo/deep/deeper1 &&
>>> 			test-tool getcwd >"$TRASH_DIRECTORY/expect" &&
>>> 			git sparse-checkout set &&
>>>
>>> 			test-tool getcwd >"$TRASH_DIRECTORY/actual" &&
>>> 			test_sparse_match git status --porcelain &&
>>> 			cd "$TRASH_DIRECTORY" &&
>>> 			test_cmp expect actual
>>> 		)
>>> 	done
>>> '
>>>
>>> Please do not let this test delay the advancement of this series. As we
>>> find these kinds of issues, we can fix them one-by-one as needed.
>> 
>> Not to pile on about "the core idea", just a question while this is
>> fresh in your mind:
>> 
>> I think that those cases would per [1] be ones where a more isolated
>> change of reading the $PWD from the environment would make all those
>> commands work as expected. Or would the "$TRASH_DIRECTORY" also
>> otherwise go away in this examples?
>
> I have read this message and the one you are referring two twice and
> I cannot understand what you are trying to say here.

I'm asking whether the WIP patch I posted at https://lore.kernel.org/git/211124.86sfvld4cl.gmgdl@evledraar.gmail.com/ would partially/entirely solve those issues you mentiened (but I see Elijah has a side-reply saying it might not in either approach).

Show 11 quoted lines

>> Anyway, just per [1] and the potential future follow-ups is this (I
>> don't think so, but maybe I'm wrong) or other examples you have things
>> that specifically need the "retain the getcwd()" part of this series?
>> 
>> Or just (as I think would be the case with that "git status") to not
>> have setup.c die quite as eagerly as it does now when getcwd() fails,
>> but it can find its way back to the .git via the environment's $PWD?
>
> Are you implying that Git will be the only thing broken by a missing
> directory after we leave in this state? I doubt that is true, and we
> should be good citizens here by leaving the directory around.

No, I'm just trying to clarify which specific thing we'd need in this/other cases to fix things *in git*. Because...

Show 7 quoted lines

>> There *are* definitely cases where not-just-that-setup.c change but also
>> the "don't remove the CWD" is an inherently better & more complete
>> solution. But I think that's mainly to do with 3rd party shellscripts &
>> other programs outside of our control.
>
> Exactly. We should take this change because it is valuable to not cause
> a confusing error in other tools.

...maybe we should fix things for other tools too, but for any future development it helps to know what was needed to fix what issues.

Show 6 quoted lines

>> I'm assuming that you were working with this on Windows, where
>> presumably there's fewer/none such shellscripts you rely on, but that's
>> now two presumes in a row, so... :)
>
> I'm working on Ubuntu, where I do all of my Git development unless there
> is a platform specific reason to do so.

*nod*, badly assumed then :)