Re: [PATCH] mergetool: clean up temp files when aborted

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 24, 2013, 19:54 UTC
Message-ID: <7vbocebblo.fsf@alter.siamese.dyndns.org>
In-Reply-To: <1359051829-21331-1-git-send-email-hordp@cisco.com>

Phil Hord <hordp@cisco.com> writes:

Show 31 quoted lines

> When handling a symlink conflict or a deleted-file conflict, mergetool
> stops to ask the user what to do. If the user chooses any option besides
> "(a)bort", then the temporary files which mergetool created in
> preparation for handling the conflict are removed.  But these temporary
> files are not removed when the user chooses to abort the operation.
>
>     $ git cherry-pick other/branch
>     error: could not apply 4e43581... Fix foo.c
>
>     $ git status --short
>     DU foo.c
>
>     $ git mergetool
>     Merging:
>     foo.c
>
>     Deleted merge conflict for 'foo.c':
>       {local}: deleted
>       {remote}: modified file
>     Use (m)odified or (d)eleted file, or (a)bort? a
>     Continue merging other unresolved paths (y/n) ? n
>
>     $ git status --short
>     DU foo.c
>     ?? foo.c.BACKUP.16929.c
>     ?? foo.c.BASE.16929.c
>     ?? foo.c.LOCAL.16929.c
>     ?? foo.c.REMOTE.16929.c
>
> These temporary files should not remain after the mergetool operation is
> completed.

Aren't there cases where people "abort" so that they can have a chance inspect them outside mergetool program? If there are no such cases, then I would agree with your claim "should not remain", but the proposed log message does not explay why it is sure that there are no such use cases.

Show 34 quoted lines

>
> Remove the temporary files by calling the cleanup_temp_files when the
> user chooses to abort the mergetool operation.
>
> It looks like 'cleanup_temp_files' without the --save-backups option is
> the correct thing to do, and this is how this commit is implemented. But
> some other paths do use --save-backups resulting in a foo.c.orig file
> being left behind.  That seems to be a different bug, though.
>
> Signed-off-by: Phil Hord <hordp@cisco.com>
> ---
>  git-mergetool.sh | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/git-mergetool.sh b/git-mergetool.sh
> index c50e18a..bb93b70 100755
> --- a/git-mergetool.sh
> +++ b/git-mergetool.sh
> @@ -90,6 +90,7 @@ resolve_symlink_merge () {
>  			return 0
>  			;;
>  		[aA]*)
> +			cleanup_temp_files
>  			return 1
>  			;;
>  		esac
> @@ -118,6 +119,7 @@ resolve_deleted_merge () {
>  			return 0
>  			;;
>  		[aA]*)
> +			cleanup_temp_files
>  			return 1
>  			;;
>  		esac