Git development
 help / color / mirror / Atom feed
From: Phillip Wood <phillip.wood123@gmail.com>
To: 重田一聖 <kazumasa.shigeta@kanamei.com>, phillip.wood@dunelm.org.uk
Cc: git@vger.kernel.org, shabbir.r.bhojani@gmail.com
Subject: Re: [PATCH] stash: expose untracked modes in create
Date: Mon, 5 Oct 2026 17:37:53 +0100	[thread overview]
Message-ID: <632af360-5797-4794-82b7-02c7dd8f7bd4@gmail.com> (raw)
In-Reply-To: <CANUHOw201hr2LgHb1ThcadiH8Y5k3zUArnhtj8g8SRVrvMsN-g@mail.gmail.com>

Hi Kazumasa

On 01/10/2026 18:44, 重田一聖 wrote:
> Hi Phillip,
> 
> Thanks for the review.
> 
> Sorry, I got a little carried away and sent v2 before replying.
> 
>> This doesn't seem to match the code changes.
> 
> For the no-change case, plain "git stash create" already checks for
> tracked changes before calling do_create_stash(), and returns 0 with
> empty output when there is nothing to create.
> 
> For -u and -a, I think we should follow that existing "create" behavior
> as well, using check_changes() for the selected mode before calling
> do_create_stash(), and returning 0 when it finds nothing to create.

I'm not really sure why that call to check_changes_tracked_files() is 
there in create_stash() as do_create_stash() repeats the same check.
I've sent a couple of patches [1] to fix that.

 > I am also thinking of mapping do_create_stash()'s internal no-change
 > status of 1 to 0 in the unlikely case where the state changes between
 > these checks.

I think that is worth doing but it is not related to adding new options 
so should be a separate patch.
> That 1 is not STASH_APPLY_CONFLICT. Following 786fc390465f
> ("stash: reserve exit status 1 for conflicts"), I do not think it should
> escape as public exit status 1, and would map it to 0 instead.

I agree we should exit 0 in that case.
>> You should pass PARSE_OPT_STOP_AT_NON_OPTION to parse_options()
>> to prevent that.
> 
> I plan to use PARSE_OPT_STOP_AT_NON_OPTION as you suggested.

That's great

Thanks

Phillip

[1] 
https://lore.kernel.org/git/cover.1791218125.git.phillip.wood@dunelm.org.uk

