Re: [PATCH 2/7] rebase -i: don't error out if $state_dir already exists

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 23, 2013, 16:35 UTC
Message-ID: <7vd2tljjcx.fsf@alter.siamese.dyndns.org>
In-Reply-To: <1366725724-1016-3-git-send-email-artagnon@gmail.com>

Ramkumar Ramachandra <artagnon@gmail.com> writes:

> Practically speaking, the only reason why a `mkdir $state_dir` would
> fail is because $state_dir already exists.  There is no problem in
> this case, and we can proceed as usual.

That was started as a cheap way to detect and avoid running the same rebase -i while another one is already in progress, way back when we did not have a separate "git-rebase--*backend*" scripts, I think.

If we have a separate safety measure to prevent it, lifting it may be safe, but in a sane case, $state_dir should _not_ exist when we start the command, and mkdir should _not_ fail to create it.

So I do not see why this change could be an improvement. I would understand it if some future changes may have already created the state dir from a separate codepath (which may be an indication of a poor design of integrating that separate codepath, but that is a different matter) and mkdir needs to be turned into "mkdir -p" here, but I do not think removal of "|| die" is warranted even in such a case.

Show 21 quoted lines

>  So, change the `mkdir` call
> to `mkdir -p`, and strip the `|| die`.
>
> Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
> ---
>  git-rebase--interactive.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
> index 048a140..cc3a9a7 100644
> --- a/git-rebase--interactive.sh
> +++ b/git-rebase--interactive.sh
> @@ -837,7 +837,7 @@ then
>  fi
>  
>  orig_head=$(git rev-parse --verify HEAD) || die "No HEAD?"
> -mkdir "$state_dir" || die "Could not create temporary $state_dir"
> +mkdir -p "$state_dir"
>  
>  : > "$state_dir"/interactive || die "Could not mark as interactive"
>  write_basic_state