Re: Trying to push into empty repo, get fatal: bad revision 'HEAD'
Junio C Hamano gits...@pobox.com writes:
A fix (or is it an enhancement) would probably look like this.
...
And this time with a pair of tests. It probably should be extended
to make sure it fails when the pushed HEAD records paths that are
floating in the working tree (as that would mean overwrting them),
but it is getting late and I am lazy, so ... ;-)
-- 8 --
Subject: [PATCH] push-to-deploy: allow pushing into an unborn branch and
updating it
Setting receive.denycurrentbranch to updateinstead and pushing into
the current branch, when the working tree and the index is truly
clean, is supposed to reset the working tree and the index to match
the tree of the pushed commit. This did not work when pushing into
an unborn branch.
The code that drives push-to-checkout hook needs no change, as the
interface is defined so that hook can decide what to do when the
push is coming to an unborn branch and take an appropriate action
since the beginning.
Signed-off-by: Junio C Hamano gits...@pobox.com
---
builtin/receive-pack.c | 21 ++-
t/t5516-fetch-push.sh | 55 +-
2 files changed, 74 insertions(+), 2 deletions(-)
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index fc8ec9c..0c0a261 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -733,6 +733,22 @@ static int update_shallow_ref(struct command *cmd, struct
shallow_info *si)
return 0;
}
+/*
+ * NEEDSWORK: we should consolidate various implementions of are we
+ * on an unborn branch? test into one, and make the unified one more
+ * robust. !get_sha1() based check used here and elsewhere would not
+ * allow us to tell an unborn branch from corrupt ref, for example.
+ * For the purpose of fixing deploy-to-update does not work when
+ * pushing into an empty repository issue, this should suffice for
+ * now.
+ */
+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,7 +761,7 @@ 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
@@ -772,6 +788,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] = head_has_history() ? HEAD : EMPTY_TREE_SHA1_HEX;
+
child_process_init(child);
child.argv = diff_index;
child.env = env-argv;
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index e4436c1..329d7d4 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -1430,8 +1430,22 @@ test_expect_success 'receive.denyCurrentBranch =
updateInstead' '
test $(git -C .. rev-parse HEAD^^) = $(git rev-parse HEAD)
git diff --quiet
test fifth = $(cat path3)
- )
+ )
+ # (5) push into void
+ rm -fr void
+ git init void
+ (
+ cd void
+ git config receive.denyCurrentBranch updateInstead
+ )
+ git push void master
+ (
+ cd void
+ test $(git -C .. rev-parse master) = $(git rev-parse HEAD)
+ git diff --quiet
+ git diff --cached --quiet
+ )
'
test_expect_success 'updateInstead with push-to-checkout hook' '
@@ -1494,6 +1508,45 @@ test_expect_success 'updateInstead with push-to-checkout
hook' '
test $(cat path5) = irrelevant
test $(git diff --name-only --cached HEAD) = path5
test $(git -C .. rev-parse HEAD) = $(git rev-parse HEAD)
+ )
+
+ # push into void
+ rm -fr void
+ git init void
+ (
+ cd void
+ git config receive.denyCurrentBranch updateInstead
+ write_script .git/hooks/push-to-checkout -\EOF
+ if git rev-parse --quiet --verify HEAD
+ then
+ has_head=yes
+ echo 2 updating from $(git rev-parse HEAD)
+ else
+ has_head=no
+ echo 2 pushing into void
+ fi
+ echo 2 updating to $1
+
+ git update-index -q --refresh
+ case $has_head in
+ yes)
+ git read-tree -u -m HEAD $1 ;;
+ no)
+ git read-tree -u -m $1 ;;
+ esac || {
+ status=$?
+
Re: Trying to push into empty repo, get fatal: bad revision 'HEAD'
Hi Samuel Junio, On 2015-04-01 03:36, Junio C Hamano wrote: Junio C Hamano gits...@pobox.com writes: Samuel Williams space.ship.travel...@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). 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. Right, I did not think about that use case at all. 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. Yeah, and we could refactor that into a global function, too. But for the moment, I think your proposed patch is good enough. Ciao, Dscho -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Trying to push into empty repo, get fatal: bad revision 'HEAD'
Hi Junio, On 2015-04-01 20:00, Junio C Hamano wrote: Johannes Schindelin johannes.schinde...@gmx.de writes: Yeah, and we could refactor that into a global function, too. But for the moment, I think your proposed patch is good enough. OK, so can I forge your Acked-by? You read my mind. Ciao, Dscho -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Trying to push into empty repo, get fatal: bad revision 'HEAD'
Johannes Schindelin johannes.schinde...@gmx.de writes: Yeah, and we could refactor that into a global function, too. But for the moment, I think your proposed patch is good enough. OK, so can I forge your Acked-by? Thanks for double checking. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Trying to push into empty repo, get fatal: bad revision 'HEAD'
Junio C Hamano gits...@pobox.com writes:
Samuel Williams space.ship.travel...@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;
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Trying to push into empty repo, get fatal: bad revision 'HEAD'
Samuel Williams space.ship.travel...@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... -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
