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?
[...]
> --- 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
--
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