{"thread":{"id":"11729","subject":"stg clean removes conflicting patch","startedAt":"2008-01-25T03:55:17Z","lastAt":"2008-01-25T10:17:27Z","messageCount":5,"participants":["Pavel Roskin","Karl Hasselström","Catalin Marinas"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"66534","messageId":"1201233317.2811.17.camel@dv","threadId":"11729","inReplyTo":null,"subject":"stg clean removes conflicting patch","fromName":"Pavel Roskin","fromEmail":"proski@gnu.org","sentAt":"2008-01-25T03:55:17Z","receivedAt":"2008-01-25T03:55:17Z","isPatch":false,"sender":{"key":"proski@gnu.org","avatar":null},"body":"Hello!\n\nIf \"stg push\" fails, the subsequent \"stg clean\" will remove the patch\nthat could not been applied. I think it's wrong. Especially when doing\n\"stg pull\", it can happen that I want to run \"stg clean\" to get rid of\nthe patches applied upstream so I can concentrate on the conflict.\nInstead, the conflicting patch is removed too.\n\nI've made a patch for the testsuite that should pass once the bug is\nfixed. Try removing \"stg clean\" from the test. and it will pass. But\n\"stg clean\" should make no difference here.\n\nAdd test to ensure that \"stg clean\" preserves conflicting patches\n\nFrom: Pavel Roskin \n\nSigned-off-by: Pavel Roskin \n---\n\n t/t2500-clean.sh | 17 +++++++++++++++++\n 1 files changed, 17 insertions(+), 0 deletions(-)\n\n\ndiff --git a/t/t2500-clean.sh b/t/t2500-clean.sh\nindex 3364c18..ad8f892 100755\n--- a/t/t2500-clean.sh\n+++ b/t/t2500-clean.sh\n@@ -24,4 +24,21 @@ test_expect_success 'Clean empty patches' '\n [ \"$(echo $(stg unapplied))\" = \"\" ]\n '\n \n+test_expect_success 'Create a conflict' '\n+ stg new p1 -m p1 &&\n+ echo bar > foo.txt &&\n+ stg refresh &&\n+ stg pop &&\n+ stg new p2 -m p2\n+ echo quux > foo.txt &&\n+ stg refresh &&\n+ ! stg push\n+'\n+\n+test_expect_success 'Make sure conflicting patches are preserved' '\n+ stg clean &&\n+ [ \"$(echo $(stg applied))\" = \"p0 p2 p1\" ] &&\n+ [ \"$(echo $(stg unapplied))\" = \"\" ]\n+'\n+\n test_done\n\n\n-- \nRegards,\nPavel Roskin\n"},{"id":"66543","messageId":"20080125080434.GA5599@diana.vm.bytemark.co.uk","threadId":"11729","inReplyTo":"1201233317.2811.17.camel@dv","subject":"Re: stg clean removes conflicting patch","fromName":"Karl Hasselström","fromEmail":"kha@treskal.com","sentAt":"2008-01-25T08:04:34Z","receivedAt":"2008-01-25T08:04:34Z","isPatch":false,"sender":{"key":"kha@treskal.com","avatar":"https://gravatar.com/avatar/f0120c734b5279b345075a28521e1ac66acb20c9913ffe9bf6ae97e53f7f3f13?d=mp&s=160"},"body":"On 2008-01-24 22:55:17 -0500, Pavel Roskin wrote:\n\n> If \"stg push\" fails, the subsequent \"stg clean\" will remove the\n> patch that could not been applied. I think it's wrong.\n\nI agree. It's consistent -- a conflicting patch is empty -- but\nclearly the wrong thing to do from a usability perspective.\n\n> I've made a patch for the testsuite that should pass once the bug is\n> fixed. Try removing \"stg clean\" from the test. and it will pass. But\n> \"stg clean\" should make no difference here.\n\nGood!\n\nFor known-to-be-failing tests, you can use test_expect_failure. I'll\namend your patch to do that when I pick it up (if Catalin doesn't beat\nme to it).\n\n-- \nKarl Hasselström, kha@treskal.com\n www.treskal.com/kalle\n"},{"id":"66547","messageId":"20080125034022.quettsgqsgck0k0o@webmail.spamcop.net","threadId":"11729","inReplyTo":"20080125080434.GA5599@diana.vm.bytemark.co.uk","subject":"Re: stg clean removes conflicting patch","fromName":"Pavel Roskin","fromEmail":"proski@gnu.org","sentAt":"2008-01-25T08:40:22Z","receivedAt":"2008-01-25T08:40:22Z","isPatch":false,"sender":{"key":"proski@gnu.org","avatar":null},"body":"Quoting Karl Hasselström :\n\n> For known-to-be-failing tests, you can use test_expect_failure. I'll\n> amend your patch to do that when I pick it up (if Catalin doesn't beat\n> me to it).\n\nYes, please go ahead with the change. I just wasn't aware of it.\n\n-- \nRegards,\nPavel Roskin\n"},{"id":"66551","messageId":"b0943d9e0801250153t30c5b9b8w4c08af107cfdf202@mail.gmail.com","threadId":"11729","inReplyTo":"20080125080434.GA5599@diana.vm.bytemark.co.uk","subject":"Re: stg clean removes conflicting patch","fromName":"Catalin Marinas","fromEmail":"catalin.marinas@gmail.com","sentAt":"2008-01-25T09:53:46Z","receivedAt":"2008-01-25T09:53:46Z","isPatch":false,"sender":{"key":"catalin.marinas@gmail.com","avatar":null},"body":"On 25/01/2008, Karl Hasselström wrote:\n> On 2008-01-24 22:55:17 -0500, Pavel Roskin wrote:\n>\n> > If \"stg push\" fails, the subsequent \"stg clean\" will remove the\n> > patch that could not been applied. I think it's wrong.\n>\n> I agree. It's consistent -- a conflicting patch is empty -- but\n> clearly the wrong thing to do from a usability perspective.\n\nGot broken by commit fe1cee2e49d9995852ba92d8fba1d064acf2fca9 which\nremoves the check_conflicts() call. As I said in a different post, we\nshould add these back (and to the 'goto' command as well) to make\nStGIT safer.\n\n> > I've made a patch for the testsuite that should pass once the bug is\n> > fixed. Try removing \"stg clean\" from the test. and it will pass. But\n> > \"stg clean\" should make no difference here.\n>\n> Good!\n>\n> For known-to-be-failing tests, you can use test_expect_failure. I'll\n> amend your patch to do that when I pick it up (if Catalin doesn't beat\n> me to it).\n\nProbably not, I'm really busy for one more week with a Linux kernel release.\n\n-- \nCatalin\n"},{"id":"66555","messageId":"20080125101727.GA7101@diana.vm.bytemark.co.uk","threadId":"11729","inReplyTo":"b0943d9e0801250153t30c5b9b8w4c08af107cfdf202@mail.gmail.com","subject":"Re: stg clean removes conflicting patch","fromName":"Karl Hasselström","fromEmail":"kha@treskal.com","sentAt":"2008-01-25T10:17:27Z","receivedAt":"2008-01-25T10:17:27Z","isPatch":false,"sender":{"key":"kha@treskal.com","avatar":"https://gravatar.com/avatar/f0120c734b5279b345075a28521e1ac66acb20c9913ffe9bf6ae97e53f7f3f13?d=mp&s=160"},"body":"On 2008-01-25 09:53:46 +0000, Catalin Marinas wrote:\n\n> On 25/01/2008, Karl Hasselström wrote:\n>\n> > On 2008-01-24 22:55:17 -0500, Pavel Roskin wrote:\n> >\n> > > If \"stg push\" fails, the subsequent \"stg clean\" will remove the\n> > > patch that could not been applied. I think it's wrong.\n> >\n> > I agree. It's consistent -- a conflicting patch is empty -- but\n> > clearly the wrong thing to do from a usability perspective.\n>\n> Got broken by commit fe1cee2e49d9995852ba92d8fba1d064acf2fca9 which\n> removes the check_conflicts() call.\n\nAh, thanks. I didn't realize it used to work.\n\n> As I said in a different post, we should add these back (and to the\n> 'goto' command as well) to make StGIT safer.\n\nThe right thing to do would be to check for conflicts before\nattempting any kind of modification, I guess.\n\n-- \nKarl Hasselström, kha@treskal.com\n www.treskal.com/kalle\n"}]}