{"thread":{"id":"60862","subject":"[PATCH] restore: allow --staged on unborn branch","startedAt":"2024-02-06T23:05:33Z","lastAt":"2024-02-07T16:39:48Z","messageCount":3,"participants":["Ghanshyam Thakkar","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"488113","messageId":"20240206230357.1097505-2-shyamthakkar001@gmail.com","threadId":"60862","inReplyTo":null,"subject":"[PATCH] restore: allow --staged on unborn branch","fromName":"Ghanshyam Thakkar","fromEmail":"shyamthakkar001@gmail.com","sentAt":"2024-02-06T23:03:50Z","receivedAt":"2024-02-06T23:05:33Z","isPatch":true,"sender":{"key":"shyamthakkar001@gmail.com","avatar":"https://avatars.githubusercontent.com/u/72698233?v=4"},"body":"Some users expect that being on an unborn branch is similar to having an\nempty tree checked out. However, when running \"git restore --staged .\"\non unborn branch having staged changes, the follwing error gets printed,\n\n    fatal: could not resolve HEAD\n\nTherefore, teach \"git restore --staged .\" without a source option, to\ntake empty tree as source on unborn branch. Note that, this assumption\nis already taken by \"git reset\" (166ec2e9). However, still disallow\nexplicitly referring to HEAD on unborn branch.\n\nSigned-off-by: Ghanshyam Thakkar <shyamthakkar001@gmail.com>\n---\n builtin/checkout.c        | 27 +++++++++++++++++++-------\n t/t2073-restore-unborn.sh | 40 +++++++++++++++++++++++++++++++++++++++\n 2 files changed, 60 insertions(+), 7 deletions(-)\n create mode 100755 t/t2073-restore-unborn.sh\n\ndiff --git a/builtin/checkout.c b/builtin/checkout.c\nindex a6e30931b5..1258ae0a59 100644\n--- a/builtin/checkout.c\n+++ b/builtin/checkout.c\n@@ -1691,6 +1691,7 @@ static int checkout_main(int argc, const char **argv, const char *prefix,\n \t\t\t struct branch_info *new_branch_info)\n {\n \tint parseopt_flags = 0;\n+\tint unborn_and_unspecified = 0;\n \n \topts->overwrite_ignore = 1;\n \topts->prefix = prefix;\n@@ -1754,12 +1755,6 @@ static int checkout_main(int argc, const char **argv, const char *prefix,\n \t}\n \tif (opts->checkout_index < 0 || opts->checkout_worktree < 0)\n \t\tBUG(\"these flags should be non-negative by now\");\n-\t/*\n-\t * convenient shortcut: \"git restore --staged [--worktree]\" equals\n-\t * \"git restore --staged [--worktree] --source HEAD\"\n-\t */\n-\tif (!opts->from_treeish && opts->checkout_index)\n-\t\topts->from_treeish = \"HEAD\";\n \n \t/*\n \t * From here on, new_branch will contain the branch to be checked out,\n@@ -1785,6 +1780,18 @@ static int checkout_main(int argc, const char **argv, const char *prefix,\n \t\topts->new_branch = argv0 + 1;\n \t}\n \n+\t/*\n+\t * convenient shortcut: \"git restore --staged [--worktree]\" equals\n+\t * \"git restore --staged [--worktree] --source HEAD\"\n+\t */\n+\tif (!opts->from_treeish && opts->checkout_index) {\n+\t\tstruct object_id oid;\n+\t\topts->from_treeish = \"HEAD\";\n+\n+\t\tif(repo_get_oid(the_repository, opts->from_treeish, &oid))\n+\t\t\tunborn_and_unspecified = 1;\n+\t}\n+\n \t/*\n \t * Extract branch name from command line arguments, so\n \t * all that is left is pathspecs.\n@@ -1812,7 +1819,13 @@ static int checkout_main(int argc, const char **argv, const char *prefix,\n \t} else if (!opts->accept_ref && opts->from_treeish) {\n \t\tstruct object_id rev;\n \n-\t\tif (repo_get_oid_mb(the_repository, opts->from_treeish, &rev))\n+\t\t/*\n+\t\t * when the branch is unborn and no revision is given, use\n+\t\t * empty tree as source\n+\t\t */\n+\t\tif(unborn_and_unspecified)\n+\t\t\toidcpy(&rev, the_hash_algo->empty_tree);\n+\t\telse if (repo_get_oid_mb(the_repository, opts->from_treeish, &rev))\n \t\t\tdie(_(\"could not resolve %s\"), opts->from_treeish);\n \n \t\tsetup_new_branch_info_and_source_tree(new_branch_info,\ndiff --git a/t/t2073-restore-unborn.sh b/t/t2073-restore-unborn.sh\nnew file mode 100755\nindex 0000000000..fbd8b2df5f\n--- /dev/null\n+++ b/t/t2073-restore-unborn.sh\n@@ -0,0 +1,40 @@\n+#!/bin/sh\n+\n+test_description='restore --staged should work on unborn branch'\n+. ./test-lib.sh\n+\n+test_expect_success 'explicitly naming HEAD on unborn should fail' '\n+\techo a >foo &&\n+\techo b >bar &&\n+\tgit add foo bar &&\n+\ttest_must_fail git restore --staged --source=HEAD .\n+'\n+\n+test_expect_success 'restore --staged .' '\n+\trm .git/index &&\n+\tgit add foo bar &&\n+\tgit restore --staged . &&\n+\tgit diff --cached --name-only >actual &&\n+\ttest_must_be_empty actual\n+'\n+\n+test_expect_success 'restore --staged $file' '\n+\trm .git/index &&\n+\tgit add foo bar &&\n+\tgit restore --staged foo &&\n+\tgit diff --cached --name-only >actual &&\n+\techo bar >expected &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'restore -p --staged' '\n+\trm .git/index &&\n+\tgit add foo bar &&\n+\ttest_write_lines y n | git restore -p --staged >output &&\n+\tgit diff --cached --name-only >actual &&\n+\techo foo >expected &&\n+\ttest_cmp expected actual &&\n+\ttest_grep \"Unstage\" output\n+'\n+\n+test_done\n-- \n2.43.0\n\n"},{"id":"488141","messageId":"xmqqle7wcl7h.fsf@gitster.g","threadId":"60862","inReplyTo":"20240206230357.1097505-2-shyamthakkar001@gmail.com","subject":"Re: [PATCH] restore: allow --staged on unborn branch","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-02-07T16:06:58Z","receivedAt":"2024-02-07T16:07:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ghanshyam Thakkar <shyamthakkar001@gmail.com> writes:\n\n> Some users expect that being on an unborn branch is similar to having an\n> empty tree checked out. However, when running \"git restore --staged .\"\n> on unborn branch having staged changes, the follwing error gets printed,\n>\n>     fatal: could not resolve HEAD\n\nSounds like a sensible behaviour---there is no HEAD so there is\nnothing to resolve.  With \"git resetore --staged .\" in such a state,\nwhat did the user try to do?  \"git reset\" (no other arguments)?\n\nBTW, \"follwing\" -> \"following\".\n"},{"id":"488147","messageId":"CYYZUAGA7I6C.2M4CHKN00W3P4@gmail.com","threadId":"60862","inReplyTo":"xmqqle7wcl7h.fsf@gitster.g","subject":"Re: [PATCH] restore: allow --staged on unborn branch","fromName":"Ghanshyam Thakkar","fromEmail":"shyamthakkar001@gmail.com","sentAt":"2024-02-07T16:39:44Z","receivedAt":"2024-02-07T16:39:48Z","isPatch":true,"sender":{"key":"shyamthakkar001@gmail.com","avatar":"https://avatars.githubusercontent.com/u/72698233?v=4"},"body":"On Wed Feb 7, 2024 at 9:36 PM IST, Junio C Hamano wrote:\n> Ghanshyam Thakkar <shyamthakkar001@gmail.com> writes:\n>\n> > Some users expect that being on an unborn branch is similar to having an\n> > empty tree checked out. However, when running \"git restore --staged .\"\n> > on unborn branch having staged changes, the follwing error gets printed,\n> >\n> >     fatal: could not resolve HEAD\n>\n> Sounds like a sensible behaviour---there is no HEAD so there is\n> nothing to resolve.  With \"git resetore --staged .\" in such a state,\n> what did the user try to do?  \"git reset\" (no other arguments)?\n\nYeah, I did \"git reset\" (I am that user btw). But I suppose this is a\ncase of UX. If a user is using \"git restore --staged .\" every time they\nwant to unstage something, then why would they expect it to fail on an\nunborn branch when something similar (e.g \"git reset\") does not?\n\nAlso that HEAD, I suppose, is our assumption of the user's intent. And\nintent of the user using \"git restore --staged .\" on unborn branch is\nto unstage the changes relative to empty tree. Besides \"git reset\" already\ndoes this so the matter of assuming empty tree on unborn, I suppose, would\nalready be settled. Therefore, wouldn't assuming empty tree on unborn\nwhen also using \"git restore --staged .\" be reasonable and consistent?\n\n"}]}