On Thu, Nov 25 2021, Elijah Newren via GitGitGadget wrote:
> Removing the current working directory causes all subsequent git
> commands run from that directory to get confused and fail with a message
> about being unable to read the current working directory:
>
> $ git status
> fatal: Unable to read current working directory: No such file or directory
>
> Non-git commands likely have similar warnings or even errors, e.g.
>
> $ bash -c 'echo hello'
> shell-init: error retrieving current directory: getcwd: cannot access parent directories: No such file or directory
> hello
Okey, but...
> diff --git a/git.c b/git.c
> index 5ff21be21f3..2c98ab48936 100644
> --- a/git.c
> +++ b/git.c
> @@ -866,6 +866,8 @@ int cmd_main(int argc, const char **argv)
>
> trace_command_performance(argv);
>
> + startup_info->original_cwd = xgetcwd();
> +
> /*
> * "git-xxxx" is the same as "git xxxx", but we obviously:
> *
> diff --git a/setup.c b/setup.c
> index 347d7181ae9..f30657723ea 100644
> --- a/setup.c
> +++ b/setup.c
> @@ -432,6 +432,54 @@ void setup_work_tree(void)
> initialized = 1;
> }
>
> +static void setup_original_cwd(void)
> +{
> + struct strbuf tmp = STRBUF_INIT;
> + const char *worktree = NULL;
> + int offset = -1;
> +
> + /*
> + * startup_info->original_cwd wass set early on in cmd_main(), unless
> + * we're an auxiliary tool like git-remote-http or test-tool.
> + */
> + if (!startup_info->original_cwd)
> + return;
This really doesn't belong in those places, by calling xgetcwd() so
early you'll be making commands that don't care about RUN_SETUP at all
die. E.g. with this change:
mkdir x &&
cd x &&
rm -rf ../x &&
git version
Will die.
So as a follow-up to my comment on this v2's 01/09 I think what's also
missing here is something that does that, but instead of "git version"
does it for all of the "while read builtin" list in t0012-help.sh.
There's other cans of worms to be had here, the discussion downthread of
the not-integrated[1] points to some of that.
I.e. how should the various commands like "ls-remote" that can work
with/without a repo behave here? That one happens to die before/after,
but as noted in that thread that's also a bug.
So anything that adds more really early dying in this area should also
think about the greater goals of what we're doing with RUN_SETUP &
related flags. I.e. does the direction make sense?
If this is moved to some soft recording of the getcwd() (and maybe the
$PWD, as in my WIP change?) shouldn't this go into common-main.c? With
git.c's cmd_main() we're excluding the test helpers and things like
git-daemon. Is that intentional?
I'd also think we'd want to do this much earlier if e.g. thing like the
trace2 setup wanted to call the remove_directory() call.
Per what Glen & mentioned I'm still not sure if I'm on board with that
idea at all, just running with the ball you put in play :) I.e. if we're
modifying all callers, let's make sure we catch all callers.
Perhaps a better approach is to intercept chdir() instead? And anything
that calls chdir() sets some GIT_* variable so we'll pass "here's our
original cwd" down to sub-processes, like we do with "git -c" for
config.
That would presumably save you the effort of in-advance whitelisting
everything like "git version", we can just move to doing this lazily. If
you're a command that does a RUN_SETUP or otherwise needs chdir() we'll
record the original getcwd(), otherwise...
1. https://lore.kernel.org/git/patch-1.1-fc26c46d39-20210722T140648Z-avarab@xxxxxxxxx/
