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. > 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. > 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. > 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. Thanks, -Stolee
