{"thread":{"id":"29417","subject":"[PATCH] optionally deny all pushes","startedAt":"2012-01-21T17:49:27Z","lastAt":"2012-02-14T19:06:24Z","messageCount":16,"participants":["Clemens Buchacher","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"182899","messageId":"20120121174927.GA7128@ecki","threadId":"29417","inReplyTo":null,"subject":"[PATCH] optionally deny all pushes","fromName":"Clemens Buchacher","fromEmail":"drizzd@aon.at","sentAt":"2012-01-21T17:49:27Z","receivedAt":"2012-01-21T17:49:27Z","isPatch":true,"sender":{"key":"drizzd@gmx.net","avatar":"https://avatars.githubusercontent.com/u/59082?v=4"},"body":"For a mirror repository which is updated only by pulling from upstream,\nit is convenient to disallow all pushes. An accidental push from a\ndownstream repository would mess up the mirror's state as well as future\nupdates.\n\nAdd a boolean configuration variable receive.denyAll. If enabled,\nreceive-pack will deny all ref updates.\n\nSigned-off-by: Clemens Buchacher <drizzd@aon.at>\n---\n Documentation/config.txt               |    4 ++++\n Documentation/git-push.txt             |    1 +\n builtin/receive-pack.c                 |   11 +++++++++++\n contrib/completion/git-completion.bash |    1 +\n t/t5400-send-pack.sh                   |   17 +++++++++++++++++\n t/t5516-fetch-push.sh                  |   12 ++++++++++++\n 6 files changed, 46 insertions(+), 0 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 04f5e19..edbddea 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -1683,6 +1683,10 @@ receive.unpackLimit::\n \tespecially on slow filesystems.  If not set, the value of\n \t`transfer.unpackLimit` is used instead.\n \n+receive.denyAll::\n+\tIf set to true, git-receive-pack will deny all ref updates.\n+\tThe repository can not be updated remotely.\n+\n receive.denyDeletes::\n \tIf set to true, git-receive-pack will deny a ref update that deletes\n \tthe ref. Use this to prevent such a ref deletion via a push.\ndiff --git a/Documentation/git-push.txt b/Documentation/git-push.txt\nindex aede488..7c6cc63 100644\n--- a/Documentation/git-push.txt\n+++ b/Documentation/git-push.txt\n@@ -219,6 +219,7 @@ remote rejected::\n \tThe remote end refused the update.  Usually caused by a hook\n \ton the remote side, or because the remote repository has one\n \tof the following safety options in effect:\n+\t`receive.denyAll` (for all pushes),\n \t`receive.denyCurrentBranch` (for pushes to the checked out\n \tbranch), `receive.denyNonFastForwards` (for forced\n \tnon-fast-forward updates), `receive.denyDeletes` or\ndiff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\nindex d2dcb7e..9cd04f9 100644\n--- a/builtin/receive-pack.c\n+++ b/builtin/receive-pack.c\n@@ -22,6 +22,7 @@ enum deny_action {\n \tDENY_REFUSE\n };\n \n+static int deny_all;\n static int deny_deletes;\n static int deny_non_fast_forwards;\n static enum deny_action deny_current_branch = DENY_UNCONFIGURED;\n@@ -57,6 +58,11 @@ static enum deny_action parse_deny_action(const char *var, const char *value)\n \n static int receive_pack_config(const char *var, const char *value, void *cb)\n {\n+\tif (strcmp(var, \"receive.denyall\") == 0) {\n+\t\tdeny_all = git_config_bool(var, value);\n+\t\treturn 0;\n+\t}\n+\n \tif (strcmp(var, \"receive.denydeletes\") == 0) {\n \t\tdeny_deletes = git_config_bool(var, value);\n \t\treturn 0;\n@@ -401,6 +407,11 @@ static const char *update(struct command *cmd)\n \tunsigned char *new_sha1 = cmd->new_sha1;\n \tstruct ref_lock *lock;\n \n+\tif (deny_all) {\n+\t\trp_error(\"denying all updates\");\n+\t\treturn \"update prohibited\";\n+\t}\n+\n \t/* only refs/... are allowed */\n \tif (prefixcmp(name, \"refs/\") || check_refname_format(name + 5, 0)) {\n \t\trp_error(\"refusing to create funny ref '%s' remotely\", name);\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex b0062ba..9d63622 100755\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -2250,6 +2250,7 @@ _git_config ()\n \t\trebase.autosquash\n \t\trebase.stat\n \t\treceive.autogc\n+\t\treceive.denyAll\n \t\treceive.denyCurrentBranch\n \t\treceive.denyDeleteCurrent\n \t\treceive.denyDeletes\ndiff --git a/t/t5400-send-pack.sh b/t/t5400-send-pack.sh\nindex 0eace37..e8c9be2 100755\n--- a/t/t5400-send-pack.sh\n+++ b/t/t5400-send-pack.sh\n@@ -222,4 +222,21 @@ test_expect_success 'deny pushing to delete current branch' '\n \t)\n '\n \n+test_expect_success 'deny pushing to any branch with denyAll' '\n+\trewound_push_setup &&\n+\t(\n+\t    cd parent &&\n+\t    git config receive.denyCurrentBranch false &&\n+\t    git config receive.denyAll true\n+\t) &&\n+\t(\n+\t    cd child &&\n+\t    test_must_fail git send-pack ../parent master 2>errs\n+\t) &&\n+\t(\n+\t    cd child &&\n+\t    test_must_fail git send-pack ../parent :refs/heads/master 2>errs\n+\t)\n+'\n+\n test_done\ndiff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh\nindex b69cf57..d1117c0 100755\n--- a/t/t5516-fetch-push.sh\n+++ b/t/t5516-fetch-push.sh\n@@ -815,6 +815,18 @@ test_expect_success 'allow push to HEAD of bare repository (bare)' '\n \t! grep \"warning: updating the current branch\" stderr\n '\n \n+test_expect_success 'deny push to HEAD of bare repository (denyAll)' '\n+\tmk_test heads/master &&\n+\t(\n+\t\tcd testrepo &&\n+\t\tgit checkout master &&\n+\t\tgit config receive.denyCurrentBranch false &&\n+\t\tgit config receive.denyAll true &&\n+\t\tgit config core.bare true\n+\t) &&\n+\ttest_must_fail git push testrepo master\n+'\n+\n test_expect_success 'allow push to HEAD of non-bare repository (config)' '\n \tmk_test heads/master &&\n \t(\n-- \n1.7.8\n"},{"id":"182907","messageId":"7vhazobto3.fsf@alter.siamese.dyndns.org","threadId":"29417","inReplyTo":"20120121174927.GA7128@ecki","subject":"Re: [PATCH] optionally deny all pushes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-01-21T22:02:52Z","receivedAt":"2012-01-21T22:02:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Clemens Buchacher <drizzd@aon.at> writes:\n\n> For a mirror repository which is updated only by pulling from upstream,\n> it is convenient to disallow all pushes. An accidental push from a\n> downstream repository would mess up the mirror's state as well as future\n> updates.\n>\n> Add a boolean configuration variable receive.denyAll. If enabled,\n> receive-pack will deny all ref updates.\n>\n> Signed-off-by: Clemens Buchacher <drizzd@aon.at>\n> ---\n\nI do not like this at all.\n\nEspecially given that this seems equivalent to:\n\n\t$ echo exit 1 >.git/hooks/pre-receive\n        $ chmod +x .git/hooks/pre-receive\n\nthis is a mere configuration bloat, isn't it?\n\nYour patch however raises an interesting point. Our pre-receive rejection\ninfrastructure does have a room for improvement.\n\nA typical \"push\" exchange goes like this:\n\n    receiver: Here are the refs I have.\n    sender:   I want to update your refs with these.\n    sender:   Here is the object data to complete the updated refs.\n    receiver: Let's see what you are trying to do... I may say:\n              - I refuse your proposal to update/delete this branch,\n                as it is the current branch.\n              - My hook tells me to refuse your proposal to update this\n                ref, as it does not fast-forward.\n\nWe can see that we could decide to reject the push to update the current\nbranch even before receiving the bulk of transfer. I suspect it might need\na protocol extension to give the receiver a chance to speak after hearing\nthe proposed set of updates before receiving the bulk of the data to add a\nclean error codepath, but at the worset case, we could immediately close\nthe connection after hearing what the sender proposes to do tries to touch\nthe current branch in a way we do not like. In other words, the exchange\ncould become:\n\n    receiver: Here are the refs I have.\n    sender:   I want to update your refs with these.\n    receiver: Let's see what you are trying to do... I may say:\n              - I can tell you without actually looking at the commits you\n                are proposing to place at the tips of my refs that I won't\n                like this branch to be updated/deleted. Go away.\n    sender:   Here is the object data to complete the updated refs.\n    receiver: Let's see what you are trying to do...\n              - My hook tells me to refuse your proposal to update this\n                ref, as it does not fast-forward.\n\nYour \"reject any and all\" better fits into that new rejection\ncodepath. Just like the configuration that rejects updates to the current\nbranch, the decision your patch wants to make can be made before we see\nany object data from the sender.\n\nAnother thing to consider is that you do not have to be limited to \"all\".\n\nOften people want to run \"git fetch\" on their mothership box receiving\ndata from their notebook, i.e.\n\n    mothership$ git fetch notebook master:refs/remotes/notebook/master\n    mothership$ git merge notebook/master\n\nbut are prevented from doing so due to networking issues (e.g. your\nnotebook may not allow incoming connections).  The right way to emulate\nthis is to initiate the connection in the reverse direction, running \"git\npush\" on their notebook sending data into their mothership box, i.e.\n\n    notebook$ git push mothership master:refs/remotes/notebook/master\n    mothership$ git merge notebook/master\n\nIn such a situation, you may want to configure your mothership repository\nto reject any push outside refs/remotes/ hierarchy to prevent mistakes.\nWe could add \"receive.allowed = refs/remotes/*\" or something to support\nthis use case.  It might turn out that we may even want to make this the\ndefault for non-bare repositories.\n\nOnce that happens, you can naturally say \"receive.allowed = none\" to do\nwhat your patch wants to do as a narrow special case. Wouldn't that be a\nmuch better approach than a \"reject any and all\" that can only serve a\nsingle narrow case that can already be done with a simple hook?\n"},{"id":"184597","messageId":"1329164235-29955-1-git-send-email-drizzd@aon.at","threadId":"29417","inReplyTo":"7vhazobto3.fsf@alter.siamese.dyndns.org","subject":"[PATCH 0/5] verify refs early","fromName":"Clemens Buchacher","fromEmail":"drizzd@aon.at","sentAt":"2012-02-13T20:17:10Z","receivedAt":"2012-02-13T20:17:10Z","isPatch":true,"sender":{"key":"drizzd@gmx.net","avatar":"https://avatars.githubusercontent.com/u/59082?v=4"},"body":"Hi Junio,\n\nThis series centers around \"[PATCH 3/5] git push: verify refs early\",\nwhich implements your suggestions in reply to my \"[PATCH] optionally\ndeny all pushes\". I am not pursuing the deny config option further,\nbecause as you noted, it is of course made redundant by a simple\npre-receive hook.\n\n[PATCH 1/5] git rev-list: fix invalid typecast\n\nI consider this one a fairly important bugfix. Although I was unable to\ndetermine under which circumstances the bug takes effect, rev-list\nprobably takes a performance hit when it does.\n\n[PATCH 2/5] do not override receive-pack errors\n[PATCH 3/5] git push: verify refs early\n\nNote that [PATCH 3/5] makes an extension to the transfer protocol.\n\n[PATCH 4/5] t5541: use configured port number\n\nA mostly unrelated bugfix.\n\n[PATCH 5/5] push/fetch/clone --no-progress suppresses progress\n\nA totally unrelated change. However, this conflicts textually\n(lexically?) with [PATCH 3/5], and therefore I am including it here.\n\nClemens\n"},{"id":"184596","messageId":"1329164235-29955-2-git-send-email-drizzd@aon.at","threadId":"29417","inReplyTo":"1329164235-29955-1-git-send-email-drizzd@aon.at","subject":"[PATCH 1/5] git rev-list: fix invalid typecast","fromName":"Clemens Buchacher","fromEmail":"drizzd@aon.at","sentAt":"2012-02-13T20:17:11Z","receivedAt":"2012-02-13T20:17:11Z","isPatch":true,"sender":{"key":"drizzd@gmx.net","avatar":"https://avatars.githubusercontent.com/u/59082?v=4"},"body":"git rev-list passes rev_list_info, not rev_list objects. Without this\nfix, rev-list enables or disables the --verify-objects option depending\non a read from an undefined memory location.\n\nSigned-off-by: Clemens Buchacher <drizzd@aon.at>\n---\n builtin/rev-list.c |    4 ++--\n t/t1450-fsck.sh    |   26 ++++++++++++++++++++++++++\n 2 files changed, 28 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/rev-list.c b/builtin/rev-list.c\nindex ab3be7c..264e3ae 100644\n--- a/builtin/rev-list.c\n+++ b/builtin/rev-list.c\n@@ -180,10 +180,10 @@ static void show_object(struct object *obj,\n \t\t\tconst struct name_path *path, const char *component,\n \t\t\tvoid *cb_data)\n {\n-\tstruct rev_info *info = cb_data;\n+\tstruct rev_list_info *info = cb_data;\n \n \tfinish_object(obj, path, component, cb_data);\n-\tif (info->verify_objects && !obj->parsed && obj->type != OBJ_COMMIT)\n+\tif (info->revs->verify_objects && !obj->parsed && obj->type != OBJ_COMMIT)\n \t\tparse_object(obj->sha1);\n \tshow_object_with_name(stdout, obj, path, component);\n }\ndiff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh\nindex 523ce9c..5b8ebd8 100755\n--- a/t/t1450-fsck.sh\n+++ b/t/t1450-fsck.sh\n@@ -191,4 +191,30 @@ test_expect_success 'cleaned up' '\n \ttest_cmp empty actual\n '\n \n+test_expect_success 'rev-list --verify-objects' '\n+\tgit rev-list --verify-objects --all >/dev/null 2>out &&\n+\ttest_cmp empty out\n+'\n+\n+test_expect_success 'rev-list --verify-objects with bad sha1' '\n+\tsha=$(echo blob | git hash-object -w --stdin) &&\n+\told=$(echo $sha | sed \"s+^..+&/+\") &&\n+\tnew=$(dirname $old)/ffffffffffffffffffffffffffffffffffffff &&\n+\tsha=\"$(dirname $new)$(basename $new)\" &&\n+\tmv .git/objects/$old .git/objects/$new &&\n+\ttest_when_finished \"remove_object $sha\" &&\n+\tgit update-index --add --cacheinfo 100644 $sha foo &&\n+\ttest_when_finished \"git read-tree -u --reset HEAD\" &&\n+\ttree=$(git write-tree) &&\n+\ttest_when_finished \"remove_object $tree\" &&\n+\tcmt=$(echo bogus | git commit-tree $tree) &&\n+\ttest_when_finished \"remove_object $cmt\" &&\n+\tgit update-ref refs/heads/bogus $cmt &&\n+\ttest_when_finished \"git update-ref -d refs/heads/bogus\" &&\n+\n+\ttest_might_fail git rev-list --verify-objects refs/heads/bogus >/dev/null 2>out &&\n+\tcat out &&\n+\tgrep -q \"error: sha1 mismatch 63ffffffffffffffffffffffffffffffffffffff\" out\n+'\n+\n test_done\n-- \n1.7.9\n"},{"id":"184599","messageId":"1329164235-29955-3-git-send-email-drizzd@aon.at","threadId":"29417","inReplyTo":"1329164235-29955-1-git-send-email-drizzd@aon.at","subject":"[PATCH 2/5] do not override receive-pack errors","fromName":"Clemens Buchacher","fromEmail":"drizzd@aon.at","sentAt":"2012-02-13T20:17:12Z","receivedAt":"2012-02-13T20:17:12Z","isPatch":true,"sender":{"key":"drizzd@gmx.net","avatar":"https://avatars.githubusercontent.com/u/59082?v=4"},"body":"Receive runs rev-list --verify-objects in order to detect missing\nobjects. However, such errors are ignored and overridden later.\nInstead, consequently ignore all update commands for which an error has\nalready been detected.\n\nSome tests in t5504 are obsoleted by this change, because invalid\nobjects are detected even if fsck is not enabled. Instead, they now test\nfor different error messages depending on whether or not fsck is turned\non. A better fix would be to force a corruption that will be detected by\nfsck but not by rev-list.\n\nSigned-off-by: Clemens Buchacher <drizzd@aon.at>\n---\n builtin/receive-pack.c          |   24 +++++++++++++++++-------\n t/t5504-fetch-receive-strict.sh |   22 ++++++++++++++++++----\n 2 files changed, 35 insertions(+), 11 deletions(-)\n\ndiff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\nindex fa7448b..0afb8b2 100644\n--- a/builtin/receive-pack.c\n+++ b/builtin/receive-pack.c\n@@ -642,8 +642,10 @@ static void check_aliased_updates(struct command *commands)\n \t}\n \tsort_string_list(&ref_list);\n \n-\tfor (cmd = commands; cmd; cmd = cmd->next)\n-\t\tcheck_aliased_update(cmd, &ref_list);\n+\tfor (cmd = commands; cmd; cmd = cmd->next) {\n+\t\tif (!cmd->error_string)\n+\t\t\tcheck_aliased_update(cmd, &ref_list);\n+\t}\n \n \tstring_list_clear(&ref_list, 0);\n }\n@@ -707,8 +709,10 @@ static void execute_commands(struct command *commands, const char *unpacker_erro\n \t\tset_connectivity_errors(commands);\n \n \tif (run_receive_hook(commands, pre_receive_hook, 0)) {\n-\t\tfor (cmd = commands; cmd; cmd = cmd->next)\n-\t\t\tcmd->error_string = \"pre-receive hook declined\";\n+\t\tfor (cmd = commands; cmd; cmd = cmd->next) {\n+\t\t\tif (!cmd->error_string)\n+\t\t\t\tcmd->error_string = \"pre-receive hook declined\";\n+\t\t}\n \t\treturn;\n \t}\n \n@@ -717,9 +721,15 @@ static void execute_commands(struct command *commands, const char *unpacker_erro\n \tfree(head_name_to_free);\n \thead_name = head_name_to_free = resolve_refdup(\"HEAD\", sha1, 0, NULL);\n \n-\tfor (cmd = commands; cmd; cmd = cmd->next)\n-\t\tif (!cmd->skip_update)\n-\t\t\tcmd->error_string = update(cmd);\n+\tfor (cmd = commands; cmd; cmd = cmd->next) {\n+\t\tif (cmd->error_string)\n+\t\t\tcontinue;\n+\n+\t\tif (cmd->skip_update)\n+\t\t\tcontinue;\n+\n+\t\tcmd->error_string = update(cmd);\n+\t}\n }\n \n static struct command *read_head_info(void)\ndiff --git a/t/t5504-fetch-receive-strict.sh b/t/t5504-fetch-receive-strict.sh\nindex 8341fc4..35ec294 100755\n--- a/t/t5504-fetch-receive-strict.sh\n+++ b/t/t5504-fetch-receive-strict.sh\n@@ -58,6 +58,11 @@ test_expect_success 'fetch with transfer.fsckobjects' '\n \t)\n '\n \n+cat >exp <<EOF\n+To dst\n+!\trefs/heads/master:refs/heads/test\t[remote rejected] (missing necessary objects)\n+EOF\n+\n test_expect_success 'push without strict' '\n \trm -rf dst &&\n \tgit init dst &&\n@@ -66,7 +71,8 @@ test_expect_success 'push without strict' '\n \t\tgit config fetch.fsckobjects false &&\n \t\tgit config transfer.fsckobjects false\n \t) &&\n-\tgit push dst master:refs/heads/test\n+\ttest_must_fail git push --porcelain dst master:refs/heads/test >act &&\n+\ttest_cmp exp act\n '\n \n test_expect_success 'push with !receive.fsckobjects' '\n@@ -77,9 +83,15 @@ test_expect_success 'push with !receive.fsckobjects' '\n \t\tgit config receive.fsckobjects false &&\n \t\tgit config transfer.fsckobjects true\n \t) &&\n-\tgit push dst master:refs/heads/test\n+\ttest_must_fail git push --porcelain dst master:refs/heads/test >act &&\n+\ttest_cmp exp act\n '\n \n+cat >exp <<EOF\n+To dst\n+!\trefs/heads/master:refs/heads/test\t[remote rejected] (n/a (unpacker error))\n+EOF\n+\n test_expect_success 'push with receive.fsckobjects' '\n \trm -rf dst &&\n \tgit init dst &&\n@@ -88,7 +100,8 @@ test_expect_success 'push with receive.fsckobjects' '\n \t\tgit config receive.fsckobjects true &&\n \t\tgit config transfer.fsckobjects false\n \t) &&\n-\ttest_must_fail git push dst master:refs/heads/test\n+\ttest_must_fail git push --porcelain dst master:refs/heads/test >act &&\n+\ttest_cmp exp act\n '\n \n test_expect_success 'push with transfer.fsckobjects' '\n@@ -98,7 +111,8 @@ test_expect_success 'push with transfer.fsckobjects' '\n \t\tcd dst &&\n \t\tgit config transfer.fsckobjects true\n \t) &&\n-\ttest_must_fail git push dst master:refs/heads/test\n+\ttest_must_fail git push --porcelain dst master:refs/heads/test >act &&\n+\ttest_cmp exp act\n '\n \n test_done\n-- \n1.7.9\n"},{"id":"184594","messageId":"1329164235-29955-4-git-send-email-drizzd@aon.at","threadId":"29417","inReplyTo":"1329164235-29955-1-git-send-email-drizzd@aon.at","subject":"[PATCH 3/5] git push: verify refs early","fromName":"Clemens Buchacher","fromEmail":"drizzd@aon.at","sentAt":"2012-02-13T20:17:13Z","receivedAt":"2012-02-13T20:17:13Z","isPatch":true,"sender":{"key":"drizzd@gmx.net","avatar":"https://avatars.githubusercontent.com/u/59082?v=4"},"body":"Receive-pack waits for the transfer of all new object data to complete\nbefore taking a closer look at the received refs. It may then decide to\nrefuse updates of refs without even looking at the data.\n\nInstead, do as much ref validation as possible before new object data is\navailable, in order to avoid a potentially expensive transfer.\nReceive-pack will then respond with \"verify-refs ok\" if at least one ref\nupdate is valid. Otherwise, it will respond with \"verify-refs no valid\nrefs\".\n\nOn the client side, send-pack will wait for \"verify-refs ok\" or skip the\nobject transfer entirely if it reads a packet line that starts with\n\"verify-refs \" but does not match \"verify-refs ok\". Any other response\nis considered an error.\n\nEarly ref validation is disabled for the smart HTTP protocol, since it\ntries to fit the entire push (ref info and object data) into a single\nPOST request.\n\nSigned-off-by: Clemens Buchacher <drizzd@aon.at>\n---\n\nI suppose with some effort, this could be done for smart HTTP as well.\nBut I am not sure if we actually want the overhead of the additional\nping-pong for HTTP.\n\n builtin/receive-pack.c |   83 ++++++++++++++++++++++++++++++++++++++----------\n builtin/send-pack.c    |   43 +++++++++++++++++--------\n send-pack.h            |    3 +-\n 3 files changed, 97 insertions(+), 32 deletions(-)\n\ndiff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\nindex 0afb8b2..0129d9c 100644\n--- a/builtin/receive-pack.c\n+++ b/builtin/receive-pack.c\n@@ -34,6 +34,8 @@ static int unpack_limit = 100;\n static int report_status;\n static int use_sideband;\n static int quiet;\n+static int verify_refs;\n+static int stateless_rpc;\n static int prefer_ofs_delta = 1;\n static int auto_update_server_info;\n static int auto_gc = 1;\n@@ -123,7 +125,7 @@ static void show_ref(const char *path, const unsigned char *sha1)\n \telse\n \t\tpacket_write(1, \"%s %s%c%s%s\\n\",\n \t\t\t     sha1_to_hex(sha1), path, 0,\n-\t\t\t     \" report-status delete-refs side-band-64k quiet\",\n+\t\t\t     \" report-status delete-refs side-band-64k quiet verify-refs\",\n \t\t\t     prefer_ofs_delta ? \" ofs-delta\" : \"\");\n \tsent_capabilities = 1;\n }\n@@ -410,14 +412,13 @@ static void refuse_unconfigured_deny_delete_current(void)\n \t\trp_error(\"%s\", refuse_unconfigured_deny_delete_current_msg[i]);\n }\n \n-static const char *update(struct command *cmd)\n+static const char *verify_ref(struct command *cmd)\n {\n \tconst char *name = cmd->ref_name;\n \tstruct strbuf namespaced_name_buf = STRBUF_INIT;\n \tconst char *namespaced_name;\n \tunsigned char *old_sha1 = cmd->old_sha1;\n \tunsigned char *new_sha1 = cmd->new_sha1;\n-\tstruct ref_lock *lock;\n \n \t/* only refs/... are allowed */\n \tif (prefixcmp(name, \"refs/\") || check_refname_format(name + 5, 0)) {\n@@ -444,12 +445,6 @@ static const char *update(struct command *cmd)\n \t\t}\n \t}\n \n-\tif (!is_null_sha1(new_sha1) && !has_sha1_file(new_sha1)) {\n-\t\terror(\"unpack should have generated %s, \"\n-\t\t      \"but I can't find it!\", sha1_to_hex(new_sha1));\n-\t\treturn \"bad pack\";\n-\t}\n-\n \tif (!is_null_sha1(old_sha1) && is_null_sha1(new_sha1)) {\n \t\tif (deny_deletes && !prefixcmp(name, \"refs/heads/\")) {\n \t\t\trp_error(\"denying ref deletion for %s\", name);\n@@ -473,6 +468,27 @@ static const char *update(struct command *cmd)\n \t\t}\n \t}\n \n+\treturn NULL;\n+}\n+\n+static const char *update(struct command *cmd)\n+{\n+\tconst char *name = cmd->ref_name;\n+\tstruct strbuf namespaced_name_buf = STRBUF_INIT;\n+\tconst char *namespaced_name;\n+\tunsigned char *old_sha1 = cmd->old_sha1;\n+\tunsigned char *new_sha1 = cmd->new_sha1;\n+\tstruct ref_lock *lock;\n+\n+\tstrbuf_addf(&namespaced_name_buf, \"%s%s\", get_git_namespace(), name);\n+\tnamespaced_name = strbuf_detach(&namespaced_name_buf, NULL);\n+\n+\tif (!is_null_sha1(new_sha1) && !has_sha1_file(new_sha1)) {\n+\t\terror(\"unpack should have generated %s, \"\n+\t\t      \"but I can't find it!\", sha1_to_hex(new_sha1));\n+\t\treturn \"bad pack\";\n+\t}\n+\n \tif (deny_non_fast_forwards && !is_null_sha1(new_sha1) &&\n \t    !is_null_sha1(old_sha1) &&\n \t    !prefixcmp(name, \"refs/heads/\")) {\n@@ -692,10 +708,41 @@ static int iterate_receive_command_list(void *cb_data, unsigned char sha1[20])\n \treturn -1; /* end of list */\n }\n \n+static int verify_ref_commands(struct command *commands)\n+{\n+\tunsigned char sha1[20];\n+\tint commands_ok;\n+\tstruct command *cmd;\n+\n+\tfree(head_name_to_free);\n+\thead_name = head_name_to_free = resolve_refdup(\"HEAD\", sha1, 0, NULL);\n+\n+\tcommands_ok = 0;\n+\tfor (cmd = commands; cmd; cmd = cmd->next) {\n+\t\tcmd->error_string = verify_ref(cmd);\n+\t\tif (!cmd->error_string)\n+\t\t\tcommands_ok++;\n+\t}\n+\n+\tif (verify_refs && !stateless_rpc) {\n+\t\tstruct strbuf buf = STRBUF_INIT;\n+\n+\t\tpacket_buf_write(&buf, \"verify-refs %s\\n\",\n+\t\t\t commands_ok > 0 ? \"ok\" : \"no valid refs\");\n+\n+\t\tif (use_sideband)\n+\t\t\tsend_sideband(1, 1, buf.buf, buf.len, use_sideband);\n+\t\telse\n+\t\t\tsafe_write(1, buf.buf, buf.len);\n+\t\tstrbuf_release(&buf);\n+\t}\n+\n+\treturn commands_ok;\n+}\n+\n static void execute_commands(struct command *commands, const char *unpacker_error)\n {\n \tstruct command *cmd;\n-\tunsigned char sha1[20];\n \n \tif (unpacker_error) {\n \t\tfor (cmd = commands; cmd; cmd = cmd->next)\n@@ -718,9 +765,6 @@ static void execute_commands(struct command *commands, const char *unpacker_erro\n \n \tcheck_aliased_updates(commands);\n \n-\tfree(head_name_to_free);\n-\thead_name = head_name_to_free = resolve_refdup(\"HEAD\", sha1, 0, NULL);\n-\n \tfor (cmd = commands; cmd; cmd = cmd->next) {\n \t\tif (cmd->error_string)\n \t\t\tcontinue;\n@@ -766,6 +810,8 @@ static struct command *read_head_info(void)\n \t\t\t\tuse_sideband = LARGE_PACKET_MAX;\n \t\t\tif (parse_feature_request(feature_list, \"quiet\"))\n \t\t\t\tquiet = 1;\n+\t\t\tif (parse_feature_request(feature_list, \"verify-refs\"))\n+\t\t\t\tverify_refs = 1;\n \t\t}\n \t\tcmd = xcalloc(1, sizeof(struct command) + len - 80);\n \t\thashcpy(cmd->old_sha1, old_sha1);\n@@ -905,7 +951,6 @@ static int delete_only(struct command *commands)\n int cmd_receive_pack(int argc, const char **argv, const char *prefix)\n {\n \tint advertise_refs = 0;\n-\tint stateless_rpc = 0;\n \tint i;\n \tchar *dir = NULL;\n \tstruct command *commands;\n@@ -962,11 +1007,15 @@ int cmd_receive_pack(int argc, const char **argv, const char *prefix)\n \t\treturn 0;\n \n \tif ((commands = read_head_info()) != NULL) {\n+\t\tint commands_ok;\n \t\tconst char *unpack_status = NULL;\n \n-\t\tif (!delete_only(commands))\n-\t\t\tunpack_status = unpack();\n-\t\texecute_commands(commands, unpack_status);\n+\t\tcommands_ok = verify_ref_commands(commands);\n+\t\tif (!verify_refs || commands_ok > 0) {\n+\t\t\tif (!delete_only(commands))\n+\t\t\t\tunpack_status = unpack();\n+\t\t\texecute_commands(commands, unpack_status);\n+\t\t}\n \t\tif (pack_lockfile)\n \t\t\tunlink_or_warn(pack_lockfile);\n \t\tif (report_status)\ndiff --git a/builtin/send-pack.c b/builtin/send-pack.c\nindex 71f258e..7d514ca 100644\n--- a/builtin/send-pack.c\n+++ b/builtin/send-pack.c\n@@ -265,6 +265,8 @@ int send_pack(struct send_pack_args *args,\n \t\tuse_sideband = 1;\n \tif (!server_supports(\"quiet\"))\n \t\targs->quiet = 0;\n+\tif (server_supports(\"verify-refs\"))\n+\t\targs->verify_refs = 1;\n \n \tif (!remote_refs) {\n \t\tfprintf(stderr, \"No refs in common and none specified; doing nothing.\\n\"\n@@ -303,12 +305,13 @@ int send_pack(struct send_pack_args *args,\n \t\t\tchar *old_hex = sha1_to_hex(ref->old_sha1);\n \t\t\tchar *new_hex = sha1_to_hex(ref->new_sha1);\n \n-\t\t\tif (!cmds_sent && (status_report || use_sideband || args->quiet)) {\n-\t\t\t\tpacket_buf_write(&req_buf, \"%s %s %s%c%s%s%s\",\n+\t\t\tif (!cmds_sent && (status_report || use_sideband || args->quiet || args->verify_refs)) {\n+\t\t\t\tpacket_buf_write(&req_buf, \"%s %s %s%c%s%s%s%s\",\n \t\t\t\t\told_hex, new_hex, ref->name, 0,\n \t\t\t\t\tstatus_report ? \" report-status\" : \"\",\n \t\t\t\t\tuse_sideband ? \" side-band-64k\" : \"\",\n-\t\t\t\t\targs->quiet ? \" quiet\" : \"\");\n+\t\t\t\t\targs->quiet ? \" quiet\" : \"\",\n+\t\t\t\t\targs->verify_refs ? \" verify-refs\" : \"\");\n \t\t\t}\n \t\t\telse\n \t\t\t\tpacket_buf_write(&req_buf, \"%s %s %s\",\n@@ -341,17 +344,29 @@ int send_pack(struct send_pack_args *args,\n \t\tin = demux.out;\n \t}\n \n-\tif (new_refs && cmds_sent) {\n-\t\tif (pack_objects(out, remote_refs, extra_have, args) < 0) {\n-\t\t\tfor (ref = remote_refs; ref; ref = ref->next)\n-\t\t\t\tref->status = REF_STATUS_NONE;\n-\t\t\tif (args->stateless_rpc)\n-\t\t\t\tclose(out);\n-\t\t\tif (git_connection_is_socket(conn))\n-\t\t\t\tshutdown(fd[0], SHUT_WR);\n-\t\t\tif (use_sideband)\n-\t\t\t\tfinish_async(&demux);\n-\t\t\treturn -1;\n+\tif (cmds_sent) {\n+\t\tint verify_refs_status = 0;\n+\n+\t\tif (args->verify_refs && !args->stateless_rpc) {\n+\t\t\tchar line[1000];\n+\t\t\tint len = packet_read_line(in, line, sizeof(line));\n+\t\t\tif (len < 15 || memcmp(line, \"verify-refs \", 12))\n+\t\t\t\treturn error(\"did not receive remote status\");\n+\t\t\tverify_refs_status = memcmp(line, \"verify-refs ok\\n\", 15);\n+\t\t}\n+\n+\t\tif (!verify_refs_status && new_refs) {\n+\t\t\tif (pack_objects(out, remote_refs, extra_have, args) < 0) {\n+\t\t\t\tfor (ref = remote_refs; ref; ref = ref->next)\n+\t\t\t\t\tref->status = REF_STATUS_NONE;\n+\t\t\t\tif (args->stateless_rpc)\n+\t\t\t\t\tclose(out);\n+\t\t\t\tif (git_connection_is_socket(conn))\n+\t\t\t\t\tshutdown(fd[0], SHUT_WR);\n+\t\t\t\tif (use_sideband)\n+\t\t\t\t\tfinish_async(&demux);\n+\t\t\t\treturn -1;\n+\t\t\t}\n \t\t}\n \t}\n \tif (args->stateless_rpc && cmds_sent)\ndiff --git a/send-pack.h b/send-pack.h\nindex 05d7ab1..87edaa5 100644\n--- a/send-pack.h\n+++ b/send-pack.h\n@@ -11,7 +11,8 @@ struct send_pack_args {\n \t\tuse_thin_pack:1,\n \t\tuse_ofs_delta:1,\n \t\tdry_run:1,\n-\t\tstateless_rpc:1;\n+\t\tstateless_rpc:1,\n+\t\tverify_refs:1;\n };\n \n int send_pack(struct send_pack_args *args,\n-- \n1.7.9\n"},{"id":"184595","messageId":"1329164235-29955-5-git-send-email-drizzd@aon.at","threadId":"29417","inReplyTo":"1329164235-29955-1-git-send-email-drizzd@aon.at","subject":"[PATCH 4/5] t5541: use configured port number","fromName":"Clemens Buchacher","fromEmail":"drizzd@aon.at","sentAt":"2012-02-13T20:17:14Z","receivedAt":"2012-02-13T20:17:14Z","isPatch":true,"sender":{"key":"drizzd@gmx.net","avatar":"https://avatars.githubusercontent.com/u/59082?v=4"},"body":"\nSigned-off-by: Clemens Buchacher <drizzd@aon.at>\n---\n t/t5541-http-push.sh |    2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/t/t5541-http-push.sh b/t/t5541-http-push.sh\nindex d66ed24..cc6f081 100755\n--- a/t/t5541-http-push.sh\n+++ b/t/t5541-http-push.sh\n@@ -106,7 +106,7 @@ cat >exp <<EOF\n remote: error: hook declined to update refs/heads/dev2\n To http://127.0.0.1:$LIB_HTTPD_PORT/smart/test_repo.git\n  ! [remote rejected] dev2 -> dev2 (hook declined)\n-error: failed to push some refs to 'http://127.0.0.1:5541/smart/test_repo.git'\n+error: failed to push some refs to 'http://127.0.0.1:$LIB_HTTPD_PORT/smart/test_repo.git'\n EOF\n \n test_expect_success 'rejected update prints status' '\n-- \n1.7.9\n"},{"id":"184598","messageId":"1329164235-29955-6-git-send-email-drizzd@aon.at","threadId":"29417","inReplyTo":"1329164235-29955-1-git-send-email-drizzd@aon.at","subject":"[PATCH 5/5] push/fetch/clone --no-progress suppresses progress output","fromName":"Clemens Buchacher","fromEmail":"drizzd@aon.at","sentAt":"2012-02-13T20:17:15Z","receivedAt":"2012-02-13T20:17:15Z","isPatch":true,"sender":{"key":"drizzd@gmx.net","avatar":"https://avatars.githubusercontent.com/u/59082?v=4"},"body":"By default, progress output is disabled if stderr is not a terminal.\nThe --progress option can be used to force progress output anyways.\nConversely, --no-progress does not force progress output. In particular,\nif stderr is a terminal, progress output is enabled.\n\nThis is unintuitive. Change --no-progress to force output off.\n\nSigned-off-by: Clemens Buchacher <drizzd@aon.at>\n---\n builtin/clone.c          |    6 +++---\n builtin/fetch-pack.c     |    2 +-\n builtin/fetch.c          |    4 ++--\n builtin/push.c           |    4 ++--\n builtin/send-pack.c      |   12 +++++++-----\n t/t5523-push-upstream.sh |    3 ++-\n transport.c              |    6 +++++-\n 7 files changed, 22 insertions(+), 15 deletions(-)\n\ndiff --git a/builtin/clone.c b/builtin/clone.c\nindex c62d4b5..09dbb09 100644\n--- a/builtin/clone.c\n+++ b/builtin/clone.c\n@@ -45,7 +45,7 @@ static char *option_branch = NULL;\n static const char *real_git_dir;\n static char *option_upload_pack = \"git-upload-pack\";\n static int option_verbosity;\n-static int option_progress;\n+static int option_progress = -1;\n static struct string_list option_config;\n static struct string_list option_reference;\n \n@@ -60,8 +60,8 @@ static int opt_parse_reference(const struct option *opt, const char *arg, int un\n \n static struct option builtin_clone_options[] = {\n \tOPT__VERBOSITY(&option_verbosity),\n-\tOPT_BOOLEAN(0, \"progress\", &option_progress,\n-\t\t\t\"force progress reporting\"),\n+\tOPT_BOOL(0, \"progress\", &option_progress,\n+\t\t \"force progress reporting\"),\n \tOPT_BOOLEAN('n', \"no-checkout\", &option_no_checkout,\n \t\t    \"don't create a checkout\"),\n \tOPT_BOOLEAN(0, \"bare\", &option_bare, \"create a bare repository\"),\ndiff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c\nindex 6207ecd..a4d3e90 100644\n--- a/builtin/fetch-pack.c\n+++ b/builtin/fetch-pack.c\n@@ -736,7 +736,7 @@ static int get_pack(int xd[2], char **pack_lockfile)\n \t}\n \telse {\n \t\t*av++ = \"unpack-objects\";\n-\t\tif (args.quiet)\n+\t\tif (args.quiet || args.no_progress)\n \t\t\t*av++ = \"-q\";\n \t}\n \tif (*hdr_arg)\ndiff --git a/builtin/fetch.c b/builtin/fetch.c\nindex ab18633..65f5f9b 100644\n--- a/builtin/fetch.c\n+++ b/builtin/fetch.c\n@@ -30,7 +30,7 @@ enum {\n };\n \n static int all, append, dry_run, force, keep, multiple, prune, update_head_ok, verbosity;\n-static int progress, recurse_submodules = RECURSE_SUBMODULES_DEFAULT;\n+static int progress = -1, recurse_submodules = RECURSE_SUBMODULES_DEFAULT;\n static int tags = TAGS_DEFAULT;\n static const char *depth;\n static const char *upload_pack;\n@@ -78,7 +78,7 @@ static struct option builtin_fetch_options[] = {\n \tOPT_BOOLEAN('k', \"keep\", &keep, \"keep downloaded pack\"),\n \tOPT_BOOLEAN('u', \"update-head-ok\", &update_head_ok,\n \t\t    \"allow updating of HEAD ref\"),\n-\tOPT_BOOLEAN(0, \"progress\", &progress, \"force progress reporting\"),\n+\tOPT_BOOL(0, \"progress\", &progress, \"force progress reporting\"),\n \tOPT_STRING(0, \"depth\", &depth, \"depth\",\n \t\t   \"deepen history of shallow clone\"),\n \t{ OPTION_STRING, 0, \"submodule-prefix\", &submodule_prefix, \"dir\",\ndiff --git a/builtin/push.c b/builtin/push.c\nindex 35cce53..6c373cf 100644\n--- a/builtin/push.c\n+++ b/builtin/push.c\n@@ -19,7 +19,7 @@ static int thin;\n static int deleterefs;\n static const char *receivepack;\n static int verbosity;\n-static int progress;\n+static int progress = -1;\n \n static const char **refspec;\n static int refspec_nr;\n@@ -260,7 +260,7 @@ int cmd_push(int argc, const char **argv, const char *prefix)\n \t\tOPT_STRING( 0 , \"exec\", &receivepack, \"receive-pack\", \"receive pack program\"),\n \t\tOPT_BIT('u', \"set-upstream\", &flags, \"set upstream for git pull/status\",\n \t\t\tTRANSPORT_PUSH_SET_UPSTREAM),\n-\t\tOPT_BOOLEAN(0, \"progress\", &progress, \"force progress reporting\"),\n+\t\tOPT_BOOL(0, \"progress\", &progress, \"force progress reporting\"),\n \t\tOPT_END()\n \t};\n \ndiff --git a/builtin/send-pack.c b/builtin/send-pack.c\nindex 7d514ca..8fc9789 100644\n--- a/builtin/send-pack.c\n+++ b/builtin/send-pack.c\n@@ -58,7 +58,7 @@ static int pack_objects(int fd, struct ref *refs, struct extra_have_objects *ext\n \t\targv[i++] = \"--thin\";\n \tif (args->use_ofs_delta)\n \t\targv[i++] = \"--delta-base-offset\";\n-\tif (args->quiet)\n+\tif (args->quiet || !args->progress)\n \t\targv[i++] = \"-q\";\n \tif (args->progress)\n \t\targv[i++] = \"--progress\";\n@@ -250,6 +250,7 @@ int send_pack(struct send_pack_args *args,\n \tint allow_deleting_refs = 0;\n \tint status_report = 0;\n \tint use_sideband = 0;\n+\tint quiet_supported = 0;\n \tunsigned cmds_sent = 0;\n \tint ret;\n \tstruct async demux;\n@@ -263,8 +264,8 @@ int send_pack(struct send_pack_args *args,\n \t\targs->use_ofs_delta = 1;\n \tif (server_supports(\"side-band-64k\"))\n \t\tuse_sideband = 1;\n-\tif (!server_supports(\"quiet\"))\n-\t\targs->quiet = 0;\n+\tif (server_supports(\"quiet\"))\n+\t\tquiet_supported = 1;\n \tif (server_supports(\"verify-refs\"))\n \t\targs->verify_refs = 1;\n \n@@ -304,13 +305,14 @@ int send_pack(struct send_pack_args *args,\n \t\t} else {\n \t\t\tchar *old_hex = sha1_to_hex(ref->old_sha1);\n \t\t\tchar *new_hex = sha1_to_hex(ref->new_sha1);\n+\t\t\tint quiet = quiet_supported && (args->quiet || !args->progress);\n \n-\t\t\tif (!cmds_sent && (status_report || use_sideband || args->quiet || args->verify_refs)) {\n+\t\t\tif (!cmds_sent && (status_report || use_sideband || quiet || args->verify_refs)) {\n \t\t\t\tpacket_buf_write(&req_buf, \"%s %s %s%c%s%s%s%s\",\n \t\t\t\t\told_hex, new_hex, ref->name, 0,\n \t\t\t\t\tstatus_report ? \" report-status\" : \"\",\n \t\t\t\t\tuse_sideband ? \" side-band-64k\" : \"\",\n-\t\t\t\t\targs->quiet ? \" quiet\" : \"\",\n+\t\t\t\t\tquiet ? \" quiet\" : \"\",\n \t\t\t\t\targs->verify_refs ? \" verify-refs\" : \"\");\n \t\t\t}\n \t\t\telse\ndiff --git a/t/t5523-push-upstream.sh b/t/t5523-push-upstream.sh\nindex 9ee52cf..3683df1 100755\n--- a/t/t5523-push-upstream.sh\n+++ b/t/t5523-push-upstream.sh\n@@ -101,10 +101,11 @@ test_expect_success TTY 'push -q suppresses progress' '\n \t! grep \"Writing objects\" err\n '\n \n-test_expect_failure TTY 'push --no-progress suppresses progress' '\n+test_expect_success TTY 'push --no-progress suppresses progress' '\n \tensure_fresh_upstream &&\n \n \ttest_terminal git push -u --no-progress upstream master >out 2>err &&\n+\t! grep \"Unpacking objects\" err &&\n \t! grep \"Writing objects\" err\n '\n \ndiff --git a/transport.c b/transport.c\nindex cac0c06..6074ee9 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -994,10 +994,14 @@ void transport_set_verbosity(struct transport *transport, int verbosity,\n \t * when a rule is satisfied):\n \t *\n \t *   1. Report progress, if force_progress is 1 (ie. --progress).\n+\t *   2. Don't report progress, if force_progress is 0 (ie. --no-progress).\n \t *   2. Don't report progress, if verbosity < 0 (ie. -q/--quiet ).\n \t *   3. Report progress if isatty(2) is 1.\n \t **/\n-\ttransport->progress = force_progress || (verbosity >= 0 && isatty(2));\n+\tif (force_progress >= 0)\n+\t\ttransport->progress = !!force_progress;\n+\telse\n+\t\ttransport->progress = verbosity >= 0 && isatty(2);\n }\n \n int transport_push(struct transport *transport,\n-- \n1.7.9\n"},{"id":"184603","messageId":"7vwr7qcvdu.fsf@alter.siamese.dyndns.org","threadId":"29417","inReplyTo":"1329164235-29955-2-git-send-email-drizzd@aon.at","subject":"Re: [PATCH 1/5] git rev-list: fix invalid typecast","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-02-13T20:48:29Z","receivedAt":"2012-02-13T20:48:29Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Clemens Buchacher <drizzd@aon.at> writes:\n\n> git rev-list passes rev_list_info, not rev_list objects. Without this\n> fix, rev-list enables or disables the --verify-objects option depending\n> on a read from an undefined memory location.\n>\n> Signed-off-by: Clemens Buchacher <drizzd@aon.at>\n> ---\n\nWow, I simply couldn't believe it, but you are right.\n\nThanks.\n"},{"id":"184608","messageId":"7vlio6cu7s.fsf@alter.siamese.dyndns.org","threadId":"29417","inReplyTo":"1329164235-29955-6-git-send-email-drizzd@aon.at","subject":"Re: [PATCH 5/5] push/fetch/clone --no-progress suppresses progress output","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-02-13T21:13:43Z","receivedAt":"2012-02-13T21:13:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Clemens Buchacher <drizzd@aon.at> writes:\n\n> diff --git a/t/t5523-push-upstream.sh b/t/t5523-push-upstream.sh\n> index 9ee52cf..3683df1 100755\n> --- a/t/t5523-push-upstream.sh\n> +++ b/t/t5523-push-upstream.sh\n> @@ -101,10 +101,11 @@ test_expect_success TTY 'push -q suppresses progress' '\n>  \t! grep \"Writing objects\" err\n>  '\n>  \n> -test_expect_failure TTY 'push --no-progress suppresses progress' '\n> +test_expect_success TTY 'push --no-progress suppresses progress' '\n>  \tensure_fresh_upstream &&\n>  \n>  \ttest_terminal git push -u --no-progress upstream master >out 2>err &&\n> +\t! grep \"Unpacking objects\" err &&\n>  \t! grep \"Writing objects\" err\n>  '\n\nVery nice to see an old expect-failure turned into expect-success.\n\n> diff --git a/transport.c b/transport.c\n> index cac0c06..6074ee9 100644\n> --- a/transport.c\n> +++ b/transport.c\n> @@ -994,10 +994,14 @@ void transport_set_verbosity(struct transport *transport, int verbosity,\n>  \t * when a rule is satisfied):\n>  \t *\n>  \t *   1. Report progress, if force_progress is 1 (ie. --progress).\n> +\t *   2. Don't report progress, if force_progress is 0 (ie. --no-progress).\n>  \t *   2. Don't report progress, if verbosity < 0 (ie. -q/--quiet ).\n>  \t *   3. Report progress if isatty(2) is 1.\n>  \t **/\n\nI'll turn this into an unnumbered bulletted list, while I tease it apart\nto remove textual dependency on the 'verify-refs' extension, which this\nfix shouldn't depend on and taken hostage to.\n\nThanks.\n"},{"id":"184610","messageId":"7vd39ictrb.fsf@alter.siamese.dyndns.org","threadId":"29417","inReplyTo":"1329164235-29955-5-git-send-email-drizzd@aon.at","subject":"Re: [PATCH 4/5] t5541: use configured port number","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-02-13T21:23:36Z","receivedAt":"2012-02-13T21:23:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Good eyes. Thanks.\n"},{"id":"184612","messageId":"7v8vk6csx9.fsf@alter.siamese.dyndns.org","threadId":"29417","inReplyTo":"1329164235-29955-3-git-send-email-drizzd@aon.at","subject":"Re: [PATCH 2/5] do not override receive-pack errors","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-02-13T21:41:38Z","receivedAt":"2012-02-13T21:41:38Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Clemens Buchacher <drizzd@aon.at> writes:\n\n> Receive runs rev-list --verify-objects in order to detect missing\n> objects. However, such errors are ignored and overridden later.\n\nThis makes me worried (not about the patch, but about the current code).\n\nAre there codepaths where an earlier pass of verify-objects mark a cmd as\nbad with a non-NULL error_string, and later code that checks other aspect\nof the push says the update does not violate its criteria, and flips the\nnon-NULL error_string back to NULL?  Or is the only offence you found in\nsuch later code that it fills error_string with its own non-NULL string\nwhen it finds a violation (and otherwise does not touch error_string)?\n\nIn other words, is this really \"ignored and overridden\", not merely\n\"overwritten\"?\n\nIn the following review, I assumed that you meant \"overwritten\".\n\n> diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\n> index fa7448b..0afb8b2 100644\n> --- a/builtin/receive-pack.c\n> +++ b/builtin/receive-pack.c\n> @@ -642,8 +642,10 @@ static void check_aliased_updates(struct command *commands)\n>  \t}\n>  \tsort_string_list(&ref_list);\n>  \n> -\tfor (cmd = commands; cmd; cmd = cmd->next)\n> -\t\tcheck_aliased_update(cmd, &ref_list);\n> +\tfor (cmd = commands; cmd; cmd = cmd->next) {\n> +\t\tif (!cmd->error_string)\n> +\t\t\tcheck_aliased_update(cmd, &ref_list);\n> +\t}\n\nWhile I agree with the general concept of this patch (i.e. if we know an\nerror exists for a particular ref update, we would want to keep the first\none without overwriting it with another error), I am not sure if this hunk\nis correct.  This checks cross reactivity between multiple cmds that can\narise when an update made by one will affect the previous value assumed\nfor another cmd because the former cmd updates a symref whose the target\nis what the later cmd wants to update.  If we have already decided the\nformer cmd is deemed to fail and skip this check, we would not catch that\nthe latter cmd is trying to make an inconsistent update request, and we\nwould end up ignoring that case.  Is that the right thing to do?\n\n> @@ -707,8 +709,10 @@ static void execute_commands(struct command *commands, const char *unpacker_erro\n>  \t\tset_connectivity_errors(commands);\n>  \n>  \tif (run_receive_hook(commands, pre_receive_hook, 0)) {\n> -\t\tfor (cmd = commands; cmd; cmd = cmd->next)\n> -\t\t\tcmd->error_string = \"pre-receive hook declined\";\n> +\t\tfor (cmd = commands; cmd; cmd = cmd->next) {\n> +\t\t\tif (!cmd->error_string)\n> +\t\t\t\tcmd->error_string = \"pre-receive hook declined\";\n> +\t\t}\n>  \t\treturn;\n>  \t}\n>  \n> @@ -717,9 +721,15 @@ static void execute_commands(struct command *commands, const char *unpacker_erro\n>  \tfree(head_name_to_free);\n>  \thead_name = head_name_to_free = resolve_refdup(\"HEAD\", sha1, 0, NULL);\n>  \n> -\tfor (cmd = commands; cmd; cmd = cmd->next)\n> -\t\tif (!cmd->skip_update)\n> -\t\t\tcmd->error_string = update(cmd);\n> +\tfor (cmd = commands; cmd; cmd = cmd->next) {\n> +\t\tif (cmd->error_string)\n> +\t\t\tcontinue;\n> +\n> +\t\tif (cmd->skip_update)\n> +\t\t\tcontinue;\n> +\n> +\t\tcmd->error_string = update(cmd);\n> +\t}\n>  }\n\nThese two hunks look good.\n"},{"id":"184613","messageId":"7v4nuucrbm.fsf@alter.siamese.dyndns.org","threadId":"29417","inReplyTo":"1329164235-29955-4-git-send-email-drizzd@aon.at","subject":"Re: [PATCH 3/5] git push: verify refs early","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-02-13T22:16:13Z","receivedAt":"2012-02-13T22:16:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Clemens Buchacher <drizzd@aon.at> writes:\n\n> I suppose with some effort, this could be done for smart HTTP as well.\n> But I am not sure if we actually want the overhead of the additional\n> ping-pong for HTTP.\n\nHrm, I am confused.\n\nThe updated protocol exchange, if I am reading your patch correctly, would\ngo like this (S stands for the sender, R for the receiver):\n\n        R: Here are the tips of my refs\n        ----\n        S: I'd like to update your refs this way\n        ----\n      + R: No you cannot because all updates will fail, go away\n        or\n      + R: You may proceed, as some updates may succeed\n        ----\n        S: Here is the packfile\n        ----\n        R: Here is how I processed your request\n\nGiven that this makes the sender stall for both smart HTTP and native\nprotocol, don't your worries about the additional ping-pong apply equally\nto both transports?\n\nIf it is not worth doing for smart HTTP, I wonder if it is worth doing for\nnative transport.  After all, \"all updates will fail\" is hopefully the\nless likely case, and with this protocol extension, we end up penalizing\nthe common case with an extra stall for everybody, regardless of the\ntransport.\n\nI dunno.\n\nAll other patches in this \"series\" looked nice fixes and improvements, but\nI am not sure about this change.  At least I am not yet convinced.\n\n>  builtin/receive-pack.c |   83 ++++++++++++++++++++++++++++++++++++++----------\n>  builtin/send-pack.c    |   43 +++++++++++++++++--------\n>  send-pack.h            |    3 +-\n>  3 files changed, 97 insertions(+), 32 deletions(-)\n>\n> diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\n> index 0afb8b2..0129d9c 100644\n> --- a/builtin/receive-pack.c\n> +++ b/builtin/receive-pack.c\n> @@ -34,6 +34,8 @@ static int unpack_limit = 100;\n>  static int report_status;\n>  static int use_sideband;\n>  static int quiet;\n> +static int verify_refs;\n> +static int stateless_rpc;\n>  static int prefer_ofs_delta = 1;\n>  static int auto_update_server_info;\n>  static int auto_gc = 1;\n> @@ -123,7 +125,7 @@ static void show_ref(const char *path, const unsigned char *sha1)\n>  \telse\n>  \t\tpacket_write(1, \"%s %s%c%s%s\\n\",\n>  \t\t\t     sha1_to_hex(sha1), path, 0,\n> -\t\t\t     \" report-status delete-refs side-band-64k quiet\",\n> +\t\t\t     \" report-status delete-refs side-band-64k quiet verify-refs\",\n>  \t\t\t     prefer_ofs_delta ? \" ofs-delta\" : \"\");\n>  \tsent_capabilities = 1;\n>  }\n> @@ -410,14 +412,13 @@ static void refuse_unconfigured_deny_delete_current(void)\n>  \t\trp_error(\"%s\", refuse_unconfigured_deny_delete_current_msg[i]);\n>  }\n>  \n> -static const char *update(struct command *cmd)\n> +static const char *verify_ref(struct command *cmd)\n>  {\n>  \tconst char *name = cmd->ref_name;\n>  \tstruct strbuf namespaced_name_buf = STRBUF_INIT;\n>  \tconst char *namespaced_name;\n>  \tunsigned char *old_sha1 = cmd->old_sha1;\n>  \tunsigned char *new_sha1 = cmd->new_sha1;\n> -\tstruct ref_lock *lock;\n>  \n>  \t/* only refs/... are allowed */\n>  \tif (prefixcmp(name, \"refs/\") || check_refname_format(name + 5, 0)) {\n> @@ -444,12 +445,6 @@ static const char *update(struct command *cmd)\n>  \t\t}\n>  \t}\n>  \n> -\tif (!is_null_sha1(new_sha1) && !has_sha1_file(new_sha1)) {\n> -\t\terror(\"unpack should have generated %s, \"\n> -\t\t      \"but I can't find it!\", sha1_to_hex(new_sha1));\n> -\t\treturn \"bad pack\";\n> -\t}\n> -\n>  \tif (!is_null_sha1(old_sha1) && is_null_sha1(new_sha1)) {\n>  \t\tif (deny_deletes && !prefixcmp(name, \"refs/heads/\")) {\n>  \t\t\trp_error(\"denying ref deletion for %s\", name);\n> @@ -473,6 +468,27 @@ static const char *update(struct command *cmd)\n>  \t\t}\n>  \t}\n>  \n> +\treturn NULL;\n> +}\n> +\n> +static const char *update(struct command *cmd)\n> +{\n> +\tconst char *name = cmd->ref_name;\n> +\tstruct strbuf namespaced_name_buf = STRBUF_INIT;\n> +\tconst char *namespaced_name;\n> +\tunsigned char *old_sha1 = cmd->old_sha1;\n> +\tunsigned char *new_sha1 = cmd->new_sha1;\n> +\tstruct ref_lock *lock;\n> +\n> +\tstrbuf_addf(&namespaced_name_buf, \"%s%s\", get_git_namespace(), name);\n> +\tnamespaced_name = strbuf_detach(&namespaced_name_buf, NULL);\n> +\n> +\tif (!is_null_sha1(new_sha1) && !has_sha1_file(new_sha1)) {\n> +\t\terror(\"unpack should have generated %s, \"\n> +\t\t      \"but I can't find it!\", sha1_to_hex(new_sha1));\n> +\t\treturn \"bad pack\";\n> +\t}\n> +\n>  \tif (deny_non_fast_forwards && !is_null_sha1(new_sha1) &&\n>  \t    !is_null_sha1(old_sha1) &&\n>  \t    !prefixcmp(name, \"refs/heads/\")) {\n> @@ -692,10 +708,41 @@ static int iterate_receive_command_list(void *cb_data, unsigned char sha1[20])\n>  \treturn -1; /* end of list */\n>  }\n>  \n> +static int verify_ref_commands(struct command *commands)\n> +{\n> +\tunsigned char sha1[20];\n> +\tint commands_ok;\n> +\tstruct command *cmd;\n> +\n> +\tfree(head_name_to_free);\n> +\thead_name = head_name_to_free = resolve_refdup(\"HEAD\", sha1, 0, NULL);\n> +\n> +\tcommands_ok = 0;\n> +\tfor (cmd = commands; cmd; cmd = cmd->next) {\n> +\t\tcmd->error_string = verify_ref(cmd);\n> +\t\tif (!cmd->error_string)\n> +\t\t\tcommands_ok++;\n> +\t}\n> +\n> +\tif (verify_refs && !stateless_rpc) {\n> +\t\tstruct strbuf buf = STRBUF_INIT;\n> +\n> +\t\tpacket_buf_write(&buf, \"verify-refs %s\\n\",\n> +\t\t\t commands_ok > 0 ? \"ok\" : \"no valid refs\");\n> +\n> +\t\tif (use_sideband)\n> +\t\t\tsend_sideband(1, 1, buf.buf, buf.len, use_sideband);\n> +\t\telse\n> +\t\t\tsafe_write(1, buf.buf, buf.len);\n> +\t\tstrbuf_release(&buf);\n> +\t}\n> +\n> +\treturn commands_ok;\n> +}\n> +\n>  static void execute_commands(struct command *commands, const char *unpacker_error)\n>  {\n>  \tstruct command *cmd;\n> -\tunsigned char sha1[20];\n>  \n>  \tif (unpacker_error) {\n>  \t\tfor (cmd = commands; cmd; cmd = cmd->next)\n> @@ -718,9 +765,6 @@ static void execute_commands(struct command *commands, const char *unpacker_erro\n>  \n>  \tcheck_aliased_updates(commands);\n>  \n> -\tfree(head_name_to_free);\n> -\thead_name = head_name_to_free = resolve_refdup(\"HEAD\", sha1, 0, NULL);\n> -\n>  \tfor (cmd = commands; cmd; cmd = cmd->next) {\n>  \t\tif (cmd->error_string)\n>  \t\t\tcontinue;\n> @@ -766,6 +810,8 @@ static struct command *read_head_info(void)\n>  \t\t\t\tuse_sideband = LARGE_PACKET_MAX;\n>  \t\t\tif (parse_feature_request(feature_list, \"quiet\"))\n>  \t\t\t\tquiet = 1;\n> +\t\t\tif (parse_feature_request(feature_list, \"verify-refs\"))\n> +\t\t\t\tverify_refs = 1;\n>  \t\t}\n>  \t\tcmd = xcalloc(1, sizeof(struct command) + len - 80);\n>  \t\thashcpy(cmd->old_sha1, old_sha1);\n> @@ -905,7 +951,6 @@ static int delete_only(struct command *commands)\n>  int cmd_receive_pack(int argc, const char **argv, const char *prefix)\n>  {\n>  \tint advertise_refs = 0;\n> -\tint stateless_rpc = 0;\n>  \tint i;\n>  \tchar *dir = NULL;\n>  \tstruct command *commands;\n> @@ -962,11 +1007,15 @@ int cmd_receive_pack(int argc, const char **argv, const char *prefix)\n>  \t\treturn 0;\n>  \n>  \tif ((commands = read_head_info()) != NULL) {\n> +\t\tint commands_ok;\n>  \t\tconst char *unpack_status = NULL;\n>  \n> -\t\tif (!delete_only(commands))\n> -\t\t\tunpack_status = unpack();\n> -\t\texecute_commands(commands, unpack_status);\n> +\t\tcommands_ok = verify_ref_commands(commands);\n> +\t\tif (!verify_refs || commands_ok > 0) {\n> +\t\t\tif (!delete_only(commands))\n> +\t\t\t\tunpack_status = unpack();\n> +\t\t\texecute_commands(commands, unpack_status);\n> +\t\t}\n>  \t\tif (pack_lockfile)\n>  \t\t\tunlink_or_warn(pack_lockfile);\n>  \t\tif (report_status)\n> diff --git a/builtin/send-pack.c b/builtin/send-pack.c\n> index 71f258e..7d514ca 100644\n> --- a/builtin/send-pack.c\n> +++ b/builtin/send-pack.c\n> @@ -265,6 +265,8 @@ int send_pack(struct send_pack_args *args,\n>  \t\tuse_sideband = 1;\n>  \tif (!server_supports(\"quiet\"))\n>  \t\targs->quiet = 0;\n> +\tif (server_supports(\"verify-refs\"))\n> +\t\targs->verify_refs = 1;\n>  \n>  \tif (!remote_refs) {\n>  \t\tfprintf(stderr, \"No refs in common and none specified; doing nothing.\\n\"\n> @@ -303,12 +305,13 @@ int send_pack(struct send_pack_args *args,\n>  \t\t\tchar *old_hex = sha1_to_hex(ref->old_sha1);\n>  \t\t\tchar *new_hex = sha1_to_hex(ref->new_sha1);\n>  \n> -\t\t\tif (!cmds_sent && (status_report || use_sideband || args->quiet)) {\n> -\t\t\t\tpacket_buf_write(&req_buf, \"%s %s %s%c%s%s%s\",\n> +\t\t\tif (!cmds_sent && (status_report || use_sideband || args->quiet || args->verify_refs)) {\n> +\t\t\t\tpacket_buf_write(&req_buf, \"%s %s %s%c%s%s%s%s\",\n>  \t\t\t\t\told_hex, new_hex, ref->name, 0,\n>  \t\t\t\t\tstatus_report ? \" report-status\" : \"\",\n>  \t\t\t\t\tuse_sideband ? \" side-band-64k\" : \"\",\n> -\t\t\t\t\targs->quiet ? \" quiet\" : \"\");\n> +\t\t\t\t\targs->quiet ? \" quiet\" : \"\",\n> +\t\t\t\t\targs->verify_refs ? \" verify-refs\" : \"\");\n>  \t\t\t}\n>  \t\t\telse\n>  \t\t\t\tpacket_buf_write(&req_buf, \"%s %s %s\",\n> @@ -341,17 +344,29 @@ int send_pack(struct send_pack_args *args,\n>  \t\tin = demux.out;\n>  \t}\n>  \n> -\tif (new_refs && cmds_sent) {\n> -\t\tif (pack_objects(out, remote_refs, extra_have, args) < 0) {\n> -\t\t\tfor (ref = remote_refs; ref; ref = ref->next)\n> -\t\t\t\tref->status = REF_STATUS_NONE;\n> -\t\t\tif (args->stateless_rpc)\n> -\t\t\t\tclose(out);\n> -\t\t\tif (git_connection_is_socket(conn))\n> -\t\t\t\tshutdown(fd[0], SHUT_WR);\n> -\t\t\tif (use_sideband)\n> -\t\t\t\tfinish_async(&demux);\n> -\t\t\treturn -1;\n> +\tif (cmds_sent) {\n> +\t\tint verify_refs_status = 0;\n> +\n> +\t\tif (args->verify_refs && !args->stateless_rpc) {\n> +\t\t\tchar line[1000];\n> +\t\t\tint len = packet_read_line(in, line, sizeof(line));\n> +\t\t\tif (len < 15 || memcmp(line, \"verify-refs \", 12))\n> +\t\t\t\treturn error(\"did not receive remote status\");\n> +\t\t\tverify_refs_status = memcmp(line, \"verify-refs ok\\n\", 15);\n> +\t\t}\n> +\n> +\t\tif (!verify_refs_status && new_refs) {\n> +\t\t\tif (pack_objects(out, remote_refs, extra_have, args) < 0) {\n> +\t\t\t\tfor (ref = remote_refs; ref; ref = ref->next)\n> +\t\t\t\t\tref->status = REF_STATUS_NONE;\n> +\t\t\t\tif (args->stateless_rpc)\n> +\t\t\t\t\tclose(out);\n> +\t\t\t\tif (git_connection_is_socket(conn))\n> +\t\t\t\t\tshutdown(fd[0], SHUT_WR);\n> +\t\t\t\tif (use_sideband)\n> +\t\t\t\t\tfinish_async(&demux);\n> +\t\t\t\treturn -1;\n> +\t\t\t}\n>  \t\t}\n>  \t}\n>  \tif (args->stateless_rpc && cmds_sent)\n> diff --git a/send-pack.h b/send-pack.h\n> index 05d7ab1..87edaa5 100644\n> --- a/send-pack.h\n> +++ b/send-pack.h\n> @@ -11,7 +11,8 @@ struct send_pack_args {\n>  \t\tuse_thin_pack:1,\n>  \t\tuse_ofs_delta:1,\n>  \t\tdry_run:1,\n> -\t\tstateless_rpc:1;\n> +\t\tstateless_rpc:1,\n> +\t\tverify_refs:1;\n>  };\n>  \n>  int send_pack(struct send_pack_args *args,\n"},{"id":"184644","messageId":"20120214083324.GA1762@ecki","threadId":"29417","inReplyTo":"7v8vk6csx9.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 2/5] do not override receive-pack errors","fromName":"Clemens Buchacher","fromEmail":"drizzd@aon.at","sentAt":"2012-02-14T08:33:24Z","receivedAt":"2012-02-14T08:33:24Z","isPatch":true,"sender":{"key":"drizzd@gmx.net","avatar":"https://avatars.githubusercontent.com/u/59082?v=4"},"body":"On Mon, Feb 13, 2012 at 01:41:38PM -0800, Junio C Hamano wrote:\n> Clemens Buchacher <drizzd@aon.at> writes:\n> \n> > Receive runs rev-list --verify-objects in order to detect missing\n> > objects. However, such errors are ignored and overridden later.\n> \n> This makes me worried (not about the patch, but about the current code).\n> \n> Are there codepaths where an earlier pass of verify-objects mark a cmd as\n> bad with a non-NULL error_string, and later code that checks other aspect\n> of the push says the update does not violate its criteria, and flips the\n> non-NULL error_string back to NULL?  Or is the only offence you found in\n> such later code that it fills error_string with its own non-NULL string\n> when it finds a violation (and otherwise does not touch error_string)?\n> \n> In other words, is this really \"ignored and overridden\", not merely\n> \"overwritten\"?\n\nYes, it really is. For example, in t5504 rev-list --verify-objects (it\nwas turned on for me if called from there) detects the corrupt object.\nBut the error string is later overwritten with the return value of\nupdate, which is NULL in this case.\n\nThat is why I had to change the t5504 tests from a successful git push\nto a test_must_fail git push with this fix. To keep the previous\nbehavior we would have to replace the corrupt blob with a more subtle\ncorruption that rev-list --verify-objects would not detect, but fsck\nwould (e.g., a malformed commit header).\n\n> > diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\n> > index fa7448b..0afb8b2 100644\n> > --- a/builtin/receive-pack.c\n> > +++ b/builtin/receive-pack.c\n> > @@ -642,8 +642,10 @@ static void check_aliased_updates(struct command *commands)\n> >  \t}\n> >  \tsort_string_list(&ref_list);\n> >  \n> > -\tfor (cmd = commands; cmd; cmd = cmd->next)\n> > -\t\tcheck_aliased_update(cmd, &ref_list);\n> > +\tfor (cmd = commands; cmd; cmd = cmd->next) {\n> > +\t\tif (!cmd->error_string)\n> > +\t\t\tcheck_aliased_update(cmd, &ref_list);\n> > +\t}\n> \n[...]\n> If we have already decided the former cmd is deemed to fail and skip\n> this check, we would not catch that the latter cmd is trying to make\n> an inconsistent update request, and we would end up ignoring that\n> case.\n\nActually, check_alias_update searches for aliases of cmd in ref_list,\nwhich is a list of refs from all commands, irrespective of their error\nstatus. So this change is correct.\n\nHowever, after re-reading the code I now have the impression that the\nalias detection is not entirely correct. It does find aliases between\nsymrefs and regular refs.  But it does not find aliases between two\nsymrefs, because ref_list will not contain the actual ref pointed to,\nand therefore the code considers neither symref an alias.\n\nBut that is independent of the hunk above.\n"},{"id":"184646","messageId":"20120214085919.GB1762@ecki","threadId":"29417","inReplyTo":"7v4nuucrbm.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 3/5] git push: verify refs early","fromName":"Clemens Buchacher","fromEmail":"drizzd@aon.at","sentAt":"2012-02-14T08:59:19Z","receivedAt":"2012-02-14T08:59:19Z","isPatch":true,"sender":{"key":"drizzd@gmx.net","avatar":"https://avatars.githubusercontent.com/u/59082?v=4"},"body":"On Mon, Feb 13, 2012 at 02:16:13PM -0800, Junio C Hamano wrote:\n> Clemens Buchacher <drizzd@aon.at> writes:\n> \n> > I suppose with some effort, this could be done for smart HTTP as well.\n> > But I am not sure if we actually want the overhead of the additional\n> > ping-pong for HTTP.\n> \n> Hrm, I am confused.\n> \n> The updated protocol exchange, if I am reading your patch correctly, would\n> go like this (S stands for the sender, R for the receiver):\n> \n>         R: Here are the tips of my refs\n>         ----\n>         S: I'd like to update your refs this way\n>         ----\n>       + R: No you cannot because all updates will fail, go away\n>         or\n>       + R: You may proceed, as some updates may succeed\n>         ----\n>         S: Here is the packfile\n>         ----\n>         R: Here is how I processed your request\n> \n> Given that this makes the sender stall for both smart HTTP and native\n> protocol, don't your worries about the additional ping-pong apply equally\n> to both transports?\n\nThat is true. However, my assumption was that the overhead is greater\nfor HTTP, because the native protocol is full-duplex, while HTTP tears\ndown the connection and starts from scratch with each request. But to be\nhonest, I am not confident that this assumption is correct.\n\nSo, the stall might be an issue for both the native and the HTTP\nprotocol, or for neither. We should probably find out and then decide\nwhether to make this change for both protocols or not at all.\n\n> If it is not worth doing for smart HTTP, I wonder if it is worth doing for\n> native transport.  After all, \"all updates will fail\" is hopefully the\n> less likely case, and with this protocol extension, we end up penalizing\n> the common case with an extra stall for everybody, regardless of the\n> transport.\n\nIndeed. I wish we could make the ref validation asynchronous. The client\nwould start sending object data right away, while listening for an\n\"abort\" command on the side-band. But if I understand correctly, that is\nnot possible for HTTP.\n"},{"id":"184670","messageId":"7vzkcl6xqn.fsf@alter.siamese.dyndns.org","threadId":"29417","inReplyTo":"20120214083324.GA1762@ecki","subject":"Re: [PATCH 2/5] do not override receive-pack errors","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-02-14T19:06:24Z","receivedAt":"2012-02-14T19:06:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Clemens Buchacher <drizzd@aon.at> writes:\n\n> Yes, it really is. For example, in t5504 rev-list --verify-objects (it\n> was turned on for me if called from there) detects the corrupt object.\n> But the error string is later overwritten with the return value of\n> update, which is NULL in this case.\n> ...\n> Actually, check_alias_update searches for aliases of cmd in ref_list,\n> which is a list of refs from all commands, irrespective of their error\n> status. So this change is correct.\n\nOk, thanks for clarificatin on both counts.\n"}]}