Re: [PATCH v5 09/10] fast-export: add support to delete refs
On 31.10.2013, at 20:41, Felipe Contreras <felipe.contreras@gmail.com> wrote:
Show 33 quoted lines
> On Thu, Oct 31, 2013 at 1:29 PM, Max Horn <max@quendi.de> wrote:
>> Actually, I just noticed one thing that I *do* have a question about:
>>
>> On 31.10.2013, at 10:36, Felipe Contreras <felipe.contreras@gmail.com> wrote:
>>
>>> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
>>> ---
>>> builtin/fast-export.c | 14 ++++++++++++++
>>> t/t9350-fast-export.sh | 11 +++++++++++
>>> 2 files changed, 25 insertions(+)
>>>
>>> diff --git a/builtin/fast-export.c b/builtin/fast-export.c
>>> index b6f623e..8ed41b4 100644
>>> --- a/builtin/fast-export.c
>>> +++ b/builtin/fast-export.c
>>> @@ -673,6 +673,19 @@ static void import_marks(char *input_file)
>>> fclose(f);
>>> }
>>>
>>> +static void handle_deletes(void)
>>> +{
>>> + int i;
>>> + for (i = 0; i < refspecs_nr; i++) {
>>> + struct refspec *refspec = &refspecs[i];
>>> + if (*refspec->src)
>>> + continue;
>>> +
>>> + printf("reset %s\nfrom %s\n\n",
>>> + refspec->dst, sha1_to_hex(null_sha1));
>>
>> If I understand it right, this issues a "reset" command in the fast-import stream, resetting a ref to an all-zero SHA1. I had a look at the git-fast-import documentation, but I found that it does not explicitly cover this case. In particular, the "reset" command does not specify that an all-zero SHA1 should be treated as "delete this ref".
>
> That's what the previous patch does.Right *facepalm*.
But then this should be documented in git-fast-import.txt, shouldn't it?
>
>> On the other hand, the docs for "reset" seem to indicate that one can omit the "from" part, although I couldn't tell for sure what that would mean, either.
>
> It means something different.
Yeah, I figured that -- just wanted to point out that this, too, is not very clear in the documentation and should be improved (not saying that I expect you to do that, just pointing it out).
Max Horn <max@quendi.de>