Patrick Steinhardt <ps@xxxxxx> writes:
> The git-p4 tool creates a bunch of temporary branches that share a
> common prefix "refs/git-p4-tmp/". These branches get cleaned up via
> git-update-ref(1) after the import has finished. Once done, we try to
> manually remove the now supposedly-empty ".git/refs/git-p4-tmp/"
> directory.
>
> This last step can fail in case there still are any temporary branches
> around that we failed to delete because `os.rmdir()` refuses to delete a
> non-empty directory. It can thus be seen as kind of a sanity check to
> verify that we really did delete all temporary branches.
Wow, thanks for being very careful. I would just have said "there
is no need for such rmdir---what's a single empty unused directory
between friends, which will be reused later when you run 'git p4'
again?" without thinking things through.
> This is a modification in behaviour for the "files" backend because the
> empty directory does not get deleted anymore. But arguably we should not
> care about such implementation details of the ref backend anyway, and
> this should not cause any user-visible change in behaviour.
Independently, it would be sensible to improve the files backend so
that it removes a subdirectory outside the hierarchies that are
created by refs/files-backend.c:files_init_db() when it becomes
empty (#leftoverbits).
And such a change would also have triggered the error from
os.rmdir(), so this patch is doubly welcomed.
Thanks.
> Signed-off-by: Patrick Steinhardt <ps@xxxxxx>
> ---
> git-p4.py | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/git-p4.py b/git-p4.py
> index 0eb3bb4c47..3ea1c405e5 100755
> --- a/git-p4.py
> +++ b/git-p4.py
> @@ -4251,7 +4251,8 @@ def run(self, args):
> if self.tempBranches != []:
> for branch in self.tempBranches:
> read_pipe(["git", "update-ref", "-d", branch])
> - os.rmdir(os.path.join(os.environ.get("GIT_DIR", ".git"), self.tempBranchLocation))
> + if len(read_pipe(["git", "for-each-ref", self.tempBranchLocation])) > 0:
> + die("There are unexpected temporary branches")
>
> # Create a symbolic ref p4/HEAD pointing to p4/<branch> to allow
> # a convenient shortcut refname "p4".
