On 11/26/2021 5:40 PM, Elijah Newren via GitGitGadget wrote: > From: Elijah Newren <newren@xxxxxxxxx> > > Since rebase spawns a `checkout` subprocess, make sure we run that from > the startup_info->original_cwd directory, so that the checkout process > knows to protect that directory. > > Signed-off-by: Elijah Newren <newren@xxxxxxxxx> > --- > sequencer.c | 3 +++ > t/t2501-cwd-empty.sh | 4 ++-- > 2 files changed, 5 insertions(+), 2 deletions(-) > > diff --git a/sequencer.c b/sequencer.c > index ea96837cde3..b71f7b8a0a6 100644 > --- a/sequencer.c > +++ b/sequencer.c > @@ -4228,6 +4228,9 @@ static int run_git_checkout(struct repository *r, struct replay_opts *opts, > > cmd.git_cmd = 1; > > + if (startup_info->original_cwd && > + !is_absolute_path(startup_info->original_cwd)) > + cmd.dir = startup_info->original_cwd; I was initially confused by the "!is_absolute_path()" because it seemed to me like it would be natural to store an absolute path there, but I see this comment in patch 2: + * For convience, we would like to have the path relative to the + * worktree instead of an absolute path. So it seems that we won't store it as an absolute path. Is there any value in this condition, then? This assignment of cmd.dir to the relative path has a lot of baked-in knowledge of this variable _and_ the current state (Git chdir()'d to the root of the worktree). If the path is always relative, then it should be a BUG() if we see an absolute path. Also, it seems like we would want cmd.dir to be a concatenation of the worktree root and the original_cwd. Or perhaps I'm being overly cautious and this could all be resolved with a comment about the expected state of the working directory and original_cwd. The tests will catch if any of those expectations change. Thanks, -Stolee