> Thanks,
> 
> Kazumasa Shigeta
> 
> 
> On Tue, 29 Sep 2026 17:08:08 +0100, Phillip Wood
> <phillip.wood123@gmail.com> wrote:
>> Hi Kazumasa
>>
>> On 29/09/2026 08:42, Kazumasa Shigeta wrote:
>>> `git stash create` always passes zero for the include_untracked parameter
>>> of do_create_stash(), even though that helper already supports untracked
>>> and ignored files and stash push/save expose those modes as
>>> -u/--include-untracked and -a/--all.
>>>
>>> Teach create to accept the same options and pass the existing mode
>>> through. Unlike push/save, create continues to only create objects: it
>>> does not update refs/stash or modify the index or working tree.
>>>
>>> When the selected mode finds no changes, do_create_stash() returns 1.
>>> Translate that to success so create keeps its existing no-object, empty
>>> output behavior.
>>
>> This doesn't seem to match the code changes. The code that prints the
>> object id when the stash is successfully created is unchanged, as far as
>> I can see what this patch does is change the exit status for "git stash
>> create" when there are no changes to stash. Instead of exiting 1, it
>> exits 0 even though it does not create a stash. That does not seem like
>> a good idea.
>>
>>> Use normal parse-options semantics, so options may appear after message
>>> arguments. A message that begins with a dash can be disambiguated with
>>
>> As "git stash create" concatenates excess arguments to use as the stash
>> message we should not be permuting options. "git stash create handle new
>> -u flag" should continue to create a stash with the message "handle new
>> -u flag" - it should not start stashing untracked files. You should pass
>> PARSE_OPT_STOP_AT_NON_OPTION to parse_options() to prevent that.
>>
>>> I proposed adding both --include-untracked and --all to
>>> "git stash create" in 2014:
>>> <1403856479-37421-1-git-send-email-shigeta@kanamei.co.jp>
>>>
>>> I should also apologize for dropping that thread after receiving review.
>>> I did not follow up on the comments at the time. Thanks to those who
>>> reviewed it then.
>>
>> Better late than never! I think the idea is fine, but the implementation
>> could do with a couple of tweaks so it is as backward compatible as
>> possible.
>>
>> Thanks
>>
>> Phillip
>>
>>> Separately, in 2017, Thomas Gummerer added an internal -u path while
>>> refactoring stash_create in 9ca6326dff29 (stash: refactor stash_create).
>>> That change explicitly kept the user interface of "git stash create"
>>> unchanged.
>>>
>>> When "stash create" was later converted to the builtin C implementation
>>> in d4788af875cc (stash: convert create to builtin), the untracked-file
>>> handling was carried into the new implementation and remains there today.
>>>
>>> More recently, Shabbir Bhojani proposed exposing --include-untracked:
>>> <pull.1892.git.1774768580147.gitgitgadget@gmail.com>
>>>
>>> This patch exposes both existing untracked modes, --include-untracked and
>>> --all, to "git stash create".
>>>
>>> Documentation/git-stash.adoc | 18 ++++++----
>>> builtin/stash.c | 36 ++++++++++++++-----
>>> t/t3903-stash.sh | 70 ++++++++++++++++++++++++++++++++++++
>>> 3 files changed, 109 insertions(+), 15 deletions(-)
>>>
>>> diff --git a/Documentation/git-stash.adoc b/Documentation/git-stash.adoc
>>> index fc6a9a0..32f0fd5 100644
>>> --- a/Documentation/git-stash.adoc
>>> +++ b/Documentation/git-stash.adoc
>>> @@ -21,7 +21,7 @@ git stash [push] [-p | --patch] [-S | --staged] [-k | --[no-]keep-index] [-q | -
>>> git stash save [-p | --patch] [-S | --staged] [-k | --[no-]keep-index] [-q | --quiet]
>>> [-u | --include-untracked] [-a | --all] [<message>]
>>> git stash clear
>>> -git stash create [<message>]
>>> +git stash create [-u | --include-untracked] [-a | --all] [<message>]
>>> git stash store [(-m | --message) <message>] [-q | --quiet] <commit>
>>> git stash export (--print | --to-ref <ref>) [<stash>...]
>>> git stash import <commit>
>>> @@ -138,10 +138,12 @@ with no conflicts.
>>> `drop [-q | --quiet] [<stash>]`::
>>> Remove a single stash entry from the list of stash entries.
>>>
>>> -`create`::
>>> +`create [-u | --include-untracked] [-a | --all]`::
>>> Create a stash entry (which is a regular commit object) and
>>> return its object name, without storing it anywhere in the ref
>>> - namespace.
>>> + namespace. The `--include-untracked` option includes untracked
>>> + files, while `--all` also includes ignored files, without modifying
>>> + the working tree.
>>> This is intended to be useful for scripts. It is probably not
>>> the command you want to use; see "push" above.
>>>
>>> @@ -167,10 +169,11 @@ OPTIONS
>>> -------
>>> `-a`::
>>> `--all`::
>>> - This option is only valid for `push` and `save` commands.
>>> + When used with the `push` and `save` commands, all ignored and
>>> + untracked files are also stashed and then cleaned up with `git clean`.
>>> +
>>> -All ignored and untracked files are also stashed and then cleaned
>>> -up with `git clean`.
>>> +When used with the `create` command, ignored and untracked files are included
>>> +in the stash entry without modifying the working tree.
>>>
>>> `-u`::
>>> `--include-untracked`::
>>> @@ -179,6 +182,9 @@ up with `git clean`.
>>> all untracked files are also stashed and then cleaned up with
>>> `git clean`.
>>> +
>>> +When used with the `create` command, untracked files are included in the
>>> +stash entry without modifying the working tree.
>>> ++
>>> When used with the `show` command, show the untracked files in the stash
>>> entry as part of the diff.
>>>
>>> diff --git a/builtin/stash.c b/builtin/stash.c
>>> index 7a98434..57a4750 100644
>>> --- a/builtin/stash.c
>>> +++ b/builtin/stash.c
>>> @@ -59,7 +59,7 @@
>>> N_("git stash save [-p | --patch] [-S | --staged] [-k | --[no-]keep-index] [-q | --quiet]\n" \
>>> " [-u | --include-untracked] [-a | --all] [<message>]")
>>> #define BUILTIN_STASH_CREATE_USAGE \
>>> - N_("git stash create [<message>]")
>>> + N_("git stash create [-u | --include-untracked] [-a | --all] [<message>]")
>>> #define BUILTIN_STASH_EXPORT_USAGE \
>>> N_("git stash export (--print | --to-ref <ref>) [<stash>...]")
>>> #define BUILTIN_STASH_IMPORT_USAGE \
>>> @@ -119,6 +119,11 @@ static const char * const git_stash_clear_usage[] = {
>>> NULL
>>> };
>>>
>>> +static const char * const git_stash_create_usage[] = {
>>> + BUILTIN_STASH_CREATE_USAGE,
>>> + NULL
>>> +};
>>> +
>>> static const char * const git_stash_store_usage[] = {
>>> BUILTIN_STASH_STORE_USAGE,
>>> NULL
>>> @@ -1643,26 +1648,39 @@ static int do_create_stash(const struct pathspec *ps, struct strbuf *stash_msg_b
>>> return ret;
>>> }
>>>
>>> -static int create_stash(int argc, const char **argv, const char *prefix UNUSED,
>>> +static int create_stash(int argc, const char **argv, const char *prefix,
>>> struct repository *repo UNUSED)
>>> {
>>> - int ret;
>>> + int ret = 0;
>>> + int include_untracked = 0;
>>> + struct option options[] = {
>>> + OPT_BOOL('u', "include-untracked", &include_untracked,
>>> + N_("include untracked files in stash")),
>>> + OPT_SET_INT('a', "all", &include_untracked,
>>> + N_("include ignored files in stash"),
>>> + INCLUDE_ALL_FILES),
>>> + OPT_END()
>>> + };
>>> struct strbuf stash_msg_buf = STRBUF_INIT;
>>> struct stash_info info = STASH_INFO_INIT;
>>> struct pathspec ps;
>>>
>>> - /* Starting with argv[1], since argv[0] is "create" */
>>> - strbuf_join_argv(&stash_msg_buf, argc - 1, ++argv, ' ');
>>> + argc = parse_options(argc, argv, prefix, options,
>>> + git_stash_create_usage, 0);
>>> + strbuf_join_argv(&stash_msg_buf, argc, argv, ' ');
>>>
>>> memset(&ps, 0, sizeof(ps));
>>> - if (!check_changes_tracked_files(&ps))
>>> - return 0;
>>> + if (!include_untracked && !check_changes_tracked_files(&ps))
>>> + goto done;
>>>
>>> - ret = do_create_stash(&ps, &stash_msg_buf, 0, 0, NULL, 0, &info,
>>> - NULL, 0);
>>> + ret = do_create_stash(&ps, &stash_msg_buf, include_untracked, 0, NULL,
>>> + 0, &info, NULL, 0);
>>> if (!ret)
>>> printf_ln("%s", oid_to_hex(&info.w_commit));
>>> + else if (ret == 1)
>>> + ret = 0;
>>>
>>> +done:
>>> free_stash_info(&info);
>>> strbuf_release(&stash_msg_buf);
>>> return ret;
>>> diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
>>> index 7211586..fe34879 100755
>>> --- a/t/t3903-stash.sh
>>> +++ b/t/t3903-stash.sh
>>> @@ -640,6 +640,76 @@ test_expect_success 'stash create - no changes' '
>>> test_must_be_empty actual
>>> '
>>>
>>> +# --all observes every untracked and ignored path in the worktree. Use one
>>> +# isolated repository for these checks so unrelated test state is not captured.
>>> +test_expect_success 'stash create with untracked options' '
>>> + test_when_finished "rm -rf stash-create-options" &&
>>> + test_create_repo stash-create-options &&
>>> + (
>>> + cd stash-create-options &&
>>> + test_commit base tracked base &&
>>> + echo create-ignored >.gitignore &&
>>> + git add .gitignore &&
>>> + git commit -m ignore &&
>>> +
>>> + git stash create -u >.git/actual &&
>>> + test_must_be_empty .git/actual &&
>>> + git stash create -a >.git/actual &&
>>> + test_must_be_empty .git/actual &&
>>> +
>>> + echo untracked >create-untracked &&
>>> + git stash create "without untracked" >.git/actual &&
>>> + test_must_be_empty .git/actual &&
>>> + short=$(git stash create "create untracked" -u) &&
>>> + long=$(git stash create --include-untracked "create untracked") &&
>>> + test_cmp_rev "$short^3^{tree}" "$long^3^{tree}" &&
>>> + echo untracked >.git/expect &&
>>> + git show "$short^3:create-untracked" >.git/actual &&
>>> + test_cmp .git/expect .git/actual &&
>>> + branch=$(git symbolic-ref --short HEAD) &&
>>> + echo "On $branch: create untracked" >.git/expect &&
>>> + git show --pretty=%s -s "$short" >.git/actual &&
>>> + test_cmp .git/expect .git/actual &&
>>> + test_path_is_file create-untracked &&
>>> +
>>> + echo ignored >create-ignored &&
>>> + with_untracked=$(git stash create -u "create options") &&
>>> + test_must_fail git cat-file -e "$with_untracked^3:create-ignored" &&
>>> + short=$(git stash create "create options" -a) &&
>>> + long=$(git stash create --all "create options") &&
>>> + test_cmp_rev "$short^3^{tree}" "$long^3^{tree}" &&
>>> + echo ignored >.git/expect &&
>>> + git show "$short^3:create-ignored" >.git/actual &&
>>> + test_cmp .git/expect .git/actual &&
>>> + test_path_is_file create-untracked &&
>>> + test_path_is_file create-ignored &&
>>> +
>>> + echo staged >staged &&
>>> + git add staged &&
>>> + echo modified >>tracked &&
>>> + git diff >.git/before-worktree &&
>>> + git diff --cached >.git/before-index &&
>>> + git status --porcelain=v1 --ignored >.git/before-status &&
>>> + test_must_fail git rev-parse --verify refs/stash >/dev/null 2>&1 &&
>>> + STASH_ID=$(git stash create -a -- -create-message) &&
>>> + git diff >.git/after-worktree &&
>>> + git diff --cached >.git/after-index &&
>>> + git status --porcelain=v1 --ignored >.git/after-status &&
>>> + test_cmp .git/before-worktree .git/after-worktree &&
>>> + test_cmp .git/before-index .git/after-index &&
>>> + test_cmp .git/before-status .git/after-status &&
>>> + test_must_fail git rev-parse --verify refs/stash >/dev/null 2>&1 &&
>>> + echo "On $branch: -create-message" >.git/expect &&
>>> + git show --pretty=%s -s "$STASH_ID" >.git/actual &&
>>> + test_cmp .git/expect .git/actual
>>> + )
>>> +'
>>> +
>>> +test_expect_success 'stash create rejects unknown options' '
>>> + test_expect_code 129 git stash create --unknown-option 2>err &&
>>> + test_grep "unknown option" err
>>> +'
>>> +
>>> test_expect_success 'stash branch - no stashes on stack, stash-like argument' '
>>> git stash clear &&
>>> test_when_finished "git reset --hard HEAD" &&


  reply	other threads:[~2026-10-05 16:37 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-29  7:42 [PATCH] stash: expose untracked modes in create Kazumasa Shigeta
2026-09-29 16:08 ` Phillip Wood
2026-10-01 17:44   ` 重田一聖
2026-10-05 16:37     ` Phillip Wood [this message]
2026-10-01  4:21 ` [PATCH v2] " Kazumasa Shigeta
2026-10-01 11:32   ` Patrick Steinhardt
2026-10-01 16:01     ` 重田一聖
2026-10-01 16:36     ` Junio C Hamano
2026-10-01 17:03   ` Junio C Hamano
2026-10-02  1:53     ` 重田一聖
2026-10-02  9:04     ` 重田一聖
2026-10-05  5:55       ` 重田一聖
2026-10-05 16:38         ` Phillip Wood
2026-10-06  9:25           ` 重田一聖
2026-10-06  9:57             ` Phillip Wood
2026-10-08  3:43               ` 重田一聖
2026-10-05 16:43         ` Junio C Hamano
2026-10-08 13:58           ` 重田一聖
2026-10-09 19:05             ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=632af360-5797-4794-82b7-02c7dd8f7bd4@gmail.com \
    --to=phillip.wood123@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=kazumasa.shigeta@kanamei.com \
    --cc=phillip.wood@dunelm.org.uk \
    --cc=shabbir.r.bhojani@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox