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

From: Elijah Newren <newren@gmail.com>
Date: Dec 7, 2021, 06:00 UTC
Message-ID: <CABPp-BGdvOhy_g8vtRogqL2vPkZEtP4+N_5x0rhAWrX9x43WWA@mail.gmail.com>
In-Reply-To: <xmqqh7bpqhf0.fsf@gitster.g>

On Mon, Dec 6, 2021 at 5:57 PM Junio C Hamano <gitster@pobox.com> wrote:

Show 22 quoted lines

>
> * 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.

Show 5 quoted lines

>
>  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?
Let me mention both:

Much of the discussion from commenters was actually related to the first point. While Peff suggested the idea, and Taylor and Phillip (Wood) spoke up in favor (and I obviously cared enough to write patches), Ævar didn't like it at first. After a lot of back and forth, we eventually discovered some misunderstanding after which Ævar, while still not a proponent, dropped his strong objection ("I'm much more sympathetic to this approach now."[1]) I was unable to determine the opinion of other reviewers/commenters on this point, though if I had to guess I'd say Junio is at least marginally in favor.

[1] https://lore.kernel.org/git/211129.868rx6g0e0.gmgdl@evledraar.gmail.com/

On the latter point, several folks provided useful suggestions. I think I have addressed all their feedback so far, though no one has explicitly verified that. Further review and/or acks to verify that I have indeed handled feedback to others' satisfaction would be welcome if anyone has the spare time.