# stg clean removes conflicting patch 5 messages from 2008-01-25 to 2008-01-25. Participants: Pavel Roskin, Karl Hasselström, Catalin Marinas. Thread: https://gitlist.dev/t/11729 ## Pavel Roskin, 2008-01-25 03:55 Subject: stg clean removes conflicting patch Message-ID: <1201233317.2811.17.camel@dv> URL: https://gitlist.dev/e/1201233317.2811.17.camel%40dv ``` Hello! If "stg push" fails, the subsequent "stg clean" will remove the patch that could not been applied. I think it's wrong. Especially when doing "stg pull", it can happen that I want to run "stg clean" to get rid of the patches applied upstream so I can concentrate on the conflict. Instead, the conflicting patch is removed too. I've made a patch for the testsuite that should pass once the bug is fixed. Try removing "stg clean" from the test. and it will pass. But "stg clean" should make no difference here. Add test to ensure that "stg clean" preserves conflicting patches From: Pavel Roskin Signed-off-by: Pavel Roskin --- t/t2500-clean.sh | 17 +++++++++++++++++ 1 files changed, 17 insertions(+), 0 deletions(-) diff --git a/t/t2500-clean.sh b/t/t2500-clean.sh index 3364c18..ad8f892 100755 --- a/t/t2500-clean.sh +++ b/t/t2500-clean.sh @@ -24,4 +24,21 @@ test_expect_success 'Clean empty patches' ' [ "$(echo $(stg unapplied))" = "" ] ' +test_expect_success 'Create a conflict' ' + stg new p1 -m p1 && + echo bar > foo.txt && + stg refresh && + stg pop && + stg new p2 -m p2 + echo quux > foo.txt && + stg refresh && + ! stg push +' + +test_expect_success 'Make sure conflicting patches are preserved' ' + stg clean && + [ "$(echo $(stg applied))" = "p0 p2 p1" ] && + [ "$(echo $(stg unapplied))" = "" ] +' + test_done -- Regards, Pavel Roskin ``` ## Karl Hasselström, 2008-01-25 08:04 Subject: Re: stg clean removes conflicting patch Message-ID: <20080125080434.GA5599@diana.vm.bytemark.co.uk> URL: https://gitlist.dev/e/20080125080434.GA5599%40diana.vm.bytemark.co.uk In-Reply-To: <1201233317.2811.17.camel@dv> ``` On 2008-01-24 22:55:17 -0500, Pavel Roskin wrote: > If "stg push" fails, the subsequent "stg clean" will remove the > patch that could not been applied. I think it's wrong. I agree. It's consistent -- a conflicting patch is empty -- but clearly the wrong thing to do from a usability perspective. > I've made a patch for the testsuite that should pass once the bug is > fixed. Try removing "stg clean" from the test. and it will pass. But > "stg clean" should make no difference here. Good! For known-to-be-failing tests, you can use test_expect_failure. I'll amend your patch to do that when I pick it up (if Catalin doesn't beat me to it). -- Karl Hasselström, kha@treskal.com www.treskal.com/kalle ``` ## Pavel Roskin, 2008-01-25 08:40 Subject: Re: stg clean removes conflicting patch Message-ID: <20080125034022.quettsgqsgck0k0o@webmail.spamcop.net> URL: https://gitlist.dev/e/20080125034022.quettsgqsgck0k0o%40webmail.spamcop.net In-Reply-To: <20080125080434.GA5599@diana.vm.bytemark.co.uk> ``` Quoting Karl Hasselström : > For known-to-be-failing tests, you can use test_expect_failure. I'll > amend your patch to do that when I pick it up (if Catalin doesn't beat > me to it). Yes, please go ahead with the change. I just wasn't aware of it. -- Regards, Pavel Roskin ``` ## Catalin Marinas, 2008-01-25 09:53 Subject: Re: stg clean removes conflicting patch Message-ID: URL: https://gitlist.dev/e/b0943d9e0801250153t30c5b9b8w4c08af107cfdf202%40mail.gmail.com In-Reply-To: <20080125080434.GA5599@diana.vm.bytemark.co.uk> ``` On 25/01/2008, Karl Hasselström wrote: > On 2008-01-24 22:55:17 -0500, Pavel Roskin wrote: > > > If "stg push" fails, the subsequent "stg clean" will remove the > > patch that could not been applied. I think it's wrong. > > I agree. It's consistent -- a conflicting patch is empty -- but > clearly the wrong thing to do from a usability perspective. Got broken by commit fe1cee2e49d9995852ba92d8fba1d064acf2fca9 which removes the check_conflicts() call. As I said in a different post, we should add these back (and to the 'goto' command as well) to make StGIT safer. > > I've made a patch for the testsuite that should pass once the bug is > > fixed. Try removing "stg clean" from the test. and it will pass. But > > "stg clean" should make no difference here. > > Good! > > For known-to-be-failing tests, you can use test_expect_failure. I'll > amend your patch to do that when I pick it up (if Catalin doesn't beat > me to it). Probably not, I'm really busy for one more week with a Linux kernel release. -- Catalin ``` ## Karl Hasselström, 2008-01-25 10:17 Subject: Re: stg clean removes conflicting patch Message-ID: <20080125101727.GA7101@diana.vm.bytemark.co.uk> URL: https://gitlist.dev/e/20080125101727.GA7101%40diana.vm.bytemark.co.uk In-Reply-To: ``` On 2008-01-25 09:53:46 +0000, Catalin Marinas wrote: > On 25/01/2008, Karl Hasselström wrote: > > > On 2008-01-24 22:55:17 -0500, Pavel Roskin wrote: > > > > > If "stg push" fails, the subsequent "stg clean" will remove the > > > patch that could not been applied. I think it's wrong. > > > > I agree. It's consistent -- a conflicting patch is empty -- but > > clearly the wrong thing to do from a usability perspective. > > Got broken by commit fe1cee2e49d9995852ba92d8fba1d064acf2fca9 which > removes the check_conflicts() call. Ah, thanks. I didn't realize it used to work. > As I said in a different post, we should add these back (and to the > 'goto' command as well) to make StGIT safer. The right thing to do would be to check for conflicts before attempting any kind of modification, I guess. -- Karl Hasselström, kha@treskal.com www.treskal.com/kalle ```