{"thread":{"id":"63904","subject":"[PATCH 0/2] remote.c: remove erroneous BUG case","startedAt":"2025-08-04T09:43:03Z","lastAt":"2025-08-08T16:06:57Z","messageCount":34,"participants":["Denton Liu","Junio C Hamano","Patrick Steinhardt","Ben Knoble","Eric Sunshine"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"523418","messageId":"cover.1754300389.git.liu.denton@gmail.com","threadId":"63904","inReplyTo":null,"subject":"[PATCH 0/2] remote.c: remove erroneous BUG case","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2025-08-04T09:42:53Z","receivedAt":"2025-08-04T09:43:03Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"In the case where one pushes a non-existent oid to an unqualified\ndestination, we encounter the following BUG\n\n\terror: The destination you provided is not a full refname (i.e.,\n\tstarting with \"refs/\"). We tried to guess what you meant by:\n\n\t- Looking for a ref that matches 'branch' on the remote side.\n\t- Checking if the <src> being pushed ('0000000000000000000000000000000000000001')\n\t  is a ref in \"refs/{heads,tags}/\". If so we add a corresponding\n\t  refs/{heads,tags}/ prefix on the remote side.\n\n\tNeither worked, so we gave up. You must fully qualify the ref.\n\tBUG: remote.c:1221: '0000000000000000000000000000000000000001' should be commit/tag/tree/blob, is '-1'\n\tfatal: the remote end hung up unexpectedly\n\tAborted (core dumped)\n\nHowever, this isn't actually a bug so replace it with an advise()\nmessage.\n\nDenton Liu (2):\n  t5516: introduce 'push ref expression with non-existent oid src'\n  remote.c: remove BUG in show_push_unqualified_ref_name_error()\n\n remote.c              | 5 +++--\n t/t5516-fetch-push.sh | 7 +++++++\n 2 files changed, 10 insertions(+), 2 deletions(-)\n\n-- \n2.50.1\n\n"},{"id":"523419","messageId":"d26f355c19c59eae30143900e218533bfeabec2a.1754300389.git.liu.denton@gmail.com","threadId":"63904","inReplyTo":"cover.1754300389.git.liu.denton@gmail.com","subject":"[PATCH 1/2] t5516: introduce 'push ref expression with non-existent oid src'","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2025-08-04T09:43:02Z","receivedAt":"2025-08-04T09:43:05Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"It is possible to trigger a Git bug by pushing a refspec where the\nsource is an oid that's non-existent. An example of the error message\nproduced is as follows:\n\n\terror: The destination you provided is not a full refname (i.e.,\n\tstarting with \"refs/\"). We tried to guess what you meant by:\n\n\t- Looking for a ref that matches 'branch' on the remote side.\n\t- Checking if the <src> being pushed ('0000000000000000000000000000000000000001')\n\t  is a ref in \"refs/{heads,tags}/\". If so we add a corresponding\n\t  refs/{heads,tags}/ prefix on the remote side.\n\n\tNeither worked, so we gave up. You must fully qualify the ref.\n\tBUG: remote.c:1221: '0000000000000000000000000000000000000001' should be commit/tag/tree/blob, is '-1'\n\tfatal: the remote end hung up unexpectedly\n\tAborted (core dumped)\n\nDocument this failure in a test case so that it can be confirmed fixed\nlater.\n\nSigned-off-by: Denton Liu <liu.denton@gmail.com>\n---\n t/t5516-fetch-push.sh | 7 +++++++\n 1 file changed, 7 insertions(+)\n\ndiff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh\nindex 4e9c27b0f2..c2fcfeca92 100755\n--- a/t/t5516-fetch-push.sh\n+++ b/t/t5516-fetch-push.sh\n@@ -509,6 +509,13 @@ test_expect_success 'push ref expression with non-existent, incomplete dest' '\n \n '\n \n+test_expect_failure 'push ref expression with non-existent oid src' '\n+\n+\tmk_test testrepo &&\n+\ttest_must_fail git push testrepo $(test_oid 001):branch\n+\n+'\n+\n for head in HEAD @\n do\n \n-- \n2.50.1\n\n"},{"id":"523420","messageId":"3eb95731ea07c5f25ed7a47cc639f53b4b18e113.1754300389.git.liu.denton@gmail.com","threadId":"63904","inReplyTo":"cover.1754300389.git.liu.denton@gmail.com","subject":"[PATCH 2/2] remote.c: remove BUG in show_push_unqualified_ref_name_error()","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2025-08-04T09:43:05Z","receivedAt":"2025-08-04T09:43:09Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"In the case where a non-existent oid is given as the <src> for a\nrefspec and the destination is unqualified, we end up hitting the BUG in\nshow_push_unqualified_ref_name_error().\n\nThis is because before hitting this advise message, the <src> is passed\nthrough repo_get_oid() which, upon receiving a fully qualified oid,\ndoesn't actually check the existence of the object and just returns\nfound. This means that it's actually possible for the\nodb_read_object_info() call to return not found under normal usage and\nthus, it's not actually a bug.\n\nReplace the BUG() with an advise() displaying a helpful message about\nthe oid possibly not existing.\n\nSigned-off-by: Denton Liu <liu.denton@gmail.com>\n---\n remote.c              | 5 +++--\n t/t5516-fetch-push.sh | 2 +-\n 2 files changed, 4 insertions(+), 3 deletions(-)\n\ndiff --git a/remote.c b/remote.c\nindex e965f022f1..9fb76049d2 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -1218,8 +1218,9 @@ static void show_push_unqualified_ref_name_error(const char *dst_value,\n \t\t\t \"'%s:refs/tags/%s'?\"),\n \t\t       matched_src_name, dst_value);\n \t} else {\n-\t\tBUG(\"'%s' should be commit/tag/tree/blob, is '%d'\",\n-\t\t    matched_src_name, type);\n+\t\tadvise(_(\"The <src> part of the refspec is an oid that doesn't exist.\\n\"\n+\t\t\t \"Please ensure that the oid '%s' is correct.\"),\n+\t\t       matched_src_name);\n \t}\n }\n \ndiff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh\nindex c2fcfeca92..e064ea7433 100755\n--- a/t/t5516-fetch-push.sh\n+++ b/t/t5516-fetch-push.sh\n@@ -509,7 +509,7 @@ test_expect_success 'push ref expression with non-existent, incomplete dest' '\n \n '\n \n-test_expect_failure 'push ref expression with non-existent oid src' '\n+test_expect_success 'push ref expression with non-existent oid src' '\n \n \tmk_test testrepo &&\n \ttest_must_fail git push testrepo $(test_oid 001):branch\n-- \n2.50.1\n\n"},{"id":"523446","messageId":"xmqq5xf39nky.fsf@gitster.g","threadId":"63904","inReplyTo":"3eb95731ea07c5f25ed7a47cc639f53b4b18e113.1754300389.git.liu.denton@gmail.com","subject":"Re: [PATCH 2/2] remote.c: remove BUG in show_push_unqualified_ref_name_error()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-08-04T14:19:09Z","receivedAt":"2025-08-04T14:19:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Denton Liu <liu.denton@gmail.com> writes:\n\n> In the case where a non-existent oid is given as the <src> for a\n> refspec and the destination is unqualified, we end up hitting the BUG in\n> show_push_unqualified_ref_name_error().\n>\n> This is because before hitting this advise message, the <src> is passed\n> through repo_get_oid() which, upon receiving a fully qualified oid,\n> doesn't actually check the existence of the object and just returns\n> found.\n\nThe tail end of the above sentence does not quite parse for me.\nStrike \"and just returns found\" out, perhaps?\n\n> This means that it's actually possible for the\n> odb_read_object_info() call to return not found under normal usage and\n> thus, it's not actually a bug.\n\nAgain it is unclear what this \"not found\", used as noun, means.\n\nOften saying \"A\" and having to follow it with \"this means B\" is a\nsign that both needs to be rewritten to clarify.  The above does it\nthree times (\"A\", \"this is because B\", \"this means C\").  How about\nflowing your thought in a slightly different order, perhaps like\nthis?\n\n    When \"git push <remote> <src>:<dst>\" does not spell out the\n    destination side of the ref fully, and when <src> is not given\n    as a reference but an object name, the code tries to give advice\n    messages based on the type of that object.\n\n    The type is determined by calling odb_read_object_info() and\n    signalled by its return value.  The code however reported a\n    programming error with BUG() when this function said that there\n    is no such object, which happens when the object name is given\n    as a full hexadecimal (if the object name is given as a partial\n    hexadecimal or an non-existing ref, the function would have died\n    without returning, so this BUG() wouldn't have triggered).  This\n    is wrong.  It is an ordinary end-user mistake to give an object\n    name that does not exist and treated as such.\n\nor something?\n\n> Replace the BUG() with an advise() displaying a helpful message about\n> the oid possibly not existing.\n\nI briefly thought this may need to be an error(), but with a larger\ncontext, this else clause is at the end of if/else if/... cascade\nfor different object types, each arm of which emits per object type\nadvice messages, so the new one being another call to advise() would\nmake sense.\n\n>  \t} else {\n> -\t\tBUG(\"'%s' should be commit/tag/tree/blob, is '%d'\",\n> -\t\t    matched_src_name, type);\n> +\t\tadvise(_(\"The <src> part of the refspec is an oid that doesn't exist.\\n\"\n> +\t\t\t \"Please ensure that the oid '%s' is correct.\"),\n> +\t\t       matched_src_name);\n\nUnlike the other existing messages, the second line after the\ndiagnosis in this new message states something that is too obvious\nto anybody---even to somebody who may be helped by an advice message\nthat says \"you seem to have a commit, perhaps you meant to create a\nbranch?\"\n\nThanks.\n\n\n"},{"id":"523512","messageId":"cover.1754375026.git.liu.denton@gmail.com","threadId":"63904","inReplyTo":"cover.1754300389.git.liu.denton@gmail.com","subject":"[PATCH v2 0/2] *** SUBJECT HERE ***","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2025-08-05T06:24:35Z","receivedAt":"2025-08-05T06:24:38Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"*** BLURB HERE ***\n\nDenton Liu (2):\n  t5516: introduce 'push ref expression with non-existent oid src'\n  remote.c: remove BUG in show_push_unqualified_ref_name_error()\n\n remote.c              | 3 +--\n t/t5516-fetch-push.sh | 7 +++++++\n 2 files changed, 8 insertions(+), 2 deletions(-)\n\nRange-diff against v1:\n1:  d26f355c19 = 1:  d26f355c19 t5516: introduce 'push ref expression with non-existent oid src'\n2:  3eb95731ea ! 2:  2bd892b26c remote.c: remove BUG in show_push_unqualified_ref_name_error()\n    @@ Metadata\n      ## Commit message ##\n         remote.c: remove BUG in show_push_unqualified_ref_name_error()\n     \n    -    In the case where a non-existent oid is given as the <src> for a\n    -    refspec and the destination is unqualified, we end up hitting the BUG in\n    -    show_push_unqualified_ref_name_error().\n    +    When \"git push <remote> <src>:<dst>\" does not spell out the\n    +    destination side of the ref fully, and when <src> is not given\n    +    as a reference but an object name, the code tries to give advice\n    +    messages based on the type of that object.\n     \n    -    This is because before hitting this advise message, the <src> is passed\n    -    through repo_get_oid() which, upon receiving a fully qualified oid,\n    -    doesn't actually check the existence of the object and just returns\n    -    found. This means that it's actually possible for the\n    -    odb_read_object_info() call to return not found under normal usage and\n    -    thus, it's not actually a bug.\n    +    The type is determined by calling odb_read_object_info() and\n    +    signalled by its return value.  The code however reported a\n    +    programming error with BUG() when this function said that there\n    +    is no such object, which happens when the object name is given\n    +    as a full hexadecimal (if the object name is given as a partial\n    +    hexadecimal or an non-existing ref, the function would have died\n    +    without returning, so this BUG() wouldn't have triggered).  This\n    +    is wrong.  It is an ordinary end-user mistake to give an object\n    +    name that does not exist and treated as such.\n     \n    -    Replace the BUG() with an advise() displaying a helpful message about\n    -    the oid possibly not existing.\n    +    Helped-by: Junio C Hamano <gitster@pobox.com>\n     \n      ## remote.c ##\n     @@ remote.c: static void show_push_unqualified_ref_name_error(const char *dst_value,\n    @@ remote.c: static void show_push_unqualified_ref_name_error(const char *dst_value\n      \t} else {\n     -\t\tBUG(\"'%s' should be commit/tag/tree/blob, is '%d'\",\n     -\t\t    matched_src_name, type);\n    -+\t\tadvise(_(\"The <src> part of the refspec is an oid that doesn't exist.\\n\"\n    -+\t\t\t \"Please ensure that the oid '%s' is correct.\"),\n    -+\t\t       matched_src_name);\n    ++\t\tadvise(_(\"The <src> part of the refspec is an oid that doesn't exist.\\n\"));\n      \t}\n      }\n      \n-- \n2.50.1\n\n"},{"id":"523513","messageId":"d26f355c19c59eae30143900e218533bfeabec2a.1754375026.git.liu.denton@gmail.com","threadId":"63904","inReplyTo":"cover.1754375026.git.liu.denton@gmail.com","subject":"[PATCH v2 1/2] t5516: introduce 'push ref expression with non-existent oid src'","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2025-08-05T06:24:37Z","receivedAt":"2025-08-05T06:24:41Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"It is possible to trigger a Git bug by pushing a refspec where the\nsource is an oid that's non-existent. An example of the error message\nproduced is as follows:\n\n\terror: The destination you provided is not a full refname (i.e.,\n\tstarting with \"refs/\"). We tried to guess what you meant by:\n\n\t- Looking for a ref that matches 'branch' on the remote side.\n\t- Checking if the <src> being pushed ('0000000000000000000000000000000000000001')\n\t  is a ref in \"refs/{heads,tags}/\". If so we add a corresponding\n\t  refs/{heads,tags}/ prefix on the remote side.\n\n\tNeither worked, so we gave up. You must fully qualify the ref.\n\tBUG: remote.c:1221: '0000000000000000000000000000000000000001' should be commit/tag/tree/blob, is '-1'\n\tfatal: the remote end hung up unexpectedly\n\tAborted (core dumped)\n\nDocument this failure in a test case so that it can be confirmed fixed\nlater.\n\nSigned-off-by: Denton Liu <liu.denton@gmail.com>\n---\n t/t5516-fetch-push.sh | 7 +++++++\n 1 file changed, 7 insertions(+)\n\ndiff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh\nindex 4e9c27b0f2..c2fcfeca92 100755\n--- a/t/t5516-fetch-push.sh\n+++ b/t/t5516-fetch-push.sh\n@@ -509,6 +509,13 @@ test_expect_success 'push ref expression with non-existent, incomplete dest' '\n \n '\n \n+test_expect_failure 'push ref expression with non-existent oid src' '\n+\n+\tmk_test testrepo &&\n+\ttest_must_fail git push testrepo $(test_oid 001):branch\n+\n+'\n+\n for head in HEAD @\n do\n \n-- \n2.50.1\n\n"},{"id":"523514","messageId":"2bd892b26c94133cd1a266d6ff4f2217418b0660.1754375026.git.liu.denton@gmail.com","threadId":"63904","inReplyTo":"cover.1754375026.git.liu.denton@gmail.com","subject":"[PATCH v2 2/2] remote.c: remove BUG in show_push_unqualified_ref_name_error()","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2025-08-05T06:24:40Z","receivedAt":"2025-08-05T06:24:43Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"When \"git push <remote> <src>:<dst>\" does not spell out the\ndestination side of the ref fully, and when <src> is not given\nas a reference but an object name, the code tries to give advice\nmessages based on the type of that object.\n\nThe type is determined by calling odb_read_object_info() and\nsignalled by its return value.  The code however reported a\nprogramming error with BUG() when this function said that there\nis no such object, which happens when the object name is given\nas a full hexadecimal (if the object name is given as a partial\nhexadecimal or an non-existing ref, the function would have died\nwithout returning, so this BUG() wouldn't have triggered).  This\nis wrong.  It is an ordinary end-user mistake to give an object\nname that does not exist and treated as such.\n\nHelped-by: Junio C Hamano <gitster@pobox.com>\nSigned-off-by: Denton Liu <liu.denton@gmail.com>\n---\nThanks, I liked the way you phrased the commit message so I copied it\nwholesale over.\n\n remote.c              | 3 +--\n t/t5516-fetch-push.sh | 2 +-\n 2 files changed, 2 insertions(+), 3 deletions(-)\n\ndiff --git a/remote.c b/remote.c\nindex e965f022f1..4ad20110e9 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -1218,8 +1218,7 @@ static void show_push_unqualified_ref_name_error(const char *dst_value,\n \t\t\t \"'%s:refs/tags/%s'?\"),\n \t\t       matched_src_name, dst_value);\n \t} else {\n-\t\tBUG(\"'%s' should be commit/tag/tree/blob, is '%d'\",\n-\t\t    matched_src_name, type);\n+\t\tadvise(_(\"The <src> part of the refspec is an oid that doesn't exist.\\n\"));\n \t}\n }\n \ndiff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh\nindex c2fcfeca92..e064ea7433 100755\n--- a/t/t5516-fetch-push.sh\n+++ b/t/t5516-fetch-push.sh\n@@ -509,7 +509,7 @@ test_expect_success 'push ref expression with non-existent, incomplete dest' '\n \n '\n \n-test_expect_failure 'push ref expression with non-existent oid src' '\n+test_expect_success 'push ref expression with non-existent oid src' '\n \n \tmk_test testrepo &&\n \ttest_must_fail git push testrepo $(test_oid 001):branch\n-- \n2.50.1\n\n"},{"id":"523547","messageId":"aJIG3TNq5eSzwSPX@pks.im","threadId":"63904","inReplyTo":"2bd892b26c94133cd1a266d6ff4f2217418b0660.1754375026.git.liu.denton@gmail.com","subject":"Re: [PATCH v2 2/2] remote.c: remove BUG in show_push_unqualified_ref_name_error()","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-08-05T13:27:57Z","receivedAt":"2025-08-05T13:28:03Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Mon, Aug 04, 2025 at 11:24:40PM -0700, Denton Liu wrote:\n> When \"git push <remote> <src>:<dst>\" does not spell out the\n> destination side of the ref fully, and when <src> is not given\n> as a reference but an object name, the code tries to give advice\n> messages based on the type of that object.\n> \n> The type is determined by calling odb_read_object_info() and\n> signalled by its return value.  The code however reported a\n> programming error with BUG() when this function said that there\n> is no such object, which happens when the object name is given\n> as a full hexadecimal (if the object name is given as a partial\n> hexadecimal or an non-existing ref, the function would have died\n> without returning, so this BUG() wouldn't have triggered).  This\n> is wrong.  It is an ordinary end-user mistake to give an object\n> name that does not exist and treated as such.\n\nYup, makes sense.\n\n> diff --git a/remote.c b/remote.c\n> index e965f022f1..4ad20110e9 100644\n> --- a/remote.c\n> +++ b/remote.c\n> @@ -1218,8 +1218,7 @@ static void show_push_unqualified_ref_name_error(const char *dst_value,\n>  \t\t\t \"'%s:refs/tags/%s'?\"),\n>  \t\t       matched_src_name, dst_value);\n>  \t} else {\n> -\t\tBUG(\"'%s' should be commit/tag/tree/blob, is '%d'\",\n> -\t\t    matched_src_name, type);\n> +\t\tadvise(_(\"The <src> part of the refspec is an oid that doesn't exist.\\n\"));\n\nI think we should rather say \"object ID\", as \"oid\" is an abbreviation\nthat might not be immediately obvious to the user. Also, should we\ncontinue to mention the object ID? Otherwise it might be hard for the\nuser to figure out which object ID doesn't exist in case they pass\nmultiple refspecs.\n\nPatrick\n"},{"id":"523548","messageId":"aJIG4lZURgqvSup1@pks.im","threadId":"63904","inReplyTo":"d26f355c19c59eae30143900e218533bfeabec2a.1754375026.git.liu.denton@gmail.com","subject":"Re: [PATCH v2 1/2] t5516: introduce 'push ref expression with non-existent oid src'","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-08-05T13:28:02Z","receivedAt":"2025-08-05T13:28:07Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Mon, Aug 04, 2025 at 11:24:37PM -0700, Denton Liu wrote:\n> diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh\n> index 4e9c27b0f2..c2fcfeca92 100755\n> --- a/t/t5516-fetch-push.sh\n> +++ b/t/t5516-fetch-push.sh\n> @@ -509,6 +509,13 @@ test_expect_success 'push ref expression with non-existent, incomplete dest' '\n>  \n>  '\n>  \n> +test_expect_failure 'push ref expression with non-existent oid src' '\n> +\n> +\tmk_test testrepo &&\n> +\ttest_must_fail git push testrepo $(test_oid 001):branch\n> +\n> +'\n> +\n>  for head in HEAD @\n>  do\n\nNit: I don't think it's necessary to implement the test in a separate\ncommit. Folks who want to check that your fix really does something can\ntrivially revert the code changes while retaining the test. I used to do\nthe same in the past, but received the same feedback.\n\nAlso, I think we can drop the empty surrounding lines in the test body.\nOther tests in this file do the same, but that is not a good reason to\nnot do better for newly added tests.\n\nPatrick\n"},{"id":"523576","messageId":"xmqqqzxpzo8b.fsf@gitster.g","threadId":"63904","inReplyTo":"aJIG4lZURgqvSup1@pks.im","subject":"Re: [PATCH v2 1/2] t5516: introduce 'push ref expression with non-existent oid src'","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-08-05T17:12:52Z","receivedAt":"2025-08-05T17:12:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt <ps@pks.im> writes:\n\n> On Mon, Aug 04, 2025 at 11:24:37PM -0700, Denton Liu wrote:\n>> diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh\n>> index 4e9c27b0f2..c2fcfeca92 100755\n>> --- a/t/t5516-fetch-push.sh\n>> +++ b/t/t5516-fetch-push.sh\n>> @@ -509,6 +509,13 @@ test_expect_success 'push ref expression with non-existent, incomplete dest' '\n>>  \n>>  '\n>>  \n>> +test_expect_failure 'push ref expression with non-existent oid src' '\n>> +\n>> +\tmk_test testrepo &&\n>> +\ttest_must_fail git push testrepo $(test_oid 001):branch\n>> +\n>> +'\n>> +\n>>  for head in HEAD @\n>>  do\n>\n> Nit: I don't think it's necessary to implement the test in a separate\n> commit. Folks who want to check that your fix really does something can\n> trivially revert the code changes while retaining the test. I used to do\n> the same in the past, but received the same feedback.\n\nA very good suggestion.\n\n> Also, I think we can drop the empty surrounding lines in the test body.\n> Other tests in this file do the same, but that is not a good reason to\n> not do better for newly added tests.\n\nYup.  The style was from a decade ago when the test suite was being\ndeveloped, and is very out of style these days.\n\nThanks.\n"},{"id":"523617","messageId":"cover.1754455931.git.liu.denton@gmail.com","threadId":"63904","inReplyTo":"cover.1754375026.git.liu.denton@gmail.com","subject":"[PATCH v3 0/2] remote.c: remove erroneous BUG case","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2025-08-06T04:53:36Z","receivedAt":"2025-08-06T04:53:40Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"In the case where one pushes a non-existent oid to an unqualified\ndestination, we encounter the following BUG\n\n\terror: The destination you provided is not a full refname (i.e.,\n\tstarting with \"refs/\"). We tried to guess what you meant by:\n\n\t- Looking for a ref that matches 'branch' on the remote side.\n\t- Checking if the <src> being pushed ('0000000000000000000000000000000000000001')\n\t  is a ref in \"refs/{heads,tags}/\". If so we add a corresponding\n\t  refs/{heads,tags}/ prefix on the remote side.\n\n\tNeither worked, so we gave up. You must fully qualify the ref.\n\tBUG: remote.c:1221: '0000000000000000000000000000000000000001' should be commit/tag/tree/blob, is '-1'\n\tfatal: the remote end hung up unexpectedly\n\tAborted (core dumped)\n\nHowever, this isn't actually a bug so replace it with an advise()\nmessage.\n\nChanges since v2:\n\n* Add t5516 cleanup patch\n* Squash test creation patch into the patch that fixes it\n* Include the erroneous object ID in the advise message\n\nDenton Liu (2):\n  t5516: remove surrounding empty lines in test bodies\n  remote.c: remove BUG in show_push_unqualified_ref_name_error()\n\n remote.c              |  4 ++--\n t/t5516-fetch-push.sh | 54 ++++---------------------------------------\n 2 files changed, 6 insertions(+), 52 deletions(-)\n\nRange-diff against v2:\n1:  d26f355c19 ! 1:  82b09af4ca t5516: introduce 'push ref expression with non-existent oid src'\n    @@ Metadata\n     Author: Denton Liu <liu.denton@gmail.com>\n     \n      ## Commit message ##\n    -    t5516: introduce 'push ref expression with non-existent oid src'\n    +    t5516: remove surrounding empty lines in test bodies\n     \n    -    It is possible to trigger a Git bug by pushing a refspec where the\n    -    source is an oid that's non-existent. An example of the error message\n    -    produced is as follows:\n    -\n    -            error: The destination you provided is not a full refname (i.e.,\n    -            starting with \"refs/\"). We tried to guess what you meant by:\n    -\n    -            - Looking for a ref that matches 'branch' on the remote side.\n    -            - Checking if the <src> being pushed ('0000000000000000000000000000000000000001')\n    -              is a ref in \"refs/{heads,tags}/\". If so we add a corresponding\n    -              refs/{heads,tags}/ prefix on the remote side.\n    -\n    -            Neither worked, so we gave up. You must fully qualify the ref.\n    -            BUG: remote.c:1221: '0000000000000000000000000000000000000001' should be commit/tag/tree/blob, is '-1'\n    -            fatal: the remote end hung up unexpectedly\n    -            Aborted (core dumped)\n    -\n    -    Document this failure in a test case so that it can be confirmed fixed\n    -    later.\n    +    This style with the empty lines in test bodies was from when the test\n    +    suite was being developed. Remove the empty lines to match the modern\n    +    test style.\n     \n      ## t/t5516-fetch-push.sh ##\n    -@@ t/t5516-fetch-push.sh: test_expect_success 'push ref expression with non-existent, incomplete dest' '\n    +@@ t/t5516-fetch-push.sh: check_push_result () {\n    + }\n      \n    + test_expect_success setup '\n    +-\n    + \t>path1 &&\n    + \tgit add path1 &&\n    + \ttest_tick &&\n    +@@ t/t5516-fetch-push.sh: test_expect_success setup '\n    + \ttest_tick &&\n    + \tgit commit -a -m second &&\n    + \tthe_commit=$(git show-ref -s --verify refs/heads/main)\n    +-\n      '\n      \n    -+test_expect_failure 'push ref expression with non-existent oid src' '\n    -+\n    -+\tmk_test testrepo &&\n    -+\ttest_must_fail git push testrepo $(test_oid 001):branch\n    -+\n    -+'\n    -+\n    - for head in HEAD @\n    - do\n    + for cmd in push fetch\n    +@@ t/t5516-fetch-push.sh: test_expect_success 'push with pushInsteadOf and explicit pushurl (pushInsteadOf\n    + '\n      \n    + test_expect_success 'push with matching heads' '\n    +-\n    + \tmk_test testrepo heads/main &&\n    + \tgit push testrepo : &&\n    + \tcheck_push_result testrepo $the_commit heads/main\n    +-\n    + '\n    + \n    + test_expect_success 'push with matching heads on the command line' '\n    +-\n    + \tmk_test testrepo heads/main &&\n    + \tgit push testrepo : &&\n    + \tcheck_push_result testrepo $the_commit heads/main\n    +-\n    + '\n    + \n    + test_expect_success 'failed (non-fast-forward) push with matching heads' '\n    +-\n    + \tmk_test testrepo heads/main &&\n    + \tgit push testrepo : &&\n    + \tgit commit --amend -massaged &&\n    + \ttest_must_fail git push testrepo &&\n    + \tcheck_push_result testrepo $the_commit heads/main &&\n    + \tgit reset --hard $the_commit\n    +-\n    + '\n    + \n    + test_expect_success 'push --force with matching heads' '\n    +-\n    + \tmk_test testrepo heads/main &&\n    + \tgit push testrepo : &&\n    + \tgit commit --amend -massaged &&\n    + \tgit push --force testrepo : &&\n    + \t! check_push_result testrepo $the_commit heads/main &&\n    + \tgit reset --hard $the_commit\n    +-\n    + '\n    + \n    + test_expect_success 'push with matching heads and forced update' '\n    +-\n    + \tmk_test testrepo heads/main &&\n    + \tgit push testrepo : &&\n    + \tgit commit --amend -massaged &&\n    + \tgit push testrepo +: &&\n    + \t! check_push_result testrepo $the_commit heads/main &&\n    + \tgit reset --hard $the_commit\n    +-\n    + '\n    + \n    + test_expect_success 'push with no ambiguity (1)' '\n    +-\n    + \tmk_test testrepo heads/main &&\n    + \tgit push testrepo main:main &&\n    + \tcheck_push_result testrepo $the_commit heads/main\n    +-\n    + '\n    + \n    + test_expect_success 'push with no ambiguity (2)' '\n    +-\n    + \tmk_test testrepo remotes/origin/main &&\n    + \tgit push testrepo main:origin/main &&\n    + \tcheck_push_result testrepo $the_commit remotes/origin/main\n    +-\n    + '\n    + \n    + test_expect_success 'push with colon-less refspec, no ambiguity' '\n    +-\n    + \tmk_test testrepo heads/main heads/t/main &&\n    + \tgit branch -f t/main main &&\n    + \tgit push testrepo main &&\n    + \tcheck_push_result testrepo $the_commit heads/main &&\n    + \tcheck_push_result testrepo $the_first_commit heads/t/main\n    +-\n    + '\n    + \n    + test_expect_success 'push with weak ambiguity (1)' '\n    +-\n    + \tmk_test testrepo heads/main remotes/origin/main &&\n    + \tgit push testrepo main:main &&\n    + \tcheck_push_result testrepo $the_commit heads/main &&\n    + \tcheck_push_result testrepo $the_first_commit remotes/origin/main\n    +-\n    + '\n    + \n    + test_expect_success 'push with weak ambiguity (2)' '\n    +-\n    + \tmk_test testrepo heads/main remotes/origin/main remotes/another/main &&\n    + \tgit push testrepo main:main &&\n    + \tcheck_push_result testrepo $the_commit heads/main &&\n    + \tcheck_push_result testrepo $the_first_commit remotes/origin/main remotes/another/main\n    +-\n    + '\n    + \n    + test_expect_success 'push with ambiguity' '\n    +-\n    + \tmk_test testrepo heads/frotz tags/frotz &&\n    + \ttest_must_fail git push testrepo main:frotz &&\n    + \tcheck_push_result testrepo $the_first_commit heads/frotz tags/frotz\n    +-\n    + '\n    + \n    + test_expect_success 'push with onelevel ref' '\n    +@@ t/t5516-fetch-push.sh: test_expect_success 'push with onelevel ref' '\n    + '\n    + \n    + test_expect_success 'push with colon-less refspec (1)' '\n    +-\n    + \tmk_test testrepo heads/frotz tags/frotz &&\n    + \tgit branch -f frotz main &&\n    + \tgit push testrepo frotz &&\n    + \tcheck_push_result testrepo $the_commit heads/frotz &&\n    + \tcheck_push_result testrepo $the_first_commit tags/frotz\n    +-\n    + '\n    + \n    + test_expect_success 'push with colon-less refspec (2)' '\n    +-\n    + \tmk_test testrepo heads/frotz tags/frotz &&\n    + \tif git show-ref --verify -q refs/heads/frotz\n    + \tthen\n    +@@ t/t5516-fetch-push.sh: test_expect_success 'push with colon-less refspec (2)' '\n    + \tgit push -f testrepo frotz &&\n    + \tcheck_push_result testrepo $the_commit tags/frotz &&\n    + \tcheck_push_result testrepo $the_first_commit heads/frotz\n    +-\n    + '\n    + \n    + test_expect_success 'push with colon-less refspec (3)' '\n    +@@ t/t5516-fetch-push.sh: test_expect_success 'push with colon-less refspec (3)' '\n    + '\n    + \n    + test_expect_success 'push with colon-less refspec (4)' '\n    +-\n    + \tmk_test testrepo &&\n    + \tif git show-ref --verify -q refs/heads/frotz\n    + \tthen\n    +@@ t/t5516-fetch-push.sh: test_expect_success 'push with colon-less refspec (4)' '\n    + \tgit push testrepo frotz &&\n    + \tcheck_push_result testrepo $the_commit tags/frotz &&\n    + \ttest 1 = $( cd testrepo && git show-ref | wc -l )\n    +-\n    + '\n    + \n    + test_expect_success 'push head with non-existent, incomplete dest' '\n    +-\n    + \tmk_test testrepo &&\n    + \tgit push testrepo main:branch &&\n    + \tcheck_push_result testrepo $the_commit heads/branch\n    +-\n    + '\n    + \n    + test_expect_success 'push tag with non-existent, incomplete dest' '\n    +-\n    + \tmk_test testrepo &&\n    + \tgit tag -f v1.0 &&\n    + \tgit push testrepo v1.0:tag &&\n    + \tcheck_push_result testrepo $the_commit tags/tag\n    +-\n    + '\n    + \n    + test_expect_success 'push oid with non-existent, incomplete dest' '\n    +-\n    + \tmk_test testrepo &&\n    + \ttest_must_fail git push testrepo $(git rev-parse main):foo\n    +-\n    + '\n    + \n    + test_expect_success 'push ref expression with non-existent, incomplete dest' '\n    +-\n    + \tmk_test testrepo &&\n    + \ttest_must_fail git push testrepo main^:branch\n    +-\n    + '\n    + \n    + for head in HEAD @\n    +@@ t/t5516-fetch-push.sh: do\n    + \t\tgit checkout main &&\n    + \t\tgit push testrepo $head:branch &&\n    + \t\tcheck_push_result testrepo $the_commit heads/branch\n    +-\n    + \t'\n    + \n    + \ttest_expect_success \"push with config remote.*.push = $head\" '\n    +@@ t/t5516-fetch-push.sh: test_expect_success 'push with remote.pushdefault' '\n    + '\n    + \n    + test_expect_success 'push with config remote.*.pushurl' '\n    +-\n    + \tmk_test testrepo heads/main &&\n    + \tgit checkout main &&\n    + \ttest_config remote.there.url test2repo &&\n    +@@ t/t5516-fetch-push.sh: test_expect_success 'push ignores \"branch.\" config without subsection' '\n    + '\n    + \n    + test_expect_success 'push with dry-run' '\n    +-\n    + \tmk_test testrepo heads/main &&\n    + \told_commit=$(git -C testrepo show-ref -s --verify refs/heads/main) &&\n    + \tgit push --dry-run testrepo : &&\n    +@@ t/t5516-fetch-push.sh: test_expect_success 'push with dry-run' '\n    + '\n    + \n    + test_expect_success 'push updates local refs' '\n    +-\n    + \tmk_test testrepo heads/main &&\n    + \tmk_child testrepo child &&\n    + \t(\n    +@@ t/t5516-fetch-push.sh: test_expect_success 'push updates local refs' '\n    + \t\ttest $(git rev-parse main) = \\\n    + \t\t\t$(git rev-parse remotes/origin/main)\n    + \t)\n    +-\n    + '\n    + \n    + test_expect_success 'push updates up-to-date local refs' '\n    +-\n    + \tmk_test testrepo heads/main &&\n    + \tmk_child testrepo child1 &&\n    + \tmk_child testrepo child2 &&\n    +@@ t/t5516-fetch-push.sh: test_expect_success 'push updates up-to-date local refs' '\n    + \t\ttest $(git rev-parse main) = \\\n    + \t\t\t$(git rev-parse remotes/origin/main)\n    + \t)\n    +-\n    + '\n    + \n    + test_expect_success 'push preserves up-to-date packed refs' '\n    +-\n    + \tmk_test testrepo heads/main &&\n    + \tmk_child testrepo child &&\n    + \t(\n    +@@ t/t5516-fetch-push.sh: test_expect_success 'push preserves up-to-date packed refs' '\n    + \t\tgit push &&\n    + \t\t! test -f .git/refs/remotes/origin/main\n    + \t)\n    +-\n    + '\n    + \n    + test_expect_success 'push does not update local refs on failure' '\n    +-\n    + \tmk_test testrepo heads/main &&\n    + \tmk_child testrepo child &&\n    + \techo \"#!/no/frobnication/today\" >testrepo/.git/hooks/pre-receive &&\n    +@@ t/t5516-fetch-push.sh: test_expect_success 'push does not update local refs on failure' '\n    + \t\ttest $(git rev-parse main) != \\\n    + \t\t\t$(git rev-parse remotes/origin/main)\n    + \t)\n    +-\n    + '\n    + \n    + test_expect_success 'allow deleting an invalid remote ref' '\n    +-\n    + \tmk_test testrepo heads/branch &&\n    + \trm -f testrepo/.git/objects/??/* &&\n    + \tgit push testrepo :refs/heads/branch &&\n    + \t(cd testrepo && test_must_fail git rev-parse --verify refs/heads/branch)\n    +-\n    + '\n    + \n    + test_expect_success 'pushing valid refs triggers post-receive and post-update hooks' '\n2:  2bd892b26c ! 2:  938dfb8d4e remote.c: remove BUG in show_push_unqualified_ref_name_error()\n    @@ Commit message\n         is wrong.  It is an ordinary end-user mistake to give an object\n         name that does not exist and treated as such.\n     \n    +    An example of the error message produced is as follows:\n    +\n    +            error: The destination you provided is not a full refname (i.e.,\n    +            starting with \"refs/\"). We tried to guess what you meant by:\n    +\n    +            - Looking for a ref that matches 'branch' on the remote side.\n    +            - Checking if the <src> being pushed ('0000000000000000000000000000000000000001')\n    +              is a ref in \"refs/{heads,tags}/\". If so we add a corresponding\n    +              refs/{heads,tags}/ prefix on the remote side.\n    +\n    +            Neither worked, so we gave up. You must fully qualify the ref.\n    +            BUG: remote.c:1221: '0000000000000000000000000000000000000001' should be commit/tag/tree/blob, is '-1'\n    +            fatal: the remote end hung up unexpectedly\n    +            Aborted (core dumped)\n    +\n         Helped-by: Junio C Hamano <gitster@pobox.com>\n     \n      ## remote.c ##\n    @@ remote.c: static void show_push_unqualified_ref_name_error(const char *dst_value\n      \t} else {\n     -\t\tBUG(\"'%s' should be commit/tag/tree/blob, is '%d'\",\n     -\t\t    matched_src_name, type);\n    -+\t\tadvise(_(\"The <src> part of the refspec is an oid that doesn't exist.\\n\"));\n    ++\t\tadvise(_(\"The <src> part of the refspec ('%s') is an object ID that doesn't exist.\\n\"),\n    ++\t\t       matched_src_name);\n      \t}\n      }\n      \n     \n      ## t/t5516-fetch-push.sh ##\n     @@ t/t5516-fetch-push.sh: test_expect_success 'push ref expression with non-existent, incomplete dest' '\n    - \n    + \ttest_must_fail git push testrepo main^:branch\n      '\n      \n    --test_expect_failure 'push ref expression with non-existent oid src' '\n     +test_expect_success 'push ref expression with non-existent oid src' '\n    ++\tmk_test testrepo &&\n    ++\ttest_must_fail git push testrepo $(test_oid 001):branch\n    ++'\n    ++\n    + for head in HEAD @\n    + do\n      \n    - \tmk_test testrepo &&\n    - \ttest_must_fail git push testrepo $(test_oid 001):branch\n-- \n2.50.1\n\n"},{"id":"523618","messageId":"82b09af4ca8e610dd06b94be560622837a35d3ff.1754455931.git.liu.denton@gmail.com","threadId":"63904","inReplyTo":"cover.1754455931.git.liu.denton@gmail.com","subject":"[PATCH v3 1/2] t5516: remove surrounding empty lines in test bodies","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2025-08-06T04:53:39Z","receivedAt":"2025-08-06T04:53:42Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"This style with the empty lines in test bodies was from when the test\nsuite was being developed. Remove the empty lines to match the modern\ntest style.\n\nSigned-off-by: Denton Liu <liu.denton@gmail.com>\n---\n t/t5516-fetch-push.sh | 51 -------------------------------------------\n 1 file changed, 51 deletions(-)\n\ndiff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh\nindex 4e9c27b0f2..8eddf3e40d 100755\n--- a/t/t5516-fetch-push.sh\n+++ b/t/t5516-fetch-push.sh\n@@ -105,7 +105,6 @@ check_push_result () {\n }\n \n test_expect_success setup '\n-\n \t>path1 &&\n \tgit add path1 &&\n \ttest_tick &&\n@@ -117,7 +116,6 @@ test_expect_success setup '\n \ttest_tick &&\n \tgit commit -a -m second &&\n \tthe_commit=$(git show-ref -s --verify refs/heads/main)\n-\n '\n \n for cmd in push fetch\n@@ -322,104 +320,82 @@ test_expect_success 'push with pushInsteadOf and explicit pushurl (pushInsteadOf\n '\n \n test_expect_success 'push with matching heads' '\n-\n \tmk_test testrepo heads/main &&\n \tgit push testrepo : &&\n \tcheck_push_result testrepo $the_commit heads/main\n-\n '\n \n test_expect_success 'push with matching heads on the command line' '\n-\n \tmk_test testrepo heads/main &&\n \tgit push testrepo : &&\n \tcheck_push_result testrepo $the_commit heads/main\n-\n '\n \n test_expect_success 'failed (non-fast-forward) push with matching heads' '\n-\n \tmk_test testrepo heads/main &&\n \tgit push testrepo : &&\n \tgit commit --amend -massaged &&\n \ttest_must_fail git push testrepo &&\n \tcheck_push_result testrepo $the_commit heads/main &&\n \tgit reset --hard $the_commit\n-\n '\n \n test_expect_success 'push --force with matching heads' '\n-\n \tmk_test testrepo heads/main &&\n \tgit push testrepo : &&\n \tgit commit --amend -massaged &&\n \tgit push --force testrepo : &&\n \t! check_push_result testrepo $the_commit heads/main &&\n \tgit reset --hard $the_commit\n-\n '\n \n test_expect_success 'push with matching heads and forced update' '\n-\n \tmk_test testrepo heads/main &&\n \tgit push testrepo : &&\n \tgit commit --amend -massaged &&\n \tgit push testrepo +: &&\n \t! check_push_result testrepo $the_commit heads/main &&\n \tgit reset --hard $the_commit\n-\n '\n \n test_expect_success 'push with no ambiguity (1)' '\n-\n \tmk_test testrepo heads/main &&\n \tgit push testrepo main:main &&\n \tcheck_push_result testrepo $the_commit heads/main\n-\n '\n \n test_expect_success 'push with no ambiguity (2)' '\n-\n \tmk_test testrepo remotes/origin/main &&\n \tgit push testrepo main:origin/main &&\n \tcheck_push_result testrepo $the_commit remotes/origin/main\n-\n '\n \n test_expect_success 'push with colon-less refspec, no ambiguity' '\n-\n \tmk_test testrepo heads/main heads/t/main &&\n \tgit branch -f t/main main &&\n \tgit push testrepo main &&\n \tcheck_push_result testrepo $the_commit heads/main &&\n \tcheck_push_result testrepo $the_first_commit heads/t/main\n-\n '\n \n test_expect_success 'push with weak ambiguity (1)' '\n-\n \tmk_test testrepo heads/main remotes/origin/main &&\n \tgit push testrepo main:main &&\n \tcheck_push_result testrepo $the_commit heads/main &&\n \tcheck_push_result testrepo $the_first_commit remotes/origin/main\n-\n '\n \n test_expect_success 'push with weak ambiguity (2)' '\n-\n \tmk_test testrepo heads/main remotes/origin/main remotes/another/main &&\n \tgit push testrepo main:main &&\n \tcheck_push_result testrepo $the_commit heads/main &&\n \tcheck_push_result testrepo $the_first_commit remotes/origin/main remotes/another/main\n-\n '\n \n test_expect_success 'push with ambiguity' '\n-\n \tmk_test testrepo heads/frotz tags/frotz &&\n \ttest_must_fail git push testrepo main:frotz &&\n \tcheck_push_result testrepo $the_first_commit heads/frotz tags/frotz\n-\n '\n \n test_expect_success 'push with onelevel ref' '\n@@ -428,17 +404,14 @@ test_expect_success 'push with onelevel ref' '\n '\n \n test_expect_success 'push with colon-less refspec (1)' '\n-\n \tmk_test testrepo heads/frotz tags/frotz &&\n \tgit branch -f frotz main &&\n \tgit push testrepo frotz &&\n \tcheck_push_result testrepo $the_commit heads/frotz &&\n \tcheck_push_result testrepo $the_first_commit tags/frotz\n-\n '\n \n test_expect_success 'push with colon-less refspec (2)' '\n-\n \tmk_test testrepo heads/frotz tags/frotz &&\n \tif git show-ref --verify -q refs/heads/frotz\n \tthen\n@@ -448,7 +421,6 @@ test_expect_success 'push with colon-less refspec (2)' '\n \tgit push -f testrepo frotz &&\n \tcheck_push_result testrepo $the_commit tags/frotz &&\n \tcheck_push_result testrepo $the_first_commit heads/frotz\n-\n '\n \n test_expect_success 'push with colon-less refspec (3)' '\n@@ -465,7 +437,6 @@ test_expect_success 'push with colon-less refspec (3)' '\n '\n \n test_expect_success 'push with colon-less refspec (4)' '\n-\n \tmk_test testrepo &&\n \tif git show-ref --verify -q refs/heads/frotz\n \tthen\n@@ -475,38 +446,29 @@ test_expect_success 'push with colon-less refspec (4)' '\n \tgit push testrepo frotz &&\n \tcheck_push_result testrepo $the_commit tags/frotz &&\n \ttest 1 = $( cd testrepo && git show-ref | wc -l )\n-\n '\n \n test_expect_success 'push head with non-existent, incomplete dest' '\n-\n \tmk_test testrepo &&\n \tgit push testrepo main:branch &&\n \tcheck_push_result testrepo $the_commit heads/branch\n-\n '\n \n test_expect_success 'push tag with non-existent, incomplete dest' '\n-\n \tmk_test testrepo &&\n \tgit tag -f v1.0 &&\n \tgit push testrepo v1.0:tag &&\n \tcheck_push_result testrepo $the_commit tags/tag\n-\n '\n \n test_expect_success 'push oid with non-existent, incomplete dest' '\n-\n \tmk_test testrepo &&\n \ttest_must_fail git push testrepo $(git rev-parse main):foo\n-\n '\n \n test_expect_success 'push ref expression with non-existent, incomplete dest' '\n-\n \tmk_test testrepo &&\n \ttest_must_fail git push testrepo main^:branch\n-\n '\n \n for head in HEAD @\n@@ -550,7 +512,6 @@ do\n \t\tgit checkout main &&\n \t\tgit push testrepo $head:branch &&\n \t\tcheck_push_result testrepo $the_commit heads/branch\n-\n \t'\n \n \ttest_expect_success \"push with config remote.*.push = $head\" '\n@@ -596,7 +557,6 @@ test_expect_success 'push with remote.pushdefault' '\n '\n \n test_expect_success 'push with config remote.*.pushurl' '\n-\n \tmk_test testrepo heads/main &&\n \tgit checkout main &&\n \ttest_config remote.there.url test2repo &&\n@@ -655,7 +615,6 @@ test_expect_success 'push ignores \"branch.\" config without subsection' '\n '\n \n test_expect_success 'push with dry-run' '\n-\n \tmk_test testrepo heads/main &&\n \told_commit=$(git -C testrepo show-ref -s --verify refs/heads/main) &&\n \tgit push --dry-run testrepo : &&\n@@ -663,7 +622,6 @@ test_expect_success 'push with dry-run' '\n '\n \n test_expect_success 'push updates local refs' '\n-\n \tmk_test testrepo heads/main &&\n \tmk_child testrepo child &&\n \t(\n@@ -673,11 +631,9 @@ test_expect_success 'push updates local refs' '\n \t\ttest $(git rev-parse main) = \\\n \t\t\t$(git rev-parse remotes/origin/main)\n \t)\n-\n '\n \n test_expect_success 'push updates up-to-date local refs' '\n-\n \tmk_test testrepo heads/main &&\n \tmk_child testrepo child1 &&\n \tmk_child testrepo child2 &&\n@@ -689,11 +645,9 @@ test_expect_success 'push updates up-to-date local refs' '\n \t\ttest $(git rev-parse main) = \\\n \t\t\t$(git rev-parse remotes/origin/main)\n \t)\n-\n '\n \n test_expect_success 'push preserves up-to-date packed refs' '\n-\n \tmk_test testrepo heads/main &&\n \tmk_child testrepo child &&\n \t(\n@@ -701,11 +655,9 @@ test_expect_success 'push preserves up-to-date packed refs' '\n \t\tgit push &&\n \t\t! test -f .git/refs/remotes/origin/main\n \t)\n-\n '\n \n test_expect_success 'push does not update local refs on failure' '\n-\n \tmk_test testrepo heads/main &&\n \tmk_child testrepo child &&\n \techo \"#!/no/frobnication/today\" >testrepo/.git/hooks/pre-receive &&\n@@ -717,16 +669,13 @@ test_expect_success 'push does not update local refs on failure' '\n \t\ttest $(git rev-parse main) != \\\n \t\t\t$(git rev-parse remotes/origin/main)\n \t)\n-\n '\n \n test_expect_success 'allow deleting an invalid remote ref' '\n-\n \tmk_test testrepo heads/branch &&\n \trm -f testrepo/.git/objects/??/* &&\n \tgit push testrepo :refs/heads/branch &&\n \t(cd testrepo && test_must_fail git rev-parse --verify refs/heads/branch)\n-\n '\n \n test_expect_success 'pushing valid refs triggers post-receive and post-update hooks' '\n-- \n2.50.1\n\n"},{"id":"523619","messageId":"938dfb8d4e37ef962c811d6e0f32122a2522deb5.1754455931.git.liu.denton@gmail.com","threadId":"63904","inReplyTo":"cover.1754455931.git.liu.denton@gmail.com","subject":"[PATCH v3 2/2] remote.c: remove BUG in show_push_unqualified_ref_name_error()","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2025-08-06T04:53:42Z","receivedAt":"2025-08-06T04:53:45Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"When \"git push <remote> <src>:<dst>\" does not spell out the\ndestination side of the ref fully, and when <src> is not given\nas a reference but an object name, the code tries to give advice\nmessages based on the type of that object.\n\nThe type is determined by calling odb_read_object_info() and\nsignalled by its return value.  The code however reported a\nprogramming error with BUG() when this function said that there\nis no such object, which happens when the object name is given\nas a full hexadecimal (if the object name is given as a partial\nhexadecimal or an non-existing ref, the function would have died\nwithout returning, so this BUG() wouldn't have triggered).  This\nis wrong.  It is an ordinary end-user mistake to give an object\nname that does not exist and treated as such.\n\nAn example of the error message produced is as follows:\n\n\terror: The destination you provided is not a full refname (i.e.,\n\tstarting with \"refs/\"). We tried to guess what you meant by:\n\n\t- Looking for a ref that matches 'branch' on the remote side.\n\t- Checking if the <src> being pushed ('0000000000000000000000000000000000000001')\n\t  is a ref in \"refs/{heads,tags}/\". If so we add a corresponding\n\t  refs/{heads,tags}/ prefix on the remote side.\n\n\tNeither worked, so we gave up. You must fully qualify the ref.\n\tBUG: remote.c:1221: '0000000000000000000000000000000000000001' should be commit/tag/tree/blob, is '-1'\n\tfatal: the remote end hung up unexpectedly\n\tAborted (core dumped)\n\nHelped-by: Junio C Hamano <gitster@pobox.com>\nSigned-off-by: Denton Liu <liu.denton@gmail.com>\n---\n remote.c              | 4 ++--\n t/t5516-fetch-push.sh | 5 +++++\n 2 files changed, 7 insertions(+), 2 deletions(-)\n\ndiff --git a/remote.c b/remote.c\nindex e965f022f1..465e0ea0eb 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -1218,8 +1218,8 @@ static void show_push_unqualified_ref_name_error(const char *dst_value,\n \t\t\t \"'%s:refs/tags/%s'?\"),\n \t\t       matched_src_name, dst_value);\n \t} else {\n-\t\tBUG(\"'%s' should be commit/tag/tree/blob, is '%d'\",\n-\t\t    matched_src_name, type);\n+\t\tadvise(_(\"The <src> part of the refspec ('%s') is an object ID that doesn't exist.\\n\"),\n+\t\t       matched_src_name);\n \t}\n }\n \ndiff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh\nindex 8eddf3e40d..46926e7bbd 100755\n--- a/t/t5516-fetch-push.sh\n+++ b/t/t5516-fetch-push.sh\n@@ -471,6 +471,11 @@ test_expect_success 'push ref expression with non-existent, incomplete dest' '\n \ttest_must_fail git push testrepo main^:branch\n '\n \n+test_expect_success 'push ref expression with non-existent oid src' '\n+\tmk_test testrepo &&\n+\ttest_must_fail git push testrepo $(test_oid 001):branch\n+'\n+\n for head in HEAD @\n do\n \n-- \n2.50.1\n\n"},{"id":"523633","messageId":"aJLywm9xWQQUADH1@pks.im","threadId":"63904","inReplyTo":"938dfb8d4e37ef962c811d6e0f32122a2522deb5.1754455931.git.liu.denton@gmail.com","subject":"Re: [PATCH v3 2/2] remote.c: remove BUG in show_push_unqualified_ref_name_error()","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-08-06T06:14:26Z","receivedAt":"2025-08-06T06:14:33Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Tue, Aug 05, 2025 at 09:53:42PM -0700, Denton Liu wrote:\n> diff --git a/remote.c b/remote.c\n> index e965f022f1..465e0ea0eb 100644\n> --- a/remote.c\n> +++ b/remote.c\n> @@ -1218,8 +1218,8 @@ static void show_push_unqualified_ref_name_error(const char *dst_value,\n>  \t\t\t \"'%s:refs/tags/%s'?\"),\n>  \t\t       matched_src_name, dst_value);\n>  \t} else {\n> -\t\tBUG(\"'%s' should be commit/tag/tree/blob, is '%d'\",\n> -\t\t    matched_src_name, type);\n> +\t\tadvise(_(\"The <src> part of the refspec ('%s') is an object ID that doesn't exist.\\n\"),\n> +\t\t       matched_src_name);\n>  \t}\n>  }\n\nThis reads a lot better, thanks. We could arguably convert the\nif-else-chain into a switch to make all of this read a bit better, but\nthat is a subjective style change and definitely not something that you\nhave to do as part of this series.\n\nRegarding the logic this looks sensible to me. We have already handled\nall valid object types in the cases leading up to this final `else`, so\nwe can be sure that we weren't able to look up the object. And warning\nabout that case feels reasonable.\n\nOne thing I wondered is whether it's okay to not die anymore via\n`BUG()`. The other error cases already don't die though, so this ought\nto be fine. Going up the callchain shows that we do bubble up the error\nas expected until we end up in `match_push_refs()`. There's multiple\ncallers of that function, and all except one perform error handling for\nit.\n\nThe only exception is git-remote(1) in `get_push_ref_states()`, where it\ngets executed via `git remote show $remote_name`. As far as I understand\nwe would end up not showing any references that are broken, and we would\nprint the above advise. Which I think is reasonable.\n\nSo all of this looks good to me, thanks!\n\nPatrick\n"},{"id":"523634","messageId":"aJLyyPpvlFjwBCIA@pks.im","threadId":"63904","inReplyTo":"82b09af4ca8e610dd06b94be560622837a35d3ff.1754455931.git.liu.denton@gmail.com","subject":"Re: [PATCH v3 1/2] t5516: remove surrounding empty lines in test bodies","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-08-06T06:14:32Z","receivedAt":"2025-08-06T06:14:37Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Tue, Aug 05, 2025 at 09:53:39PM -0700, Denton Liu wrote:\n> This style with the empty lines in test bodies was from when the test\n> suite was being developed. Remove the empty lines to match the modern\n> test style.\n\nThanks for going the extra mile. Tacking on while-at-it fixes like this\nis what ensures that overall the Git codebase is trending towards our\nmodern code style.\n\nPatrick\n"},{"id":"523660","messageId":"xmqqv7n0wkbv.fsf@gitster.g","threadId":"63904","inReplyTo":"aJLywm9xWQQUADH1@pks.im","subject":"Re: [PATCH v3 2/2] remote.c: remove BUG in show_push_unqualified_ref_name_error()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-08-06T15:17:40Z","receivedAt":"2025-08-06T15:17:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt <ps@pks.im> writes:\n\n> This reads a lot better, thanks. We could arguably convert the\n> if-else-chain into a switch to make all of this read a bit better, but\n> that is a subjective style change and definitely not something that you\n> have to do as part of this series.\n\nI concur.  I admit that using switch never occured to me but I agree\n100% with you that it would make the result nicer, and that it does\nnot have to be part of this series.\n\n> One thing I wondered is whether it's okay to not die anymore via\n> `BUG()`. The other error cases already don't die though, so this ought\n> to be fine. Going up the callchain shows that we do bubble up the error\n> as expected until we end up in `match_push_refs()`. There's multiple\n> callers of that function, and all except one perform error handling for\n> it.\n>\n> The only exception is git-remote(1) in `get_push_ref_states()`, where it\n> gets executed via `git remote show $remote_name`. As far as I understand\n> we would end up not showing any references that are broken, and we would\n> print the above advise. Which I think is reasonable.\n>\n> So all of this looks good to me, thanks!\n\nNice to see somebody thinks through the potential impact for all the\ncallers.  Very much appreciated.\n\nLet's merge the topic to 'next'.\n\nThanks.\n"},{"id":"523698","messageId":"5866818859be97c091c40602974629eb7e463623.1754540903.git.liu.denton@gmail.com","threadId":"63904","inReplyTo":"xmqqv7n0wkbv.fsf@gitster.g","subject":"[PATCH] remote.c: convert if-else tower to switch","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2025-08-07T04:30:20Z","receivedAt":"2025-08-07T04:30:23Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"For better readability, convert the if-else tower into a switch\nstatement.\n\nSigned-off-by: Denton Liu <liu.denton@gmail.com>\n---\nThanks for the suggestion, both. Please queue this patch wherever it\nmakes the most sense to do so (either with the existing series or on its\nown separate branch).\n\n remote.c | 16 +++++++++++-----\n 1 file changed, 11 insertions(+), 5 deletions(-)\n\ndiff --git a/remote.c b/remote.c\nindex 465e0ea0eb..c7ae18fcfa 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -1197,29 +1197,35 @@ static void show_push_unqualified_ref_name_error(const char *dst_value,\n \t\t    \"match_explicit_lhs() should catch this!\",\n \t\t    matched_src_name);\n \ttype = odb_read_object_info(the_repository->objects, &oid, NULL);\n-\tif (type == OBJ_COMMIT) {\n+\tswitch (type) {\n+\tcase OBJ_COMMIT:\n \t\tadvise(_(\"The <src> part of the refspec is a commit object.\\n\"\n \t\t\t \"Did you mean to create a new branch by pushing to\\n\"\n \t\t\t \"'%s:refs/heads/%s'?\"),\n \t\t       matched_src_name, dst_value);\n-\t} else if (type == OBJ_TAG) {\n+\t\tbreak;\n+\tcase OBJ_TAG:\n \t\tadvise(_(\"The <src> part of the refspec is a tag object.\\n\"\n \t\t\t \"Did you mean to create a new tag by pushing to\\n\"\n \t\t\t \"'%s:refs/tags/%s'?\"),\n \t\t       matched_src_name, dst_value);\n-\t} else if (type == OBJ_TREE) {\n+\t\tbreak;\n+\tcase OBJ_TREE:\n \t\tadvise(_(\"The <src> part of the refspec is a tree object.\\n\"\n \t\t\t \"Did you mean to tag a new tree by pushing to\\n\"\n \t\t\t \"'%s:refs/tags/%s'?\"),\n \t\t       matched_src_name, dst_value);\n-\t} else if (type == OBJ_BLOB) {\n+\t\tbreak;\n+\tcase OBJ_BLOB:\n \t\tadvise(_(\"The <src> part of the refspec is a blob object.\\n\"\n \t\t\t \"Did you mean to tag a new blob by pushing to\\n\"\n \t\t\t \"'%s:refs/tags/%s'?\"),\n \t\t       matched_src_name, dst_value);\n-\t} else {\n+\t\tbreak;\n+\tdefault:\n \t\tadvise(_(\"The <src> part of the refspec ('%s') is an object ID that doesn't exist.\\n\"),\n \t\t       matched_src_name);\n+\t\tbreak;\n \t}\n }\n \n-- \n2.50.1\n\n"},{"id":"523699","messageId":"aJQtrgZ1fldaIy4E@pks.im","threadId":"63904","inReplyTo":"5866818859be97c091c40602974629eb7e463623.1754540903.git.liu.denton@gmail.com","subject":"Re: [PATCH] remote.c: convert if-else tower to switch","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-08-07T04:38:06Z","receivedAt":"2025-08-07T04:38:13Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Wed, Aug 06, 2025 at 09:30:20PM -0700, Denton Liu wrote:\n> For better readability, convert the if-else tower into a switch\n> statement.\n> \n> Signed-off-by: Denton Liu <liu.denton@gmail.com>\n> ---\n> Thanks for the suggestion, both. Please queue this patch wherever it\n> makes the most sense to do so (either with the existing series or on its\n> own separate branch).\n> \n>  remote.c | 16 +++++++++++-----\n>  1 file changed, 11 insertions(+), 5 deletions(-)\n> \n> diff --git a/remote.c b/remote.c\n> index 465e0ea0eb..c7ae18fcfa 100644\n> --- a/remote.c\n> +++ b/remote.c\n> @@ -1197,29 +1197,35 @@ static void show_push_unqualified_ref_name_error(const char *dst_value,\n>  \t\t    \"match_explicit_lhs() should catch this!\",\n>  \t\t    matched_src_name);\n>  \ttype = odb_read_object_info(the_repository->objects, &oid, NULL);\n\nNit: we can also drop the `type` variable, we don't need it for anything\nbut the value of the switch as far as I can see.\n\nThanks!\n\nPatrick\n"},{"id":"523742","messageId":"54a16614e2a38117f533ede3321b4d8ee2eabe8c.1754558302.git.liu.denton@gmail.com","threadId":"63904","inReplyTo":"5866818859be97c091c40602974629eb7e463623.1754540903.git.liu.denton@gmail.com","subject":"[PATCH v2] remote.c: convert if-else tower to switch","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2025-08-07T09:20:03Z","receivedAt":"2025-08-07T09:20:07Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"For better readability, convert the if-else tower into a switch\nstatement.\n\nSigned-off-by: Denton Liu <liu.denton@gmail.com>\n---\n remote.c | 19 ++++++++++++-------\n 1 file changed, 12 insertions(+), 7 deletions(-)\n\ndiff --git a/remote.c b/remote.c\nindex 465e0ea0eb..029b1fa93b 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -1171,7 +1171,6 @@ static void show_push_unqualified_ref_name_error(const char *dst_value,\n \t\t\t\t\t\t const char *matched_src_name)\n {\n \tstruct object_id oid;\n-\tenum object_type type;\n \n \t/*\n \t * TRANSLATORS: \"matches '%s'%\" is the <dst> part of \"git push\n@@ -1196,30 +1195,36 @@ static void show_push_unqualified_ref_name_error(const char *dst_value,\n \t\tBUG(\"'%s' is not a valid object, \"\n \t\t    \"match_explicit_lhs() should catch this!\",\n \t\t    matched_src_name);\n-\ttype = odb_read_object_info(the_repository->objects, &oid, NULL);\n-\tif (type == OBJ_COMMIT) {\n+\n+\tswitch (odb_read_object_info(the_repository->objects, &oid, NULL)) {\n+\tcase OBJ_COMMIT:\n \t\tadvise(_(\"The <src> part of the refspec is a commit object.\\n\"\n \t\t\t \"Did you mean to create a new branch by pushing to\\n\"\n \t\t\t \"'%s:refs/heads/%s'?\"),\n \t\t       matched_src_name, dst_value);\n-\t} else if (type == OBJ_TAG) {\n+\t\tbreak;\n+\tcase OBJ_TAG:\n \t\tadvise(_(\"The <src> part of the refspec is a tag object.\\n\"\n \t\t\t \"Did you mean to create a new tag by pushing to\\n\"\n \t\t\t \"'%s:refs/tags/%s'?\"),\n \t\t       matched_src_name, dst_value);\n-\t} else if (type == OBJ_TREE) {\n+\t\tbreak;\n+\tcase OBJ_TREE:\n \t\tadvise(_(\"The <src> part of the refspec is a tree object.\\n\"\n \t\t\t \"Did you mean to tag a new tree by pushing to\\n\"\n \t\t\t \"'%s:refs/tags/%s'?\"),\n \t\t       matched_src_name, dst_value);\n-\t} else if (type == OBJ_BLOB) {\n+\t\tbreak;\n+\tcase OBJ_BLOB:\n \t\tadvise(_(\"The <src> part of the refspec is a blob object.\\n\"\n \t\t\t \"Did you mean to tag a new blob by pushing to\\n\"\n \t\t\t \"'%s:refs/tags/%s'?\"),\n \t\t       matched_src_name, dst_value);\n-\t} else {\n+\t\tbreak;\n+\tdefault:\n \t\tadvise(_(\"The <src> part of the refspec ('%s') is an object ID that doesn't exist.\\n\"),\n \t\t       matched_src_name);\n+\t\tbreak;\n \t}\n }\n \n\nRange-diff against v1:\n1:  5866818859 ! 1:  54a16614e2 remote.c: convert if-else tower to switch\n    @@ Commit message\n     \n      ## remote.c ##\n     @@ remote.c: static void show_push_unqualified_ref_name_error(const char *dst_value,\n    + \t\t\t\t\t\t const char *matched_src_name)\n    + {\n    + \tstruct object_id oid;\n    +-\tenum object_type type;\n    + \n    + \t/*\n    + \t * TRANSLATORS: \"matches '%s'%\" is the <dst> part of \"git push\n    +@@ remote.c: static void show_push_unqualified_ref_name_error(const char *dst_value,\n    + \t\tBUG(\"'%s' is not a valid object, \"\n      \t\t    \"match_explicit_lhs() should catch this!\",\n      \t\t    matched_src_name);\n    - \ttype = odb_read_object_info(the_repository->objects, &oid, NULL);\n    +-\ttype = odb_read_object_info(the_repository->objects, &oid, NULL);\n     -\tif (type == OBJ_COMMIT) {\n    -+\tswitch (type) {\n    ++\n    ++\tswitch (odb_read_object_info(the_repository->objects, &oid, NULL)) {\n     +\tcase OBJ_COMMIT:\n      \t\tadvise(_(\"The <src> part of the refspec is a commit object.\\n\"\n      \t\t\t \"Did you mean to create a new branch by pushing to\\n\"\n-- \n2.50.1\n\n"},{"id":"523746","messageId":"F3252723-7E5E-4E84-94E4-5FC00298BAB2@gmail.com","threadId":"63904","inReplyTo":"54a16614e2a38117f533ede3321b4d8ee2eabe8c.1754558302.git.liu.denton@gmail.com","subject":"Re: [PATCH v2] remote.c: convert if-else tower to switch","fromName":"Ben Knoble","fromEmail":"ben.knoble@gmail.com","sentAt":"2025-08-07T12:35:17Z","receivedAt":"2025-08-07T12:35:30Z","isPatch":true,"sender":{"key":"ben.knoble@gmail.com","avatar":"https://avatars.githubusercontent.com/u/22802209?v=4"},"body":"\n> Le 7 août 2025 à 05:20, Denton Liu <liu.denton@gmail.com> a écrit :\n> \n> For better readability, convert the if-else tower into a switch\n> statement.\n> \n> Signed-off-by: Denton Liu <liu.denton@gmail.com>\n> ---\n> remote.c | 19 ++++++++++++-------\n> 1 file changed, 12 insertions(+), 7 deletions(-)\n> \n> diff --git a/remote.c b/remote.c\n> index 465e0ea0eb..029b1fa93b 100644\n> --- a/remote.c\n> +++ b/remote.c\n> @@ -1171,7 +1171,6 @@ static void show_push_unqualified_ref_name_error(const char *dst_value,\n>                         const char *matched_src_name)\n> {\n>    struct object_id oid;\n> -    enum object_type type;\n> \n>    /*\n>     * TRANSLATORS: \"matches '%s'%\" is the <dst> part of \"git push\n> @@ -1196,30 +1195,36 @@ static void show_push_unqualified_ref_name_error(const char *dst_value,\n>        BUG(\"'%s' is not a valid object, \"\n>            \"match_explicit_lhs() should catch this!\",\n>            matched_src_name);\n> -    type = odb_read_object_info(the_repository->objects, &oid, NULL);\n> -    if (type == OBJ_COMMIT) {\n> +\n> +    switch (odb_read_object_info(the_repository->objects, &oid, NULL)) {\n> +    case OBJ_COMMIT:\n>        advise(_(\"The <src> part of the refspec is a commit object.\\n\"\n>             \"Did you mean to create a new branch by pushing to\\n\"\n>             \"'%s:refs/heads/%s'?\"),\n>               matched_src_name, dst_value);\n> -    } else if (type == OBJ_TAG) {\n> +        break;\n> +    case OBJ_TAG:\n>        advise(_(\"The <src> part of the refspec is a tag object.\\n\"\n>             \"Did you mean to create a new tag by pushing to\\n\"\n>             \"'%s:refs/tags/%s'?\"),\n>               matched_src_name, dst_value);\n> -    } else if (type == OBJ_TREE) {\n> +        break;\n> +    case OBJ_TREE:\n>        advise(_(\"The <src> part of the refspec is a tree object.\\n\"\n>             \"Did you mean to tag a new tree by pushing to\\n\"\n>             \"'%s:refs/tags/%s'?\"),\n>               matched_src_name, dst_value);\n> -    } else if (type == OBJ_BLOB) {\n> +        break;\n> +    case OBJ_BLOB:\n>        advise(_(\"The <src> part of the refspec is a blob object.\\n\"\n>             \"Did you mean to tag a new blob by pushing to\\n\"\n>             \"'%s:refs/tags/%s'?\"),\n>               matched_src_name, dst_value);\n> -    } else {\n> +        break;\n> +    default:\n>        advise(_(\"The <src> part of the refspec ('%s') is an object ID that doesn't exist.\\n\"),\n>               matched_src_name);\n> +        break;\n>    }\n> }\n> \n> \n> Range-diff against v1:\n\nDon’t we normally put single-patch notes like a range-diff right after the triple dash? I have a feeling this format breaks git-am on the receiving side, though I haven’t actually tried it. \n\n> 1:  5866818859 ! 1:  54a16614e2 remote.c: convert if-else tower to switch\n>    @@ Commit message\n> \n>      ## remote.c ##\n>     @@ remote.c: static void show_push_unqualified_ref_name_error(const char *dst_value,\n>    +                         const char *matched_src_name)\n>    + {\n>    +    struct object_id oid;\n>    +-    enum object_type type;\n>    +\n>    +    /*\n>    +     * TRANSLATORS: \"matches '%s'%\" is the <dst> part of \"git push\n>    +@@ remote.c: static void show_push_unqualified_ref_name_error(const char *dst_value,\n>    +        BUG(\"'%s' is not a valid object, \"\n>                  \"match_explicit_lhs() should catch this!\",\n>                  matched_src_name);\n>    -    type = odb_read_object_info(the_repository->objects, &oid, NULL);\n>    +-    type = odb_read_object_info(the_repository->objects, &oid, NULL);\n>     -    if (type == OBJ_COMMIT) {\n>    -+    switch (type) {\n>    ++\n>    ++    switch (odb_read_object_info(the_repository->objects, &oid, NULL)) {\n>     +    case OBJ_COMMIT:\n>              advise(_(\"The <src> part of the refspec is a commit object.\\n\"\n>                   \"Did you mean to create a new branch by pushing to\\n\"\n> --\n> 2.50.1\n> \n> \n"},{"id":"523750","messageId":"xmqqsei3rx81.fsf@gitster.g","threadId":"63904","inReplyTo":"5866818859be97c091c40602974629eb7e463623.1754540903.git.liu.denton@gmail.com","subject":"Re: [PATCH] remote.c: convert if-else tower to switch","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-08-07T15:02:38Z","receivedAt":"2025-08-07T15:02:41Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Denton Liu <liu.denton@gmail.com> writes:\n\n> For better readability, convert the if-else tower into a switch\n> statement.\n\nThe reference to \"tower\" is something new to me.  A quick search\nseems to tell me that \"if-else cascade\", which is what I've been\nusing around here, is not popular, either.  \"if-else ladder\" is the\nterm more often used, it seems.\n\n\n> Signed-off-by: Denton Liu <liu.denton@gmail.com>\n> ---\n> Thanks for the suggestion, both. Please queue this patch wherever it\n> makes the most sense to do so (either with the existing series or on its\n> own separate branch).\n>\n>  remote.c | 16 +++++++++++-----\n>  1 file changed, 11 insertions(+), 5 deletions(-)\n\nOK.  Sitting down and thinking about it, the reason is obvious, but\nTIL that switch/case is slightly more verbose ;-).\n\n> +\tcase OBJ_BLOB:\n>  \t\tadvise(_(\"The <src> part of the refspec is a blob object.\\n\"\n>  \t\t\t \"Did you mean to tag a new blob by pushing to\\n\"\n>  \t\t\t \"'%s:refs/tags/%s'?\"),\n>  \t\t       matched_src_name, dst_value);\n> -\t} else {\n> +\t\tbreak;\n> +\tdefault:\n>  \t\tadvise(_(\"The <src> part of the refspec ('%s') is an object ID that doesn't exist.\\n\"),\n\nThis line alone is overly long; it is not part of _this_ patch but\nis showing the state after that BUG()->advise() fix, so it should be\nfixed there, I think?\n\n>  \t\t       matched_src_name);\n> +\t\tbreak;\n>  \t}\n>  }\n"},{"id":"523766","messageId":"CAPig+cQW2t+PC6R7YKaBTPHr96oyBerXv5UwCUhGpXYUqn-HgA@mail.gmail.com","threadId":"63904","inReplyTo":"F3252723-7E5E-4E84-94E4-5FC00298BAB2@gmail.com","subject":"Re: [PATCH v2] remote.c: convert if-else tower to switch","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2025-08-07T17:19:34Z","receivedAt":"2025-08-07T17:19:46Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Thu, Aug 7, 2025 at 8:35 AM Ben Knoble <ben.knoble@gmail.com> wrote:\n> > Le 7 août 2025 à 05:20, Denton Liu <liu.denton@gmail.com> a écrit :\n> > For better readability, convert the if-else tower into a switch\n> > statement.\n> >\n> > Signed-off-by: Denton Liu <liu.denton@gmail.com>\n> > ---\n> > diff --git a/remote.c b/remote.c\n> > index 465e0ea0eb..029b1fa93b 100644\n> > --- a/remote.c\n> > +++ b/remote.c\n> > @@ -1171,7 +1171,6 @@ static void show_push_unqualified_ref_name_error(const char *dst_value,\n> >\n> > Range-diff against v1:\n>\n> Don’t we normally put single-patch notes like a range-diff right after the triple dash? I have a feeling this format breaks git-am on the receiving side, though I haven’t actually tried it.\n\nNot since 2fa04cebfb (format-patch: move range/inter diff at the end\nof a single patch output, 2024-05-24).\n"},{"id":"523794","messageId":"cover.1754627874.git.liu.denton@gmail.com","threadId":"63904","inReplyTo":"cover.1754455931.git.liu.denton@gmail.com","subject":"[PATCH v4 0/3] remote.c: remove erroneous BUG case","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2025-08-08T04:41:06Z","receivedAt":"2025-08-08T04:41:09Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"In the case where one pushes a non-existent oid to an unqualified\ndestination, we encounter the following BUG\n\n\terror: The destination you provided is not a full refname (i.e.,\n\tstarting with \"refs/\"). We tried to guess what you meant by:\n\n\t- Looking for a ref that matches 'branch' on the remote side.\n\t- Checking if the <src> being pushed ('0000000000000000000000000000000000000001')\n\t  is a ref in \"refs/{heads,tags}/\". If so we add a corresponding\n\t  refs/{heads,tags}/ prefix on the remote side.\n\n\tNeither worked, so we gave up. You must fully qualify the ref.\n\tBUG: remote.c:1221: '0000000000000000000000000000000000000001' should be commit/tag/tree/blob, is '-1'\n\tfatal: the remote end hung up unexpectedly\n\tAborted (core dumped)\n\nHowever, this isn't actually a bug so replace it with an advise()\nmessage.\n\nChanges since v3:\n\n* Include the switch statement refactoring patch as a prelude to the\n  functional patch\n* Change \"if-else tower\" to \"if-else ladder\"\n* Shortened the overly long advise() line\n* Rebased on latest 'master' to avoid merge conflict introduced earlier\n  in the merge cycle (this should be fine since we haven't merged to\n  'next' yet right?)\n\nChanges since v2:\n\n* Add t5516 cleanup patch\n* Squash test creation patch into the patch that fixes it\n* Include the erroneous object ID in the advise message\n\nDenton Liu (3):\n  t5516: remove surrounding empty lines in test bodies\n  remote.c: convert if-else ladder to switch\n  remote.c: remove BUG in show_push_unqualified_ref_name_error()\n\n remote.c              | 24 +++++++++++--------\n t/t5516-fetch-push.sh | 54 ++++---------------------------------------\n 2 files changed, 19 insertions(+), 59 deletions(-)\n\nRange-diff against v3:\n1:  82b09af4ca = 1:  d31f320fdb t5516: remove surrounding empty lines in test bodies\n2:  938dfb8d4e ! 2:  ee6d69bcaf remote.c: remove BUG in show_push_unqualified_ref_name_error()\n[... deleted the diff of diff because it's mostly noise]\n-:  ---------- > 3:  3d84072dc7 remote.c: remove BUG in show_push_unqualified_ref_name_error()\n-- \n2.50.1\n\n"},{"id":"523795","messageId":"d31f320fdbb375cda9365df501e9b684ee84360c.1754627874.git.liu.denton@gmail.com","threadId":"63904","inReplyTo":"cover.1754627874.git.liu.denton@gmail.com","subject":"[PATCH v4 1/3] t5516: remove surrounding empty lines in test bodies","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2025-08-08T04:41:09Z","receivedAt":"2025-08-08T04:41:12Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"This style with the empty lines in test bodies was from when the test\nsuite was being developed. Remove the empty lines to match the modern\ntest style.\n\nSigned-off-by: Denton Liu <liu.denton@gmail.com>\n---\n t/t5516-fetch-push.sh | 51 -------------------------------------------\n 1 file changed, 51 deletions(-)\n\ndiff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh\nindex 4e9c27b0f2..8eddf3e40d 100755\n--- a/t/t5516-fetch-push.sh\n+++ b/t/t5516-fetch-push.sh\n@@ -105,7 +105,6 @@ check_push_result () {\n }\n \n test_expect_success setup '\n-\n \t>path1 &&\n \tgit add path1 &&\n \ttest_tick &&\n@@ -117,7 +116,6 @@ test_expect_success setup '\n \ttest_tick &&\n \tgit commit -a -m second &&\n \tthe_commit=$(git show-ref -s --verify refs/heads/main)\n-\n '\n \n for cmd in push fetch\n@@ -322,104 +320,82 @@ test_expect_success 'push with pushInsteadOf and explicit pushurl (pushInsteadOf\n '\n \n test_expect_success 'push with matching heads' '\n-\n \tmk_test testrepo heads/main &&\n \tgit push testrepo : &&\n \tcheck_push_result testrepo $the_commit heads/main\n-\n '\n \n test_expect_success 'push with matching heads on the command line' '\n-\n \tmk_test testrepo heads/main &&\n \tgit push testrepo : &&\n \tcheck_push_result testrepo $the_commit heads/main\n-\n '\n \n test_expect_success 'failed (non-fast-forward) push with matching heads' '\n-\n \tmk_test testrepo heads/main &&\n \tgit push testrepo : &&\n \tgit commit --amend -massaged &&\n \ttest_must_fail git push testrepo &&\n \tcheck_push_result testrepo $the_commit heads/main &&\n \tgit reset --hard $the_commit\n-\n '\n \n test_expect_success 'push --force with matching heads' '\n-\n \tmk_test testrepo heads/main &&\n \tgit push testrepo : &&\n \tgit commit --amend -massaged &&\n \tgit push --force testrepo : &&\n \t! check_push_result testrepo $the_commit heads/main &&\n \tgit reset --hard $the_commit\n-\n '\n \n test_expect_success 'push with matching heads and forced update' '\n-\n \tmk_test testrepo heads/main &&\n \tgit push testrepo : &&\n \tgit commit --amend -massaged &&\n \tgit push testrepo +: &&\n \t! check_push_result testrepo $the_commit heads/main &&\n \tgit reset --hard $the_commit\n-\n '\n \n test_expect_success 'push with no ambiguity (1)' '\n-\n \tmk_test testrepo heads/main &&\n \tgit push testrepo main:main &&\n \tcheck_push_result testrepo $the_commit heads/main\n-\n '\n \n test_expect_success 'push with no ambiguity (2)' '\n-\n \tmk_test testrepo remotes/origin/main &&\n \tgit push testrepo main:origin/main &&\n \tcheck_push_result testrepo $the_commit remotes/origin/main\n-\n '\n \n test_expect_success 'push with colon-less refspec, no ambiguity' '\n-\n \tmk_test testrepo heads/main heads/t/main &&\n \tgit branch -f t/main main &&\n \tgit push testrepo main &&\n \tcheck_push_result testrepo $the_commit heads/main &&\n \tcheck_push_result testrepo $the_first_commit heads/t/main\n-\n '\n \n test_expect_success 'push with weak ambiguity (1)' '\n-\n \tmk_test testrepo heads/main remotes/origin/main &&\n \tgit push testrepo main:main &&\n \tcheck_push_result testrepo $the_commit heads/main &&\n \tcheck_push_result testrepo $the_first_commit remotes/origin/main\n-\n '\n \n test_expect_success 'push with weak ambiguity (2)' '\n-\n \tmk_test testrepo heads/main remotes/origin/main remotes/another/main &&\n \tgit push testrepo main:main &&\n \tcheck_push_result testrepo $the_commit heads/main &&\n \tcheck_push_result testrepo $the_first_commit remotes/origin/main remotes/another/main\n-\n '\n \n test_expect_success 'push with ambiguity' '\n-\n \tmk_test testrepo heads/frotz tags/frotz &&\n \ttest_must_fail git push testrepo main:frotz &&\n \tcheck_push_result testrepo $the_first_commit heads/frotz tags/frotz\n-\n '\n \n test_expect_success 'push with onelevel ref' '\n@@ -428,17 +404,14 @@ test_expect_success 'push with onelevel ref' '\n '\n \n test_expect_success 'push with colon-less refspec (1)' '\n-\n \tmk_test testrepo heads/frotz tags/frotz &&\n \tgit branch -f frotz main &&\n \tgit push testrepo frotz &&\n \tcheck_push_result testrepo $the_commit heads/frotz &&\n \tcheck_push_result testrepo $the_first_commit tags/frotz\n-\n '\n \n test_expect_success 'push with colon-less refspec (2)' '\n-\n \tmk_test testrepo heads/frotz tags/frotz &&\n \tif git show-ref --verify -q refs/heads/frotz\n \tthen\n@@ -448,7 +421,6 @@ test_expect_success 'push with colon-less refspec (2)' '\n \tgit push -f testrepo frotz &&\n \tcheck_push_result testrepo $the_commit tags/frotz &&\n \tcheck_push_result testrepo $the_first_commit heads/frotz\n-\n '\n \n test_expect_success 'push with colon-less refspec (3)' '\n@@ -465,7 +437,6 @@ test_expect_success 'push with colon-less refspec (3)' '\n '\n \n test_expect_success 'push with colon-less refspec (4)' '\n-\n \tmk_test testrepo &&\n \tif git show-ref --verify -q refs/heads/frotz\n \tthen\n@@ -475,38 +446,29 @@ test_expect_success 'push with colon-less refspec (4)' '\n \tgit push testrepo frotz &&\n \tcheck_push_result testrepo $the_commit tags/frotz &&\n \ttest 1 = $( cd testrepo && git show-ref | wc -l )\n-\n '\n \n test_expect_success 'push head with non-existent, incomplete dest' '\n-\n \tmk_test testrepo &&\n \tgit push testrepo main:branch &&\n \tcheck_push_result testrepo $the_commit heads/branch\n-\n '\n \n test_expect_success 'push tag with non-existent, incomplete dest' '\n-\n \tmk_test testrepo &&\n \tgit tag -f v1.0 &&\n \tgit push testrepo v1.0:tag &&\n \tcheck_push_result testrepo $the_commit tags/tag\n-\n '\n \n test_expect_success 'push oid with non-existent, incomplete dest' '\n-\n \tmk_test testrepo &&\n \ttest_must_fail git push testrepo $(git rev-parse main):foo\n-\n '\n \n test_expect_success 'push ref expression with non-existent, incomplete dest' '\n-\n \tmk_test testrepo &&\n \ttest_must_fail git push testrepo main^:branch\n-\n '\n \n for head in HEAD @\n@@ -550,7 +512,6 @@ do\n \t\tgit checkout main &&\n \t\tgit push testrepo $head:branch &&\n \t\tcheck_push_result testrepo $the_commit heads/branch\n-\n \t'\n \n \ttest_expect_success \"push with config remote.*.push = $head\" '\n@@ -596,7 +557,6 @@ test_expect_success 'push with remote.pushdefault' '\n '\n \n test_expect_success 'push with config remote.*.pushurl' '\n-\n \tmk_test testrepo heads/main &&\n \tgit checkout main &&\n \ttest_config remote.there.url test2repo &&\n@@ -655,7 +615,6 @@ test_expect_success 'push ignores \"branch.\" config without subsection' '\n '\n \n test_expect_success 'push with dry-run' '\n-\n \tmk_test testrepo heads/main &&\n \told_commit=$(git -C testrepo show-ref -s --verify refs/heads/main) &&\n \tgit push --dry-run testrepo : &&\n@@ -663,7 +622,6 @@ test_expect_success 'push with dry-run' '\n '\n \n test_expect_success 'push updates local refs' '\n-\n \tmk_test testrepo heads/main &&\n \tmk_child testrepo child &&\n \t(\n@@ -673,11 +631,9 @@ test_expect_success 'push updates local refs' '\n \t\ttest $(git rev-parse main) = \\\n \t\t\t$(git rev-parse remotes/origin/main)\n \t)\n-\n '\n \n test_expect_success 'push updates up-to-date local refs' '\n-\n \tmk_test testrepo heads/main &&\n \tmk_child testrepo child1 &&\n \tmk_child testrepo child2 &&\n@@ -689,11 +645,9 @@ test_expect_success 'push updates up-to-date local refs' '\n \t\ttest $(git rev-parse main) = \\\n \t\t\t$(git rev-parse remotes/origin/main)\n \t)\n-\n '\n \n test_expect_success 'push preserves up-to-date packed refs' '\n-\n \tmk_test testrepo heads/main &&\n \tmk_child testrepo child &&\n \t(\n@@ -701,11 +655,9 @@ test_expect_success 'push preserves up-to-date packed refs' '\n \t\tgit push &&\n \t\t! test -f .git/refs/remotes/origin/main\n \t)\n-\n '\n \n test_expect_success 'push does not update local refs on failure' '\n-\n \tmk_test testrepo heads/main &&\n \tmk_child testrepo child &&\n \techo \"#!/no/frobnication/today\" >testrepo/.git/hooks/pre-receive &&\n@@ -717,16 +669,13 @@ test_expect_success 'push does not update local refs on failure' '\n \t\ttest $(git rev-parse main) != \\\n \t\t\t$(git rev-parse remotes/origin/main)\n \t)\n-\n '\n \n test_expect_success 'allow deleting an invalid remote ref' '\n-\n \tmk_test testrepo heads/branch &&\n \trm -f testrepo/.git/objects/??/* &&\n \tgit push testrepo :refs/heads/branch &&\n \t(cd testrepo && test_must_fail git rev-parse --verify refs/heads/branch)\n-\n '\n \n test_expect_success 'pushing valid refs triggers post-receive and post-update hooks' '\n-- \n2.50.1\n\n"},{"id":"523796","messageId":"ee6d69bcafeda9d8a2cdfd1f8bb62c28c13941f9.1754627874.git.liu.denton@gmail.com","threadId":"63904","inReplyTo":"cover.1754627874.git.liu.denton@gmail.com","subject":"[PATCH v4 2/3] remote.c: convert if-else ladder to switch","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2025-08-08T04:41:11Z","receivedAt":"2025-08-08T04:41:14Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"For better readability, convert the if-else ladder into a switch\nstatement.\n\nSuggested-by: Patrick Steinhardt <ps@pks.im>\nSigned-off-by: Denton Liu <liu.denton@gmail.com>\n---\n remote.c | 18 +++++++++++-------\n 1 file changed, 11 insertions(+), 7 deletions(-)\n\ndiff --git a/remote.c b/remote.c\nindex 88f991795b..61e2c9951a 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -1171,7 +1171,6 @@ static void show_push_unqualified_ref_name_error(const char *dst_value,\n \t\t\t\t\t\t const char *matched_src_name)\n {\n \tstruct object_id oid;\n-\tenum object_type type;\n \n \t/*\n \t * TRANSLATORS: \"matches '%s'%\" is the <dst> part of \"git push\n@@ -1196,28 +1195,33 @@ static void show_push_unqualified_ref_name_error(const char *dst_value,\n \t\tBUG(\"'%s' is not a valid object, \"\n \t\t    \"match_explicit_lhs() should catch this!\",\n \t\t    matched_src_name);\n-\ttype = odb_read_object_info(the_repository->objects, &oid, NULL);\n-\tif (type == OBJ_COMMIT) {\n+\n+\tswitch (odb_read_object_info(the_repository->objects, &oid, NULL)) {\n+\tcase OBJ_COMMIT:\n \t\tadvise(_(\"The <src> part of the refspec is a commit object.\\n\"\n \t\t\t \"Did you mean to create a new branch by pushing to\\n\"\n \t\t\t \"'%s:refs/heads/%s'?\"),\n \t\t       matched_src_name, dst_value);\n-\t} else if (type == OBJ_TAG) {\n+\t\tbreak;\n+\tcase OBJ_TAG:\n \t\tadvise(_(\"The <src> part of the refspec is a tag object.\\n\"\n \t\t\t \"Did you mean to create a new tag by pushing to\\n\"\n \t\t\t \"'%s:refs/tags/%s'?\"),\n \t\t       matched_src_name, dst_value);\n-\t} else if (type == OBJ_TREE) {\n+\t\tbreak;\n+\tcase OBJ_TREE:\n \t\tadvise(_(\"The <src> part of the refspec is a tree object.\\n\"\n \t\t\t \"Did you mean to tag a new tree by pushing to\\n\"\n \t\t\t \"'%s:refs/tags/%s'?\"),\n \t\t       matched_src_name, dst_value);\n-\t} else if (type == OBJ_BLOB) {\n+\t\tbreak;\n+\tcase OBJ_BLOB:\n \t\tadvise(_(\"The <src> part of the refspec is a blob object.\\n\"\n \t\t\t \"Did you mean to tag a new blob by pushing to\\n\"\n \t\t\t \"'%s:refs/tags/%s'?\"),\n \t\t       matched_src_name, dst_value);\n-\t} else {\n+\t\tbreak;\n+\tdefault:\n \t\tBUG(\"'%s' should be commit/tag/tree/blob, is '%d'\",\n \t\t    matched_src_name, type);\n \t}\n-- \n2.50.1\n\n"},{"id":"523797","messageId":"3d84072dc7910026e203ca35e32ef026d0efc131.1754627874.git.liu.denton@gmail.com","threadId":"63904","inReplyTo":"cover.1754627874.git.liu.denton@gmail.com","subject":"[PATCH v4 3/3] remote.c: remove BUG in show_push_unqualified_ref_name_error()","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2025-08-08T04:41:14Z","receivedAt":"2025-08-08T04:41:17Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"When \"git push <remote> <src>:<dst>\" does not spell out the\ndestination side of the ref fully, and when <src> is not given\nas a reference but an object name, the code tries to give advice\nmessages based on the type of that object.\n\nThe type is determined by calling odb_read_object_info() and\nsignalled by its return value.  The code however reported a\nprogramming error with BUG() when this function said that there\nis no such object, which happens when the object name is given\nas a full hexadecimal (if the object name is given as a partial\nhexadecimal or an non-existing ref, the function would have died\nwithout returning, so this BUG() wouldn't have triggered).  This\nis wrong.  It is an ordinary end-user mistake to give an object\nname that does not exist and treated as such.\n\nAn example of the error message produced is as follows:\n\n\terror: The destination you provided is not a full refname (i.e.,\n\tstarting with \"refs/\"). We tried to guess what you meant by:\n\n\t- Looking for a ref that matches 'branch' on the remote side.\n\t- Checking if the <src> being pushed ('0000000000000000000000000000000000000001')\n\t  is a ref in \"refs/{heads,tags}/\". If so we add a corresponding\n\t  refs/{heads,tags}/ prefix on the remote side.\n\n\tNeither worked, so we gave up. You must fully qualify the ref.\n\tBUG: remote.c:1221: '0000000000000000000000000000000000000001' should be commit/tag/tree/blob, is '-1'\n\tfatal: the remote end hung up unexpectedly\n\tAborted (core dumped)\n\nHelped-by: Junio C Hamano <gitster@pobox.com>\nSigned-off-by: Denton Liu <liu.denton@gmail.com>\n---\n remote.c              | 6 ++++--\n t/t5516-fetch-push.sh | 5 +++++\n 2 files changed, 9 insertions(+), 2 deletions(-)\n\ndiff --git a/remote.c b/remote.c\nindex 61e2c9951a..df88914716 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -1222,8 +1222,10 @@ static void show_push_unqualified_ref_name_error(const char *dst_value,\n \t\t       matched_src_name, dst_value);\n \t\tbreak;\n \tdefault:\n-\t\tBUG(\"'%s' should be commit/tag/tree/blob, is '%d'\",\n-\t\t    matched_src_name, type);\n+\t\tadvise(_(\"The <src> part of the refspec ('%s') \"\n+\t\t\t \"is an object ID that doesn't exist.\\n\"),\n+\t\t       matched_src_name);\n+\t\tbreak;\n \t}\n }\n \ndiff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh\nindex 8eddf3e40d..46926e7bbd 100755\n--- a/t/t5516-fetch-push.sh\n+++ b/t/t5516-fetch-push.sh\n@@ -471,6 +471,11 @@ test_expect_success 'push ref expression with non-existent, incomplete dest' '\n \ttest_must_fail git push testrepo main^:branch\n '\n \n+test_expect_success 'push ref expression with non-existent oid src' '\n+\tmk_test testrepo &&\n+\ttest_must_fail git push testrepo $(test_oid 001):branch\n+'\n+\n for head in HEAD @\n do\n \n-- \n2.50.1\n\n"},{"id":"523798","messageId":"aJWOcDN2LZaMzaqH@pks.im","threadId":"63904","inReplyTo":"ee6d69bcafeda9d8a2cdfd1f8bb62c28c13941f9.1754627874.git.liu.denton@gmail.com","subject":"Re: [PATCH v4 2/3] remote.c: convert if-else ladder to switch","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-08-08T05:43:12Z","receivedAt":"2025-08-08T05:43:24Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Thu, Aug 07, 2025 at 09:41:11PM -0700, Denton Liu wrote:\n> diff --git a/remote.c b/remote.c\n> index 88f991795b..61e2c9951a 100644\n> --- a/remote.c\n> +++ b/remote.c\n> @@ -1171,7 +1171,6 @@ static void show_push_unqualified_ref_name_error(const char *dst_value,\n>  \t\t\t\t\t\t const char *matched_src_name)\n>  {\n>  \tstruct object_id oid;\n> -\tenum object_type type;\n\n> @@ -1196,28 +1195,33 @@ static void show_push_unqualified_ref_name_error(const char *dst_value,\n>  \t\tBUG(\"'%s' is not a valid object, \"\n>  \t\t    \"match_explicit_lhs() should catch this!\",\n>  \t\t    matched_src_name);\n> -\ttype = odb_read_object_info(the_repository->objects, &oid, NULL);\n> -\tif (type == OBJ_COMMIT) {\n> +\n> +\tswitch (odb_read_object_info(the_repository->objects, &oid, NULL)) {\n> +\tcase OBJ_COMMIT:\n>  \t\tadvise(_(\"The <src> part of the refspec is a commit object.\\n\"\n>  \t\t\t \"Did you mean to create a new branch by pushing to\\n\"\n>  \t\t\t \"'%s:refs/heads/%s'?\"),\n>  \t\t       matched_src_name, dst_value);\n> -\t} else if (type == OBJ_TAG) {\n> +\t\tbreak;\n> +\tcase OBJ_TAG:\n>  \t\tadvise(_(\"The <src> part of the refspec is a tag object.\\n\"\n>  \t\t\t \"Did you mean to create a new tag by pushing to\\n\"\n>  \t\t\t \"'%s:refs/tags/%s'?\"),\n>  \t\t       matched_src_name, dst_value);\n> -\t} else if (type == OBJ_TREE) {\n> +\t\tbreak;\n> +\tcase OBJ_TREE:\n>  \t\tadvise(_(\"The <src> part of the refspec is a tree object.\\n\"\n>  \t\t\t \"Did you mean to tag a new tree by pushing to\\n\"\n>  \t\t\t \"'%s:refs/tags/%s'?\"),\n>  \t\t       matched_src_name, dst_value);\n> -\t} else if (type == OBJ_BLOB) {\n> +\t\tbreak;\n> +\tcase OBJ_BLOB:\n>  \t\tadvise(_(\"The <src> part of the refspec is a blob object.\\n\"\n>  \t\t\t \"Did you mean to tag a new blob by pushing to\\n\"\n>  \t\t\t \"'%s:refs/tags/%s'?\"),\n>  \t\t       matched_src_name, dst_value);\n> -\t} else {\n> +\t\tbreak;\n> +\tdefault:\n>  \t\tBUG(\"'%s' should be commit/tag/tree/blob, is '%d'\",\n>  \t\t    matched_src_name, type);\n\nWe can't remove the `type` variable in this patch already -- it's still\nused by this call to `BUG()`. But we can drop the variable in the next\npatch, where that call is converted to `advise()`.\n\nSo I'd recommend to either move this patch to after the next patch or to\nkeep the `type` variable here and remove it in the next patch.\n\nPatrick\n"},{"id":"523803","messageId":"aJWjxbiyQ0cuiQku@generichostname","threadId":"63904","inReplyTo":"aJWOcDN2LZaMzaqH@pks.im","subject":"Re: [PATCH v4 2/3] remote.c: convert if-else ladder to switch","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2025-08-08T07:14:13Z","receivedAt":"2025-08-08T07:14:16Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"On Fri, Aug 08, 2025 at 07:43:12AM +0200, Patrick Steinhardt wrote:\n> We can't remove the `type` variable in this patch already -- it's still\n> used by this call to `BUG()`. But we can drop the variable in the next\n> patch, where that call is converted to `advise()`.\n\nUgh, that's what I get for rushing this patchset out without doing a\ntest compile :/\n\nThanks for catching that. Another patchset incoming\n\n-Denton\n"},{"id":"523804","messageId":"cover.1754637849.git.liu.denton@gmail.com","threadId":"63904","inReplyTo":"cover.1754627874.git.liu.denton@gmail.com","subject":"[PATCH v5 0/3] remote.c: remove erroneous BUG case","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2025-08-08T07:24:39Z","receivedAt":"2025-08-08T07:24:43Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"In the case where one pushes a non-existent oid to an unqualified\ndestination, we encounter the following BUG\n\n\terror: The destination you provided is not a full refname (i.e.,\n\tstarting with \"refs/\"). We tried to guess what you meant by:\n\n\t- Looking for a ref that matches 'branch' on the remote side.\n\t- Checking if the <src> being pushed ('0000000000000000000000000000000000000001')\n\t  is a ref in \"refs/{heads,tags}/\". If so we add a corresponding\n\t  refs/{heads,tags}/ prefix on the remote side.\n\n\tNeither worked, so we gave up. You must fully qualify the ref.\n\tBUG: remote.c:1221: '0000000000000000000000000000000000000001' should be commit/tag/tree/blob, is '-1'\n\tfatal: the remote end hung up unexpectedly\n\tAborted (core dumped)\n\nHowever, this isn't actually a bug so replace it with an advise()\nmessage.\n\nChanges since v4:\n\n* Put the switch statement refactoring patch last so that we don't get\n  compile errors from a missing variable\n\nChanges since v3:\n\n* Include the switch statement refactoring patch as a prelude to the\n  functional patch\n* Change \"if-else tower\" to \"if-else ladder\"\n* Shortened the overly long advise() line\n* Rebased on latest 'master' to avoid merge conflict introduced earlier\n  in the merge cycle (this should be fine since we haven't merged to\n  'next' yet right?)\n\nChanges since v2:\n\n* Add t5516 cleanup patch\n* Squash test creation patch into the patch that fixes it\n* Include the erroneous object ID in the advise message\n\nDenton Liu (3):\n  t5516: remove surrounding empty lines in test bodies\n  remote.c: remove BUG in show_push_unqualified_ref_name_error()\n  remote.c: convert if-else ladder to switch\n\n remote.c              | 24 +++++++++++--------\n t/t5516-fetch-push.sh | 54 ++++---------------------------------------\n 2 files changed, 19 insertions(+), 59 deletions(-)\n\nRange-diff against v4:\n1:  d31f320fdb = 1:  d31f320fdb t5516: remove surrounding empty lines in test bodies\n3:  3d84072dc7 ! 2:  d21612fca6 remote.c: remove BUG in show_push_unqualified_ref_name_error()\n    @@ Commit message\n     \n      ## remote.c ##\n     @@ remote.c: static void show_push_unqualified_ref_name_error(const char *dst_value,\n    + \t\t\t \"'%s:refs/tags/%s'?\"),\n      \t\t       matched_src_name, dst_value);\n    - \t\tbreak;\n    - \tdefault:\n    + \t} else {\n     -\t\tBUG(\"'%s' should be commit/tag/tree/blob, is '%d'\",\n     -\t\t    matched_src_name, type);\n     +\t\tadvise(_(\"The <src> part of the refspec ('%s') \"\n     +\t\t\t \"is an object ID that doesn't exist.\\n\"),\n     +\t\t       matched_src_name);\n    -+\t\tbreak;\n      \t}\n      }\n      \n2:  ee6d69bcaf ! 3:  cbda61af5c remote.c: convert if-else ladder to switch\n    @@ remote.c: static void show_push_unqualified_ref_name_error(const char *dst_value\n     -\t} else {\n     +\t\tbreak;\n     +\tdefault:\n    - \t\tBUG(\"'%s' should be commit/tag/tree/blob, is '%d'\",\n    - \t\t    matched_src_name, type);\n    + \t\tadvise(_(\"The <src> part of the refspec ('%s') \"\n    + \t\t\t \"is an object ID that doesn't exist.\\n\"),\n    + \t\t       matched_src_name);\n    ++\t\tbreak;\n      \t}\n    + }\n    + \n-- \n2.50.1\n\n"},{"id":"523805","messageId":"d31f320fdbb375cda9365df501e9b684ee84360c.1754637850.git.liu.denton@gmail.com","threadId":"63904","inReplyTo":"cover.1754637849.git.liu.denton@gmail.com","subject":"[PATCH v5 1/3] t5516: remove surrounding empty lines in test bodies","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2025-08-08T07:24:42Z","receivedAt":"2025-08-08T07:24:45Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"This style with the empty lines in test bodies was from when the test\nsuite was being developed. Remove the empty lines to match the modern\ntest style.\n\nSigned-off-by: Denton Liu <liu.denton@gmail.com>\n---\n t/t5516-fetch-push.sh | 51 -------------------------------------------\n 1 file changed, 51 deletions(-)\n\ndiff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh\nindex 4e9c27b0f2..8eddf3e40d 100755\n--- a/t/t5516-fetch-push.sh\n+++ b/t/t5516-fetch-push.sh\n@@ -105,7 +105,6 @@ check_push_result () {\n }\n \n test_expect_success setup '\n-\n \t>path1 &&\n \tgit add path1 &&\n \ttest_tick &&\n@@ -117,7 +116,6 @@ test_expect_success setup '\n \ttest_tick &&\n \tgit commit -a -m second &&\n \tthe_commit=$(git show-ref -s --verify refs/heads/main)\n-\n '\n \n for cmd in push fetch\n@@ -322,104 +320,82 @@ test_expect_success 'push with pushInsteadOf and explicit pushurl (pushInsteadOf\n '\n \n test_expect_success 'push with matching heads' '\n-\n \tmk_test testrepo heads/main &&\n \tgit push testrepo : &&\n \tcheck_push_result testrepo $the_commit heads/main\n-\n '\n \n test_expect_success 'push with matching heads on the command line' '\n-\n \tmk_test testrepo heads/main &&\n \tgit push testrepo : &&\n \tcheck_push_result testrepo $the_commit heads/main\n-\n '\n \n test_expect_success 'failed (non-fast-forward) push with matching heads' '\n-\n \tmk_test testrepo heads/main &&\n \tgit push testrepo : &&\n \tgit commit --amend -massaged &&\n \ttest_must_fail git push testrepo &&\n \tcheck_push_result testrepo $the_commit heads/main &&\n \tgit reset --hard $the_commit\n-\n '\n \n test_expect_success 'push --force with matching heads' '\n-\n \tmk_test testrepo heads/main &&\n \tgit push testrepo : &&\n \tgit commit --amend -massaged &&\n \tgit push --force testrepo : &&\n \t! check_push_result testrepo $the_commit heads/main &&\n \tgit reset --hard $the_commit\n-\n '\n \n test_expect_success 'push with matching heads and forced update' '\n-\n \tmk_test testrepo heads/main &&\n \tgit push testrepo : &&\n \tgit commit --amend -massaged &&\n \tgit push testrepo +: &&\n \t! check_push_result testrepo $the_commit heads/main &&\n \tgit reset --hard $the_commit\n-\n '\n \n test_expect_success 'push with no ambiguity (1)' '\n-\n \tmk_test testrepo heads/main &&\n \tgit push testrepo main:main &&\n \tcheck_push_result testrepo $the_commit heads/main\n-\n '\n \n test_expect_success 'push with no ambiguity (2)' '\n-\n \tmk_test testrepo remotes/origin/main &&\n \tgit push testrepo main:origin/main &&\n \tcheck_push_result testrepo $the_commit remotes/origin/main\n-\n '\n \n test_expect_success 'push with colon-less refspec, no ambiguity' '\n-\n \tmk_test testrepo heads/main heads/t/main &&\n \tgit branch -f t/main main &&\n \tgit push testrepo main &&\n \tcheck_push_result testrepo $the_commit heads/main &&\n \tcheck_push_result testrepo $the_first_commit heads/t/main\n-\n '\n \n test_expect_success 'push with weak ambiguity (1)' '\n-\n \tmk_test testrepo heads/main remotes/origin/main &&\n \tgit push testrepo main:main &&\n \tcheck_push_result testrepo $the_commit heads/main &&\n \tcheck_push_result testrepo $the_first_commit remotes/origin/main\n-\n '\n \n test_expect_success 'push with weak ambiguity (2)' '\n-\n \tmk_test testrepo heads/main remotes/origin/main remotes/another/main &&\n \tgit push testrepo main:main &&\n \tcheck_push_result testrepo $the_commit heads/main &&\n \tcheck_push_result testrepo $the_first_commit remotes/origin/main remotes/another/main\n-\n '\n \n test_expect_success 'push with ambiguity' '\n-\n \tmk_test testrepo heads/frotz tags/frotz &&\n \ttest_must_fail git push testrepo main:frotz &&\n \tcheck_push_result testrepo $the_first_commit heads/frotz tags/frotz\n-\n '\n \n test_expect_success 'push with onelevel ref' '\n@@ -428,17 +404,14 @@ test_expect_success 'push with onelevel ref' '\n '\n \n test_expect_success 'push with colon-less refspec (1)' '\n-\n \tmk_test testrepo heads/frotz tags/frotz &&\n \tgit branch -f frotz main &&\n \tgit push testrepo frotz &&\n \tcheck_push_result testrepo $the_commit heads/frotz &&\n \tcheck_push_result testrepo $the_first_commit tags/frotz\n-\n '\n \n test_expect_success 'push with colon-less refspec (2)' '\n-\n \tmk_test testrepo heads/frotz tags/frotz &&\n \tif git show-ref --verify -q refs/heads/frotz\n \tthen\n@@ -448,7 +421,6 @@ test_expect_success 'push with colon-less refspec (2)' '\n \tgit push -f testrepo frotz &&\n \tcheck_push_result testrepo $the_commit tags/frotz &&\n \tcheck_push_result testrepo $the_first_commit heads/frotz\n-\n '\n \n test_expect_success 'push with colon-less refspec (3)' '\n@@ -465,7 +437,6 @@ test_expect_success 'push with colon-less refspec (3)' '\n '\n \n test_expect_success 'push with colon-less refspec (4)' '\n-\n \tmk_test testrepo &&\n \tif git show-ref --verify -q refs/heads/frotz\n \tthen\n@@ -475,38 +446,29 @@ test_expect_success 'push with colon-less refspec (4)' '\n \tgit push testrepo frotz &&\n \tcheck_push_result testrepo $the_commit tags/frotz &&\n \ttest 1 = $( cd testrepo && git show-ref | wc -l )\n-\n '\n \n test_expect_success 'push head with non-existent, incomplete dest' '\n-\n \tmk_test testrepo &&\n \tgit push testrepo main:branch &&\n \tcheck_push_result testrepo $the_commit heads/branch\n-\n '\n \n test_expect_success 'push tag with non-existent, incomplete dest' '\n-\n \tmk_test testrepo &&\n \tgit tag -f v1.0 &&\n \tgit push testrepo v1.0:tag &&\n \tcheck_push_result testrepo $the_commit tags/tag\n-\n '\n \n test_expect_success 'push oid with non-existent, incomplete dest' '\n-\n \tmk_test testrepo &&\n \ttest_must_fail git push testrepo $(git rev-parse main):foo\n-\n '\n \n test_expect_success 'push ref expression with non-existent, incomplete dest' '\n-\n \tmk_test testrepo &&\n \ttest_must_fail git push testrepo main^:branch\n-\n '\n \n for head in HEAD @\n@@ -550,7 +512,6 @@ do\n \t\tgit checkout main &&\n \t\tgit push testrepo $head:branch &&\n \t\tcheck_push_result testrepo $the_commit heads/branch\n-\n \t'\n \n \ttest_expect_success \"push with config remote.*.push = $head\" '\n@@ -596,7 +557,6 @@ test_expect_success 'push with remote.pushdefault' '\n '\n \n test_expect_success 'push with config remote.*.pushurl' '\n-\n \tmk_test testrepo heads/main &&\n \tgit checkout main &&\n \ttest_config remote.there.url test2repo &&\n@@ -655,7 +615,6 @@ test_expect_success 'push ignores \"branch.\" config without subsection' '\n '\n \n test_expect_success 'push with dry-run' '\n-\n \tmk_test testrepo heads/main &&\n \told_commit=$(git -C testrepo show-ref -s --verify refs/heads/main) &&\n \tgit push --dry-run testrepo : &&\n@@ -663,7 +622,6 @@ test_expect_success 'push with dry-run' '\n '\n \n test_expect_success 'push updates local refs' '\n-\n \tmk_test testrepo heads/main &&\n \tmk_child testrepo child &&\n \t(\n@@ -673,11 +631,9 @@ test_expect_success 'push updates local refs' '\n \t\ttest $(git rev-parse main) = \\\n \t\t\t$(git rev-parse remotes/origin/main)\n \t)\n-\n '\n \n test_expect_success 'push updates up-to-date local refs' '\n-\n \tmk_test testrepo heads/main &&\n \tmk_child testrepo child1 &&\n \tmk_child testrepo child2 &&\n@@ -689,11 +645,9 @@ test_expect_success 'push updates up-to-date local refs' '\n \t\ttest $(git rev-parse main) = \\\n \t\t\t$(git rev-parse remotes/origin/main)\n \t)\n-\n '\n \n test_expect_success 'push preserves up-to-date packed refs' '\n-\n \tmk_test testrepo heads/main &&\n \tmk_child testrepo child &&\n \t(\n@@ -701,11 +655,9 @@ test_expect_success 'push preserves up-to-date packed refs' '\n \t\tgit push &&\n \t\t! test -f .git/refs/remotes/origin/main\n \t)\n-\n '\n \n test_expect_success 'push does not update local refs on failure' '\n-\n \tmk_test testrepo heads/main &&\n \tmk_child testrepo child &&\n \techo \"#!/no/frobnication/today\" >testrepo/.git/hooks/pre-receive &&\n@@ -717,16 +669,13 @@ test_expect_success 'push does not update local refs on failure' '\n \t\ttest $(git rev-parse main) != \\\n \t\t\t$(git rev-parse remotes/origin/main)\n \t)\n-\n '\n \n test_expect_success 'allow deleting an invalid remote ref' '\n-\n \tmk_test testrepo heads/branch &&\n \trm -f testrepo/.git/objects/??/* &&\n \tgit push testrepo :refs/heads/branch &&\n \t(cd testrepo && test_must_fail git rev-parse --verify refs/heads/branch)\n-\n '\n \n test_expect_success 'pushing valid refs triggers post-receive and post-update hooks' '\n-- \n2.50.1\n\n"},{"id":"523806","messageId":"d21612fca63794df8cb405280d795799b374e1cd.1754637850.git.liu.denton@gmail.com","threadId":"63904","inReplyTo":"cover.1754637849.git.liu.denton@gmail.com","subject":"[PATCH v5 2/3] remote.c: remove BUG in show_push_unqualified_ref_name_error()","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2025-08-08T07:24:45Z","receivedAt":"2025-08-08T07:24:48Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"When \"git push <remote> <src>:<dst>\" does not spell out the\ndestination side of the ref fully, and when <src> is not given\nas a reference but an object name, the code tries to give advice\nmessages based on the type of that object.\n\nThe type is determined by calling odb_read_object_info() and\nsignalled by its return value.  The code however reported a\nprogramming error with BUG() when this function said that there\nis no such object, which happens when the object name is given\nas a full hexadecimal (if the object name is given as a partial\nhexadecimal or an non-existing ref, the function would have died\nwithout returning, so this BUG() wouldn't have triggered).  This\nis wrong.  It is an ordinary end-user mistake to give an object\nname that does not exist and treated as such.\n\nAn example of the error message produced is as follows:\n\n\terror: The destination you provided is not a full refname (i.e.,\n\tstarting with \"refs/\"). We tried to guess what you meant by:\n\n\t- Looking for a ref that matches 'branch' on the remote side.\n\t- Checking if the <src> being pushed ('0000000000000000000000000000000000000001')\n\t  is a ref in \"refs/{heads,tags}/\". If so we add a corresponding\n\t  refs/{heads,tags}/ prefix on the remote side.\n\n\tNeither worked, so we gave up. You must fully qualify the ref.\n\tBUG: remote.c:1221: '0000000000000000000000000000000000000001' should be commit/tag/tree/blob, is '-1'\n\tfatal: the remote end hung up unexpectedly\n\tAborted (core dumped)\n\nHelped-by: Junio C Hamano <gitster@pobox.com>\nSigned-off-by: Denton Liu <liu.denton@gmail.com>\n---\n remote.c              | 5 +++--\n t/t5516-fetch-push.sh | 5 +++++\n 2 files changed, 8 insertions(+), 2 deletions(-)\n\ndiff --git a/remote.c b/remote.c\nindex 88f991795b..00761604a8 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -1218,8 +1218,9 @@ static void show_push_unqualified_ref_name_error(const char *dst_value,\n \t\t\t \"'%s:refs/tags/%s'?\"),\n \t\t       matched_src_name, dst_value);\n \t} else {\n-\t\tBUG(\"'%s' should be commit/tag/tree/blob, is '%d'\",\n-\t\t    matched_src_name, type);\n+\t\tadvise(_(\"The <src> part of the refspec ('%s') \"\n+\t\t\t \"is an object ID that doesn't exist.\\n\"),\n+\t\t       matched_src_name);\n \t}\n }\n \ndiff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh\nindex 8eddf3e40d..46926e7bbd 100755\n--- a/t/t5516-fetch-push.sh\n+++ b/t/t5516-fetch-push.sh\n@@ -471,6 +471,11 @@ test_expect_success 'push ref expression with non-existent, incomplete dest' '\n \ttest_must_fail git push testrepo main^:branch\n '\n \n+test_expect_success 'push ref expression with non-existent oid src' '\n+\tmk_test testrepo &&\n+\ttest_must_fail git push testrepo $(test_oid 001):branch\n+'\n+\n for head in HEAD @\n do\n \n-- \n2.50.1\n\n"},{"id":"523807","messageId":"cbda61af5c33a5ca4e7dcf2cfa5a2068e9483a6c.1754637850.git.liu.denton@gmail.com","threadId":"63904","inReplyTo":"cover.1754637849.git.liu.denton@gmail.com","subject":"[PATCH v5 3/3] remote.c: convert if-else ladder to switch","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2025-08-08T07:24:48Z","receivedAt":"2025-08-08T07:24:52Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"For better readability, convert the if-else ladder into a switch\nstatement.\n\nSuggested-by: Patrick Steinhardt <ps@pks.im>\nSigned-off-by: Denton Liu <liu.denton@gmail.com>\n---\n remote.c | 19 ++++++++++++-------\n 1 file changed, 12 insertions(+), 7 deletions(-)\n\ndiff --git a/remote.c b/remote.c\nindex 00761604a8..df88914716 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -1171,7 +1171,6 @@ static void show_push_unqualified_ref_name_error(const char *dst_value,\n \t\t\t\t\t\t const char *matched_src_name)\n {\n \tstruct object_id oid;\n-\tenum object_type type;\n \n \t/*\n \t * TRANSLATORS: \"matches '%s'%\" is the <dst> part of \"git push\n@@ -1196,31 +1195,37 @@ static void show_push_unqualified_ref_name_error(const char *dst_value,\n \t\tBUG(\"'%s' is not a valid object, \"\n \t\t    \"match_explicit_lhs() should catch this!\",\n \t\t    matched_src_name);\n-\ttype = odb_read_object_info(the_repository->objects, &oid, NULL);\n-\tif (type == OBJ_COMMIT) {\n+\n+\tswitch (odb_read_object_info(the_repository->objects, &oid, NULL)) {\n+\tcase OBJ_COMMIT:\n \t\tadvise(_(\"The <src> part of the refspec is a commit object.\\n\"\n \t\t\t \"Did you mean to create a new branch by pushing to\\n\"\n \t\t\t \"'%s:refs/heads/%s'?\"),\n \t\t       matched_src_name, dst_value);\n-\t} else if (type == OBJ_TAG) {\n+\t\tbreak;\n+\tcase OBJ_TAG:\n \t\tadvise(_(\"The <src> part of the refspec is a tag object.\\n\"\n \t\t\t \"Did you mean to create a new tag by pushing to\\n\"\n \t\t\t \"'%s:refs/tags/%s'?\"),\n \t\t       matched_src_name, dst_value);\n-\t} else if (type == OBJ_TREE) {\n+\t\tbreak;\n+\tcase OBJ_TREE:\n \t\tadvise(_(\"The <src> part of the refspec is a tree object.\\n\"\n \t\t\t \"Did you mean to tag a new tree by pushing to\\n\"\n \t\t\t \"'%s:refs/tags/%s'?\"),\n \t\t       matched_src_name, dst_value);\n-\t} else if (type == OBJ_BLOB) {\n+\t\tbreak;\n+\tcase OBJ_BLOB:\n \t\tadvise(_(\"The <src> part of the refspec is a blob object.\\n\"\n \t\t\t \"Did you mean to tag a new blob by pushing to\\n\"\n \t\t\t \"'%s:refs/tags/%s'?\"),\n \t\t       matched_src_name, dst_value);\n-\t} else {\n+\t\tbreak;\n+\tdefault:\n \t\tadvise(_(\"The <src> part of the refspec ('%s') \"\n \t\t\t \"is an object ID that doesn't exist.\\n\"),\n \t\t       matched_src_name);\n+\t\tbreak;\n \t}\n }\n \n-- \n2.50.1\n\n"},{"id":"523808","messageId":"aJWnMSmEXNTG1lL1@pks.im","threadId":"63904","inReplyTo":"cover.1754637849.git.liu.denton@gmail.com","subject":"Re: [PATCH v5 0/3] remote.c: remove erroneous BUG case","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-08-08T07:28:49Z","receivedAt":"2025-08-08T07:28:55Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Fri, Aug 08, 2025 at 12:24:39AM -0700, Denton Liu wrote:\n> In the case where one pushes a non-existent oid to an unqualified\n> destination, we encounter the following BUG\n> \n> \terror: The destination you provided is not a full refname (i.e.,\n> \tstarting with \"refs/\"). We tried to guess what you meant by:\n> \n> \t- Looking for a ref that matches 'branch' on the remote side.\n> \t- Checking if the <src> being pushed ('0000000000000000000000000000000000000001')\n> \t  is a ref in \"refs/{heads,tags}/\". If so we add a corresponding\n> \t  refs/{heads,tags}/ prefix on the remote side.\n> \n> \tNeither worked, so we gave up. You must fully qualify the ref.\n> \tBUG: remote.c:1221: '0000000000000000000000000000000000000001' should be commit/tag/tree/blob, is '-1'\n> \tfatal: the remote end hung up unexpectedly\n> \tAborted (core dumped)\n> \n> However, this isn't actually a bug so replace it with an advise()\n> message.\n> \n> Changes since v4:\n> \n> * Put the switch statement refactoring patch last so that we don't get\n>   compile errors from a missing variable\n\nThanks, this version looks good to me.\n\nPatrick\n"},{"id":"523829","messageId":"xmqqo6spiyqp.fsf@gitster.g","threadId":"63904","inReplyTo":"aJWnMSmEXNTG1lL1@pks.im","subject":"Re: [PATCH v5 0/3] remote.c: remove erroneous BUG case","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-08-08T16:06:54Z","receivedAt":"2025-08-08T16:06:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt <ps@pks.im> writes:\n\n> On Fri, Aug 08, 2025 at 12:24:39AM -0700, Denton Liu wrote:\n>> In the case where one pushes a non-existent oid to an unqualified\n>> destination, we encounter the following BUG\n>> \n>> \terror: The destination you provided is not a full refname (i.e.,\n>> \tstarting with \"refs/\"). We tried to guess what you meant by:\n>> \n>> \t- Looking for a ref that matches 'branch' on the remote side.\n>> \t- Checking if the <src> being pushed ('0000000000000000000000000000000000000001')\n>> \t  is a ref in \"refs/{heads,tags}/\". If so we add a corresponding\n>> \t  refs/{heads,tags}/ prefix on the remote side.\n>> \n>> \tNeither worked, so we gave up. You must fully qualify the ref.\n>> \tBUG: remote.c:1221: '0000000000000000000000000000000000000001' should be commit/tag/tree/blob, is '-1'\n>> \tfatal: the remote end hung up unexpectedly\n>> \tAborted (core dumped)\n>> \n>> However, this isn't actually a bug so replace it with an advise()\n>> message.\n>> \n>> Changes since v4:\n>> \n>> * Put the switch statement refactoring patch last so that we don't get\n>>   compile errors from a missing variable\n>\n> Thanks, this version looks good to me.\n\nYeah, this looks good.  Let's mark it for 'next'.\n\nThanks, both.\n"}]}