{"thread":{"id":"62928","subject":"[PATCH] contrib/subtree: verify HEAD is valid before adding a subtree","startedAt":"2025-02-10T02:11:58Z","lastAt":"2025-02-11T01:13:43Z","messageCount":3,"participants":["Bingwu Zhang","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"512166","messageId":"20250210021128.31083-2-xtex@envs.net","threadId":"62928","inReplyTo":null,"subject":"[PATCH] contrib/subtree: verify HEAD is valid before adding a subtree","fromName":"Bingwu Zhang","fromEmail":"xtex@envs.net","sentAt":"2025-02-10T02:11:26Z","receivedAt":"2025-02-10T02:11:58Z","isPatch":true,"sender":{"key":"xtex@envs.net","avatar":null},"body":"From: Bingwu Zhang \n\nAfter initializing a new repository or switching to a orphan branch,\nHEAD is a symbolic reference to refs/heads/xxx while the pointed branch\nhead does not exist until a initial commit.\n\n\"git subtree add\" will try to ensure that working tree and index are\nclean, but as HEAD is invalid, diff-index always fails:\n fatal: ambiguous argument 'HEAD': unknown revision or path not in the working tree.\n Use '--' to separate paths from revisions, like this:\n 'git [...] -- [...]'\n fatal: working tree has modifications. Cannot add.\nIt says \"working tree has modifications\" but it is not the case.\n\nAdd a check using \"git show-ref --verify\" to ensure that HEAD is a valid\nreference and give a clearer error message.\n\nSigned-off-by: Bingwu Zhang \n---\n contrib/subtree/git-subtree.sh | 5 +++++\n 1 file changed, 5 insertions(+)\n\ndiff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh\nindex 15ae86db1b27..41eb816e454a 100755\n--- a/contrib/subtree/git-subtree.sh\n+++ b/contrib/subtree/git-subtree.sh\n@@ -770,6 +770,11 @@ copy_or_skip () {\n # Usage: ensure_clean\n ensure_clean () {\n \tassert test $# = 0\n+\t# verify HEAD, or else \"git diff-index HEAD\" will fail\n+\tif ! git show-ref --verify --quiet HEAD 2>&1\n+\tthen\n+\t\tdie \"fatal: HEAD is not a valid reference. Subtree cannot be committed as the first commit of a branch.\"\n+\tfi\n \tif ! git diff-index HEAD --exit-code --quiet 2>&1\n \tthen\n \t\tdie \"fatal: working tree has modifications. Cannot add.\"\n\nbase-commit: f93ff170b93a1782659637824b25923245ac9dd1\n-- \n2.48.1\n\n"},{"id":"512181","messageId":"xmqq8qqddarl.fsf@gitster.g","threadId":"62928","inReplyTo":"20250210021128.31083-2-xtex@envs.net","subject":"Re: [PATCH] contrib/subtree: verify HEAD is valid before adding a subtree","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-02-10T16:28:46Z","receivedAt":"2025-02-10T16:28:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Bingwu Zhang writes:\n\n> From: Bingwu Zhang \n>\n> After initializing a new repository or switching to a orphan branch,\n> HEAD is a symbolic reference to refs/heads/xxx while the pointed branch\n> head does not exist until a initial commit.\n>\n> \"git subtree add\" will try to ensure that working tree and index are\n> clean, but as HEAD is invalid, diff-index always fails:\n> fatal: ambiguous argument 'HEAD': unknown revision or path not in the working tree.\n> Use '--' to separate paths from revisions, like this:\n> 'git [...] -- [...]'\n> fatal: working tree has modifications. Cannot add.\n\n[Disclaimer. I do not use \"git subtree\" at all myself, and I may be\nmissing the usual expectation of end-users of that command in the\nfollowing description.]\n\nGood finding. I can see that an unborn HEAD would cause the command\nfail.\n\n> It says \"working tree has modifications\" but it is not the case.\n\nI am not sure if \"it is not the case\" is true, though. If the index\nis empty (i.e. nothing has been added yet) and the HEAD is unborn,\nshouldn't that state be considered that working tree has no\nmodifications? IOW, wouldn't this part of the code that uses\n\"diff-index HEAD\" want to consider that an unborn HEAD equivalent to\nan empty tree for the purpose of the comparison?\n\nIn short, \"is not\" -> \"may not be\", perhaps?\n\n> Add a check using \"git show-ref --verify\" to ensure that HEAD is a valid\n> reference and give a clearer error message.\n\nAnd if we want to treat an unborn HEAD equivalent to an empty tree,\nthen dying upon seeing \"show-ref\" fail would not be a good solution\nto the problem, no? Shouldn't the updated logic to deal with an\nunborn HEAD be more like \"if we see that the HEAD is unborn, then we\nare happy iff the index is empty; if HEAD already points at a\ncommit, then we are happy iff the working tree has no changes\nrelative to it\"?\n\n> Signed-off-by: Bingwu Zhang \n> ---\n> contrib/subtree/git-subtree.sh | 5 +++++\n> 1 file changed, 5 insertions(+)\n>\n> diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh\n> index 15ae86db1b27..41eb816e454a 100755\n> --- a/contrib/subtree/git-subtree.sh\n> +++ b/contrib/subtree/git-subtree.sh\n> @@ -770,6 +770,11 @@ copy_or_skip () {\n> # Usage: ensure_clean\n> ensure_clean () {\n> \tassert test $# = 0\n> +\t# verify HEAD, or else \"git diff-index HEAD\" will fail\n> +\tif ! git show-ref --verify --quiet HEAD 2>&1\n> +\tthen\n> +\t\tdie \"fatal: HEAD is not a valid reference. Subtree cannot be committed as the first commit of a branch.\"\n> +\tfi\n> \tif ! git diff-index HEAD --exit-code --quiet 2>&1\n> \tthen\n> \t\tdie \"fatal: working tree has modifications. Cannot add.\"\n>\n> base-commit: f93ff170b93a1782659637824b25923245ac9dd1\n"},{"id":"512205","messageId":"12606159.O9o76ZdvQC@xtex1","threadId":"62928","inReplyTo":"xmqq8qqddarl.fsf@gitster.g","subject":"Re: [PATCH] contrib/subtree: verify HEAD is valid before adding a subtree","fromName":"Bingwu Zhang","fromEmail":"xtex@envs.net","sentAt":"2025-02-11T01:13:26Z","receivedAt":"2025-02-11T01:13:43Z","isPatch":true,"sender":{"key":"xtex@envs.net","avatar":null},"body":"On Tuesday, February 11, 2025 12:28:46 AM GMT+8 Junio C Hamano wrote:\n> In short, \"is not\" -> \"may not be\", perhaps?\n\nYes, thank you!\n\n> And if we want to treat an unborn HEAD equivalent to an empty tree,\n> then dying upon seeing \"show-ref\" fail would not be a good solution\n> to the problem, no? Shouldn't the updated logic to deal with an\n> unborn HEAD be more like \"if we see that the HEAD is unborn, then we\n> are happy iff the index is empty; if HEAD already points at a\n> commit, then we are happy iff the working tree has no changes\n> relative to it\"?\n\nI did consider treating an unborn HEAD as a empty tree and test if index is \nempty. However I searched and failed to find out a git subcommand to do that \ntest (:\nDo you have any suggestions?\n\n-- \nBingwu Zhang\nxtex @ Tue, 11 Feb 2025 01:10:17 +0000\n\n\n\n"}]}