Re: en/keep-cwd (Was: Re: What's cooking in git.git (Dec 2021, #01; Fri, 3))

[Elijah Newren <newren@gmail.com>]

On Tue, Dec 7, 2021 at 1:25 AM Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:
>
> On Mon, Dec 06 2021, Elijah Newren wrote:
>
> > On Mon, Dec 6, 2021 at 5:57 PM Junio C Hamano <gitster@pobox.com> wrote:
> >>
> >> * en/keep-cwd (2021-12-01) 11 commits
> >>  - t2501: simplify the tests since we can now assume desired behavior
> >>  - dir: new flag to remove_dir_recurse() to spare the original_cwd
> >>  - dir: avoid incidentally removing the original_cwd in remove_path()
> >>  - stash: do not attempt to remove startup_info->original_cwd
> >>  - rebase: do not attempt to remove startup_info->original_cwd
> >>  - clean: do not attempt to remove startup_info->original_cwd
> >>  - symlinks: do not include startup_info->original_cwd in dir removal
> >>  - unpack-trees: add special cwd handling
> >>  - unpack-trees: refuse to remove startup_info->original_cwd
> >>  - setup: introduce startup_info->original_cwd
> >>  - t2501: add various tests for removing the current working directory
> >>
> >>  Many git commands that deal with working tree files try to remove a
> >>  directory that becomes empty (i.e. "git switch" from a branch that
> >>  has the directory to another branch that does not would attempt
> >>  remove all files in the directory and the directory itself).  This
> >>  drops users into an unfamiliar situation if the command was run in
> >>  a subdirectory that becomes subject to removal due to the command.
> >>  The commands have been taught to keep an empty directory if it is
> >>  the directory they were started in to avoid surprising users.
> >
> > Very nicely written summary.
> >
> >>
> >>  Needs review.
> >>  There are some comments on earlier rounds; the latest one needs a
> >>  serious review or at least Acks from past commentors.
> >>  source: <pull.1140.v5.git.git.1638340854.gitgitgadget@gmail.com>
> >
> > If it helps, there are two parts to the review:
> > - Do we want this feature?
> > - Does this patch series implement the feature correctly?
...
> So I've got no objections to these changes going in as they stand.

Thanks.

> If
> anyone is interested in pulling at the "let's not make it die" thread
> that can always be done later, or we can make it configurable or
> whatever.
>
> I don't think this needs to be part of your initial series, but just a
> question:
>
> Isn't a logical follow-up after it to change the various things like
> "git bisect that refuse to run in subdirectories to happily do so?
> I.e. the reason for those is specifically this "cwd goes away". Both
> because of our bad handling of it, and due to any user confusion that'll
> be addressed by that cwd sticking around".

Both sound like potentially relevant logical follow-ups.