{"thread":{"id":"47438","subject":"Apparent bug in 'git stash push <subdir>' loses untracked files","startedAt":"2017-12-13T17:32:35Z","lastAt":"2018-01-06T00:22:27Z","messageCount":10,"participants":["Reid Price","Igor Djordjevic","Thomas Gummerer","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"334763","messageId":"CA+HNv10i7AvWXjrQjxxy1LNJTmhr7LE4TwxhHUYBiWtmJCOf_A@mail.gmail.com","threadId":"47438","inReplyTo":null,"subject":"Apparent bug in 'git stash push <subdir>' loses untracked files","fromName":"Reid Price","fromEmail":"reid.price@gmail.com","sentAt":"2017-12-13T17:32:09Z","receivedAt":"2017-12-13T17:32:35Z","isPatch":false,"sender":{"key":"reid.price@gmail.com","avatar":null},"body":"When running 'git stash push <subdir>' if there are both tracked and\nuntracked files in this subdirectory, the tracked files are stashed\nbut the untracked files are discarded.\n\nI can reproduce this on my system (OSX, git 2.14.1) by running the\nbelow script as\n\n    bash -x ./stashbug.sh &> output.txt\n\nI could not find this indicated anywhere as an existing issue by\nperforming generic searches, apologies if this is known.\n\n  -Reid\n\nContents of stashbug.sh\n------------------------\n    #!/bin/sh\n\n    uname -a\n    git --version\n    mkdir -p stashbug\n    cd stashbug\n    git init\n    mkdir dir\n    touch dir/tracked\n    git add dir/tracked\n    git commit -m 'initial'\n    tree; git status\n    mkdir dir/untracked_dir\n    touch dir/untracked_dir/casualty1\n    touch dir/casualty2\n    echo 'contents' > dir/tracked\n    tree; git status\n    git stash push dir/\n    git stash show -v\n    tree; git status\n    git stash pop\n    tree; git status\n------------------------\n\nResulting output.txt\n---------------------\n    + uname -a\n    Darwin Reids-MacBook-Pro.local 15.6.0 Darwin Kernel Version\n15.6.0: Tue Apr 11 16:00:51 PDT 2017;\nroot:xnu-3248.60.11.5.3~1/RELEASE_X86_64 x86_64\n    + git --version\n    git version 2.14.1\n    + mkdir -p stashbug\n    + cd stashbug\n    + git init\n    Initialized empty Git repository in /Users/reid/git/stashbug/.git/\n    + mkdir dir\n    + touch dir/tracked\n    + git add dir/tracked\n    + git commit -m initial\n    [master (root-commit) 895197e] initial\n     1 file changed, 0 insertions(+), 0 deletions(-)\n     create mode 100644 dir/tracked\n    + tree\n    .\n    └── dir\n        └── tracked\n\n    1 directory, 1 file\n    + git status\n    On branch master\n    nothing to commit, working tree clean\n    + mkdir dir/untracked_dir\n    + touch dir/untracked_dir/casualty1\n    + touch dir/casualty2\n    + echo contents\n    + tree\n    .\n    └── dir\n        ├── casualty2\n        ├── tracked\n        └── untracked_dir\n            └── casualty1\n\n    2 directories, 3 files\n    + git status\n    On branch master\n    Changes not staged for commit:\n      (use \"git add <file>...\" to update what will be committed)\n      (use \"git checkout -- <file>...\" to discard changes in working directory)\n\n        modified:   dir/tracked\n\n    Untracked files:\n      (use \"git add <file>...\" to include in what will be committed)\n\n        dir/casualty2\n        dir/untracked_dir/\n\n    no changes added to commit (use \"git add\" and/or \"git commit -a\")\n    + git stash push dir/\n    Saved working directory and index state WIP on master: 895197e initial\n    + git stash show -v\n    diff --git a/dir/tracked b/dir/tracked\n    index e69de29..12f00e9 100644\n    --- a/dir/tracked\n    +++ b/dir/tracked\n    @@ -0,0 +1 @@\n    +contents\n    + tree\n    .\n    └── dir\n        └── tracked\n\n    1 directory, 1 file\n    + git status\n    On branch master\n    nothing to commit, working tree clean\n    + git stash pop\n    On branch master\n    Changes not staged for commit:\n      (use \"git add <file>...\" to update what will be committed)\n      (use \"git checkout -- <file>...\" to discard changes in working directory)\n\n        modified:   dir/tracked\n\n    no changes added to commit (use \"git add\" and/or \"git commit -a\")\n    Dropped refs/stash@{0} (93ceee344b947ecd8a27a672e3aedd2b2e1acc99)\n    + tree\n    .\n    └── dir\n        └── tracked\n\n    1 directory, 1 file\n    + git status\n    On branch master\n    Changes not staged for commit:\n      (use \"git add <file>...\" to update what will be committed)\n      (use \"git checkout -- <file>...\" to discard changes in working directory)\n\n        modified:   dir/tracked\n\n    no changes added to commit (use \"git add\" and/or \"git commit -a\")\n---------------------\n"},{"id":"334780","messageId":"0692d5b8-5fc7-c34c-0712-11a913267441@gmail.com","threadId":"47438","inReplyTo":"CA+HNv10i7AvWXjrQjxxy1LNJTmhr7LE4TwxhHUYBiWtmJCOf_A@mail.gmail.com","subject":"Re: Apparent bug in 'git stash push <subdir>' loses untracked files","fromName":"Igor Djordjevic","fromEmail":"igor.d.djordjevic@gmail.com","sentAt":"2017-12-13T21:20:26Z","receivedAt":"2017-12-13T21:20:37Z","isPatch":false,"sender":{"key":"igor.d.djordjevic@gmail.com","avatar":null},"body":"Hi Reid,\n\nOn 13/12/2017 18:32, Reid Price wrote:\n> \n> When running 'git stash push <subdir>' if there are both tracked and\n> untracked files in this subdirectory, the tracked files are stashed\n> but the untracked files are discarded.\n\nI can reproduce this as well (git version 2.15.1.windows.2).\n\nFor what it`s worth, using `git stash save <subdir>` instead seems to \n(still) work as expected... but on the other hand, `git-stash`[1] \nmanpage seems not to mention this usage (\"save\" with \"pathspec\")?\n\nRegards, Buga\n\n[1] https://git-scm.com/docs/git-stash\n"},{"id":"334784","messageId":"20171213230547.GC24597@hank","threadId":"47438","inReplyTo":"CA+HNv10i7AvWXjrQjxxy1LNJTmhr7LE4TwxhHUYBiWtmJCOf_A@mail.gmail.com","subject":"Re: Apparent bug in 'git stash push <subdir>' loses untracked files","fromName":"Thomas Gummerer","fromEmail":"t.gummerer@gmail.com","sentAt":"2017-12-13T23:05:47Z","receivedAt":"2017-12-13T23:04:11Z","isPatch":false,"sender":{"key":"t.gummerer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/191004?v=4"},"body":"Hi,\n\nOn 12/13, Reid Price wrote:\n> When running 'git stash push <subdir>' if there are both tracked and\n> untracked files in this subdirectory, the tracked files are stashed\n> but the untracked files are discarded.\n> \n> I can reproduce this on my system (OSX, git 2.14.1) by running the\n> below script as\n> \n>     bash -x ./stashbug.sh &> output.txt\n> \n> I could not find this indicated anywhere as an existing issue by\n> performing generic searches, apologies if this is known.\n\nThanks for the detailed bug report below.  This is indeed a bug, and\nindeed has been broken ever since I introduced the pathspec feature.\nSorry about that.\n\nWhile I did implement this feature in the first place, I must admit\nthat I'm a bit out of my depth in terms of my shell foo to fix this\ncleanly here.\n\nJust to demonstrate what is wrong, the below diff fixes it (but it\ndoes so in an ugly way that needs a temporary file, and needs a\nspecific version of the 'read' utility, that supports the '-d' flag,\nwhich isn't in POSIX).\n\nFor the pathspec case what we're doing after creating the stash is to\nessentially emulate what 'git reset --hard <pathspec>' would do.  As\n'git reset --hard <pathspec>' doesn't exist, we do so using the\nsequence below of reset -> checkout-index -> clean for the given\npathspec.\n\nThis works when the pathspec doesn't match any untracked files, but if\nit matches untracked files such as in your case it removes them as\nwell as the files it should have removed.\n\nOne solution to that would be to limit the pathspec given to 'git\nclean'.  And that is where my shell foo doesn't go far enough for me\nbeing able to do that, as lists of filenames, which can include spaces\ntend to get quite hairy.\n\nMaybe the best solution would be to introduce 'git reset --hard --\n<pathspec>', or maybe someone who knows shell programming a little\nbetter than me has an idea? \n\ndiff --git a/git-stash.sh b/git-stash.sh\nindex 1114005ce2..01bf74015e 100755\n--- a/git-stash.sh\n+++ b/git-stash.sh\n@@ -322,10 +322,15 @@ push_stash () {\n \n                if test $# != 0\n                then\n+                       git ls-files -z >\"$(git rev-parse --git-dir)\"/stash-to-remove\n                        git reset -q -- \"$@\"\n                        git ls-files -z --modified -- \"$@\" |\n                        git checkout-index -z --force --stdin\n-                       git clean --force -q -d -- \"$@\"\n+                       while read -r -d '' to_delete\n+                       do\n+                               git clean --force -q -d -- \"$to_delete\"\n+                       done <\"$(git rev-parse --git-dir)\"/stash-to-remove\n+                       rm \"$(git rev-parse --git-dir)\"/stash-to-remove\n                else\n                        git reset --hard -q\n                fi\ndiff --git a/t/t3903-stash.sh b/t/t3903-stash.sh\nindex 39c7f2ebd7..6952a031b2 100755\n--- a/t/t3903-stash.sh\n+++ b/t/t3903-stash.sh\n@@ -1064,4 +1064,20 @@ test_expect_success 'stash -k -- <pathspec> leaves unstaged files intact' '\n        test foo,bar = $(cat foo),$(cat bar)\n '\n \n+test_expect_success 'stash -- <subdir> leaves untracked files in subdir intact' '\n+       git reset &&\n+       >subdir/untracked &&\n+       >subdir/tracked1 &&\n+       >subdir/tracked2 &&\n+       git add subdir/tracked* &&\n+       git stash -- subdir/ &&\n+       test_path_is_missing subdir/tracked1 &&\n+       test_path_is_missing subdir/tracked2 &&\n+       test_path_is_file subdir/untracked &&\n+       git stash pop &&\n+       test_path_is_file subdir/tracked1 &&\n+       test_path_is_file subdir/tracked2 &&\n+       test_path_is_file subdir/untracked\n+'\n+\n test_done\n\n>   -Reid\n> \n> Contents of stashbug.sh\n> ------------------------\n>     #!/bin/sh\n> \n>     uname -a\n>     git --version\n>     mkdir -p stashbug\n>     cd stashbug\n>     git init\n>     mkdir dir\n>     touch dir/tracked\n>     git add dir/tracked\n>     git commit -m 'initial'\n>     tree; git status\n>     mkdir dir/untracked_dir\n>     touch dir/untracked_dir/casualty1\n>     touch dir/casualty2\n>     echo 'contents' > dir/tracked\n>     tree; git status\n>     git stash push dir/\n>     git stash show -v\n>     tree; git status\n>     git stash pop\n>     tree; git status\n> ------------------------\n> \n> Resulting output.txt\n> ---------------------\n>     + uname -a\n>     Darwin Reids-MacBook-Pro.local 15.6.0 Darwin Kernel Version\n> 15.6.0: Tue Apr 11 16:00:51 PDT 2017;\n> root:xnu-3248.60.11.5.3~1/RELEASE_X86_64 x86_64\n>     + git --version\n>     git version 2.14.1\n>     + mkdir -p stashbug\n>     + cd stashbug\n>     + git init\n>     Initialized empty Git repository in /Users/reid/git/stashbug/.git/\n>     + mkdir dir\n>     + touch dir/tracked\n>     + git add dir/tracked\n>     + git commit -m initial\n>     [master (root-commit) 895197e] initial\n>      1 file changed, 0 insertions(+), 0 deletions(-)\n>      create mode 100644 dir/tracked\n>     + tree\n>     .\n>     └── dir\n>         └── tracked\n> \n>     1 directory, 1 file\n>     + git status\n>     On branch master\n>     nothing to commit, working tree clean\n>     + mkdir dir/untracked_dir\n>     + touch dir/untracked_dir/casualty1\n>     + touch dir/casualty2\n>     + echo contents\n>     + tree\n>     .\n>     └── dir\n>         ├── casualty2\n>         ├── tracked\n>         └── untracked_dir\n>             └── casualty1\n> \n>     2 directories, 3 files\n>     + git status\n>     On branch master\n>     Changes not staged for commit:\n>       (use \"git add <file>...\" to update what will be committed)\n>       (use \"git checkout -- <file>...\" to discard changes in working directory)\n> \n>         modified:   dir/tracked\n> \n>     Untracked files:\n>       (use \"git add <file>...\" to include in what will be committed)\n> \n>         dir/casualty2\n>         dir/untracked_dir/\n> \n>     no changes added to commit (use \"git add\" and/or \"git commit -a\")\n>     + git stash push dir/\n>     Saved working directory and index state WIP on master: 895197e initial\n>     + git stash show -v\n>     diff --git a/dir/tracked b/dir/tracked\n>     index e69de29..12f00e9 100644\n>     --- a/dir/tracked\n>     +++ b/dir/tracked\n>     @@ -0,0 +1 @@\n>     +contents\n>     + tree\n>     .\n>     └── dir\n>         └── tracked\n> \n>     1 directory, 1 file\n>     + git status\n>     On branch master\n>     nothing to commit, working tree clean\n>     + git stash pop\n>     On branch master\n>     Changes not staged for commit:\n>       (use \"git add <file>...\" to update what will be committed)\n>       (use \"git checkout -- <file>...\" to discard changes in working directory)\n> \n>         modified:   dir/tracked\n> \n>     no changes added to commit (use \"git add\" and/or \"git commit -a\")\n>     Dropped refs/stash@{0} (93ceee344b947ecd8a27a672e3aedd2b2e1acc99)\n>     + tree\n>     .\n>     └── dir\n>         └── tracked\n> \n>     1 directory, 1 file\n>     + git status\n>     On branch master\n>     Changes not staged for commit:\n>       (use \"git add <file>...\" to update what will be committed)\n>       (use \"git checkout -- <file>...\" to discard changes in working directory)\n> \n>         modified:   dir/tracked\n> \n>     no changes added to commit (use \"git add\" and/or \"git commit -a\")\n> ---------------------\n"},{"id":"334786","messageId":"20171213231414.GD24597@hank","threadId":"47438","inReplyTo":"0692d5b8-5fc7-c34c-0712-11a913267441@gmail.com","subject":"Re: Apparent bug in 'git stash push <subdir>' loses untracked files","fromName":"Thomas Gummerer","fromEmail":"t.gummerer@gmail.com","sentAt":"2017-12-13T23:14:14Z","receivedAt":"2017-12-13T23:12:34Z","isPatch":false,"sender":{"key":"t.gummerer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/191004?v=4"},"body":"On 12/13, Igor Djordjevic wrote:\n> Hi Reid,\n> \n> On 13/12/2017 18:32, Reid Price wrote:\n> > \n> > When running 'git stash push <subdir>' if there are both tracked and\n> > untracked files in this subdirectory, the tracked files are stashed\n> > but the untracked files are discarded.\n> \n> I can reproduce this as well (git version 2.15.1.windows.2).\n> \n> For what it`s worth, using `git stash save <subdir>` instead seems to \n> (still) work as expected...\n\nI think that depends on what you expect ;)  'git stash save <subdir>'\nwill create a stash of the whole working directory with the message\n\"<subdir>\".  So while it would indeed work for the presumably\nsimplified example Reid provided, it would not do what you'd expect if\nthere are any tracked and modified files outside of the <subdir>.\n\nIn that case 'git stash save <subdir>' would include the tracked files\noutside of <subdir>, while what I assume Reid wanted is to keep them\nin place, and only stash the files in <subdir>.\n\n> but on the other hand, `git-stash`[1] \n> manpage seems not to mention this usage (\"save\" with \"pathspec\")?\n\n\"stash save\" with \"pathspec\" doesn't exist, and it will probably never\nexist.  We decided to introduce a new \"push\" verb for 'git stash'\nbecause the command line for 'git stash save' takes a message as its\nlast argument, instead of taking the message with a -m flag like other\ncommands do.  Introducing a pathspec argument for \"git stash save\"\nwould have either broken backward compatibility, or it would have had\nsome syntax that's very inconsistent with other git commands.\n\n> Regards, Buga\n> \n> [1] https://git-scm.com/docs/git-stash\n"},{"id":"334789","messageId":"ee9e6b8b-278e-0b86-cd35-c5d490ed7700@gmail.com","threadId":"47438","inReplyTo":"20171213231414.GD24597@hank","subject":"Re: Apparent bug in 'git stash push <subdir>' loses untracked files","fromName":"Igor Djordjevic","fromEmail":"igor.d.djordjevic@gmail.com","sentAt":"2017-12-13T23:46:21Z","receivedAt":"2017-12-13T23:46:31Z","isPatch":false,"sender":{"key":"igor.d.djordjevic@gmail.com","avatar":null},"body":"Hi Thomas,\n\nOn 14/12/2017 00:14, Thomas Gummerer wrote:\n> \n> > For what it`s worth, using `git stash save <subdir>` instead seems\n> > to (still) work as expected...\n> \n> I think that depends on what you expect ;) 'git stash save <subdir>' \n> will create a stash of the whole working directory with the message \n> \"<subdir>\". So while it would indeed work for the presumably \n> simplified example Reid provided, it would not do what you'd expect\n> if there are any tracked and modified files outside of the <subdir>.\n> \n> In that case 'git stash save <subdir>' would include the tracked\n> files outside of <subdir>, while what I assume Reid wanted is to keep\n> them in place, and only stash the files in <subdir>.\n\nIndeed, I didn`t pay enough attention to the fact that even `git \nstash save\\push` produced different output messages, the difference \nbeing exactly automatic (push) versus provided (save) stash message.\n\nAnd I did use `git stash save <message>` in the past... :$ Not too \noften, I guess.\n\n> > but on the other hand, `git-stash`[1] manpage seems not to mention\n> > this usage (\"save\" with \"pathspec\")?\n> \n> \"stash save\" with \"pathspec\" doesn't exist, and it will probably\n> never exist. We decided to introduce a new \"push\" verb for 'git\n> stash' because the command line for 'git stash save' takes a message\n> as its last argument, instead of taking the message with a -m flag\n> like other commands do. Introducing a pathspec argument for \"git\n> stash save\" would have either broken backward compatibility, or it\n> would have had some syntax that's very inconsistent with other git\n> commands.\n\nYeah, I`m aware of the \"transition\", thus teaching myself to use `git \nstash push` lately. That`s also what made me curious to try out the \n\"old\" `git stash save` behavior, but obviously in a bit hasty manner.\n\nSorry for the confusion, and thanks, for both clarification \nand your work.\n\nRegards, Buga\n"},{"id":"334939","messageId":"xmqq4loqplou.fsf@gitster.mtv.corp.google.com","threadId":"47438","inReplyTo":"20171213230547.GC24597@hank","subject":"Re: Apparent bug in 'git stash push <subdir>' loses untracked files","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-12-16T18:33:21Z","receivedAt":"2017-12-16T18:33:33Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thomas Gummerer <t.gummerer@gmail.com> writes:\n\n> Maybe the best solution would be to introduce 'git reset --hard --\n> <pathspec>', or maybe someone who knows shell programming a little\n> better than me has an idea? \n>\n> diff --git a/git-stash.sh b/git-stash.sh\n> index 1114005ce2..01bf74015e 100755\n> --- a/git-stash.sh\n> +++ b/git-stash.sh\n> @@ -322,10 +322,15 @@ push_stash () {\n>  \n>                 if test $# != 0\n>                 then\n> +                       git ls-files -z >\"$(git rev-parse --git-dir)\"/stash-to-remove\n>                         git reset -q -- \"$@\"\n>                         git ls-files -z --modified -- \"$@\" |\n>                         git checkout-index -z --force --stdin\n> -                       git clean --force -q -d -- \"$@\"\n> +                       while read -r -d '' to_delete\n> +                       do\n> +                               git clean --force -q -d -- \"$to_delete\"\n> +                       done <\"$(git rev-parse --git-dir)\"/stash-to-remove\n> +                       rm \"$(git rev-parse --git-dir)\"/stash-to-remove\n>                 else\n>                         git reset --hard -q\n>                 fi\n\nHmph.  I personally did not care (nor use) pathspec limited stash\ncreation, so I admit that I do not offhand know what this code\n(before or after the above fix) is trying to do, even though it has\nbeen in our tree for some time X-<.  But judging from the fact that\nthe other side is a mere \"git reset --hard\" (for the whole tree), I\nguess for each and every path in the index and/or in the HEAD that\nmatch pathspec \"$@\", you are trying to\n\n - remove it from the index if it is not in HEAD;\n\n - add it to the index if it is not in the index but is in HEAD; or\n\n - reset the index to the version in HEAD if it is in the index.\n\nand then match the working tree version to that of the index?\n\nI am not convinced that \"git reset --hard -- <pathspec>\" is a good\nUI [*1*] but I agree that a low level facility that does these would\nbe quite helpful to implement the then-clause of the above.\n\nBut I suspect that you do not have to do shell \"logic\" nor low-level\nplumbing update in this case.  Would the attached work?\n\n[Footnote]\n\n*1* Primarily because \"git reset [<tree-ish>] -- <pathspec>\" that\n    already exists is not a good UI.  The remainder of \"git reset\"\n    is about repointing the HEAD, and the various operation modes\n    are there to adjust the index and the working tree to the\n    updated HEAD depending on why the user wants to repoint the HEAD\n    in the first place.  On the other hand, the pathspec limited\n    \"reset\" is all about affecting the index without moving the HEAD\n    and operation modes, if --hard is also taught not to reject\n    pathspec, will become \"do we do index-only or do we do both?\"\n\n    But updating the index and the working tree is what \"git\n    checkout [<tree-ish>] -- <pathspec>\" is for, and the \"git reset\n    [<tree-ish>] -- <pathspec>\" should have been \"checkout --cached\n    [<tree-ish>] -- <pathspec>\" (and the \"reset --hard\" equivalent\n    should be the \"default\" for checkout).\n\n    There is a small glitch in the current behaviour of \"checkout --\n    <pathspec>\" to complicate the above vision, but assuming that it\n    is surmountable, and if we were going to update the UI for\n    consistency along the above line, then I do not mind making \"git\n    checkout\" solely for \"checking out paths\" and adding a different\n    subcommannd to \"checking out a branch to work on\".\n\n    \"reset.<commit>\" is about resetting the HEAD and \"reset\n    [<tree-ish>] -- <pathspec>\" is about resetting the index, which\n    is the same duality some people complain about \"git checkout\"\n    that is both about checking out the paths and checking out a\n    branch to work on.  Adding more mode(s) to \"git reset\" that\n    makes the command not about repointing the HEAD is piling more\n    on top.\n\n    The \"small glitch\" is that \"checkout <tree-ish> -- <pathspec>\"\n    is defined as an overlay operation---take the current index (and\n    the working tree), then for each (path, contents) in <tree-ish>\n    that match the <pathspec>, add it in if path does not exist in\n    the current index, or replace if path does.  If we have a path\n    in the index that match <pathspec>, and that path does not exist\n    in <tree-ish>, \"checkout <tree-ish> -- <pathspec>\" does not\n    remove that path from the index.\n\n    So even the index+working tree mode (which is how \"reset --hard\"\n    would want to work) may need to take a new option.  Between two\n    modes of \"checkout <tree-ish> -- <pathspec>\" that allows\n    deletion, \"--index\" would be the one that works on both the\n    index and the working tree, and \"--cached\" would be the one that\n    works only on the index (this follows gitcli convention that is\n    used by existing commands), and the current \"overlay\" mode will\n    stay to be the default, at least until we know that the \"--index\"\n    mode should become the default and decide to transition.\n\n\n\n\n---\n\n git-stash.sh     |  5 ++---\n t/t3903-stash.sh | 16 ++++++++++++++++\n 2 files changed, 18 insertions(+), 3 deletions(-)\n\ndiff --git a/git-stash.sh b/git-stash.sh\nindex 1114005ce2..a979bfb665 100755\n--- a/git-stash.sh\n+++ b/git-stash.sh\n@@ -322,10 +322,9 @@ push_stash () {\n \n \t\tif test $# != 0\n \t\tthen\n-\t\t\tgit reset -q -- \"$@\"\n-\t\t\tgit ls-files -z --modified -- \"$@\" |\n+\t\t\tgit ls-files -z -- \"$@\" |\n \t\t\tgit checkout-index -z --force --stdin\n-\t\t\tgit clean --force -q -d -- \"$@\"\n+\t\t\tgit diff-index -p HEAD -- \"$@\" | git apply --index -R\n \t\telse\n \t\t\tgit reset --hard -q\n \t\tfi\ndiff --git a/t/t3903-stash.sh b/t/t3903-stash.sh\nindex 39c7f2ebd7..6952a031b2 100755\n--- a/t/t3903-stash.sh\n+++ b/t/t3903-stash.sh\n@@ -1064,4 +1064,20 @@ test_expect_success 'stash -k -- <pathspec> leaves unstaged files intact' '\n \ttest foo,bar = $(cat foo),$(cat bar)\n '\n \n+test_expect_success 'stash -- <subdir> leaves untracked files in subdir intact' '\n+\tgit reset &&\n+\t>subdir/untracked &&\n+\t>subdir/tracked1 &&\n+\t>subdir/tracked2 &&\n+\tgit add subdir/tracked* &&\n+\tgit stash -- subdir/ &&\n+\ttest_path_is_missing subdir/tracked1 &&\n+\ttest_path_is_missing subdir/tracked2 &&\n+\ttest_path_is_file subdir/untracked &&\n+\tgit stash pop &&\n+\ttest_path_is_file subdir/tracked1 &&\n+\ttest_path_is_file subdir/tracked2 &&\n+\ttest_path_is_file subdir/untracked\n+'\n+\n test_done\n"},{"id":"334951","messageId":"20171217180511.GA2641@hank","threadId":"47438","inReplyTo":"xmqq4loqplou.fsf@gitster.mtv.corp.google.com","subject":"Re: Apparent bug in 'git stash push <subdir>' loses untracked files","fromName":"Thomas Gummerer","fromEmail":"t.gummerer@gmail.com","sentAt":"2017-12-17T18:05:11Z","receivedAt":"2017-12-17T18:03:30Z","isPatch":false,"sender":{"key":"t.gummerer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/191004?v=4"},"body":"On 12/16, Junio C Hamano wrote:\n> Thomas Gummerer <t.gummerer@gmail.com> writes:\n> \n> > Maybe the best solution would be to introduce 'git reset --hard --\n> > <pathspec>', or maybe someone who knows shell programming a little\n> > better than me has an idea? \n> >\n> > diff --git a/git-stash.sh b/git-stash.sh\n> > index 1114005ce2..01bf74015e 100755\n> > --- a/git-stash.sh\n> > +++ b/git-stash.sh\n> > @@ -322,10 +322,15 @@ push_stash () {\n> >  \n> >                 if test $# != 0\n> >                 then\n> > +                       git ls-files -z >\"$(git rev-parse --git-dir)\"/stash-to-remove\n> >                         git reset -q -- \"$@\"\n> >                         git ls-files -z --modified -- \"$@\" |\n> >                         git checkout-index -z --force --stdin\n> > -                       git clean --force -q -d -- \"$@\"\n> > +                       while read -r -d '' to_delete\n> > +                       do\n> > +                               git clean --force -q -d -- \"$to_delete\"\n> > +                       done <\"$(git rev-parse --git-dir)\"/stash-to-remove\n> > +                       rm \"$(git rev-parse --git-dir)\"/stash-to-remove\n> >                 else\n> >                         git reset --hard -q\n> >                 fi\n> \n> Hmph.  I personally did not care (nor use) pathspec limited stash\n> creation, so I admit that I do not offhand know what this code\n> (before or after the above fix) is trying to do, even though it has\n> been in our tree for some time X-<.\n\nYeah unfortunately this has been broken for pathspecs that match\nuntracked files since I introduced it :(\n\n> But judging from the fact that\n> the other side is a mere \"git reset --hard\" (for the whole tree), I\n> guess for each and every path in the index and/or in the HEAD that\n> match pathspec \"$@\", you are trying to\n> \n>  - remove it from the index if it is not in HEAD;\n> \n>  - add it to the index if it is not in the index but is in HEAD; or\n> \n>  - reset the index to the version in HEAD if it is in the index.\n> \n> and then match the working tree version to that of the index?\n\nThat's correct.  That's how I would have imagined 'git reset --hard --\n<pathspec>'.\n\n> I am not convinced that \"git reset --hard -- <pathspec>\" is a good\n> UI [*1*] but I agree that a low level facility that does these would\n> be quite helpful to implement the then-clause of the above.\n\nThanks a lot for spelling this out below.  After reading that I\ndefinitely agree that 'git reset --hard -- <pathspec>' wouldn't be a\ngood UI, and something like that would belong in checkout.\n\n> But I suspect that you do not have to do shell \"logic\" nor low-level\n> plumbing update in this case.  Would the attached work?\n\nAh interesting, what you have below looks good to me indeed, it\nmatches what I'd expect it to do and fixes the bug that was reported.\nThanks! \n\nI've taken the liberty to take what you have below and turned into a\nproper patch, giving you authorship of it, as you wrote the code for\nthe fix.  Hope that's the appropriate credit for you for this patch.\n\n[...]\n\n--- >8 ---\nFrom: Junio C Hamano <gitster@pobox.com>\nSubject: [PATCH] stash: don't delete untracked files that match pathspec\n\nCurrently when 'git stash push -- <pathspec>' is used, untracked files\nthat match the pathspec will be deleted, even though they do not end up\nin a stash anywhere.\n\nUntracked files should be left along by 'git stash push -- <pathspec>'\nunless the --untracked or --all flags are given.  Fix that.\n\nReported-by: Reid Price <reid.price@gmail.com>\nTest-by: Thomas Gummerer <t.gummerer@gmail.com>\nSigned-off-by: Thomas Gummerer <t.gummerer@gmail.com>\n---\n git-stash.sh     |  5 ++---\n t/t3903-stash.sh | 16 ++++++++++++++++\n 2 files changed, 18 insertions(+), 3 deletions(-)\n\ndiff --git a/git-stash.sh b/git-stash.sh\nindex 1114005ce2..a979bfb665 100755\n--- a/git-stash.sh\n+++ b/git-stash.sh\n@@ -322,10 +322,9 @@ push_stash () {\n \n \t\tif test $# != 0\n \t\tthen\n-\t\t\tgit reset -q -- \"$@\"\n-\t\t\tgit ls-files -z --modified -- \"$@\" |\n+\t\t\tgit ls-files -z -- \"$@\" |\n \t\t\tgit checkout-index -z --force --stdin\n-\t\t\tgit clean --force -q -d -- \"$@\"\n+\t\t\tgit diff-index -p HEAD -- \"$@\" | git apply --index -R\n \t\telse\n \t\t\tgit reset --hard -q\n \t\tfi\ndiff --git a/t/t3903-stash.sh b/t/t3903-stash.sh\nindex 39c7f2ebd7..6952a031b2 100755\n--- a/t/t3903-stash.sh\n+++ b/t/t3903-stash.sh\n@@ -1064,4 +1064,20 @@ test_expect_success 'stash -k -- <pathspec> leaves unstaged files intact' '\n \ttest foo,bar = $(cat foo),$(cat bar)\n '\n \n+test_expect_success 'stash -- <subdir> leaves untracked files in subdir intact' '\n+\tgit reset &&\n+\t>subdir/untracked &&\n+\t>subdir/tracked1 &&\n+\t>subdir/tracked2 &&\n+\tgit add subdir/tracked* &&\n+\tgit stash -- subdir/ &&\n+\ttest_path_is_missing subdir/tracked1 &&\n+\ttest_path_is_missing subdir/tracked2 &&\n+\ttest_path_is_file subdir/untracked &&\n+\tgit stash pop &&\n+\ttest_path_is_file subdir/tracked1 &&\n+\ttest_path_is_file subdir/tracked2 &&\n+\ttest_path_is_file subdir/untracked\n+'\n+\n test_done\n-- \n2.15.1.390.ga48b24a585.dirty\n\n"},{"id":"334978","messageId":"xmqqpo7byjwb.fsf@gitster.mtv.corp.google.com","threadId":"47438","inReplyTo":"20171217180511.GA2641@hank","subject":"Re: Apparent bug in 'git stash push <subdir>' loses untracked files","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-12-18T18:24:04Z","receivedAt":"2017-12-18T18:24:13Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thomas Gummerer <t.gummerer@gmail.com> writes:\n\n> Ah interesting, what you have below looks good to me indeed, it\n> matches what I'd expect it to do and fixes the bug that was reported.\n> Thanks! \n>\n> I've taken the liberty to take what you have below and turned into a\n> proper patch, giving you authorship of it, as you wrote the code for\n> the fix.  Hope that's the appropriate credit for you for this patch.\n\nNot so fast.\n\nI know why the updated code works like \"--hard -- <pathspec>\", but I\ndo not quite get what the original was trying to do and how it is\ndifferent.  Even with your proposed log message, which describes a\nsymptom (i.e. \"untracked files ... be deleted\"), it is unclear why\nthis deletion was happening in the first place.  Specifically, it is\nunclear why that \"clean --force -q -d\" was in there, and are we\nbreaking other cases by rewriting this codepath without it?\n\nIn any case, instead of the \n\n\tls-files -z -- \"$@\" | checkout-index -z --force --stdin\n\tdiff-index -p HEAD -- \"$@\" | apply --index -R\n\nsequence, a shorter variant that should work is\n\n\tadd -u -- \"$@\"\n\tdiff-index -p --cached HEAD -- \"$@\" | apply --index -R\n\nBoth of these share the same idea.  \n\n - The first step is to match what is in the index and what is in\n   the working tree (i.e. make \"diff-files\" silent).  The version\n   you have does so by checking out what is in the index to the\n   working tree.  The shorter one goes the other way and updates the\n   index with what is in the working tree.\n\n - Once that is done, we ask diff-index what got changed since the\n   HEAD in the working tree (or in the index in the updated\n   one---after the first step that makes the two match, comparing\n   with the working tree and comparing with the index should result\n   in the same diff; I have a suspicion that \"--cached\" is faster,\n   but we need to benchmark to pick), and ask apply to get rid of\n   all these changes, which includes removal of added files, and\n   resurrection of removed files.\n\nI think the original that did 'git reset -- \"$@\"' upfront lost new\npaths from the index (without removing it from the working tree), I\nam guessing that it is why \"clean\" was there to get rid of them, and\nif that is the reason, I can understand why the original code was\nbehaving incorrectly---it would get rid of new files that did not\nexist in HEAD correctly, but it also would remove untracked ones,\nbecause after that first 'reset', the code couldn't tell between\nthem.\n\nAnd I think that is what we want, i.e. why the original was wrong\nand how the new one fixes it, to describe in the log message to\njustify this change.\n\nOne thing that I didn't think through and you need to verify is if\nwe need to do anything extra to deal with binary files (in the old\ndays, we needed --full-index and --binary options to produce and\napply a binary patch; I do not offhand know if that is still the\ncase) in the final \"diff-index piped to apply -R --index\" dance.\n\nSo I am asking you to fill in quite a lot of gaps that I didn't do\nwith only the above two-liner ;-)  You should take the authorship\nand, if you like, credit me with helped-by: or something.\n\nThanks.\n\n\n>\n> [...]\n>\n> --- >8 ---\n> From: Junio C Hamano <gitster@pobox.com>\n> Subject: [PATCH] stash: don't delete untracked files that match pathspec\n>\n> Currently when 'git stash push -- <pathspec>' is used, untracked files\n> that match the pathspec will be deleted, even though they do not end up\n> in a stash anywhere.\n>\n> Untracked files should be left along by 'git stash push -- <pathspec>'\n> unless the --untracked or --all flags are given.  Fix that.\n>\n> Reported-by: Reid Price <reid.price@gmail.com>\n> Test-by: Thomas Gummerer <t.gummerer@gmail.com>\n> Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>\n> ---\n>  git-stash.sh     |  5 ++---\n>  t/t3903-stash.sh | 16 ++++++++++++++++\n>  2 files changed, 18 insertions(+), 3 deletions(-)\n>\n> diff --git a/git-stash.sh b/git-stash.sh\n> index 1114005ce2..a979bfb665 100755\n> --- a/git-stash.sh\n> +++ b/git-stash.sh\n> @@ -322,10 +322,9 @@ push_stash () {\n>  \n>  \t\tif test $# != 0\n>  \t\tthen\n> -\t\t\tgit reset -q -- \"$@\"\n> -\t\t\tgit ls-files -z --modified -- \"$@\" |\n> +\t\t\tgit ls-files -z -- \"$@\" |\n>  \t\t\tgit checkout-index -z --force --stdin\n> -\t\t\tgit clean --force -q -d -- \"$@\"\n> +\t\t\tgit diff-index -p HEAD -- \"$@\" | git apply --index -R\n>  \t\telse\n>  \t\t\tgit reset --hard -q\n>  \t\tfi\n> diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh\n> index 39c7f2ebd7..6952a031b2 100755\n> --- a/t/t3903-stash.sh\n> +++ b/t/t3903-stash.sh\n> @@ -1064,4 +1064,20 @@ test_expect_success 'stash -k -- <pathspec> leaves unstaged files intact' '\n>  \ttest foo,bar = $(cat foo),$(cat bar)\n>  '\n>  \n> +test_expect_success 'stash -- <subdir> leaves untracked files in subdir intact' '\n> +\tgit reset &&\n> +\t>subdir/untracked &&\n> +\t>subdir/tracked1 &&\n> +\t>subdir/tracked2 &&\n> +\tgit add subdir/tracked* &&\n> +\tgit stash -- subdir/ &&\n> +\ttest_path_is_missing subdir/tracked1 &&\n> +\ttest_path_is_missing subdir/tracked2 &&\n> +\ttest_path_is_file subdir/untracked &&\n> +\tgit stash pop &&\n> +\ttest_path_is_file subdir/tracked1 &&\n> +\ttest_path_is_file subdir/tracked2 &&\n> +\ttest_path_is_file subdir/untracked\n> +'\n> +\n>  test_done\n"},{"id":"335952","messageId":"20180105200313.GF2641@hank","threadId":"47438","inReplyTo":"xmqqpo7byjwb.fsf@gitster.mtv.corp.google.com","subject":"Re: Apparent bug in 'git stash push <subdir>' loses untracked files","fromName":"Thomas Gummerer","fromEmail":"t.gummerer@gmail.com","sentAt":"2018-01-05T20:03:13Z","receivedAt":"2018-01-05T20:01:12Z","isPatch":false,"sender":{"key":"t.gummerer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/191004?v=4"},"body":"On 12/18, Junio C Hamano wrote:\n> Thomas Gummerer <t.gummerer@gmail.com> writes:\n> \n> > Ah interesting, what you have below looks good to me indeed, it\n> > matches what I'd expect it to do and fixes the bug that was reported.\n> > Thanks! \n> >\n> > I've taken the liberty to take what you have below and turned into a\n> > proper patch, giving you authorship of it, as you wrote the code for\n> > the fix.  Hope that's the appropriate credit for you for this patch.\n> \n> Not so fast.\n> \n> I know why the updated code works like \"--hard -- <pathspec>\", but I\n> do not quite get what the original was trying to do and how it is\n> different.  Even with your proposed log message, which describes a\n> symptom (i.e. \"untracked files ... be deleted\"), it is unclear why\n> this deletion was happening in the first place.  Specifically, it is\n> unclear why that \"clean --force -q -d\" was in there, and are we\n> breaking other cases by rewriting this codepath without it?\n\nAs you hint at below, the intention was to get rid of the files that\nwere unstaged by 'git reset', but didn't realize that it would also\ndelete files that were already untracked before the 'git reset' and\nthus not in the stash.\n\nIt behaves fine if the pathspec matches a filename exactly, which is\nwhy it worked in all my tests, but is obviously broken in the case\nwhere the pathspec matches untracked files that are around.\n\nI completely failed to realize this, and we wouldn't break any\nintended usecases with the change.  I'll update the commit message to\ninclude this information. \n\n> In any case, instead of the \n> \n> \tls-files -z -- \"$@\" | checkout-index -z --force --stdin\n> \tdiff-index -p HEAD -- \"$@\" | apply --index -R\n> \n> sequence, a shorter variant that should work is\n> \n> \tadd -u -- \"$@\"\n> \tdiff-index -p --cached HEAD -- \"$@\" | apply --index -R\n> \n> Both of these share the same idea.  \n> \n>  - The first step is to match what is in the index and what is in\n>    the working tree (i.e. make \"diff-files\" silent).  The version\n>    you have does so by checking out what is in the index to the\n>    working tree.  The shorter one goes the other way and updates the\n>    index with what is in the working tree.\n> \n>  - Once that is done, we ask diff-index what got changed since the\n>    HEAD in the working tree (or in the index in the updated\n>    one---after the first step that makes the two match, comparing\n>    with the working tree and comparing with the index should result\n>    in the same diff; I have a suspicion that \"--cached\" is faster,\n>    but we need to benchmark to pick), and ask apply to get rid of\n>    all these changes, which includes removal of added files, and\n>    resurrection of removed files.\n\nThanks for the explanation.\n\n> I think the original that did 'git reset -- \"$@\"' upfront lost new\n> paths from the index (without removing it from the working tree), I\n> am guessing that it is why \"clean\" was there to get rid of them, and\n> if that is the reason, I can understand why the original code was\n> behaving incorrectly---it would get rid of new files that did not\n> exist in HEAD correctly, but it also would remove untracked ones,\n> because after that first 'reset', the code couldn't tell between\n> them.\n> \n> And I think that is what we want, i.e. why the original was wrong\n> and how the new one fixes it, to describe in the log message to\n> justify this change.\n> \n> One thing that I didn't think through and you need to verify is if\n> we need to do anything extra to deal with binary files (in the old\n> days, we needed --full-index and --binary options to produce and\n> apply a binary patch; I do not offhand know if that is still the\n> case) in the final \"diff-index piped to apply -R --index\" dance.\n> \n> So I am asking you to fill in quite a lot of gaps that I didn't do\n> with only the above two-liner ;-)  You should take the authorship\n> and, if you like, credit me with helped-by: or something.\n\nThanks, I'll fill in the gaps, and send a new patch, hopefully over\nthe weekend.\n\n> Thanks.\n> \n> \n> >\n> > [...]\n> >\n> > --- >8 ---\n> > From: Junio C Hamano <gitster@pobox.com>\n> > Subject: [PATCH] stash: don't delete untracked files that match pathspec\n> >\n> > Currently when 'git stash push -- <pathspec>' is used, untracked files\n> > that match the pathspec will be deleted, even though they do not end up\n> > in a stash anywhere.\n> >\n> > Untracked files should be left along by 'git stash push -- <pathspec>'\n> > unless the --untracked or --all flags are given.  Fix that.\n> >\n> > Reported-by: Reid Price <reid.price@gmail.com>\n> > Test-by: Thomas Gummerer <t.gummerer@gmail.com>\n> > Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>\n> > ---\n> >  git-stash.sh     |  5 ++---\n> >  t/t3903-stash.sh | 16 ++++++++++++++++\n> >  2 files changed, 18 insertions(+), 3 deletions(-)\n> >\n> > diff --git a/git-stash.sh b/git-stash.sh\n> > index 1114005ce2..a979bfb665 100755\n> > --- a/git-stash.sh\n> > +++ b/git-stash.sh\n> > @@ -322,10 +322,9 @@ push_stash () {\n> >  \n> >  \t\tif test $# != 0\n> >  \t\tthen\n> > -\t\t\tgit reset -q -- \"$@\"\n> > -\t\t\tgit ls-files -z --modified -- \"$@\" |\n> > +\t\t\tgit ls-files -z -- \"$@\" |\n> >  \t\t\tgit checkout-index -z --force --stdin\n> > -\t\t\tgit clean --force -q -d -- \"$@\"\n> > +\t\t\tgit diff-index -p HEAD -- \"$@\" | git apply --index -R\n> >  \t\telse\n> >  \t\t\tgit reset --hard -q\n> >  \t\tfi\n> > diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh\n> > index 39c7f2ebd7..6952a031b2 100755\n> > --- a/t/t3903-stash.sh\n> > +++ b/t/t3903-stash.sh\n> > @@ -1064,4 +1064,20 @@ test_expect_success 'stash -k -- <pathspec> leaves unstaged files intact' '\n> >  \ttest foo,bar = $(cat foo),$(cat bar)\n> >  '\n> >  \n> > +test_expect_success 'stash -- <subdir> leaves untracked files in subdir intact' '\n> > +\tgit reset &&\n> > +\t>subdir/untracked &&\n> > +\t>subdir/tracked1 &&\n> > +\t>subdir/tracked2 &&\n> > +\tgit add subdir/tracked* &&\n> > +\tgit stash -- subdir/ &&\n> > +\ttest_path_is_missing subdir/tracked1 &&\n> > +\ttest_path_is_missing subdir/tracked2 &&\n> > +\ttest_path_is_file subdir/untracked &&\n> > +\tgit stash pop &&\n> > +\ttest_path_is_file subdir/tracked1 &&\n> > +\ttest_path_is_file subdir/tracked2 &&\n> > +\ttest_path_is_file subdir/untracked\n> > +'\n> > +\n> >  test_done\n"},{"id":"336034","messageId":"20180106002420.10467-1-t.gummerer@gmail.com","threadId":"47438","inReplyTo":"20180105200313.GF2641@hank","subject":"[PATCH v2] stash: don't delete untracked files that match pathspec","fromName":"Thomas Gummerer","fromEmail":"t.gummerer@gmail.com","sentAt":"2018-01-06T00:24:20Z","receivedAt":"2018-01-06T00:22:27Z","isPatch":true,"sender":{"key":"t.gummerer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/191004?v=4"},"body":"Currently when 'git stash push -- <pathspec>' is used, untracked files\nthat match the pathspec will be deleted, even though they do not end up\nin a stash anywhere.\n\nThis is because the original commit introducing the pathspec feature in\ngit stash push (df6bba0937 (\"stash: teach 'push' (and 'create_stash') to\nhonor pathspec\", 2017-02-28)) used the sequence of 'git reset <pathspec>\n&& git ls-files --modified <pathspec> | git checkout-index && git clean\n<pathspec>'.\n\nThe intention was to emulate what 'git reset --hard -- <pathspec>' would\ndo.  The call to 'git clean' was supposed to clean up the files that\nwere unstaged by 'git reset'.  This would work fine if the pathspec\ndoesn't match any files that were untracked before 'git stash push --\n<pathspec>'.  However if <pathspec> matches a file that was untracked\nbefore invoking the 'stash' command, all untracked files matching the\npathspec would inadvertently be deleted as well, even though they\nwouldn't end up in the stash, and are therefore lost.\n\nThis behaviour was never what was intended, only blobs that also end up\nin the stash should be reset to their state in HEAD, previously\nuntracked files should be left alone.\n\nTo achieve this, first match what's in the index and what's in the\nworking tree by adding all changes to the index, ask diff-index what\nchanged between HEAD and the current index, and then apply that patch in\nreverse to get rid of the changes, which includes removal of added\nfiles and resurrection of removed files.\n\nReported-by: Reid Price <reid.price@gmail.com>\nHelped-by: Junio C Hamano <gitster@pobox.com>\nSigned-off-by: Thomas Gummerer <t.gummerer@gmail.com>\n---\n> Thanks, I'll fill in the gaps, and send a new patch, hopefully over\n> the weekend.\n\nHere it is :)\n\nChanges since the previous version:\n - handle binary files correctly and add a test for that\n - updated commit message\n - took back authorship, and added Helped-by: Junio as he mentioned\n   would be the best way earlier in the thread.\n\n git-stash.sh     |  5 ++---\n t/t3903-stash.sh | 32 ++++++++++++++++++++++++++++++++\n 2 files changed, 34 insertions(+), 3 deletions(-)\n\ndiff --git a/git-stash.sh b/git-stash.sh\nindex 1114005ce2..fc8f8ae640 100755\n--- a/git-stash.sh\n+++ b/git-stash.sh\n@@ -322,10 +322,9 @@ push_stash () {\n \n \t\tif test $# != 0\n \t\tthen\n-\t\t\tgit reset -q -- \"$@\"\n-\t\t\tgit ls-files -z --modified -- \"$@\" |\n+\t\t\tgit add -u -- \"$@\" |\n \t\t\tgit checkout-index -z --force --stdin\n-\t\t\tgit clean --force -q -d -- \"$@\"\n+\t\t\tgit diff-index -p --cached --binary HEAD -- \"$@\" | git apply --index -R\n \t\telse\n \t\t\tgit reset --hard -q\n \t\tfi\ndiff --git a/t/t3903-stash.sh b/t/t3903-stash.sh\nindex 39c7f2ebd7..aefde7b172 100755\n--- a/t/t3903-stash.sh\n+++ b/t/t3903-stash.sh\n@@ -1064,4 +1064,36 @@ test_expect_success 'stash -k -- <pathspec> leaves unstaged files intact' '\n \ttest foo,bar = $(cat foo),$(cat bar)\n '\n \n+test_expect_success 'stash -- <subdir> leaves untracked files in subdir intact' '\n+\tgit reset &&\n+\t>subdir/untracked &&\n+\t>subdir/tracked1 &&\n+\t>subdir/tracked2 &&\n+\tgit add subdir/tracked* &&\n+\tgit stash -- subdir/ &&\n+\ttest_path_is_missing subdir/tracked1 &&\n+\ttest_path_is_missing subdir/tracked2 &&\n+\ttest_path_is_file subdir/untracked &&\n+\tgit stash pop &&\n+\ttest_path_is_file subdir/tracked1 &&\n+\ttest_path_is_file subdir/tracked2 &&\n+\ttest_path_is_file subdir/untracked\n+'\n+\n+test_expect_success 'stash -- <subdir> works with binary files' '\n+\tgit reset &&\n+\t>subdir/untracked &&\n+\t>subdir/tracked &&\n+\tcp \"$TEST_DIRECTORY\"/test-binary-1.png subdir/tracked-binary &&\n+\tgit add subdir/tracked* &&\n+\tgit stash -- subdir/ &&\n+\ttest_path_is_missing subdir/tracked &&\n+\ttest_path_is_missing subdir/tracked-binary &&\n+\ttest_path_is_file subdir/untracked &&\n+\tgit stash pop &&\n+\ttest_path_is_file subdir/tracked &&\n+\ttest_path_is_file subdir/tracked-binary &&\n+\ttest_path_is_file subdir/untracked\n+'\n+\n test_done\n-- \n2.15.1.620.gb9897f4670\n\n"}]}