Re: FYI: git-am allows creation of empty commits.
ebiederm@xmission.com (Eric W. Biederman) writes:
Show 6 quoted lines
> Is this something that we always want to test for in the porcelain
> or do we want to move a check into git-commit-tree?
>
> For getting a reasonable error message where you have the test
> seems to be the only sane place, but having the check deeper
> down would be more likely to catch this kind of thing.
I think 99.9% of the time it is a mistake if a single-parented commit has the same tree as its parent commit has, so having a check in commit-tree may not be a bad idea.
If you want to do it, however, you need to be a bit careful about merges. In a multi-parented commit, it is legitimate if the merge result exactly match one of the parents (in fact "git-merge -s ours" can be used to create a merge that matches its first parent).
The commit-tree program is one of the oldest part of the system, and as a mechanism, does not set nor enforce policies like this. It has been more liberal than the best current practice, such as (1) don't do single-parent empty commit; (2) don't list the same parent twice. The second one was introduced in mid-June last year, which is "long after it was invented" in git timescale.
On the other hand, it is more anal than it could be. It does not take a tag that points to a commit to its -p parameter. That's because we did not have tag objects in the beginning, and by the time tags were introduced, nobody ran commit-tree by hand anymore.
-- >8 -- [PATCH] commit-tree: disallow an empty single-parent commit.
Also allow "git-commit-tree -p v2.6.16-rc2", instead of having to say "git-commit-tree -p v2.6.16-rc2^0".
Signed-off-by: Junio C Hamano <junkio@cox.net>
---
diff --git a/commit-tree.c b/commit-tree.c
index 88871b0..14a7552 100644
--- a/commit-tree.c
+++ b/commit-tree.c
@@ -45,12 +45,14 @@ static void check_valid(unsigned char *s
{
void *buf;
char type[20];
+ unsigned char real_sha1[20];
unsigned long size;
- buf = read_sha1_file(sha1, type, &size);
- if (!buf || strcmp(type, expect))
+ buf = read_object_with_reference(sha1, expect, &size, real_sha1);
+ if (!buf)
die("%s is not a valid '%s' object", sha1_to_hex(sha1), expect);
free(buf);
+ memcpy(sha1, real_sha1, 20);
}
/*
@@ -75,6 +77,19 @@ static int new_parent(int idx)
return 1;
}
+static int check_empty_commit(const unsigned char *tree_sha1,
+ const unsigned char *parent_sha1)
+{
+ unsigned char sha1[20];
+ char refit[50];
+ sprintf(refit, "%s^{tree}", sha1_to_hex(parent_sha1));
+ if (get_sha1(refit, sha1))
+ die("cannot determine tree in parent commit.");
+ if (!memcmp(sha1, tree_sha1, 20))
+ return error ("empty commit? aborting.");
+ return 0;
+}
+
int main(int argc, char **argv)
{
int i;
@@ -105,6 +120,9 @@ int main(int argc, char **argv)
}
if (!parents)
fprintf(stderr, "Committing initial tree %s\n", argv[1]);
+ if (parents == 1)
+ if (check_empty_commit(tree_sha1, parent_sha1[0]))
+ exit(1);
init_buffer(&buffer, &size);
add_buffer(&buffer, &size, "tree %s\n", sha1_to_hex(tree_sha1));
Eric W. Biederman