Re: [PATCH 5/6] Comment important codepaths regarding nuking untracked files/dirs

[Luke Diamand <luke@diamand.org>]

On Sun, 19 Sept 2021 at 00:15, Elijah Newren via GitGitGadget
<gitgitgadget@gmail.com> wrote:
>
> From: Elijah Newren <newren@gmail.com>
>
> In the last few commits we focused on code in unpack-trees.c that
> mistakenly removed untracked files or directories.  There may be more of
> those, but in this commit we change our focus: callers of toplevel
> commands that are expected to remove untracked files or directories.
>
> As noted previously, we have toplevel commands that are expected to
> delete untracked files such as 'read-tree --reset', 'reset --hard', and
> 'checkout --force'.  However, that does not mean that other highlevel
> commands that happen to call these other commands thought about or
> conveyed to users the possibility that untracked files could be removed.
> Audit the code for such callsites, and add comments near existing
> callsites to mention whether these are safe or not.
>
> My auditing is somewhat incomplete, though; it skipped several cases:
>   * git-rebase--preserve-merges.sh: is in the process of being
>     deprecated/removed, so I won't leave a note that there are
>     likely more bugs in that script.
>   * contrib/git-new-workdir: why is the -f flag being used in a new
>     empty directory??  It shouldn't hurt, but it seems useless.
>   * git-p4.py: Don't see why -f is needed for a new dir (maybe it's
>     not and is just superfluous), but I'm not at all familiar with
>     the p4 stuff

Assuming you're talking about this code in git-p4.py:

            print("Synchronizing p4 checkout...")
            if new_client_dir:
                # old one was destroyed, and maybe nobody told p4
                p4_sync("...", "-f")
            else:
                p4_sync("...")

This is doing a Perforce sync in the P4 repo, not the git repo.

In the usual/happy case, this directory already exists, the Perforce
server knows about its state, and a normal "p4 sync ..." will bring it
up to date.

But, if someone manually deleted the directory then "p4 sync ..." will
only update modified files, and all sorts of things will then go wrong
(e.g. the files we updated in the git view won't be present, and
git-p4 will fall flat on its face).

So in this case, do a forced sync, which syncs everything ignoring the
P4 server's idea of what files are/not present.

Luke