Re: Trying to push into empty repo, get fatal: bad revision 'HEAD'

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 1, 2015, 01:36 UTC
Message-ID: <xmqq8uecocjg.fsf@gitster.dls.corp.google.com>
In-Reply-To: <xmqqego4ogxw.fsf@gitster.dls.corp.google.com>

Junio C Hamano <gitster@pobox.com> writes:

Show 16 quoted lines

> Samuel Williams <space.ship.traveller@gmail.com> writes:
>
>> I would expect if you push to an empty repo, it would update it
>> (because denyCurrentBranch = updateInstead).
>
> Good finding.
>
> I think the current implementation of updateInstead is set up to
> bootstrap from an empty repository but only supports incremental
> updates once the receiving repository and its working tree gets set
> up.  But I do not think it was a conscious design decison to forbid
> bootstrapping an empty repository, but was a mere gap in the
> implementation.  At least, I do not think of a reason why we should
> forbid it (and I am Cc'ing Dscho to confirm).
>
> Fixing it should not be too hard, but I am on a bus right now so...

A fix (or is it an enhancement) would probably look like this.

This is a tangent but I think we should unify the "do we already have history behind HEAD, or is the current branch still unborn" test done by various commands and tighten it. As a quick and dirty hack, I just mimicked what builtin/merge.c seems to do, but this would tell a detached HEAD that points at a nonsense object name (i.e. "abcde" not a full 40-hex) as "unborn", where we would be better off stopping the operation instead of making the repository breakage worse by doing further damage.

I originally suspected I'd need to fix the push_to_checkout() codepath, too, but it turns out that the detection of unborn-ness of the current branch is also outsourced to the push-to-checkout hook, so I do not have to do anything special ;-)

 builtin/receive-pack.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index fc8ec9c..758b0b3 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -733,6 +733,13 @@ static int update_shallow_ref(struct command *cmd, struct shallow_info *si)
 	return 0;
 }
 
+static int head_has_history(void)
+{
+	unsigned char sha1[20];
+
+	return !get_sha1("HEAD", sha1);
+}
+
 static const char *push_to_deploy(unsigned char *sha1,
 				  struct argv_array *env,
 				  const char *work_tree)
@@ -745,13 +752,15 @@ static const char *push_to_deploy(unsigned char *sha1,
 	};
 	const char *diff_index[] = {
 		"diff-index", "--quiet", "--cached", "--ignore-submodules",
-		"HEAD", "--", NULL
+		NULL, "--", NULL
 	};
 	const char *read_tree[] = {
 		"read-tree", "-u", "-m", NULL, NULL
 	};
 	struct child_process child = CHILD_PROCESS_INIT;
 
+	int empty = !head_has_history();
+
 	child.argv = update_refresh;
 	child.env = env->argv;
 	child.dir = work_tree;
@@ -772,6 +781,9 @@ static const char *push_to_deploy(unsigned char *sha1,
 	if (run_command(&child))
 		return "Working directory has unstaged changes";
 
+	/* diff-index with either HEAD or an empty tree */
+	diff_index[4] = empty ? EMPTY_TREE_SHA1_HEX : "HEAD";
+
 	child_process_init(&child);
 	child.argv = diff_index;
 	child.env = env->argv;