{"thread":{"id":"39530","subject":"Re: [PATCH 3/3] stash: require a clean index to apply","startedAt":"2015-06-05T00:43:00Z","lastAt":"2015-06-26T04:15:34Z","messageCount":16,"participants":["Jonathan Kamens","Jeff King","bär","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"262989","messageId":"5570F094.10007@quantopian.com","threadId":"39530","inReplyTo":null,"subject":"Re: [PATCH 3/3] stash: require a clean index to apply","fromName":"Jonathan Kamens","fromEmail":"jkamens@quantopian.com","sentAt":"2015-06-05T00:43:00Z","receivedAt":"2015-06-05T00:43:00Z","isPatch":true,"sender":{"key":"jkamens@quantopian.com","avatar":null},"body":"I'm writing about the patch that Jeff King submitted on April 22, in \n<20150422193101.GC27945@peff.net>, in particular, \nhttps://github.com/git/git/commit/ed178ef13a26136d86ff4e33bb7b1afb5033f908 \n. It appears that this patch was included in git 2.4.2, and it breaks my \nworkflow.\n\nIn particular, I have a pre-commit hook whith does the following:\n\n1. Stash unstaged changes (\"git stash -k\").\n2. Run flake8 over all staged changes.\n3. If flake8 complains, then error out of the commit.\n4. Otherwise, apply the stash and exit.\n\nThis way I am prevented from committing staged changes that don't pass \nflake8. I can't imagine that this is a terribly uncommon workflow.\n\nThis worked fine until the aforementioned comment, after which my hook \ncomplains, \"Cannot apply stash: Your index contains uncommitted changes.\"\n\nThe reason I have to do things this way is as follows. Suppose I did the \nfollowing:\n\n1. Stage changes that have a flake8 violation.\n2. Fix the flake8 violation in the unstaged version of the staged file.\n3. Commit the previously staged changes.\n\nIf my commit hook runs over the unstaged version of the file, then it \nwon't detect the flake8 violation, and as a result the violation will be \ncommitted.\n\nIf anyone has a suggestion for how I can achieve the desired goal within \nthe constraints of the 2.4.2 version of git-stash.sh, I'd love to hear \nit. Otherwise, I'd like to ask for this patch to be reconsidered.\n\nThank you,\n\nJonathan Kamens\n"},{"id":"263148","messageId":"20150607124001.GA11042@peff.net","threadId":"39530","inReplyTo":"5570F094.10007@quantopian.com","subject":"Re: [PATCH 3/3] stash: require a clean index to apply","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-06-07T12:40:05Z","receivedAt":"2015-06-07T12:40:05Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Jun 04, 2015 at 08:43:00PM -0400, Jonathan Kamens wrote:\n\n> I'm writing about the patch that Jeff King submitted on April 22, in\n> <20150422193101.GC27945@peff.net>, in particular,\n> https://github.com/git/git/commit/ed178ef13a26136d86ff4e33bb7b1afb5033f908 .\n> It appears that this patch was included in git 2.4.2, and it breaks my\n> workflow.\n> \n> In particular, I have a pre-commit hook whith does the following:\n> \n> 1. Stash unstaged changes (\"git stash -k\").\n> 2. Run flake8 over all staged changes.\n> 3. If flake8 complains, then error out of the commit.\n> 4. Otherwise, apply the stash and exit.\n\nHrm. The new protection in v2.4.2 is meant to prevent you from losing\nyour index state during step 4 when we run into a conflict. But here you\nknow something that git doesn't: that we just created the stash based on\nthis same state, so it should apply cleanly.\n\nSo besides the obvious fix of reverting the patch, we could perhaps do\nsomething along the lines of:\n\n 1. Add a --force option to tell git to do it anyway.\n\n 2. Only have the protection kick in when we would in fact have a\n conflict. This is probably hard to implement, though, as we don't\n know until we do the merge (so it would probably involve teaching\n merge a mode where it bails immediately on conflicts rather than\n writing out the conflicted entries to the index).\n\nHowever, I am puzzled by something in your workflow: does it work when\nyou have staged and working tree changes to the same hunk? For example:\n\n [differing content for HEAD, index, and working tree]\n $ git init\n $ echo base >file && git add file && git commit -m base\n $ echo index >file && git add file\n $ echo working >file\n\n [make our stash]\n $ git stash -k\n Saved working directory and index state WIP on master: dc7f0dd base\n HEAD is now at dc7f0dd base\n\n [new version]\n $ git.v2.4.2 stash apply\n Cannot apply stash: Your index contains uncommitted changes.\n\n [old version]\n $ git.v2.4.1 stash apply\n Auto-merging file\n CONFLICT (content): Merge conflict in file\n $ git diff\n diff --cc file\n index 9015a7a,d26b33d..0000000\n --- a/file\n +++ b/file\n @@@ -1,1 -1,1 +1,5 @@@\n ++<<<<<<< Updated upstream\n +index\n ++=======\n + working\n ++>>>>>>> Stashed changes\n\nSo v2.4.1 shows a conflict, and loses the index state you had. Is there\nsomething more to your hook that I'm missing (stash options, or\nsomething else) that covers this case?\n\n> The reason I have to do things this way is as follows. Suppose I did the\n> following:\n> \n> 1. Stage changes that have a flake8 violation.\n> 2. Fix the flake8 violation in the unstaged version of the staged file.\n> 3. Commit the previously staged changes.\n> \n> If my commit hook runs over the unstaged version of the file, then it won't\n> detect the flake8 violation, and as a result the violation will be\n> committed.\n\nYeah, the fundamental pattern makes sense. You want to test what is\ngoing into the commit, not what happens to be in the working tree.\n\nOne way to do that would be to checkout the proposed index to a\ntemporary directory and operate on that. But of course that's\ninefficient if most of the files are unchanged.\n\nAre you running flake8 across the whole tree, or just on the files with\nproposed changes? Does it need to see the whole tree? If you can get\naway with feeding it single files, then it should be very efficient to\nloop over the output of \"git diff-index HEAD\" and feed the proposed\nupdated blobs to it. If you can get away with feeding single lines, then\nfeeding the actual diffs to it would be even better, but I assume that\nis not enough (I do not use flake8 myself).\n\n-Peff\n"},{"id":"263149","messageId":"20150607124714.GA17632@peff.net","threadId":"39530","inReplyTo":"20150607124001.GA11042@peff.net","subject":"Re: [PATCH 3/3] stash: require a clean index to apply","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-06-07T12:47:15Z","receivedAt":"2015-06-07T12:47:15Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Jun 07, 2015 at 08:40:01AM -0400, Jeff King wrote:\n\n> Are you running flake8 across the whole tree, or just on the files with\n> proposed changes? Does it need to see the whole tree? If you can get\n> away with feeding it single files, then it should be very efficient to\n> loop over the output of \"git diff-index HEAD\" and feed the proposed\n> updated blobs to it. If you can get away with feeding single lines, then\n> feeding the actual diffs to it would be even better, but I assume that\n> is not enough (I do not use flake8 myself).\n\nLike I said, I do not use it, but peeking at the flake8 source code, it\nhas an \"--install-hook\" option to set up a pre-commit hook. It looks\nlike the hook runs \"git diff-index --cached --name-only HEAD\" to get\nthe list of modified files, filters that only for \"*.py\", and then\ncopies the results into a temporary directory to operate on.\n\n-Peff\n"},{"id":"263514","messageId":"CADQvvWWpjZoXt7=8yAEt110pBTNtBfg7XfPOTbJRfPUH=J3OKw@mail.gmail.com","threadId":"39530","inReplyTo":"20150607124001.GA11042@peff.net","subject":"Re: [PATCH 3/3] stash: require a clean index to apply","fromName":"bär","fromEmail":"crashcookie@gmail.com","sentAt":"2015-06-10T18:19:41Z","receivedAt":"2015-06-10T18:19:41Z","isPatch":true,"sender":{"key":"crashcookie@gmail.com","avatar":"https://gravatar.com/avatar/32bf7cf3a471aefb063df50c6f41fa7ce41aea1241e91018bc9318e4a5560a02?d=mp&s=160"},"body":"On Sun, Jun 7, 2015 at 9:40 AM, Jeff King wrote:\n> Hrm. The new protection in v2.4.2 is meant to prevent you from losing\n> your index state during step 4 when we run into a conflict. But here you\n> know something that git doesn't: that we just created the stash based on\n> this same state, so it should apply cleanly.\n\n\nIt is strange that `git stash --keep-index && git stash pop` while\nhaving something in the index, fails with a \"Cannot apply stash: Your\nindex contains uncommitted changes.\" error, even if we have a\n`--force` param it find it awkward that one needs to force\napplying/pop'ing even though the stash was created from the same\nsnapshot where the stash is being merged with.\n\nI understand the original issue, but at least it was expected, when\nyou stash save/pop/apply, you should know what you are doing anyways.\n\n-- \nBer Clausen\n"},{"id":"263519","messageId":"20150610185635.GA22800@peff.net","threadId":"39530","inReplyTo":"CADQvvWWpjZoXt7=8yAEt110pBTNtBfg7XfPOTbJRfPUH=J3OKw@mail.gmail.com","subject":"Re: [PATCH 3/3] stash: require a clean index to apply","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-06-10T18:56:36Z","receivedAt":"2015-06-10T18:56:36Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jun 10, 2015 at 03:19:41PM -0300, bär wrote:\n\n> On Sun, Jun 7, 2015 at 9:40 AM, Jeff King wrote:\n> > Hrm. The new protection in v2.4.2 is meant to prevent you from losing\n> > your index state during step 4 when we run into a conflict. But here you\n> > know something that git doesn't: that we just created the stash based on\n> > this same state, so it should apply cleanly.\n> \n> \n> It is strange that `git stash --keep-index && git stash pop` while\n> having something in the index, fails with a \"Cannot apply stash: Your\n> index contains uncommitted changes.\" error, even if we have a\n> `--force` param it find it awkward that one needs to force\n> applying/pop'ing even though the stash was created from the same\n> snapshot where the stash is being merged with.\n> \n> I understand the original issue, but at least it was expected, when\n> you stash save/pop/apply, you should know what you are doing anyways.\n\nYeah, I would expect \"stash && pop\" to work in general. But I find it\npuzzling that it does not always with \"-k\" (this is with v2.3.x):\n\n $ git init -q\n $ echo head >file && git add file && git commit -qm head\n $ echo index >file && git add file\n $ echo tree >file\n $ git stash --keep-index && git stash pop\n Saved working directory and index state WIP on master: 74f6d33 head\n HEAD is now at 74f6d33 head\n Auto-merging file\n CONFLICT (content): Merge conflict in file\n\nSo I am trying to figure out what the use case here is. Clearly the\nabove is a toy case, but why is \"stash -k\" followed by a quick pop\nuseful in general? Certainly I use \"stash\" (without \"-k\") and a quick\npop all the time, and I think that is what stash was designed for.\n\nThe best use case I can think of is Jonathan's original: to see only the\nstaged content in the working tree, and then restore the original state.\nBut stash does not currently work very well for that, as shown above.\n\n-Peff\n"},{"id":"263524","messageId":"xmqq616vl6gm.fsf@gitster.dls.corp.google.com","threadId":"39530","inReplyTo":"20150610185635.GA22800@peff.net","subject":"Re: [PATCH 3/3] stash: require a clean index to apply","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-06-10T19:16:25Z","receivedAt":"2015-06-10T19:16:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> So I am trying to figure out what the use case here is. Clearly the\n> above is a toy case, but why is \"stash -k\" followed by a quick pop\n> useful in general? Certainly I use \"stash\" (without \"-k\") and a quick\n> pop all the time, and I think that is what stash was designed for.\n>\n> The best use case I can think of is Jonathan's original: to see only the\n> staged content in the working tree, and then restore the original state.\n> But stash does not currently work very well for that, as shown above.\n\nThe canonical use case for \"stash -k\" is to see only the content to\nbe committed (for testing), commit it after testing and then pop on\ntop of the committed result, which is the same as what you saw in\nthe working tree and the index when you did \"stash -k\". I do not\nthink \"stash -k && stash pop\" was in the design parameter when \"-k\"\nwas added (as you demonstrated, it would not fundamentally work\nreliably depending on the differences between HEAD-Index-Worktree).\n"},{"id":"263525","messageId":"20150610192734.GA23715@peff.net","threadId":"39530","inReplyTo":"xmqq616vl6gm.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 3/3] stash: require a clean index to apply","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-06-10T19:27:35Z","receivedAt":"2015-06-10T19:27:35Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jun 10, 2015 at 12:16:25PM -0700, Junio C Hamano wrote:\n\n> Jeff King writes:\n> \n> > So I am trying to figure out what the use case here is. Clearly the\n> > above is a toy case, but why is \"stash -k\" followed by a quick pop\n> > useful in general? Certainly I use \"stash\" (without \"-k\") and a quick\n> > pop all the time, and I think that is what stash was designed for.\n> >\n> > The best use case I can think of is Jonathan's original: to see only the\n> > staged content in the working tree, and then restore the original state.\n> > But stash does not currently work very well for that, as shown above.\n> \n> The canonical use case for \"stash -k\" is to see only the content to\n> be committed (for testing), commit it after testing and then pop on\n> top of the committed result, which is the same as what you saw in\n> the working tree and the index when you did \"stash -k\". I do not\n> think \"stash -k && stash pop\" was in the design parameter when \"-k\"\n> was added (as you demonstrated, it would not fundamentally work\n> reliably depending on the differences between HEAD-Index-Worktree).\n\nIt seems like applying a stash made with \"-k\" is fundamentally\nmisdesigned in the current code. We would want to apply to the working\ntree the difference between the index and the working tree, but instead\nwe try to apply the difference between the HEAD and the working tree.\nWhich is nonsensical for this use case (i.e., to apply the diff between\n$stash and $stash^2, not $stash^1).\n\nI don't think there is any way to tell that \"-k\" was used, though. But\neven if the user knew that, I do not think there is any option to tell\n\"stash apply\" to do it this way.\n\nI dunno. With respect to the original patch, I am OK if we just want to\nrevert it. This area of stash seems a bit under-designed IMHO, but if\npeople were happy enough with it before, I do not think the safety\nbenefit from ed178ef is that great (it is not saving you from destroying\nworking tree content, only the index state; the individual content blobs\nare still available from git-fsck).\n\n-Peff\n"},{"id":"263538","messageId":"CADQvvWWS35vO9XZAEc6Zb3PPRU1XBOn_Xe+1P3-RVOcSFjbHKw@mail.gmail.com","threadId":"39530","inReplyTo":"20150610192734.GA23715@peff.net","subject":"Re: [PATCH 3/3] stash: require a clean index to apply","fromName":"bär","fromEmail":"crashcookie@gmail.com","sentAt":"2015-06-10T21:54:35Z","receivedAt":"2015-06-10T21:54:35Z","isPatch":true,"sender":{"key":"crashcookie@gmail.com","avatar":"https://gravatar.com/avatar/32bf7cf3a471aefb063df50c6f41fa7ce41aea1241e91018bc9318e4a5560a02?d=mp&s=160"},"body":"On Wed, Jun 10, 2015 at 4:27 PM, Jeff King wrote:\n> I dunno. With respect to the original patch, I am OK if we just want to\n> revert it. This area of stash seems a bit under-designed IMHO, but if\n> people were happy enough with it before, I do not think the safety\n> benefit from ed178ef is that great (it is not saving you from destroying\n> working tree content, only the index state; the individual content blobs\n> are still available from git-fsck).\n\nI feel the same way, in fact I'm +1 to revert it until we figure out a\nbetter way to deal with this properly.\n\n-- \nBer Clausen\n"},{"id":"263869","messageId":"xmqqsi9samx1.fsf@gitster.dls.corp.google.com","threadId":"39530","inReplyTo":"20150610192734.GA23715@peff.net","subject":"Re: [PATCH 3/3] stash: require a clean index to apply","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-06-15T17:42:18Z","receivedAt":"2015-06-15T17:42:18Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> It seems like applying a stash made with \"-k\" is fundamentally\n> misdesigned in the current code. We would want to apply to the working\n> tree the difference between the index and the working tree, but instead\n> we try to apply the difference between the HEAD and the working tree.\n> Which is nonsensical for this use case (i.e., to apply the diff between\n> $stash and $stash^2, not $stash^1).\n>\n> I don't think there is any way to tell that \"-k\" was used, though. But\n> even if the user knew that, I do not think there is any option to tell\n> \"stash apply\" to do it this way.\n>\n> I dunno. With respect to the original patch, I am OK if we just want to\n> revert it. This area of stash seems a bit under-designed IMHO, but if\n> people were happy enough with it before, I do not think the safety\n> benefit from ed178ef is that great (it is not saving you from destroying\n> working tree content, only the index state; the individual content blobs\n> are still available from git-fsck).\n\nYeah, I agree. Somebody care to do the log message?\n\nThis is a tangent, but I'd actually not just agree with \"'stash -k'\nis a bit under-designed\" but also would say something stronger than\nthat, IMHO ;-)\n\nThe very original idea of \"git stash\" to ssave away working tree\nchanges with respect to HEAD without bothering the index was at\nleast internally consistent. You save away and then may later pop\nthe change on top of a different commit after dealing with an\nemergency, at which time there may be conflicts that you would need\nto resolve, and because it is only between HEAD and working tree,\nyou can safely use the index to resolve the conflicts just like in\nany other situation when you help Git to resolve them. In that\ncontext, the flaw ed178ef1 (stash: require a clean index to apply,\n2015-04-22) tried to correct made a lot of sense.\n\nWhat broke the entire system was the throwing the \"save the index,\ntoo\" into the mix, which was done without careful thinking. While\nit made the command more useful (when it works), it made the command\ninternally inconsistent---the command has to punt restoring index\nstate when it can't, for example, losing the state it tried to save.\nI think that is where the \"under-designed\" started, I would think;\nof course, \"-k\" needed to build on top of \"save also the index\",\nbut that is not the primary culprit.\n"},{"id":"263875","messageId":"20150615182721.GA4041@peff.net","threadId":"39530","inReplyTo":"xmqqsi9samx1.fsf@gitster.dls.corp.google.com","subject":"[PATCH] Revert \"stash: require a clean index to apply\"","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-06-15T18:27:22Z","receivedAt":"2015-06-15T18:27:22Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Jun 15, 2015 at 10:42:18AM -0700, Junio C Hamano wrote:\n\n> > I dunno. With respect to the original patch, I am OK if we just want to\n> > revert it. This area of stash seems a bit under-designed IMHO, but if\n> > people were happy enough with it before, I do not think the safety\n> > benefit from ed178ef is that great (it is not saving you from destroying\n> > working tree content, only the index state; the individual content blobs\n> > are still available from git-fsck).\n> \n> Yeah, I agree. Somebody care to do the log message?\n\nPatch is below. It's a straight revert. The other option would be to\nallow it with \"--force\", and teach people to use that. I'm not sure it's\nworth the effort.\n\n> This is a tangent, but I'd actually not just agree with \"'stash -k'\n> is a bit under-designed\" but also would say something stronger than\n> that, IMHO ;-)\n\nYeah, I agree with everything you said here. :)\n\n-- >8 --\nSubject: Revert \"stash: require a clean index to apply\"\n\nThis reverts commit ed178ef13a26136d86ff4e33bb7b1afb5033f908.\n\nThat commit was an attempt to improve the safety of applying\na stash, because the application process may create\nconflicted index entries, after which it is hard to restore\nthe original index state.\n\nUnfortunately, this hurts some common workflows around \"git\nstash -k\", like:\n\n git add -p ;# (1) stage set of proposed changes\n git stash -k ;# (2) get rid of everything else\n make test ;# (3) make sure proposal is reasonable\n git stash apply ;# (4) restore original working tree\n\nIf you \"git commit\" between steps (3) and (4), then this\njust works. However, if these steps are part of a pre-commit\nhook, you don't have that opportunity (you have to restore\nthe original state regardless of whether the tests passed or\nfailed).\n\nIt's possible that we could provide better tools for this\nsort of workflow. In particular, even before ed178ef, it\ncould fail with a conflict if there were conflicting hunks\nin the working tree and index (since the \"stash -k\" puts the\nindex version into the working tree, and we then attempt to\napply the differences between HEAD and the old working tree\non top of that). But the fact remains that people have been\nusing it happily for a while, and the safety provided by\ned178ef is simply not that great. Let's revert it for now.\nIn the long run, people can work on improving stash for this\nsort of workflow, but the safety tradeoff is not worth it in\nthe meantime.\n\nSigned-off-by: Jeff King \n---\nThis is directly on jk/stash-require-clean-index, but it should merge\nfine up to master.\n\n git-stash.sh | 2 --\n t/t3903-stash.sh | 7 -------\n 2 files changed, 9 deletions(-)\n\ndiff --git a/git-stash.sh b/git-stash.sh\nindex cc28368..d4cf818 100755\n--- a/git-stash.sh\n+++ b/git-stash.sh\n@@ -442,8 +442,6 @@ apply_stash () {\n \tassert_stash_like \"$@\"\n \n \tgit update-index -q --refresh || die \"$(gettext \"unable to refresh index\")\"\n-\tgit diff-index --cached --quiet --ignore-submodules HEAD -- ||\n-\t\tdie \"$(gettext \"Cannot apply stash: Your index contains uncommitted changes.\")\"\n \n \t# current index state\n \tc_tree=$(git write-tree) ||\ndiff --git a/t/t3903-stash.sh b/t/t3903-stash.sh\nindex 0746eee..f179c93 100755\n--- a/t/t3903-stash.sh\n+++ b/t/t3903-stash.sh\n@@ -45,13 +45,6 @@ test_expect_success 'applying bogus stash does nothing' '\n \ttest_cmp expect file\n '\n \n-test_expect_success 'apply requires a clean index' '\n-\ttest_when_finished \"git reset --hard\" &&\n-\techo changed >other-file &&\n-\tgit add other-file &&\n-\ttest_must_fail git stash apply\n-'\n-\n test_expect_success 'apply does not need clean working directory' '\n \techo 4 >other-file &&\n \tgit stash apply &&\n-- \n2.4.3.699.g84b4da7\n"},{"id":"263897","messageId":"xmqqy4jk7muq.fsf@gitster.dls.corp.google.com","threadId":"39530","inReplyTo":"20150615182721.GA4041@peff.net","subject":"Re: [PATCH] Revert \"stash: require a clean index to apply\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-06-15T20:11:57Z","receivedAt":"2015-06-15T20:11:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> Subject: Revert \"stash: require a clean index to apply\"\n>\n> This reverts commit ed178ef13a26136d86ff4e33bb7b1afb5033f908.\n\nThanks.\n"},{"id":"264911","messageId":"fab199f5-b69a-4e07-9fb4-6a13b3ed81dd.maildroid@localhost","threadId":"39530","inReplyTo":"xmqqy4jk7muq.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] Revert \"stash: require a clean index to apply\"","fromName":"Jonathan Kamens","fromEmail":"jkamens@quantopian.com","sentAt":"2015-06-25T21:51:40Z","receivedAt":"2015-06-25T21:51:40Z","isPatch":true,"sender":{"key":"jkamens@quantopian.com","avatar":null},"body":"Is this revert going to be applied and released?\n\nSent from my Android device\n\n\n\n-----Original Message-----\nFrom: Junio C Hamano \nTo: Jeff King \nCc: \"bär\" , Jonathan Kamens , Git List \nSent: Mon, 15 Jun 2015 4:11 PM\nSubject: Re: [PATCH] Revert \"stash: require a clean index to apply\"\n\nJeff King writes:\n\n> Subject: Revert \"stash: require a clean index to apply\"\n>\n> This reverts commit ed178ef13a26136d86ff4e33bb7b1afb5033f908.\n\nThanks.\n"},{"id":"264919","messageId":"20150626002706.GA6367@peff.net","threadId":"39530","inReplyTo":"f06e573d-02e3-47e9-85d8-3bb6551d72f5.maildroid@localhost","subject":"Re: [PATCH] Revert \"stash: require a clean index to apply\"","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-06-26T00:27:07Z","receivedAt":"2015-06-26T00:27:07Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Jun 25, 2015 at 05:49:11PM -0400, Jonathan Kamens wrote:\n\n> Is this revert going to be applied and released?\n\nIt is on \"master\", and part of v2.5.0-rc0 (it is not part of v2.4.x, because\nthe original problem was not there, either).\n\n-Peff\n"},{"id":"264929","messageId":"558CA717.2040400@quantopian.com","threadId":"39530","inReplyTo":"20150626002706.GA6367@peff.net","subject":"Re: [PATCH] Revert \"stash: require a clean index to apply\"","fromName":"Jonathan Kamens","fromEmail":"jkamens@quantopian.com","sentAt":"2015-06-26T01:12:55Z","receivedAt":"2015-06-26T01:12:55Z","isPatch":true,"sender":{"key":"jkamens@quantopian.com","avatar":null},"body":"Um.\n\nI encountered this issue in git 2.4.3 on Fedora 22.\n\nThese lines appear in /usr/libexec/git-core/git-stash on my Fedora 22 \nsystem:\n\n git diff-index --cached --quiet --ignore-submodules HEAD -- ||\n die \"$(gettext \"Cannot apply stash: Your index contains \nuncommitted changes.\")\"\n\nThey also appear in \nhttps://github.com/git/git/blob/69f9a6e54a46c4a75dff680047a465d04cca20ca/git-stash.sh#L445 \n, which is the commit tagged v2.4.3.\n\nIn fact, it appears they were released in v2.4.2, at least according to \nhttps://github.com/git/git/compare/v2.4.1...v2.4.2 .\n\nSo it appears to me that this patch was, in fact, released in v2.4.x and \ntherefore needs to be reverted in v2.4.x.\n\n jik\n\nOn 06/25/2015 08:27 PM, Jeff King wrote:\n> On Thu, Jun 25, 2015 at 05:49:11PM -0400, Jonathan Kamens wrote:\n>\n>> Is this revert going to be applied and released?\n> It is on \"master\", and part of v2.5.0-rc0 (it is not part of v2.4.x, because\n> the original problem was not there, either).\n>\n> -Peff\n"},{"id":"264930","messageId":"20150626040325.GA8559@peff.net","threadId":"39530","inReplyTo":"558CA717.2040400@quantopian.com","subject":"Re: [PATCH] Revert \"stash: require a clean index to apply\"","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-06-26T04:03:25Z","receivedAt":"2015-06-26T04:03:25Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Jun 25, 2015 at 09:12:55PM -0400, Jonathan Kamens wrote:\n\n> I encountered this issue in git 2.4.3 on Fedora 22.\n\nAh, sorry, you're right. I must have fed the wrong sha1 to \"git tag\n--contains\" earlier.\n\nI agree it can probably go onto the v2.4.x maintenance track. It is\nalready in v2.5.0-rc0.\n\n-Peff\n"},{"id":"264932","messageId":"CAPc5daUXw-ZUHrM+7Ff8-KOPFiPXLEKGZ3O4sPeZ0KCKbgrUig@mail.gmail.com","threadId":"39530","inReplyTo":"20150626040325.GA8559@peff.net","subject":"Re: [PATCH] Revert \"stash: require a clean index to apply\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-06-26T04:15:34Z","receivedAt":"2015-06-26T04:15:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"On Thu, Jun 25, 2015 at 9:03 PM, Jeff King wrote:\n> On Thu, Jun 25, 2015 at 09:12:55PM -0400, Jonathan Kamens wrote:\n>\n>> I encountered this issue in git 2.4.3 on Fedora 22.\n>\n> Ah, sorry, you're right. I must have fed the wrong sha1 to \"git tag\n> --contains\" earlier.\n>\n> I agree it can probably go onto the v2.4.x maintenance track. It is\n> already in v2.5.0-rc0.\n\nYeah, thanks for clarifying while I was away. As with any other changes,\na revert also follows the \"master first and then maintenance tracks later\"\npattern. Perhaps in 2.4.6 which I expect we would do during the 2.5-rc\nperiod.\n"}]}