{"thread":{"id":"63449","subject":"[BUG] git stash incorrectly showing submodule branch instead of superproject branch","startedAt":"2025-05-12T15:19:33Z","lastAt":"2025-06-11T16:00:40Z","messageCount":17,"participants":["Stuart MacDonald","K Jayatheerth","JAYATHEERTH K","Junio C Hamano","Eric Sunshine","René Scharfe"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"517859","messageId":"TO1PPF29324B4CE6D3518208073452C3C51CD97A@TO1PPF29324B4CE.CANPRD01.PROD.OUTLOOK.COM","threadId":"63449","inReplyTo":null,"subject":"[BUG] git stash incorrectly showing submodule branch instead of superproject branch","fromName":"Stuart MacDonald","fromEmail":"smacdonald@kaimaging.com","sentAt":"2025-05-12T15:19:30Z","receivedAt":"2025-05-12T15:19:33Z","isPatch":false,"sender":{"key":"smacdonald@kaimaging.com","avatar":null},"body":"Bug report inline below. I am willing to provide further information but I am not subscribed to this list.\n\n\n\n\nThank you for filling out a Git bug report!\nPlease answer the following questions to help us understand your issue.\n\nWhat did you do before the bug happened? (Steps to reproduce your issue)\n\nI am working on a UI that includes the hardware's SDK as a submodule.\nBoth the UI and the SDK are our own code. A colleague is working on the\nSDK.\n\n- clone out a project that has a submodule\n- set up submodule properly\n- create feature branch 'feature_foo'\n- move submodule to colleague's feature branch 'feature_sdk_foo'\n- develop for a while\n\n- in superproject discover bug, fix bug\n- create new superproject branch to contain bug fix 'bugfix_bar'\n- leave submodule branch as is; I'm intending to return to it momentarily\n- commit fix via 'git add --patch file1 file2 ...'\n- save bugfix debugging via 'git stash push -m \"debugging\" file1 file2 ...'\n\nWhat did you expect to happen? (Expected behavior)\n\n'git stash list' shows\nstash@{0}: On bugfix_bar: debugging\n\nWhat happened instead? (Actual behavior)\n\n'git stash list' shows\nstash@{0}: On feature_sdk_foo: debugging\n\nWhat's different between what you expected and what actually happened?\n\nThe \"On <branch>\" says the wrong branch; specifically it's the\nsubmodule's currently checked out branch. I have a number of branches on\nthe go, and need to rely on stash telling me the branch I was on when\npushing to determine when I no longer need the debugging.\nThe said branch doesn't even exist in the superproject.\n\nAnything else you want to add:\n\nUsing Git For Windows on Windows 10. Using the \"git bash\" command line\ngit, not the UI git.\n\nI'm pretty sure this used to work correctly, but it's been several years\nsince my last repo with a submodule (circa 2021).\n\nThe Git For Windows bash PS1 prompt lists the current branch I'm on in the\nsuperproject, so the error is quite visible.\n\nReading the 'git stash' documentation didn't shed any light.\nGoogling didn't reveal anyone encountering this problem.\nSearching git@vger.kernel.org didn't reveal anyone reporting this.\n\nPlease review the rest of the bug report below.\nYou can delete any lines you don't wish to share.\n\n\n[System Info]\ngit version:\ngit version 2.49.0.windows.1\ncpu: x86_64\nbuilt from commit: cca1f38702730b35f52c29efd62864b85e85ddcc\nsizeof-long: 4\nsizeof-size_t: 8\nshell-path: D:/git-sdk-64-build-installers/usr/bin/sh\nfeature: fsmonitor--daemon\nlibcurl: 8.12.1\nOpenSSL: OpenSSL 3.2.4 11 Feb 2025\nzlib: 1.3.1\nuname: Windows 10.0 26100\ncompiler info: gnuc: 14.2\nlibc info: no libc information available\n$SHELL (typically, interactive shell): C:\\Program Files\\Git\\usr\\bin\\bash.exe\n\n\n[Enabled Hooks]\nCONFIDENTIALITY NOTICE: The contents of this email, including any attachments, may contain private and confidential information intended to be reviewed only by the individual(s) or organization to whom it is addressed and may be legally protected from disclosure. If you are not the intended recipient or an authorized representative of the intended recipient, please be notified that any review, distribution, copying, saving or disclosure is strictly prohibited. If you have received this email in error, please immediately notify the sender by return email and delete this email from your system, including from the deleted items folder. Thank you for your cooperation.\n"},{"id":"517863","messageId":"20250512161253.58427-1-jayatheerthkulkarni2005@gmail.com","threadId":"63449","inReplyTo":"TO1PPF29324B4CE6D3518208073452C3C51CD97A@TO1PPF29324B4CE.CANPRD01.PROD.OUTLOOK.COM","subject":"Re [BUG] git stash incorrectly showing submodule branch instead of superproject branch","fromName":"K Jayatheerth","fromEmail":"jayatheerthkulkarni2005@gmail.com","sentAt":"2025-05-12T16:12:52Z","receivedAt":"2025-05-12T16:12:59Z","isPatch":false,"sender":{"key":"jayatheerthkulkarni2005@gmail.com","avatar":"https://avatars.githubusercontent.com/u/148841023?v=4"},"body":"\n\nThank you for reporting the bug, Stuart.\n\nThis is genuinely one of the most interesting Git bugs I’ve seen in a while \nI was able to reproduce it with the following minimal steps:\n\n    mkdir sdk && cd sdk\n    git init\n    echo \"SDK file\" > sdk.txt\n    git add sdk.txt\n    git commit -m \"Initial commit in SDK\"\n    cd ..\n\n    mkdir ui && cd ui\n    git init\n    git -c protocol.file.allow=always submodule add ../sdk\n    git commit -m \"Add SDK as submodule\"\n\n    git checkout -b feature_foo        # in main repo\n    cd sdk\n    git checkout -b feature_sdk_foo    # in submodule\n    cd ..\n    git checkout -b bugfix_bar         # still in main repo\n\n    echo \"Bugfix content\" > fix.txt\n    git add fix.txt\n    git stash push -m \"debugging\"\n\n    git stash list\n\nAfter this, the stash message shows:\n    stash@{0}: On feature_sdk_foo: debugging\n\nWhich clearly leaks the submodule’s branch name into the superproject’s stash label.\n\nThis is indeed a `git stash` problem \nI verified that:\n- Branches were unchanged after stashing\n- Submodule state remained untouched\n- `git status` correctly reported the superproject branch\n- Yet the stash commit was labeled with the submodule’s branch name\n\nSo the stash mechanism seems to be pulling `HEAD` information from the submodule \ncontext by mistake, even when the stash is purely for the superproject.\n\nAlso confirming that this is *not Windows-specific* — I reproduced it on Fedora as well.\n\nThe historical detail you shared (that this used to work fine around 2021) is helpful. \nI'll dig into the stash code paths to see if I can isolate a regression or misbehavior.\n\n \n-Jayatheerth\n"},{"id":"517866","messageId":"TO1PPF29324B4CE2F822159A3F1C6A5F3B4CD97A@TO1PPF29324B4CE.CANPRD01.PROD.OUTLOOK.COM","threadId":"63449","inReplyTo":"20250512161253.58427-1-jayatheerthkulkarni2005@gmail.com","subject":"Re: Re [BUG] git stash incorrectly showing submodule branch instead of superproject branch","fromName":"Stuart MacDonald","fromEmail":"smacdonald@kaimaging.com","sentAt":"2025-05-12T16:26:23Z","receivedAt":"2025-05-12T16:26:30Z","isPatch":false,"sender":{"key":"smacdonald@kaimaging.com","avatar":null},"body":"> This is genuinely one of the most interesting Git bugs I’ve seen in a while\n\nThanks. I think. :-)\n\n> The historical detail you shared (that this used to work fine around 2021) is helpful.\n\nI said I _think_ it used to work; I am not 100% on this.\nI can say 100% that\n- I've been stashing for a long time; 2016 or before\n- the repo with the submodules was in the Apr 2021 - Apr 2022 window\n- I don't recall seeing this bug at that time, BUT, I may have just not stashed anything in a way that triggered it\n\nSo it feels like a regression but I am not 100% certain.\nCONFIDENTIALITY NOTICE: The contents of this email, including any attachments, may contain private and confidential information intended to be reviewed only by the individual(s) or organization to whom it is addressed and may be legally protected from disclosure. If you are not the intended recipient or an authorized representative of the intended recipient, please be notified that any review, distribution, copying, saving or disclosure is strictly prohibited. If you have received this email in error, please immediately notify the sender by return email and delete this email from your system, including from the deleted items folder. Thank you for your cooperation.\n"},{"id":"517867","messageId":"20250512164001.62065-1-jayatheerthkulkarni2005@gmail.com","threadId":"63449","inReplyTo":"TO1PPF29324B4CE2F822159A3F1C6A5F3B4CD97A@TO1PPF29324B4CE.CANPRD01.PROD.OUTLOOK.COM","subject":"[PATCH] stash: fix incorrect branch name in stash message","fromName":"K Jayatheerth","fromEmail":"jayatheerthkulkarni2005@gmail.com","sentAt":"2025-05-12T16:40:01Z","receivedAt":"2025-05-12T16:40:21Z","isPatch":true,"sender":{"key":"jayatheerthkulkarni2005@gmail.com","avatar":"https://avatars.githubusercontent.com/u/148841023?v=4"},"body":"When creating a stash, Git builds the stash commit message using the current\nbranch name of the superproject. However, in repositories that include\nsubmodules, the stash message may incorrectly show the submodule branch\ninstead of the superproject.\n\nThis happens because `branch_name` is obtained via `refs_resolve_ref_unsafe()`,\nwhich returns a pointer to a static buffer. Later calls to the same function\n(e.g., during commit creation or submodule reference resolution) overwrite\nthis buffer, causing the `branch_name` used in the final stash message to be\nincorrect.\n\nReported-by: Stuart <smacdonald@kaimaging.com>\nSigned-off-by: K Jayatheerth <jayatheerthkulkarni2005@gmail.com>\n---\n builtin/stash.c | 20 +++++++++++++++-----\n 1 file changed, 15 insertions(+), 5 deletions(-)\n\ndiff --git a/builtin/stash.c b/builtin/stash.c\nindex cfbd92852a..6a375a3430 100644\n--- a/builtin/stash.c\n+++ b/builtin/stash.c\n@@ -1377,6 +1377,7 @@ static int do_create_stash(const struct pathspec *ps, struct strbuf *stash_msg_b\n \tstruct strbuf msg = STRBUF_INIT;\n \tstruct strbuf commit_tree_label = STRBUF_INIT;\n \tstruct strbuf untracked_files = STRBUF_INIT;\n+\tchar *branch_name_buf = NULL;\n \n \tprepare_fallback_ident(\"git stash\", \"git@stash\");\n \n@@ -1404,11 +1405,20 @@ static int do_create_stash(const struct pathspec *ps, struct strbuf *stash_msg_b\n \n \tbranch_ref = refs_resolve_ref_unsafe(get_main_ref_store(the_repository),\n \t\t\t\t\t     \"HEAD\", 0, NULL, &flags);\n-\tif (flags & REF_ISSYMREF)\n-\t\tskip_prefix(branch_ref, \"refs/heads/\", &branch_name);\n-\thead_short_sha1 = repo_find_unique_abbrev(the_repository,\n-\t\t\t\t\t\t  &head_commit->object.oid,\n-\t\t\t\t\t\t  DEFAULT_ABBREV);\n+\n+\tif (flags & REF_ISSYMREF) {\n+\t\tconst char *tmp = NULL;\n+\t\tif (skip_prefix(branch_ref, \"refs/heads/\", &tmp))\n+\t\t\tbranch_name_buf = xstrdup(tmp); \n+\t}\n+\t\tif (branch_name_buf)\n+\t\t\tbranch_name = branch_name_buf;\n+\t\telse\n+\t\t\tbranch_name = \"(no branch)\";\n+\t\t\t\t\t    \n+\t\thead_short_sha1 = repo_find_unique_abbrev(the_repository,\n+\t\t\t\t\t\t\t&head_commit->object.oid,\n+\t\t\t\t\t\t\tDEFAULT_ABBREV);\t\t\t\t\t    \n \tstrbuf_addf(&msg, \"%s: %s \", branch_name, head_short_sha1);\n \tpp_commit_easy(CMIT_FMT_ONELINE, head_commit, &msg);\n \n-- \n2.49.GIT\n\n"},{"id":"517869","messageId":"CA+rGoLfv6E7=TCm99yt4SYsZ1P-QRxD+vMuaHtDWX=ufneiYUA@mail.gmail.com","threadId":"63449","inReplyTo":"20250512164001.62065-1-jayatheerthkulkarni2005@gmail.com","subject":"Re: [PATCH] stash: fix incorrect branch name in stash message","fromName":"JAYATHEERTH K","fromEmail":"jayatheerthkulkarni2005@gmail.com","sentAt":"2025-05-12T16:42:50Z","receivedAt":"2025-05-12T16:43:01Z","isPatch":true,"sender":{"key":"jayatheerthkulkarni2005@gmail.com","avatar":"https://avatars.githubusercontent.com/u/148841023?v=4"},"body":"Changing this I found results to be correct\n\nI think you can use this patch if you want a solution on an immediate basis,\ntesting and merging this into the master usually takes some time.\n\nAfter testing with similar use case listed above\n\njayatheerth@fedora:~/Documents/code/test$ # Clean up previous test directories\nrm -rf ~/Documents/code/test/ui ~/Documents/code/test/sdk\nmkdir -p ~/Documents/code/test && cd ~/Documents/code/test\n\n# Step 1: Create the submodule repo\nmkdir sdk && cd sdk\ngit init\necho \"SDK file\" > sdk.txt\ngit add sdk.txt\ngit commit -m \"Initial commit in SDK\"\ngit stash listk stash messagextsh itbmodule add ../sdk\nhint: Using 'master' as the name for the initial branch. This default\nbranch name\nhint: is subject to change. To configure the initial branch name to use in all\nhint: of your new repositories, which will suppress this warning, call:\nhint:\nhint:   git config --global init.defaultBranch <name>\nhint:\nhint: Names commonly chosen instead of 'master' are 'main', 'trunk' and\nhint: 'development'. The just-created branch can be renamed via this command:\nhint:\nhint:   git branch -m <name>\nhint:\nhint: Disable this message with \"git config set advice.defaultBranchName false\"\nInitialized empty Git repository in\n/home/jayatheerth/Documents/code/test/sdk/.git/\n[master (root-commit) 033e72f] Initial commit in SDK\n 1 file changed, 1 insertion(+)\n create mode 100644 sdk.txt\nhint: Using 'master' as the name for the initial branch. This default\nbranch name\nhint: is subject to change. To configure the initial branch name to use in all\nhint: of your new repositories, which will suppress this warning, call:\nhint:\nhint:   git config --global init.defaultBranch <name>\nhint:\nhint: Names commonly chosen instead of 'master' are 'main', 'trunk' and\nhint: 'development'. The just-created branch can be renamed via this command:\nhint:\nhint:   git branch -m <name>\nhint:\nhint: Disable this message with \"git config set advice.defaultBranchName false\"\nInitialized empty Git repository in\n/home/jayatheerth/Documents/code/test/ui/.git/\nCloning into '/home/jayatheerth/Documents/code/test/ui/sdk'...\ndone.\n[master (root-commit) 870c575] Add SDK as submodule\n 2 files changed, 4 insertions(+)\n create mode 100644 .gitmodules\n create mode 160000 sdk\nSwitched to a new branch 'feature_foo'\nSwitched to a new branch 'feature_sdk_foo'\nSwitched to a new branch 'bugfix_bar'\nSaved working directory and index state On bugfix_bar: debugging\nstash@{0}: On bugfix_bar: debugging\n\nI found it to be on the bugfix_bar correctly as intended.\n\n-Jayatheerth\n"},{"id":"517877","messageId":"TO1PPF29324B4CEE0C0BCD7CA1F2FAFE505CD97A@TO1PPF29324B4CE.CANPRD01.PROD.OUTLOOK.COM","threadId":"63449","inReplyTo":"CA+rGoLfv6E7=TCm99yt4SYsZ1P-QRxD+vMuaHtDWX=ufneiYUA@mail.gmail.com","subject":"Re: [PATCH] stash: fix incorrect branch name in stash message","fromName":"Stuart MacDonald","fromEmail":"smacdonald@kaimaging.com","sentAt":"2025-05-12T17:09:13Z","receivedAt":"2025-05-12T17:09:17Z","isPatch":true,"sender":{"key":"smacdonald@kaimaging.com","avatar":null},"body":"> Changing this I found results to be correct\n\nThe patch looks reasonable to me.\n\n> I think you can use this patch if you want a solution on an immediate basis\n\nI would if I knew how to update/rebuild Git For Window's git.\n\nFor now, I'll count this as fixed and try to remember to check it in the future.\n\nThanks!\n...Stu\nCONFIDENTIALITY NOTICE: The contents of this email, including any attachments, may contain private and confidential information intended to be reviewed only by the individual(s) or organization to whom it is addressed and may be legally protected from disclosure. If you are not the intended recipient or an authorized representative of the intended recipient, please be notified that any review, distribution, copying, saving or disclosure is strictly prohibited. If you have received this email in error, please immediately notify the sender by return email and delete this email from your system, including from the deleted items folder. Thank you for your cooperation.\n"},{"id":"517889","messageId":"xmqqtt5psor5.fsf@gitster.g","threadId":"63449","inReplyTo":"20250512164001.62065-1-jayatheerthkulkarni2005@gmail.com","subject":"Re: [PATCH] stash: fix incorrect branch name in stash message","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-05-12T17:49:34Z","receivedAt":"2025-05-12T17:49:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"K Jayatheerth <jayatheerthkulkarni2005@gmail.com> writes:\n\n> @@ -1377,6 +1377,7 @@ static int do_create_stash(const struct pathspec *ps, struct strbuf *stash_msg_b\n>  \tstruct strbuf msg = STRBUF_INIT;\n>  \tstruct strbuf commit_tree_label = STRBUF_INIT;\n>  \tstruct strbuf untracked_files = STRBUF_INIT;\n> +\tchar *branch_name_buf = NULL;\n>  \n>  \tprepare_fallback_ident(\"git stash\", \"git@stash\");\n>  \n> @@ -1404,11 +1405,20 @@ static int do_create_stash(const struct pathspec *ps, struct strbuf *stash_msg_b\n>  \n>  \tbranch_ref = refs_resolve_ref_unsafe(get_main_ref_store(the_repository),\n>  \t\t\t\t\t     \"HEAD\", 0, NULL, &flags);\n> -\tif (flags & REF_ISSYMREF)\n> -\t\tskip_prefix(branch_ref, \"refs/heads/\", &branch_name);\n> -\thead_short_sha1 = repo_find_unique_abbrev(the_repository,\n> -\t\t\t\t\t\t  &head_commit->object.oid,\n> -\t\t\t\t\t\t  DEFAULT_ABBREV);\n> +\n> +\tif (flags & REF_ISSYMREF) {\n> +\t\tconst char *tmp = NULL;\n> +\t\tif (skip_prefix(branch_ref, \"refs/heads/\", &tmp))\n> +\t\t\tbranch_name_buf = xstrdup(tmp); \n> +\t}\n> +\t\tif (branch_name_buf)\n> +\t\t\tbranch_name = branch_name_buf;\n> +\t\telse\n> +\t\t\tbranch_name = \"(no branch)\";\n> +\t\t\t\t\t    \n> +\t\thead_short_sha1 = repo_find_unique_abbrev(the_repository,\n> +\t\t\t\t\t\t\t&head_commit->object.oid,\n> +\t\t\t\t\t\t\tDEFAULT_ABBREV);\t\t\t\t\t    \n>  \tstrbuf_addf(&msg, \"%s: %s \", branch_name, head_short_sha1);\n>  \tpp_commit_easy(CMIT_FMT_ONELINE, head_commit, &msg);\n\nFunny indentation, and branch_name_buf needs to be free'ed after use\nbut other than that, nice digging!\n\nThanks.\n"},{"id":"517900","messageId":"CAPig+cSbwGdhq66tSk_A5oNGXqpbqFA=ZO5GS1E-ENb+_rY70A@mail.gmail.com","threadId":"63449","inReplyTo":"xmqqtt5psor5.fsf@gitster.g","subject":"Re: [PATCH] stash: fix incorrect branch name in stash message","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2025-05-12T18:54:41Z","receivedAt":"2025-05-12T18:54:53Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Mon, May 12, 2025 at 1:50 PM Junio C Hamano <gitster@pobox.com> wrote:\n> K Jayatheerth <jayatheerthkulkarni2005@gmail.com> writes:\n> > @@ -1404,11 +1405,20 @@ static int do_create_stash(const struct pathspec *ps, struct strbuf *stash_msg_b\n> > +     if (flags & REF_ISSYMREF) {\n> > +             const char *tmp = NULL;\n> > +             if (skip_prefix(branch_ref, \"refs/heads/\", &tmp))\n> > +                     branch_name_buf = xstrdup(tmp);\n> > +     }\n> > +             if (branch_name_buf)\n> > +                     branch_name = branch_name_buf;\n> > +             else\n> > +                     branch_name = \"(no branch)\";\n> > +\n> > +             head_short_sha1 = repo_find_unique_abbrev(the_repository,\n> > +                                                     &head_commit->object.oid,\n> > +                                                     DEFAULT_ABBREV);\n> >       strbuf_addf(&msg, \"%s: %s \", branch_name, head_short_sha1);\n> >       pp_commit_easy(CMIT_FMT_ONELINE, head_commit, &msg);\n>\n> Funny indentation, and branch_name_buf needs to be free'ed after use\n> but other than that, nice digging!\n\nThis change should also be accompanied by a new test to verify the\nfixed behavior, right?\n"},{"id":"517921","messageId":"CA+rGoLfpV7XLB01-5evp=qNX3g9-=za9zH33ceBRnzYe3Og=uA@mail.gmail.com","threadId":"63449","inReplyTo":"CAPig+cSbwGdhq66tSk_A5oNGXqpbqFA=ZO5GS1E-ENb+_rY70A@mail.gmail.com","subject":"Re: [PATCH] stash: fix incorrect branch name in stash message","fromName":"JAYATHEERTH K","fromEmail":"jayatheerthkulkarni2005@gmail.com","sentAt":"2025-05-13T01:21:22Z","receivedAt":"2025-05-13T01:21:35Z","isPatch":true,"sender":{"key":"jayatheerthkulkarni2005@gmail.com","avatar":"https://avatars.githubusercontent.com/u/148841023?v=4"},"body":"On Tue, May 13, 2025 at 12:24 AM Eric Sunshine <sunshine@sunshineco.com> wrote:\n>\n> On Mon, May 12, 2025 at 1:50 PM Junio C Hamano <gitster@pobox.com> wrote:\n> > K Jayatheerth <jayatheerthkulkarni2005@gmail.com> writes:\n> > > @@ -1404,11 +1405,20 @@ static int do_create_stash(const struct pathspec *ps, struct strbuf *stash_msg_b\n> > > +     if (flags & REF_ISSYMREF) {\n> > > +             const char *tmp = NULL;\n> > > +             if (skip_prefix(branch_ref, \"refs/heads/\", &tmp))\n> > > +                     branch_name_buf = xstrdup(tmp);\n> > > +     }\n> > > +             if (branch_name_buf)\n> > > +                     branch_name = branch_name_buf;\n> > > +             else\n> > > +                     branch_name = \"(no branch)\";\n> > > +\n> > > +             head_short_sha1 = repo_find_unique_abbrev(the_repository,\n> > > +                                                     &head_commit->object.oid,\n> > > +                                                     DEFAULT_ABBREV);\n> > >       strbuf_addf(&msg, \"%s: %s \", branch_name, head_short_sha1);\n> > >       pp_commit_easy(CMIT_FMT_ONELINE, head_commit, &msg);\n> >\n> > Funny indentation, and branch_name_buf needs to be free'ed after use\n> > but other than that, nice digging!\n>\n> This change should also be accompanied by a new test to verify the\n> fixed behavior, right?\n\nI will be writing a patch series after checking CI and include tests,\nthe only reason I sent this patch was\nif the bug reporter needed an immediate fix.\nWill send a new patch with test case and improved indentation\n\nThanks for pointing it out\n\n-Jayatheerth\n"},{"id":"518036","messageId":"xmqq7c2jcodj.fsf@gitster.g","threadId":"63449","inReplyTo":"CA+rGoLfpV7XLB01-5evp=qNX3g9-=za9zH33ceBRnzYe3Og=uA@mail.gmail.com","subject":"Re: [PATCH] stash: fix incorrect branch name in stash message","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-05-14T13:28:56Z","receivedAt":"2025-05-14T13:28:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"JAYATHEERTH K <jayatheerthkulkarni2005@gmail.com> writes:\n\n> On Tue, May 13, 2025 at 12:24 AM Eric Sunshine <sunshine@sunshineco.com> wrote:\n>>\n>> On Mon, May 12, 2025 at 1:50 PM Junio C Hamano <gitster@pobox.com> wrote:\n>> > K Jayatheerth <jayatheerthkulkarni2005@gmail.com> writes:\n>> > > @@ -1404,11 +1405,20 @@ static int do_create_stash(const struct pathspec *ps, struct strbuf *stash_msg_b\n>> > > +     if (flags & REF_ISSYMREF) {\n>> > > +             const char *tmp = NULL;\n>> > > +             if (skip_prefix(branch_ref, \"refs/heads/\", &tmp))\n>> > > +                     branch_name_buf = xstrdup(tmp);\n>> > > +     }\n>> > > +             if (branch_name_buf)\n>> > > +                     branch_name = branch_name_buf;\n>> > > +             else\n>> > > +                     branch_name = \"(no branch)\";\n>> > > +\n>> > > +             head_short_sha1 = repo_find_unique_abbrev(the_repository,\n>> > > +                                                     &head_commit->object.oid,\n>> > > +                                                     DEFAULT_ABBREV);\n>> > >       strbuf_addf(&msg, \"%s: %s \", branch_name, head_short_sha1);\n>> > >       pp_commit_easy(CMIT_FMT_ONELINE, head_commit, &msg);\n>> >\n>> > Funny indentation, and branch_name_buf needs to be free'ed after use\n>> > but other than that, nice digging!\n>>\n>> This change should also be accompanied by a new test to verify the\n>> fixed behavior, right?\n>\n> I will be writing a patch series after checking CI and include tests,\n> the only reason I sent this patch was\n> if the bug reporter needed an immediate fix.\n> Will send a new patch with test case and improved indentation\n\nDon't forget to plug the leak, too ;-)\n\n> Thanks for pointing it out\n\n"},{"id":"519909","messageId":"20250608063537.233243-1-jayatheerthkulkarni2005@gmail.com","threadId":"63449","inReplyTo":"xmqq7c2jcodj.fsf@gitster.g","subject":"[PATCH] stash: fix incorrect branch name in stash message","fromName":"K Jayatheerth","fromEmail":"jayatheerthkulkarni2005@gmail.com","sentAt":"2025-06-08T06:35:37Z","receivedAt":"2025-06-08T06:35:51Z","isPatch":true,"sender":{"key":"jayatheerthkulkarni2005@gmail.com","avatar":"https://avatars.githubusercontent.com/u/148841023?v=4"},"body":"When creating a stash, Git uses the current branch name\nof the superproject to construct the stash commit message.\nHowever, in repositories with submodules,\nthe message may mistakenly display the submodule branch name instead.\n\nThis is because `refs_resolve_ref_unsafe()` returns a pointer to a static buffer.\nSubsequent calls to the same function overwrite the buffer,\ncorrupting the originally fetched `branch_name` used for the stash message.\n\nUse `xstrdup()` to duplicate the branch name immediately after resolving it,\nso that later buffer overwrites do not affect the stash message.\n\nSigned-off-by: K Jayatheerth <jayatheerthkulkarni2005@gmail.com>\n---\nThis this patch was long due, and submodules patchs had a design choices \nRefined this patch Added tests and freed the leak as intended.\n\n builtin/stash.c  | 19 +++++++++++++++----\n t/t3903-stash.sh | 39 +++++++++++++++++++++++++++++++++++++++\n 2 files changed, 54 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/stash.c b/builtin/stash.c\nindex cfbd92852a..13606efb12 100644\n--- a/builtin/stash.c\n+++ b/builtin/stash.c\n@@ -1372,6 +1372,7 @@ static int do_create_stash(const struct pathspec *ps, struct strbuf *stash_msg_b\n \tconst char *head_short_sha1 = NULL;\n \tconst char *branch_ref = NULL;\n \tconst char *branch_name = \"(no branch)\";\n+\tchar *branch_name_buf = NULL;\n \tstruct commit *head_commit = NULL;\n \tstruct commit_list *parents = NULL;\n \tstruct strbuf msg = STRBUF_INIT;\n@@ -1401,11 +1402,20 @@ static int do_create_stash(const struct pathspec *ps, struct strbuf *stash_msg_b\n \t\tret = 1;\n \t\tgoto done;\n \t}\n-\n-\tbranch_ref = refs_resolve_ref_unsafe(get_main_ref_store(the_repository),\n+\t\n+\tbranch_ref = refs_resolve_ref_unsafe(get_main_ref_store(the_repository), \n \t\t\t\t\t     \"HEAD\", 0, NULL, &flags);\n-\tif (flags & REF_ISSYMREF)\n-\t\tskip_prefix(branch_ref, \"refs/heads/\", &branch_name);\n+\n+\tif (flags & REF_ISSYMREF) {\n+\t\tconst char *tmp = NULL;\n+\t\tif (skip_prefix(branch_ref, \"refs/heads/\", &tmp))\n+\t\t\tbranch_name_buf = xstrdup(tmp); \n+\t}\n+\tif (branch_name_buf)\n+\t\tbranch_name = branch_name_buf;\n+\telse\n+\t\tbranch_name = \"(no branch)\";\n+\t\t\t\t\t    \n \thead_short_sha1 = repo_find_unique_abbrev(the_repository,\n \t\t\t\t\t\t  &head_commit->object.oid,\n \t\t\t\t\t\t  DEFAULT_ABBREV);\n@@ -1495,6 +1505,7 @@ static int do_create_stash(const struct pathspec *ps, struct strbuf *stash_msg_b\n \tstrbuf_release(&msg);\n \tstrbuf_release(&untracked_files);\n \tfree_commit_list(parents);\n+\tfree(branch_name_buf);\n \treturn ret;\n }\n \ndiff --git a/t/t3903-stash.sh b/t/t3903-stash.sh\nindex 74666ff3e4..5d5aac8b15 100755\n--- a/t/t3903-stash.sh\n+++ b/t/t3903-stash.sh\n@@ -1592,4 +1592,43 @@ test_expect_success 'stash apply reports a locked index' '\n \t)\n '\n \n+test_expect_success 'stash reflog message uses superproject branch, not submodule branch' '\n+\tgit init main_project &&\n+\t(\n+\t\tcd main_project &&\n+\t\techo \"Initial content in main_project\" > main_file.txt &&\n+\t\tgit add main_file.txt &&\n+\t\tgit commit -q -m \"Initial commit in main_project\"\n+\t) &&\n+\n+\tgit init sub_project &&\n+\t(\n+\t\tcd sub_project &&\n+\t\techo \"Initial content in sub_project\" > sub_file.txt &&\n+\t\tgit add sub_file.txt &&\n+\t\tgit commit -q -m \"Initial commit in sub_project\"\n+\t) &&\n+\n+\t(\n+\t\tcd main_project &&\n+\t\tgit -c protocol.file.allow=always submodule add --quiet ../sub_project sub &&\n+\t\tgit commit -q -m \"Added submodule sub_project\" &&\n+\n+\t\tgit checkout -q -b feature_main &&\n+\t\t(\n+\t\t\tcd sub &&\n+\t\t\tgit checkout -q -b feature_sub\n+\t\t) &&\n+\n+\t\tgit checkout -q -b work_branch &&\n+\n+\t\techo \"Important work to be stashed\" > work_item.txt &&\n+\t\tgit add work_item.txt &&\n+\t\tgit stash push -q -m \"custom stash for work_branch\" &&\n+\n+\t\tgit stash list > ../actual_stash_list.txt &&\n+\t\tgrep \"On work_branch: custom stash for work_branch\" ../actual_stash_list.txt\n+\t)\n+'\n+\n test_done\n-- \n2.49.GIT\n\n"},{"id":"519934","messageId":"f46443ac-eb7f-47db-8f4b-a06384e6fde5@web.de","threadId":"63449","inReplyTo":"20250608063537.233243-1-jayatheerthkulkarni2005@gmail.com","subject":"Re: [PATCH] stash: fix incorrect branch name in stash message","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2025-06-08T13:11:58Z","receivedAt":"2025-06-08T13:17:23Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 08.06.25 um 08:35 schrieb K Jayatheerth:\n> When creating a stash, Git uses the current branch name\n> of the superproject to construct the stash commit message.\n> However, in repositories with submodules,\n> the message may mistakenly display the submodule branch name instead.\n> \n> This is because `refs_resolve_ref_unsafe()` returns a pointer to a static buffer.\n> Subsequent calls to the same function overwrite the buffer,\n> corrupting the originally fetched `branch_name` used for the stash message.\n> \n> Use `xstrdup()` to duplicate the branch name immediately after resolving it,\n> so that later buffer overwrites do not affect the stash message.\n\nMakes sense.\n\n> \n> Signed-off-by: K Jayatheerth <jayatheerthkulkarni2005@gmail.com>\n> ---\n> This this patch was long due, and submodules patchs had a design choices \n> Refined this patch Added tests and freed the leak as intended.\n> \n>  builtin/stash.c  | 19 +++++++++++++++----\n>  t/t3903-stash.sh | 39 +++++++++++++++++++++++++++++++++++++++\n>  2 files changed, 54 insertions(+), 4 deletions(-)\n> \n> diff --git a/builtin/stash.c b/builtin/stash.c\n> index cfbd92852a..13606efb12 100644\n> --- a/builtin/stash.c\n> +++ b/builtin/stash.c\n> @@ -1372,6 +1372,7 @@ static int do_create_stash(const struct pathspec *ps, struct strbuf *stash_msg_b\n>  \tconst char *head_short_sha1 = NULL;\n>  \tconst char *branch_ref = NULL;\n>  \tconst char *branch_name = \"(no branch)\";\n> +\tchar *branch_name_buf = NULL;\n>  \tstruct commit *head_commit = NULL;\n>  \tstruct commit_list *parents = NULL;\n>  \tstruct strbuf msg = STRBUF_INIT;\n> @@ -1401,11 +1402,20 @@ static int do_create_stash(const struct pathspec *ps, struct strbuf *stash_msg_b\n>  \t\tret = 1;\n>  \t\tgoto done;\n>  \t}\n> -\n> -\tbranch_ref = refs_resolve_ref_unsafe(get_main_ref_store(the_repository),\n> +\t\n> +\tbranch_ref = refs_resolve_ref_unsafe(get_main_ref_store(the_repository), \n\nThis part just adds whitespace at the end of the lines.  Please don't.\n\n>  \t\t\t\t\t     \"HEAD\", 0, NULL, &flags);\n> -\tif (flags & REF_ISSYMREF)\n> -\t\tskip_prefix(branch_ref, \"refs/heads/\", &branch_name);\n> +\n> +\tif (flags & REF_ISSYMREF) {\n> +\t\tconst char *tmp = NULL;\n> +\t\tif (skip_prefix(branch_ref, \"refs/heads/\", &tmp))\n> +\t\t\tbranch_name_buf = xstrdup(tmp); \n> +\t}\n> +\tif (branch_name_buf)\n> +\t\tbranch_name = branch_name_buf;\n> +\telse\n> +\t\tbranch_name = \"(no branch)\";\n> +\t\t\t\t\t    \n\nLooks complicated.\n\nThe if/else can be avoided by assigning both variables in the same\nblock, or in the same statement even.  You can see this pattern in the\ncode base by searching for \"to_free\".\n\nAnd I'm not sure the tmp variable is providing much benefit.\n\n\tif (flags & REF_ISSYMREF) {\n\t\tif (skip_prefix(branch_ref, \"refs/heads/\", &branch_name))\n\t\t\tbranch_name = branch_name_buf = xstrdup(branch_name);\n\t}\n\n>  \thead_short_sha1 = repo_find_unique_abbrev(the_repository,\n>  \t\t\t\t\t\t  &head_commit->object.oid,\n>  \t\t\t\t\t\t  DEFAULT_ABBREV);\n> @@ -1495,6 +1505,7 @@ static int do_create_stash(const struct pathspec *ps, struct strbuf *stash_msg_b\n>  \tstrbuf_release(&msg);\n>  \tstrbuf_release(&untracked_files);\n>  \tfree_commit_list(parents);\n> +\tfree(branch_name_buf);\n>  \treturn ret;\n>  }\n>  \n> diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh\n> index 74666ff3e4..5d5aac8b15 100755\n> --- a/t/t3903-stash.sh\n> +++ b/t/t3903-stash.sh\n> @@ -1592,4 +1592,43 @@ test_expect_success 'stash apply reports a locked index' '\n>  \t)\n>  '\n>  \n> +test_expect_success 'stash reflog message uses superproject branch, not submodule branch' '\n> +\tgit init main_project &&\n> +\t(\n> +\t\tcd main_project &&\n> +\t\techo \"Initial content in main_project\" > main_file.txt &&\n\nDocumentation/CodingGuidelines recommends: \"Redirection operators should\nbe written with space before, but no space after them.\", i.e., write\n'>main_file.txt' instead of '> main_file.txt'.\n\n> +\t\tgit add main_file.txt &&\n> +\t\tgit commit -q -m \"Initial commit in main_project\"\n> +\t) &&\n> +\n> +\tgit init sub_project &&\n> +\t(\n> +\t\tcd sub_project &&\n> +\t\techo \"Initial content in sub_project\" > sub_file.txt &&\n\nSame here.\n\n> +\t\tgit add sub_file.txt &&\n> +\t\tgit commit -q -m \"Initial commit in sub_project\"\n> +\t) &&> +\n> +\t(\n> +\t\tcd main_project &&\n> +\t\tgit -c protocol.file.allow=always submodule add --quiet ../sub_project sub &&\n> +\t\tgit commit -q -m \"Added submodule sub_project\" &&\n> +\n> +\t\tgit checkout -q -b feature_main &&\n> +\t\t(\n> +\t\t\tcd sub &&\n> +\t\t\tgit checkout -q -b feature_sub\n> +\t\t) &&\n> +\n> +\t\tgit checkout -q -b work_branch &&\n> +\n> +\t\techo \"Important work to be stashed\" > work_item.txt &&\n\nSame.\n\n> +\t\tgit add work_item.txt &&\n> +\t\tgit stash push -q -m \"custom stash for work_branch\" &&\n> +\n> +\t\tgit stash list > ../actual_stash_list.txt &&\n\nAnd here.\n\n> +\t\tgrep \"On work_branch: custom stash for work_branch\" ../actual_stash_list.txt\n> +\t)\n\nYou could avoid spawning one subshell by building sub_project first and\nthen doing all the work in main_project without interruption afterwards.\n\n> +'\n> +\n>  test_done\n\n"},{"id":"519935","messageId":"20250608144542.275836-1-jayatheerthkulkarni2005@gmail.com","threadId":"63449","inReplyTo":"f46443ac-eb7f-47db-8f4b-a06384e6fde5@web.de","subject":"[PATCH v2] stash: fix incorrect branch name in stash message","fromName":"K Jayatheerth","fromEmail":"jayatheerthkulkarni2005@gmail.com","sentAt":"2025-06-08T14:45:42Z","receivedAt":"2025-06-08T14:46:18Z","isPatch":true,"sender":{"key":"jayatheerthkulkarni2005@gmail.com","avatar":"https://avatars.githubusercontent.com/u/148841023?v=4"},"body":"When creating a stash, Git uses the current branch name\nof the superproject to construct the stash commit message.\nHowever, in repositories with submodules,\nthe message may mistakenly display the submodule branch name instead.\n\nThis is because `refs_resolve_ref_unsafe()` returns a pointer to a static buffer.\nSubsequent calls to the same function overwrite the buffer,\ncorrupting the originally fetched `branch_name` used for the stash message.\n\nUse `xstrdup()` to duplicate the branch name immediately after resolving it,\nso that later buffer overwrites do not affect the stash message.\n\nSigned-off-by: K Jayatheerth <jayatheerthkulkarni2005@gmail.com>\n---\n\nAll good points,\nRemoved multiple if else blocks and kept them \nwithin a single if/else block.\n\nRemoved the white space at the branch_ref = refs_r... line\n\nremoved the spaces in the test case.\nThank you for the feedback.\n\n builtin/stash.c  | 13 ++++++++++---\n t/t3903-stash.sh | 34 ++++++++++++++++++++++++++++++++++\n 2 files changed, 44 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/stash.c b/builtin/stash.c\nindex cfbd92852a..c71e20a8cd 100644\n--- a/builtin/stash.c\n+++ b/builtin/stash.c\n@@ -1372,6 +1372,7 @@ static int do_create_stash(const struct pathspec *ps, struct strbuf *stash_msg_b\n \tconst char *head_short_sha1 = NULL;\n \tconst char *branch_ref = NULL;\n \tconst char *branch_name = \"(no branch)\";\n+\tchar *branch_name_buf = NULL;\n \tstruct commit *head_commit = NULL;\n \tstruct commit_list *parents = NULL;\n \tstruct strbuf msg = STRBUF_INIT;\n@@ -1401,11 +1402,16 @@ static int do_create_stash(const struct pathspec *ps, struct strbuf *stash_msg_b\n \t\tret = 1;\n \t\tgoto done;\n \t}\n-\n+\t\n \tbranch_ref = refs_resolve_ref_unsafe(get_main_ref_store(the_repository),\n \t\t\t\t\t     \"HEAD\", 0, NULL, &flags);\n-\tif (flags & REF_ISSYMREF)\n-\t\tskip_prefix(branch_ref, \"refs/heads/\", &branch_name);\n+\n+\tif (flags & REF_ISSYMREF) {\n+\t\tif (skip_prefix(branch_ref, \"refs/heads/\", &branch_name))\n+\t\t\tbranch_name = branch_name_buf = xstrdup(branch_name);\n+\t} else\n+\t\tbranch_name = \"(no branch)\";\n+\t\t\t\t\t    \n \thead_short_sha1 = repo_find_unique_abbrev(the_repository,\n \t\t\t\t\t\t  &head_commit->object.oid,\n \t\t\t\t\t\t  DEFAULT_ABBREV);\n@@ -1495,6 +1501,7 @@ static int do_create_stash(const struct pathspec *ps, struct strbuf *stash_msg_b\n \tstrbuf_release(&msg);\n \tstrbuf_release(&untracked_files);\n \tfree_commit_list(parents);\n+\tfree(branch_name_buf);\n \treturn ret;\n }\n \ndiff --git a/t/t3903-stash.sh b/t/t3903-stash.sh\nindex 74666ff3e4..b606827a73 100755\n--- a/t/t3903-stash.sh\n+++ b/t/t3903-stash.sh\n@@ -1592,4 +1592,38 @@ test_expect_success 'stash apply reports a locked index' '\n \t)\n '\n \n+test_expect_success 'stash reflog message uses superproject branch, not submodule branch' '\n+\tgit init sub_project &&\n+\t(\n+\t\tcd sub_project &&\n+\t\techo \"Initial content in sub_project\" >sub_file.txt &&\n+\t\tgit add sub_file.txt &&\n+\t\tgit commit -q -m \"Initial commit in sub_project\"\n+\t) &&\n+\n+\tgit init main_project &&\n+\t(\n+\t\tcd main_project &&\n+\t\techo \"Initial content in main_project\" >main_file.txt &&\n+\t\tgit add main_file.txt &&\n+\t\tgit commit -q -m \"Initial commit in main_project\" &&\n+\n+\t\tgit -c protocol.file.allow=always submodule add --quiet ../sub_project sub &&\n+\t\tgit commit -q -m \"Added submodule sub_project\" &&\n+\n+\t\tgit checkout -q -b feature_main &&\n+\t\tcd sub &&\n+\t\tgit checkout -q -b feature_sub &&\n+\t\tcd .. &&\n+\n+\t\tgit checkout -q -b work_branch &&\n+\t\techo \"Important work to be stashed\" >work_item.txt &&\n+\t\tgit add work_item.txt &&\n+\t\tgit stash push -q -m \"custom stash for work_branch\" &&\n+\n+\t\tgit stash list >../actual_stash_list.txt &&\n+\t\tgrep \"On work_branch: custom stash for work_branch\" ../actual_stash_list.txt\n+\t)\n+'\n+\n test_done\n-- \n2.49.GIT\n\n"},{"id":"519937","messageId":"xmqqo6uyw6h4.fsf@gitster.g","threadId":"63449","inReplyTo":"20250608144542.275836-1-jayatheerthkulkarni2005@gmail.com","subject":"Re: [PATCH v2] stash: fix incorrect branch name in stash message","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-06-08T16:20:07Z","receivedAt":"2025-06-08T16:20:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"K Jayatheerth <jayatheerthkulkarni2005@gmail.com> writes:\n\n> When creating a stash, Git uses the current branch name\n> of the superproject to construct the stash commit message.\n> However, in repositories with submodules,\n> the message may mistakenly display the submodule branch name instead.\n>\n> This is because `refs_resolve_ref_unsafe()` returns a pointer to a static buffer.\n> Subsequent calls to the same function overwrite the buffer,\n> corrupting the originally fetched `branch_name` used for the stash message.\n>\n> Use `xstrdup()` to duplicate the branch name immediately after resolving it,\n> so that later buffer overwrites do not affect the stash message.\n>\n> Signed-off-by: K Jayatheerth <jayatheerthkulkarni2005@gmail.com>\n> ---\n\nNicely described.\n\n> @@ -1372,6 +1372,7 @@ static int do_create_stash(const struct pathspec *ps, struct strbuf *stash_msg_b\n>  \tconst char *head_short_sha1 = NULL;\n>  \tconst char *branch_ref = NULL;\n>  \tconst char *branch_name = \"(no branch)\";\n> +\tchar *branch_name_buf = NULL;\n>  \tstruct commit *head_commit = NULL;\n>  \tstruct commit_list *parents = NULL;\n>  \tstruct strbuf msg = STRBUF_INIT;\n> @@ -1401,11 +1402,16 @@ static int do_create_stash(const struct pathspec *ps, struct strbuf *stash_msg_b\n>  \t\tret = 1;\n>  \t\tgoto done;\n>  \t}\n> -\n> +\t\n\nAddition of trailing whitespace?\n\nYou can avoid such mistakes in the future by enabling our sample\npre-commit hook, which essentially does\n\n\tgit diff-index --check --cached $against --\n\nwhere $against is HEAD (or an empty tree object while preparing for\nan initial commit).\n\n>  \tbranch_ref = refs_resolve_ref_unsafe(get_main_ref_store(the_repository),\n>  \t\t\t\t\t     \"HEAD\", 0, NULL, &flags);\n> -\tif (flags & REF_ISSYMREF)\n> -\t\tskip_prefix(branch_ref, \"refs/heads/\", &branch_name);\n> +\n> +\tif (flags & REF_ISSYMREF) {\n> +\t\tif (skip_prefix(branch_ref, \"refs/heads/\", &branch_name))\n> +\t\t\tbranch_name = branch_name_buf = xstrdup(branch_name);\n> +\t} else\n> +\t\tbranch_name = \"(no branch)\";\n\nDo we need the else clause?  The original did not have it and showed\nthe \"(no branch)\" message without an issue, and I do not see anything\nis changed by what happens inside the other side of this if statement.\nAm I missing something?\n\n> @@ -1495,6 +1501,7 @@ static int do_create_stash(const struct pathspec *ps, struct strbuf *stash_msg_b\n>  \tstrbuf_release(&msg);\n>  \tstrbuf_release(&untracked_files);\n>  \tfree_commit_list(parents);\n> +\tfree(branch_name_buf);\n>  \treturn ret;\n>  }\n\nMakes sense.\n\nThis is a common pattern we use with a variable whose name contains\n\"to_free\" (e.g., \"branch_name_to_free\"), but \"branch_name_buf\" is\npleanty readable and easy to understand what is going on.\n\n> +test_expect_success 'stash reflog message uses superproject branch, not submodule branch' '\n\nThe title looks a bit on the overly-long side.  Would\n\n    stash message records the superproject branch\n\nbe sufficient?  The fact that the stash is implemented as reflog\nis invidible and irrelevant at this level, so \"reflog message\" is\nwasting bytes without adding any useful information.\n\nWhat we want to make sure is that the message records the current\nbranch name, whether the project has any submodules or not, and from\nthat point of view,\n\n    stash message records the correct branch name\n\nought to be good, but not quite, because this test is trying to\ntrigger a bug that was present only when there are submodules, so\nnot mentioning superproject/submodule at all would not work well.\n\nWould\n\n    submodules does not affect the branch recorded in stash message\n\nwork?  That is the best one I can come up with offhand.\n\n> +\tgit init sub_project &&\n> +\t(\n> +\t\tcd sub_project &&\n> +\t\techo \"Initial content in sub_project\" >sub_file.txt &&\n> +\t\tgit add sub_file.txt &&\n> +\t\tgit commit -q -m \"Initial commit in sub_project\"\n> +\t) &&\n\nIt is easier to debug the test script if you avoid using --quiet too\nmuch.  Regular \"sh ./t3903-stash.sh\" will squelch these output\nanyway, and they can be seen when the test script is run with \"-v\".\n\n> +\tgit init main_project &&\n> +\t(\n> +\t\tcd main_project &&\n> +\t\techo \"Initial content in main_project\" >main_file.txt &&\n> +\t\tgit add main_file.txt &&\n> +\t\tgit commit -q -m \"Initial commit in main_project\" &&\n> +\n> +\t\tgit -c protocol.file.allow=always submodule add --quiet ../sub_project sub &&\n> +\t\tgit commit -q -m \"Added submodule sub_project\" &&\n> +\n> +\t\tgit checkout -q -b feature_main &&\n\n\n> +\t\tcd sub &&\n> +\t\tgit checkout -q -b feature_sub &&\n> +\t\tcd .. &&\n\nThese three lines can be written more compactly as:\n\n\t\tgit -C sub checkout -b feature_sub &&\n\n> +\t\tgit checkout -q -b work_branch &&\n> +\t\techo \"Important work to be stashed\" >work_item.txt &&\n> +\t\tgit add work_item.txt &&\n> +\t\tgit stash push -q -m \"custom stash for work_branch\" &&\n> +\n> +\t\tgit stash list >../actual_stash_list.txt &&\n> +\t\tgrep \"On work_branch: custom stash for work_branch\" ../actual_stash_list.txt\n> +\t)\n> +'\n> +\n>  test_done\n\nThanks.\n"},{"id":"520072","messageId":"CA+rGoLdLOYGt8tsWtqWoBCct0Q5HkE_dY1X+MaSoyyfo6mCqFw@mail.gmail.com","threadId":"63449","inReplyTo":"xmqqo6uyw6h4.fsf@gitster.g","subject":"Re: [PATCH v2] stash: fix incorrect branch name in stash message","fromName":"JAYATHEERTH K","fromEmail":"jayatheerthkulkarni2005@gmail.com","sentAt":"2025-06-11T01:32:57Z","receivedAt":"2025-06-11T01:33:10Z","isPatch":true,"sender":{"key":"jayatheerthkulkarni2005@gmail.com","avatar":"https://avatars.githubusercontent.com/u/148841023?v=4"},"body":"> > +\n>\n> Addition of trailing whitespace?\n>\n> You can avoid such mistakes in the future by enabling our sample\n> pre-commit hook, which essentially does\n>\n>         git diff-index --check --cached $against --\n>\n\nThank you, I needed something like this, I think I'm going to steal\nthis to other projects too : )\n\n> where $against is HEAD (or an empty tree object while preparing for\n> an initial commit).\n>\n> >       branch_ref = refs_resolve_ref_unsafe(get_main_ref_store(the_repository),\n> >                                            \"HEAD\", 0, NULL, &flags);\n> > -     if (flags & REF_ISSYMREF)\n> > -             skip_prefix(branch_ref, \"refs/heads/\", &branch_name);\n> > +\n> > +     if (flags & REF_ISSYMREF) {\n> > +             if (skip_prefix(branch_ref, \"refs/heads/\", &branch_name))\n> > +                     branch_name = branch_name_buf = xstrdup(branch_name);\n> > +     } else\n> > +             branch_name = \"(no branch)\";\n>\n> Do we need the else clause?  The original did not have it and showed\n> the \"(no branch)\" message without an issue, and I do not see anything\n> is changed by what happens inside the other side of this if statement.\n> Am I missing something?\n\nNo, that's just a random idea, I get the point I will remove that in\nthe new patch\n\n>\n> > @@ -1495,6 +1501,7 @@ static int do_create_stash(const struct pathspec *ps, struct strbuf *stash_msg_b\n> >       strbuf_release(&msg);\n> >       strbuf_release(&untracked_files);\n> >       free_commit_list(parents);\n> > +     free(branch_name_buf);\n> >       return ret;\n> >  }\n>\n> Makes sense.\n>\n> This is a common pattern we use with a variable whose name contains\n> \"to_free\" (e.g., \"branch_name_to_free\"), but \"branch_name_buf\" is\n> pleanty readable and easy to understand what is going on.\n>\n> > +test_expect_success 'stash reflog message uses superproject branch, not submodule branch' '\n>\n> The title looks a bit on the overly-long side.  Would\n>\n>     stash message records the superproject branch\n>\n> be sufficient?  The fact that the stash is implemented as reflog\n> is invidible and irrelevant at this level, so \"reflog message\" is\n> wasting bytes without adding any useful information.\n>\n> What we want to make sure is that the message records the current\n> branch name, whether the project has any submodules or not, and from\n> that point of view,\n>\n>     stash message records the correct branch name\n>\n> ought to be good, but not quite, because this test is trying to\n> trigger a bug that was present only when there are submodules, so\n> not mentioning superproject/submodule at all would not work well.\n>\n> Would\n>\n>     submodules does not affect the branch recorded in stash message\n>\n> work?  That is the best one I can come up with offhand.\n>\n\nWorks, I will use this as is, I think this sounds good.\n\n> > +     git init sub_project &&\n> > +     (\n> > +             cd sub_project &&\n> > +             echo \"Initial content in sub_project\" >sub_file.txt &&\n> > +             git add sub_file.txt &&\n> > +             git commit -q -m \"Initial commit in sub_project\"\n> > +     ) &&\n>\n> It is easier to debug the test script if you avoid using --quiet too\n> much.  Regular \"sh ./t3903-stash.sh\" will squelch these output\n> anyway, and they can be seen when the test script is run with \"-v\".\n>\n\nOk will remove all the -q tests\n\n> > +     git init main_project &&\n> > +     (\n> > +             cd main_project &&\n> > +             echo \"Initial content in main_project\" >main_file.txt &&\n> > +             git add main_file.txt &&\n> > +             git commit -q -m \"Initial commit in main_project\" &&\n> > +\n> > +             git -c protocol.file.allow=always submodule add --quiet ../sub_project sub &&\n> > +             git commit -q -m \"Added submodule sub_project\" &&\n> > +\n> > +             git checkout -q -b feature_main &&\n>\n>\n> > +             cd sub &&\n> > +             git checkout -q -b feature_sub &&\n> > +             cd .. &&\n>\n> These three lines can be written more compactly as:\n>\n>                 git -C sub checkout -b feature_sub &&\n>\n\nTrue.\n\n> > +             git checkout -q -b work_branch &&\n> > +             echo \"Important work to be stashed\" >work_item.txt &&\n> > +             git add work_item.txt &&\n> > +             git stash push -q -m \"custom stash for work_branch\" &&\n> > +\n> > +             git stash list >../actual_stash_list.txt &&\n> > +             grep \"On work_branch: custom stash for work_branch\" ../actual_stash_list.txt\n> > +     )\n> > +'\n> > +\n> >  test_done\n>\n> Thanks.\n\nWill send a patch soon\nThank you again\n\n- Jayatheerth\n"},{"id":"520073","messageId":"20250611014204.24994-1-jayatheerthkulkarni2005@gmail.com","threadId":"63449","inReplyTo":"xmqqo6uyw6h4.fsf@gitster.g","subject":"[PATCH v3] stash: fix incorrect branch name in stash message","fromName":"K Jayatheerth","fromEmail":"jayatheerthkulkarni2005@gmail.com","sentAt":"2025-06-11T01:42:03Z","receivedAt":"2025-06-11T01:42:16Z","isPatch":true,"sender":{"key":"jayatheerthkulkarni2005@gmail.com","avatar":"https://avatars.githubusercontent.com/u/148841023?v=4"},"body":"When creating a stash, Git uses the current branch name\nof the superproject to construct the stash commit message.\nHowever, in repositories with submodules,\nthe message may mistakenly display the submodule branch name instead.\n\nThis is because `refs_resolve_ref_unsafe()` returns a pointer to a static buffer.\nSubsequent calls to the same function overwrite the buffer,\ncorrupting the originally fetched `branch_name` used for the stash message.\n\nUse `xstrdup()` to duplicate the branch name immediately after resolving it,\nso that later buffer overwrites do not affect the stash message.\n\nSigned-off-by: K Jayatheerth <jayatheerthkulkarni2005@gmail.com>\n---\n1. Used the hook to remove trailing whitespaces\n2. Removed the else clause \n3. Changed the test title to the suggested one \n4. Avoided --quiet Flags in Tests\n5. Used git -C sub ... instead of cd sub && ... && cd ...\n\n builtin/stash.c  | 12 +++++++++---\n t/t3903-stash.sh | 32 ++++++++++++++++++++++++++++++++\n 2 files changed, 41 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/stash.c b/builtin/stash.c\nindex cfbd92852a..4215ee781c 100644\n--- a/builtin/stash.c\n+++ b/builtin/stash.c\n@@ -1372,6 +1372,7 @@ static int do_create_stash(const struct pathspec *ps, struct strbuf *stash_msg_b\n \tconst char *head_short_sha1 = NULL;\n \tconst char *branch_ref = NULL;\n \tconst char *branch_name = \"(no branch)\";\n+\tchar *branch_name_buf = NULL;\n \tstruct commit *head_commit = NULL;\n \tstruct commit_list *parents = NULL;\n \tstruct strbuf msg = STRBUF_INIT;\n@@ -1401,11 +1402,15 @@ static int do_create_stash(const struct pathspec *ps, struct strbuf *stash_msg_b\n \t\tret = 1;\n \t\tgoto done;\n \t}\n-\n+\t\n \tbranch_ref = refs_resolve_ref_unsafe(get_main_ref_store(the_repository),\n \t\t\t\t\t     \"HEAD\", 0, NULL, &flags);\n-\tif (flags & REF_ISSYMREF)\n-\t\tskip_prefix(branch_ref, \"refs/heads/\", &branch_name);\n+\n+\tif (flags & REF_ISSYMREF) {\n+\t\tif (skip_prefix(branch_ref, \"refs/heads/\", &branch_name))\n+\t\t\tbranch_name = branch_name_buf = xstrdup(branch_name);\n+\t}\n+\t\t\t\t\t    \n \thead_short_sha1 = repo_find_unique_abbrev(the_repository,\n \t\t\t\t\t\t  &head_commit->object.oid,\n \t\t\t\t\t\t  DEFAULT_ABBREV);\n@@ -1495,6 +1500,7 @@ static int do_create_stash(const struct pathspec *ps, struct strbuf *stash_msg_b\n \tstrbuf_release(&msg);\n \tstrbuf_release(&untracked_files);\n \tfree_commit_list(parents);\n+\tfree(branch_name_buf);\n \treturn ret;\n }\n \ndiff --git a/t/t3903-stash.sh b/t/t3903-stash.sh\nindex 74666ff3e4..5678223cfb 100755\n--- a/t/t3903-stash.sh\n+++ b/t/t3903-stash.sh\n@@ -1592,4 +1592,36 @@ test_expect_success 'stash apply reports a locked index' '\n \t)\n '\n \n+test_expect_success 'submodules does not affect the branch recorded in stash message' '\n+\tgit init sub_project &&\n+\t(\n+\t\tcd sub_project &&\n+\t\techo \"Initial content in sub_project\" >sub_file.txt &&\n+\t\tgit add sub_file.txt &&\n+\t\tgit commit -m \"Initial commit in sub_project\"\n+\t) &&\n+\n+\tgit init main_project &&\n+\t(\n+\t\tcd main_project &&\n+\t\techo \"Initial content in main_project\" >main_file.txt &&\n+\t\tgit add main_file.txt &&\n+\t\tgit commit -m \"Initial commit in main_project\" &&\n+\n+\t\tgit -c protocol.file.allow=always submodule add ../sub_project sub &&\n+\t\tgit commit -m \"Added submodule sub_project\" &&\n+\n+\t\tgit checkout -b feature_main &&\n+\t\tgit -C sub checkout -b feature_sub &&\n+\n+\t\tgit checkout -b work_branch &&\n+\t\techo \"Important work to be stashed\" >work_item.txt &&\n+\t\tgit add work_item.txt &&\n+\t\tgit stash push -m \"custom stash for work_branch\" &&\n+\n+\t\tgit stash list >../actual_stash_list.txt &&\n+\t\tgrep \"On work_branch: custom stash for work_branch\" ../actual_stash_list.txt\n+\t)\n+'\n+\n test_done\n-- \n2.49.GIT\n\n"},{"id":"520113","messageId":"xmqqqzzqi7yy.fsf@gitster.g","threadId":"63449","inReplyTo":"20250611014204.24994-1-jayatheerthkulkarni2005@gmail.com","subject":"Re: [PATCH v3] stash: fix incorrect branch name in stash message","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-06-11T16:00:37Z","receivedAt":"2025-06-11T16:00:40Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"K Jayatheerth <jayatheerthkulkarni2005@gmail.com> writes:\n\n> When creating a stash, Git uses the current branch name\n> of the superproject to construct the stash commit message.\n> However, in repositories with submodules,\n> the message may mistakenly display the submodule branch name instead.\n>\n> This is because `refs_resolve_ref_unsafe()` returns a pointer to a static buffer.\n> Subsequent calls to the same function overwrite the buffer,\n> corrupting the originally fetched `branch_name` used for the stash message.\n>\n> Use `xstrdup()` to duplicate the branch name immediately after resolving it,\n> so that later buffer overwrites do not affect the stash message.\n>\n> Signed-off-by: K Jayatheerth <jayatheerthkulkarni2005@gmail.com>\n> ---\n> 1. Used the hook to remove trailing whitespaces\n\nHuh?  The pre-commit hook trick I showed was to detect and prevent\nyou from creating such a commit; it does not remove them for you.\nYour trailing whitespaces still remain in the patch just fine ;-)\n\n> @@ -1401,11 +1402,15 @@ static int do_create_stash(const struct pathspec *ps, struct strbuf *stash_msg_b\n>  \t\tret = 1;\n>  \t\tgoto done;\n>  \t}\n> -\n> +\t\n>  \tbranch_ref = refs_resolve_ref_unsafe(get_main_ref_store(the_repository),\n\nwhich causes\n\n    .git/rebase-apply/patch:29: trailing whitespace.\n\n    .git/rebase-apply/patch:39: trailing whitespace.\n\n    warning: 2 lines applied after fixing whitespace errors.\n    Applying: stash: fix incorrect branch name in stash message\n\nOther than that, looing good.\n\nThanks, will queue.\n"}]}