# [PATCH] add test case for rebase of empty commit 8 messages from 2012-06-27 to 2012-07-03. Participants: Martin von Zweigbergk, Junio C Hamano, Neil Horman. Thread: https://gitlist.dev/t/30916 ## Martin von Zweigbergk, 2012-06-27 16:22 Subject: [PATCH] add test case for rebase of empty commit Message-ID: <1340814121-23813-1-git-send-email-martin.von.zweigbergk@gmail.com> URL: https://gitlist.dev/e/1340814121-23813-1-git-send-email-martin.von.zweigbergk%40gmail.com ``` --- t/t3401-rebase-partial.sh | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/t/t3401-rebase-partial.sh b/t/t3401-rebase-partial.sh index 7ba1797..7f8693b 100755 --- a/t/t3401-rebase-partial.sh +++ b/t/t3401-rebase-partial.sh @@ -42,4 +42,12 @@ test_expect_success 'rebase --merge topic branch that was partially merged upstr test_path_is_missing .git/rebase-merge ' +test_expect_success 'rebase ignores empty commit' ' + git reset --hard A && + git commit --allow-empty -m empty && + test_commit D && + git rebase C && + test $(git log --format=%s C..) = "D" +' + test_done -- 1.7.9.3.327.g2980b ``` ## Junio C Hamano, 2012-06-27 21:02 Subject: Re: [PATCH] add test case for rebase of empty commit Message-ID: <7vr4t079jp.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7vr4t079jp.fsf%40alter.siamese.dyndns.org In-Reply-To: <1340814121-23813-1-git-send-email-martin.von.zweigbergk@gmail.com> ``` Thanks. We recently had a topic to add an option to allow rebase to carry empty commits forward, but I notice that it only had tests for the component cherry-pick to keep empty or redundant commits, so it may not be a bad idea to add tests for that series to the same t3401 after this commit (Neil Horman CC'ed). ``` ## Neil Horman, 2012-06-28 11:30 Subject: Re: [PATCH] add test case for rebase of empty commit Message-ID: <20120628113057.GA29277@hmsreliant.think-freely.org> URL: https://gitlist.dev/e/20120628113057.GA29277%40hmsreliant.think-freely.org In-Reply-To: <7vr4t079jp.fsf@alter.siamese.dyndns.org> ``` On Wed, Jun 27, 2012 at 02:02:34PM -0700, Junio C Hamano wrote: > Thanks. > > We recently had a topic to add an option to allow rebase to carry > empty commits forward, but I notice that it only had tests for the > component cherry-pick to keep empty or redundant commits, so it may > not be a bad idea to add tests for that series to the same t3401 > after this commit (Neil Horman CC'ed). > > So if I understand correctly, the desire is to augment t3401 such that it adds a test in which both of the commits in the local branch are empty, and still correctly identifies the one that was cherry-picked and only adds the remaining one during the rebase? Yes, I think that sounds like a good idea. I'm in the middle of an sctp project at the moment, but I expect to complete it in the next few days. I can look into writing this next week if you like. Thanks & Regards Neil ``` ## Neil Horman, 2012-07-03 18:20 Subject: Re: [PATCH] add test case for rebase of empty commit Message-ID: <20120703182000.GB10864@hmsreliant.think-freely.org> URL: https://gitlist.dev/e/20120703182000.GB10864%40hmsreliant.think-freely.org In-Reply-To: <7vr4t079jp.fsf@alter.siamese.dyndns.org> ``` On Wed, Jun 27, 2012 at 02:02:34PM -0700, Junio C Hamano wrote: > Thanks. > > We recently had a topic to add an option to allow rebase to carry > empty commits forward, but I notice that it only had tests for the > component cherry-pick to keep empty or redundant commits, so it may > not be a bad idea to add tests for that series to the same t3401 > after this commit (Neil Horman CC'ed). > > So, I've been thinking about this some, and I'm a bit stuck on it. Reading the test description for t3401, I see that we're testing gits ability to detect patches merged upstream when doing a rebase. That said, how are we supposed to differentiate between upstream empty patches that have been cherry-picked or merged, and local branch empty changes that haven't. As humans we can see that the changelog might be the same, but git has no way to detect that, and if --allow-empty is specified will just apply any empty patch it finds between the two branches merge base and the topic branch head. Does anyone have an idea as to how we should detect such duplication? Neil ``` ## Junio C Hamano, 2012-07-03 19:00 Subject: Re: [PATCH] add test case for rebase of empty commit Message-ID: <7vtxxovfec.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7vtxxovfec.fsf%40alter.siamese.dyndns.org In-Reply-To: <20120703182000.GB10864@hmsreliant.think-freely.org> ``` Neil Horman writes: > So, I've been thinking about this some, and I'm a bit stuck on it. Reading the > test description for t3401, I see that we're testing gits ability to detect > patches merged upstream when doing a rebase. That said, how are we supposed to > differentiate between upstream empty patches that have been cherry-picked or > merged, and local branch empty changes that haven't. As humans we can see that > the changelog might be the same, but git has no way to detect that, and if > --allow-empty is specified will just apply any empty patch it finds between the > two branches merge base and the topic branch head. Does anyone have an idea as > to how we should detect such duplication? The changelog might be similar or textually identical, but it is entirely a different matter if it makes sense taken out of the context (i.e. cherry-picked). So I would personally do not bother "filtering" about them too much---if you ask for empties, you will get all empties. ``` ## Neil Horman, 2012-07-03 20:31 Subject: Re: [PATCH] add test case for rebase of empty commit Message-ID: <20120703203136.GC10864@hmsreliant.think-freely.org> URL: https://gitlist.dev/e/20120703203136.GC10864%40hmsreliant.think-freely.org In-Reply-To: <7vtxxovfec.fsf@alter.siamese.dyndns.org> ``` On Tue, Jul 03, 2012 at 12:00:27PM -0700, Junio C Hamano wrote: > Neil Horman writes: > > > So, I've been thinking about this some, and I'm a bit stuck on it. Reading the > > test description for t3401, I see that we're testing gits ability to detect > > patches merged upstream when doing a rebase. That said, how are we supposed to > > differentiate between upstream empty patches that have been cherry-picked or > > merged, and local branch empty changes that haven't. As humans we can see that > > the changelog might be the same, but git has no way to detect that, and if > > --allow-empty is specified will just apply any empty patch it finds between the > > two branches merge base and the topic branch head. Does anyone have an idea as > > to how we should detect such duplication? > > The changelog might be similar or textually identical, but it is > entirely a different matter if it makes sense taken out of the > context (i.e. cherry-picked). So I would personally do not bother > "filtering" about them too much---if you ask for empties, you will > get all empties. > Ok, copy that. Thanks! Neil ``` ## Junio C Hamano, 2012-07-03 21:13 Subject: Re: [PATCH] add test case for rebase of empty commit Message-ID: <7vpq8ctune.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7vpq8ctune.fsf%40alter.siamese.dyndns.org In-Reply-To: <20120703203136.GC10864@hmsreliant.think-freely.org> ``` Neil Horman writes: > On Tue, Jul 03, 2012 at 12:00:27PM -0700, Junio C Hamano wrote: >> >> The changelog might be similar or textually identical, but it is >> entirely a different matter if it makes sense taken out of the >> context (i.e. cherry-picked). So I would personally do not bother >> "filtering" about them too much---if you ask for empties, you will >> get all empties. >> > Ok, copy that. That was somewhat unexpected, though ;-) It was 30% tongue-in-cheek comment. People who want to keep the empty commits in the history may want some filtering. As I am not among them, I do not think of anything useful (other than "filter all empty ones away", that is). ``` ## Neil Horman, 2012-07-03 23:40 Subject: Re: [PATCH] add test case for rebase of empty commit Message-ID: <20120703234048.GA10224@neilslaptop.think-freely.org> URL: https://gitlist.dev/e/20120703234048.GA10224%40neilslaptop.think-freely.org In-Reply-To: <7vpq8ctune.fsf@alter.siamese.dyndns.org> ``` On Tue, Jul 03, 2012 at 02:13:57PM -0700, Junio C Hamano wrote: > Neil Horman writes: > > > On Tue, Jul 03, 2012 at 12:00:27PM -0700, Junio C Hamano wrote: > >> > >> The changelog might be similar or textually identical, but it is > >> entirely a different matter if it makes sense taken out of the > >> context (i.e. cherry-picked). So I would personally do not bother > >> "filtering" about them too much---if you ask for empties, you will > >> get all empties. > >> > > Ok, copy that. > > That was somewhat unexpected, though ;-) It was 30% tongue-in-cheek > comment. People who want to keep the empty commits in the history > may want some filtering. As I am not among them, I do not think of > anything useful (other than "filter all empty ones away", that is). > > I understand what you're saying (for the record, I'm ok with the duplicates, to be fixed up at a later date, as opposed to dropping them all). But the fact remains, theres not obvious differentiator, other than some fuzzy search on the changelog we can use to differentiate empty commits. Let me think about it some more, maybe theres some sort of policy specification we can make regarding the changelog that would let us intellegently filter empty commits appropriately. Best Neil ```