From: Felipe Contreras <felipe.contreras@gmail.com>
To: Max Horn <max@quendi.de>
Cc: git@vger.kernel.org, Sverre Rabbelier <srabbelier@gmail.com>,
Richard Hansen <rhansen@bbn.com>
Subject: Re: [PATCH v5 09/10] fast-export: add support to delete refs
Date: Thu, 31 Oct 2013 13:41:15 -0600 [thread overview]
Message-ID: <CAMP44s3vxfN5Wc+S+VjimisUgZGQMzR7Z2YkO8J4UEk7+cPy_Q@mail.gmail.com> (raw)
In-Reply-To: <6F276334-DFDB-40B8-8B24-38FFB6DBED9B@quendi.de>
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.
> 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.
> I have not yet tried to dive into the code to figure out what actually happens, but I assume Felipe did that resp. tested it, and so supposedly doing this works, at least for git resp. existing transport helpers. But I wonder if other implementers of the fast-import format would handle it properly?
Try this:
--- a/t/t9350-fast-export.sh
+++ b/t/t9350-fast-export.sh
@@ -519,7 +519,9 @@ test_expect_success 'delete refspec' '
from 0000000000000000000000000000000000000000
EOF
- test_cmp expected actual
+ test_cmp expected actual &&
+ git fast-import < actual &&
+ test_must_fail git rev-parse -q --verify refs/heads/to-delete
'
test_done
--
Felipe Contreras
next prev parent reply other threads:[~2013-10-31 19:41 UTC|newest]
Thread overview: 45+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-10-31 9:36 [PATCH v5 10/10] transport-helper: add support to delete branches Felipe Contreras
2013-10-31 9:36 ` [PATCH v5 00/10] transport-helper: updates Felipe Contreras
2013-10-31 17:59 ` Max Horn
2013-10-31 18:10 ` Junio C Hamano
2013-10-31 18:30 ` Felipe Contreras
2013-10-31 18:34 ` Junio C Hamano
2013-10-31 18:41 ` Max Horn
2013-11-11 5:09 ` [PATCH v5 11/10] test-hg.sh: tests are now expected to pass Richard Hansen
2013-11-11 5:10 ` [PATCH v5 12/10] remote-bzr: support the new 'force' option Richard Hansen
2013-11-11 11:51 ` Felipe Contreras
2013-11-11 18:12 ` Richard Hansen
2013-11-11 18:15 ` Felipe Contreras
2013-10-31 9:36 ` [PATCH v5 01/10] transport-helper: fix extra lines Felipe Contreras
2013-10-31 18:16 ` Junio C Hamano
2013-10-31 9:36 ` [PATCH v5 08/10] fast-import: add support to delete refs Felipe Contreras
2013-10-31 18:41 ` Max Horn
2013-10-31 9:36 ` [PATCH v5 05/10] fast-export: improve argument parsing Felipe Contreras
2013-10-31 9:36 ` [PATCH v5 06/10] fast-export: add new --refspec option Felipe Contreras
2013-10-31 18:26 ` Junio C Hamano
2013-10-31 18:41 ` Felipe Contreras
2013-11-06 21:00 ` Junio C Hamano
2013-11-06 22:14 ` Jeff King
2013-11-06 22:31 ` Junio C Hamano
2013-11-06 22:41 ` Junio C Hamano
2013-11-08 23:44 ` Junio C Hamano
2013-11-09 0:06 ` Jeff King
2013-11-07 1:00 ` Felipe Contreras
2013-10-31 9:36 ` [PATCH v5 03/10] transport-helper: add 'force' to 'export' helpers Felipe Contreras
2013-10-31 18:21 ` Junio C Hamano
2013-10-31 18:55 ` Felipe Contreras
2013-10-31 19:07 ` Junio C Hamano
2013-11-01 14:49 ` Junio C Hamano
2013-11-01 15:37 ` Felipe Contreras
2013-11-01 16:35 ` Junio C Hamano
2013-11-01 17:28 ` Eric Sunshine
2013-11-11 5:01 ` [PATCH] fixup! " Richard Hansen
2013-10-31 9:36 ` [PATCH v5 09/10] fast-export: add support to delete refs Felipe Contreras
2013-10-31 19:29 ` Max Horn
2013-10-31 19:41 ` Felipe Contreras [this message]
2013-10-31 19:47 ` Max Horn
2013-10-31 19:53 ` Felipe Contreras
2013-10-31 22:38 ` Max Horn
2013-10-31 9:36 ` [PATCH v5 07/10] transport-helper: add support for old:new refspec Felipe Contreras
2013-10-31 9:36 ` [PATCH v5 04/10] transport-helper: check for 'forced update' message Felipe Contreras
2013-10-31 9:36 ` [PATCH v5 02/10] transport-helper: don't update refs in dry-run Felipe Contreras
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAMP44s3vxfN5Wc+S+VjimisUgZGQMzR7Z2YkO8J4UEk7+cPy_Q@mail.gmail.com \
--to=felipe.contreras@gmail.com \
--cc=git@vger.kernel.org \
--cc=max@quendi.de \
--cc=rhansen@bbn.com \
--cc=srabbelier@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).