Re: [PATCH v2 2/3] fast-import: allow "merge $null_sha1" command
From: Jonathan Nieder <hidden>
Date: 2016-06-15 22:54:11
Dmitry Ivankov wrote:
"from $null_sha1" and "merge $empty_branch" are already allowed so allow "merge $null_sha1" command too.
The reader might not realize that null_sha1 means 0000000000000000000000000000000000000000 until she reads the test script. Is it possible to help her save time? [...]
quoted hunk ↗ jump to hunk
--- a/fast-import.c +++ b/fast-import.c@@ -2631,12 +2631,14 @@ static struct hash_list *parse_merge(unsigned int *count) die("Mark :%" PRIuMAX " not a commit", idnum); hashcpy(n->sha1, oe->idx.sha1); } else if (!get_sha1(from, n->sha1)) { - unsigned long size; - char *buf = read_object_with_reference(n->sha1, - commit_type, &size, n->sha1); - if (!buf || size < 46) - die("Not a valid commit: %s", from); - free(buf); + if (!is_null_sha1(n->sha1)) { + unsigned long size; + char *buf = read_object_with_reference(n->sha1, + commit_type, &size, n->sha1); + if (!buf || size < 46) + die("Not a valid commit: %s", from); + free(buf); + }
Hm, ok. Maybe the "peel onion" call guarded by this "if" could be a
separate function to make this cleaner (and avoid some duplication of
code with other functions while at it)?
e.g.,
static int peel_to_commit(unsigned char sha1[20])
{
unsigned long size;
char *buf = read_object_with_reference(...);
if (!buf)
return -1;
free(buf);
if (size < strlen("commit ") + 40)
return -1;
return 0;
}
...
if (is_null_sha1(n->sha1))
; /* ok */
else if (peel_to_commit(n->sha1))
die("Not a valid commit: %s", from);
I like the direction, but as it is, this patch feels kind of "meh" to
me.
Thanks again and hope that helps,
Jonathan