{"thread":{"id":"66088","subject":"[PATCH 0/2] git stash drop stash@{2.days.ago}","startedAt":"2026-07-30T03:41:10Z","lastAt":"2026-07-30T20:29:46Z","messageCount":6,"participants":["Junio C Hamano","Ben Knoble"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"549256","messageId":"20260730034108.765430-1-gitster@pobox.com","threadId":"66088","inReplyTo":null,"subject":"[PATCH 0/2] git stash drop stash@{2.days.ago}","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-07-30T03:41:06Z","receivedAt":"2026-07-30T03:41:10Z","isPatch":true,"body":"Because 'stash' is implemented in terms of the reflog, it can accept\nnot only a small integer index (such as 'stash@{4}') but also a\ntime-based reference.  This is not a good thing.\n\n - 'git stash pop stash@{2.days.ago}' picks the first stash entry\n   that is no younger than the specified time and uses it to modify\n   the working tree and the index, but then removes all stash\n   entries that are no younger than that specified time.\n\n - 'git stash drop stash@{2.days.ago}' does the same, except that\n   no entry is used to affect the working tree and the index.\n\nThese two patches forbid passing time-based stash references to the\n'git stash drop' and 'git stash pop' commands as minor safety\nimprovements.\n\n 1/2: stash: record positional index in 'struct stash_info'\n 2/2: stash: reject time-based selectors in drop and pop\n\n Documentation/git-stash.adoc |  8 ++++++++\n builtin/stash.c              | 18 ++++++++++++++++++\n t/t3903-stash.sh             | 13 +++++++++++++\n 3 files changed, 39 insertions(+)\n\n-- \n2.55.0-597-ge6126a35d6\n\n"},{"id":"549257","messageId":"20260730034108.765430-2-gitster@pobox.com","threadId":"66088","inReplyTo":"20260730034108.765430-1-gitster@pobox.com","subject":"[PATCH 1/2] stash: record positional index in 'struct stash_info'","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-07-30T03:41:07Z","receivedAt":"2026-07-30T03:41:12Z","isPatch":true,"body":"get_stash_info() resolves revision arguments (such as 'stash@{0}'\nor '2') and checks whether they refer to 'refs/stash', but it\ndoes not allow callers to determine the 0-based positional\nreflog index.\n\nRecord '.stash_idx' in 'struct stash_info'.  Populate it in\nget_stash_info(), setting it to 0 when omitted (defaulting\nto the latest stash), to 'n' when a valid positional index\n'@{n}' is specified, or to -1 when the index specification\nis invalid or non-positional (such as a time-based reference).\n\nSubcommands that manipulate reflog entries by index can use\n'.stash_idx' directly, instead of parsing the revision arguments\nthemselves.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n builtin/stash.c | 15 +++++++++++++++\n 1 file changed, 15 insertions(+)\n\ndiff --git a/builtin/stash.c b/builtin/stash.c\nindex c4809f299a..5041a9ba81 100644\n--- a/builtin/stash.c\n+++ b/builtin/stash.c\n@@ -175,6 +175,7 @@ struct stash_info {\n \tstruct strbuf revision;\n \tint is_stash_ref;\n \tint has_u;\n+\tint stash_idx;\n };\n \n #define STASH_INFO_INIT { \\\n@@ -248,6 +249,7 @@ static int get_stash_info(struct stash_info *info, int argc, const char **argv)\n \tchar *expanded_ref;\n \tconst char *revision;\n \tconst char *commit = NULL;\n+\tconst char *at;\n \tstruct object_id dummy;\n \tstruct strbuf symbolic = STRBUF_INIT;\n \n@@ -300,6 +302,19 @@ static int get_stash_info(struct stash_info *info, int argc, const char **argv)\n \t}\n \n \tfree(expanded_ref);\n+\n+\tat = strstr(revision, \"@{\");\n+\tif (at) {\n+\t\tchar *ep;\n+\t\tunsigned long u = strtoul(at + 2, &ep, 10);\n+\t\tif (ep > at + 2 && *ep == '}' && u < 100000000)\n+\t\t\tinfo->stash_idx = (int)u;\n+\t\telse\n+\t\t\tinfo->stash_idx = -1;\n+\t} else {\n+\t\tinfo->stash_idx = 0;\n+\t}\n+\n \treturn !(ret == 0 || ret == 1);\n }\n \n-- \n2.55.0-597-ge6126a35d6\n\n"},{"id":"549258","messageId":"20260730034108.765430-3-gitster@pobox.com","threadId":"66088","inReplyTo":"20260730034108.765430-1-gitster@pobox.com","subject":"[PATCH 2/2] stash: reject time-based selectors in drop and pop","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-07-30T03:41:08Z","receivedAt":"2026-07-30T03:41:13Z","isPatch":true,"body":"get_stash_info_assert() verifies that a revision is a stash\nreference ('.is_stash_ref'), but it does not verify whether the\nreference is a positional reflog index as opposed to a time-based\nselector (such as 'stash@{2.days}').\n\nWhen subcommands such as 'git stash drop' or 'git stash pop' pass\ntime-based selectors to reflog_delete(), reflog_delete() treats\nnon-integer selectors as expiration cutoff timestamps.\nConsequently,\n\n - 'git stash drop stash@{2.days.ago}' deletes all stash entries\n   older than two days instead of dropping a single entry, and\n\n - 'git stash pop stash@{2.days.ago}' applies a single stash entry\n   at that timestamp and then deletes all stash entries older than\n   two days.\n\nWhile the former might be remotely useful, the latter is certainly\nnot.  In get_stash_info_assert(), reject references where\n'.stash_idx' is negative (i.e., a time-based reference was used),\nensuring that 'git stash drop' and 'git stash pop' fail early on\ninvalid or date-based stash references.\n\nDocument that 'git reflog expire --expire=<time> refs/stash' should\nbe used to prune stashes by age, and add unit tests covering\nrejection of time-based selectors for 'drop' and 'pop'.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n Documentation/git-stash.adoc |  8 ++++++++\n builtin/stash.c              |  3 +++\n t/t3903-stash.sh             | 13 +++++++++++++\n 3 files changed, 24 insertions(+)\n\ndiff --git a/Documentation/git-stash.adoc b/Documentation/git-stash.adoc\nindex 50bb89f483..6711157421 100644\n--- a/Documentation/git-stash.adoc\n+++ b/Documentation/git-stash.adoc\n@@ -106,6 +106,10 @@ command to control what is shown and how. See linkgit:git-log[1].\n \toperation of `git stash push`. The working directory must\n \tmatch the index.\n +\n+When _<stash>_ is specified, it must be a positional stash index\n+of the form `stash@{<n>}` or `<n>`. Time-based reflog selectors\n+(e.g. `stash@{2.days.ago}`) are not accepted.\n++\n Applying the state can fail with conflicts; in this case, it is not\n removed from the stash list. You need to resolve the conflicts by hand\n and call `git stash drop` manually afterwards.\n@@ -137,6 +141,10 @@ with no conflicts.\n \n `drop [-q | --quiet] [<stash>]`::\n \tRemove a single stash entry from the list of stash entries.\n+\tWhen _<stash>_ is specified, it must be a positional stash index\n+\tof the form `stash@{<n>}` or `<n>`. Time-based reflog selectors\n+\t(e.g. `stash@{2.days.ago}`) are not accepted. To prune stashes older\n+\tthan a given timestamp, use `git reflog expire --expire=<time> refs/stash`.\n \n `create`::\n \tCreate a stash entry (which is a regular commit object) and\ndiff --git a/builtin/stash.c b/builtin/stash.c\nindex 5041a9ba81..6f9561ee3a 100644\n--- a/builtin/stash.c\n+++ b/builtin/stash.c\n@@ -865,6 +865,9 @@ static int get_stash_info_assert(struct stash_info *info, int argc,\n \tif (!info->is_stash_ref)\n \t\treturn error(_(\"'%s' is not a stash reference\"), info->revision.buf);\n \n+\tif (info->stash_idx < 0)\n+\t\treturn error(_(\"'%s' is not a valid stash index\"), info->revision.buf);\n+\n \treturn 0;\n }\n \ndiff --git a/t/t3903-stash.sh b/t/t3903-stash.sh\nindex da27a6599a..01d59c8ef4 100755\n--- a/t/t3903-stash.sh\n+++ b/t/t3903-stash.sh\n@@ -808,6 +808,19 @@ test_expect_success 'pop: fail early if specified stash is not a stash ref' '\n \tgit reset --hard HEAD\n '\n \n+test_expect_success 'drop and pop reject time-based reflog selectors' '\n+\tgit stash clear &&\n+\ttest_when_finished \"git reset --hard HEAD && git stash clear\" &&\n+\tgit reset --hard &&\n+\techo foo >file &&\n+\tgit stash &&\n+\ttest_must_fail git stash drop stash@{2.days.ago} 2>err &&\n+\ttest_grep \"is not a valid stash index\" err &&\n+\ttest_must_fail git stash pop stash@{2.days.ago} 2>err &&\n+\ttest_grep \"is not a valid stash index\" err &&\n+\tgit stash drop\n+'\n+\n test_expect_success 'ref with non-existent reflog' '\n \tgit stash clear &&\n \techo bar5 >file &&\n-- \n2.55.0-597-ge6126a35d6\n\n"},{"id":"549263","messageId":"AA402B97-B3DC-4085-AF53-C6D80792C3DF@gmail.com","threadId":"66088","inReplyTo":"20260730034108.765430-2-gitster@pobox.com","subject":"Re: [PATCH 1/2] stash: record positional index in 'struct stash_info'","fromName":"Ben Knoble","fromEmail":"ben.knoble@gmail.com","sentAt":"2026-07-30T07:43:07Z","receivedAt":"2026-07-30T07:43:19Z","isPatch":true,"body":"[on mobile, so only looking at patch context]\n\n> Le 30 juil. 2026 à 12:41, Junio C Hamano <gitster@pobox.com> a écrit :\n> \n> get_stash_info() resolves revision arguments (such as 'stash@{0}'\n> or '2') and checks whether they refer to 'refs/stash', but it\n> does not allow callers to determine the 0-based positional\n> reflog index.\n> \n> Record '.stash_idx' in 'struct stash_info'.  Populate it in\n> get_stash_info(), setting it to 0 when omitted (defaulting\n> to the latest stash), to 'n' when a valid positional index\n> '@{n}' is specified, or to -1 when the index specification\n> is invalid or non-positional (such as a time-based reference).\n> \n> Subcommands that manipulate reflog entries by index can use\n> '.stash_idx' directly, instead of parsing the revision arguments\n> themselves.\n\nI notice even after 2/2 we don’t have any users of this index yet (except rejecting invalid entries as the series goal).\n\n> Signed-off-by: Junio C Hamano <gitster@pobox.com>\n> ---\n> builtin/stash.c | 15 +++++++++++++++\n> 1 file changed, 15 insertions(+)\n> \n> diff --git a/builtin/stash.c b/builtin/stash.c\n> index c4809f299a..5041a9ba81 100644\n> --- a/builtin/stash.c\n> +++ b/builtin/stash.c\n> @@ -175,6 +175,7 @@ struct stash_info {\n>   struct strbuf revision;\n>   int is_stash_ref;\n>   int has_u;\n> +    int stash_idx;\n> };\n> \n> #define STASH_INFO_INIT { \\\n> @@ -248,6 +249,7 @@ static int get_stash_info(struct stash_info *info, int argc, const char **argv)\n>   char *expanded_ref;\n>   const char *revision;\n>   const char *commit = NULL;\n> +    const char *at;\n>   struct object_id dummy;\n>   struct strbuf symbolic = STRBUF_INIT;\n> \n> @@ -300,6 +302,19 @@ static int get_stash_info(struct stash_info *info, int argc, const char **argv)\n>   }\n> \n>   free(expanded_ref);\n> +\n> +    at = strstr(revision, \"@{\");\n> +    if (at) {\n> +        char *ep;\n> +        unsigned long u = strtoul(at + 2, &ep, 10);\n> +        if (ep > at + 2 && *ep == '}' && u < 100000000)\n> +            info->stash_idx = (int)u;\n\nWhat’s the purpose of the 1e8 constant/comparison? I see we truncate the unsigned long to an int, but even on 32-bit platforms 1e8 is a small portion of the integer range, right? So my read is that we are limiting the valid « n » in @{n}. I’m not totally sure why, though, or if that matches with the rest of the stash manipulation code.  \n\n> +        else\n> +            info->stash_idx = -1;\n> +    } else {\n> +        info->stash_idx = 0;\n> +    }\n> +\n>   return !(ret == 0 || ret == 1);\n> }\n> \n> --\n> 2.55.0-597-ge6126a35d6\n"},{"id":"549299","messageId":"87h5lgtxw4.fsf@gitster.g","threadId":"66088","inReplyTo":"AA402B97-B3DC-4085-AF53-C6D80792C3DF@gmail.com","subject":"Re: [PATCH 1/2] stash: record positional index in 'struct stash_info'","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-07-30T13:22:51Z","receivedAt":"2026-07-30T13:22:56Z","isPatch":true,"body":"Ben Knoble <ben.knoble@gmail.com> writes:\n\n>> +    if (at) {\n>> +        char *ep;\n>> +        unsigned long u = strtoul(at + 2, &ep, 10);\n>> +        if (ep > at + 2 && *ep == '}' && u < 100000000)\n>> +            info->stash_idx = (int)u;\n>\n\n> What’s the purpose of the 1e8 constant/comparison? I see we\n> truncate the unsigned long to an int, but even on 32-bit platforms\n> 1e8 is a small portion of the integer range, right? So my read is\n> that we are limiting the valid « n » in @{n}. I’m not totally\n> sure why, though, or if that matches with the rest of the stash\n> manipulation code.\n\nThis mirrors what approxidate does.  An integer that is too big is\ntaken as number-of-seconds-since-epoch.\n\n"},{"id":"549325","messageId":"xmqqjyqcjk5o.fsf@gitster.g","threadId":"66088","inReplyTo":"20260730034108.765430-1-gitster@pobox.com","subject":"Re: [PATCH 0/2] git stash drop stash@{2.days.ago}","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-07-30T20:29:39Z","receivedAt":"2026-07-30T20:29:46Z","isPatch":true,"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Because 'stash' is implemented in terms of the reflog, it can accept\n> not only a small integer index (such as 'stash@{4}') but also a\n> time-based reference.  This is not a good thing.\n>\n>  - 'git stash pop stash@{2.days.ago}' picks the first stash entry\n>    that is no younger than the specified time and uses it to modify\n>    the working tree and the index, but then removes all stash\n>    entries that are no younger than that specified time.\n>\n>  - 'git stash drop stash@{2.days.ago}' does the same, except that\n>    no entry is used to affect the working tree and the index.\n>\n> These two patches forbid passing time-based stash references to the\n> 'git stash drop' and 'git stash pop' commands as minor safety\n> improvements.\n>\n>  1/2: stash: record positional index in 'struct stash_info'\n>  2/2: stash: reject time-based selectors in drop and pop\n>\n>  Documentation/git-stash.adoc |  8 ++++++++\n>  builtin/stash.c              | 18 ++++++++++++++++++\n>  t/t3903-stash.sh             | 13 +++++++++++++\n>  3 files changed, 39 insertions(+)\n\nSorry, it turns out that the collateral damange claim was completely\nbogus.  We do abuse the reflog expiration machinery but make sure we\nonly remove a single entry, it seems, so only one entry is consumed\nand then removed.  Consider these patches retracted.\n\nThanks.\n"}]}