Re: [PATCH] pull: fix 'git pull --all' when current branch is tracking remote that is not last in the list of remotes

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Michael Lukashov <michael.lukashov@xxxxxxxxx> writes:

> On Wed, Feb 24, 2010 at 2:02 AM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
>> Michael Lukashov <michael.lukashov@xxxxxxxxx> writes:
>>
>>> diff --git a/git-pull.sh b/git-pull.sh
>>> index 38331a8..fcde096 100755
>>> --- a/git-pull.sh
>>> +++ b/git-pull.sh
>>> @@ -214,7 +214,11 @@ test true = "$rebase" && {
>>>       done
>>>  }
>>>  orig_head=$(git rev-parse -q --verify HEAD)
>>> -git fetch $verbosity --update-head-ok "$@" || exit 1
>>> +if test -e "$GIT_DIR"/FETCH_HEAD
>>> +then
>>> +     rm "$GIT_DIR"/FETCH_HEAD 2>/dev/null
>>> +fi
>>
>> When is it sane to ignore an error from this "rm", especially after you
>> made sure that it exists?
>
> The file "$GIT_DIR"/FETCH_HEAD is rewritten
> in subsequent call to 'git fetch', thus it is safe to ignore all errors.

You are not answering my question.

You found out that the thing exists, and you want to overwrite it later.
You _need_ that file to either not exist, or at least be empty, because
you will be _appending_ to it, unlike the earlier code.

Now, you expected you would be able to remove it, and that is why you
called "rm".  Suppose that removal has failed for some reason.  The file
stays.  It is not emptied, either.

Why is it sane to ignore that error and let fetch --append to run, as if
it is starting from either non-existing file or an empty one?  You already
diagnosed that the file is in some _funny_ state.  It is not sensible to
continue further at that point, knowing that there is something wrong.

If the new code you introduced were

	rm -f "$GIT_DIR/FETCH_HEAD" || exit

then I would understand it.  But your patch doesn't make sense to me;
neither your "thus it is safe".
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]