{"thread":{"id":"30916","subject":"[PATCH] add test case for rebase of empty commit","startedAt":"2012-06-27T16:22:01Z","lastAt":"2012-07-03T23:40:48Z","messageCount":8,"participants":["Martin von Zweigbergk","Junio C Hamano","Neil Horman"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"194357","messageId":"1340814121-23813-1-git-send-email-martin.von.zweigbergk@gmail.com","threadId":"30916","inReplyTo":null,"subject":"[PATCH] add test case for rebase of empty commit","fromName":"Martin von Zweigbergk","fromEmail":"martin.von.zweigbergk@gmail.com","sentAt":"2012-06-27T16:22:01Z","receivedAt":"2012-06-27T16:22:01Z","isPatch":true,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"---\n t/t3401-rebase-partial.sh | 8 ++++++++\n 1 file changed, 8 insertions(+)\n\ndiff --git a/t/t3401-rebase-partial.sh b/t/t3401-rebase-partial.sh\nindex 7ba1797..7f8693b 100755\n--- a/t/t3401-rebase-partial.sh\n+++ b/t/t3401-rebase-partial.sh\n@@ -42,4 +42,12 @@ test_expect_success 'rebase --merge topic branch that was partially merged upstr\n \ttest_path_is_missing .git/rebase-merge\n '\n \n+test_expect_success 'rebase ignores empty commit' '\n+\tgit reset --hard A &&\n+\tgit commit --allow-empty -m empty &&\n+\ttest_commit D &&\n+\tgit rebase C &&\n+\ttest $(git log --format=%s C..) = \"D\"\n+'\n+\n test_done\n-- \n1.7.9.3.327.g2980b\n"},{"id":"194377","messageId":"7vr4t079jp.fsf@alter.siamese.dyndns.org","threadId":"30916","inReplyTo":"1340814121-23813-1-git-send-email-martin.von.zweigbergk@gmail.com","subject":"Re: [PATCH] add test case for rebase of empty commit","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-06-27T21:02:34Z","receivedAt":"2012-06-27T21:02:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thanks.\n\nWe recently had a topic to add an option to allow rebase to carry\nempty commits forward, but I notice that it only had tests for the\ncomponent cherry-pick to keep empty or redundant commits, so it may\nnot be a bad idea to add tests for that series to the same t3401\nafter this commit (Neil Horman CC'ed).\n"},{"id":"194410","messageId":"20120628113057.GA29277@hmsreliant.think-freely.org","threadId":"30916","inReplyTo":"7vr4t079jp.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] add test case for rebase of empty commit","fromName":"Neil Horman","fromEmail":"nhorman@tuxdriver.com","sentAt":"2012-06-28T11:30:57Z","receivedAt":"2012-06-28T11:30:57Z","isPatch":true,"sender":{"key":"nhorman@tuxdriver.com","avatar":"https://avatars.githubusercontent.com/u/1032926?v=4"},"body":"On Wed, Jun 27, 2012 at 02:02:34PM -0700, Junio C Hamano wrote:\n> Thanks.\n> \n> We recently had a topic to add an option to allow rebase to carry\n> empty commits forward, but I notice that it only had tests for the\n> component cherry-pick to keep empty or redundant commits, so it may\n> not be a bad idea to add tests for that series to the same t3401\n> after this commit (Neil Horman CC'ed).\n> \n> \nSo if I understand correctly, the desire is to augment t3401 such that it adds a\ntest in which both of the commits in the local branch are empty, and still\ncorrectly identifies the one that was cherry-picked and only adds the remaining\none during the rebase?\n\nYes, I think that sounds like a good idea. I'm in the middle of an sctp project\nat the moment, but I expect to complete it in the next few days. I can look\ninto writing this next week if you like.\n\nThanks & Regards\nNeil\n \n"},{"id":"194539","messageId":"20120703182000.GB10864@hmsreliant.think-freely.org","threadId":"30916","inReplyTo":"7vr4t079jp.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] add test case for rebase of empty commit","fromName":"Neil Horman","fromEmail":"nhorman@tuxdriver.com","sentAt":"2012-07-03T18:20:00Z","receivedAt":"2012-07-03T18:20:00Z","isPatch":true,"sender":{"key":"nhorman@tuxdriver.com","avatar":"https://avatars.githubusercontent.com/u/1032926?v=4"},"body":"On Wed, Jun 27, 2012 at 02:02:34PM -0700, Junio C Hamano wrote:\n> Thanks.\n> \n> We recently had a topic to add an option to allow rebase to carry\n> empty commits forward, but I notice that it only had tests for the\n> component cherry-pick to keep empty or redundant commits, so it may\n> not be a bad idea to add tests for that series to the same t3401\n> after this commit (Neil Horman CC'ed).\n> \n> \nSo, I've been thinking about this some, and I'm a bit stuck on it. Reading the\ntest description for t3401, I see that we're testing gits ability to detect\npatches merged upstream when doing a rebase. That said, how are we supposed to\ndifferentiate between upstream empty patches that have been cherry-picked or\nmerged, and local branch empty changes that haven't. As humans we can see that\nthe changelog might be the same, but git has no way to detect that, and if\n--allow-empty is specified will just apply any empty patch it finds between the\ntwo branches merge base and the topic branch head. Does anyone have an idea as\nto how we should detect such duplication?\n\nNeil\n"},{"id":"194545","messageId":"7vtxxovfec.fsf@alter.siamese.dyndns.org","threadId":"30916","inReplyTo":"20120703182000.GB10864@hmsreliant.think-freely.org","subject":"Re: [PATCH] add test case for rebase of empty commit","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-07-03T19:00:27Z","receivedAt":"2012-07-03T19:00:27Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Neil Horman writes:\n\n> So, I've been thinking about this some, and I'm a bit stuck on it. Reading the\n> test description for t3401, I see that we're testing gits ability to detect\n> patches merged upstream when doing a rebase. That said, how are we supposed to\n> differentiate between upstream empty patches that have been cherry-picked or\n> merged, and local branch empty changes that haven't. As humans we can see that\n> the changelog might be the same, but git has no way to detect that, and if\n> --allow-empty is specified will just apply any empty patch it finds between the\n> two branches merge base and the topic branch head. Does anyone have an idea as\n> to how we should detect such duplication?\n\nThe changelog might be similar or textually identical, but it is\nentirely a different matter if it makes sense taken out of the\ncontext (i.e. cherry-picked). So I would personally do not bother\n\"filtering\" about them too much---if you ask for empties, you will\nget all empties.\n"},{"id":"194556","messageId":"20120703203136.GC10864@hmsreliant.think-freely.org","threadId":"30916","inReplyTo":"7vtxxovfec.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] add test case for rebase of empty commit","fromName":"Neil Horman","fromEmail":"nhorman@tuxdriver.com","sentAt":"2012-07-03T20:31:36Z","receivedAt":"2012-07-03T20:31:36Z","isPatch":true,"sender":{"key":"nhorman@tuxdriver.com","avatar":"https://avatars.githubusercontent.com/u/1032926?v=4"},"body":"On Tue, Jul 03, 2012 at 12:00:27PM -0700, Junio C Hamano wrote:\n> Neil Horman writes:\n> \n> > So, I've been thinking about this some, and I'm a bit stuck on it. Reading the\n> > test description for t3401, I see that we're testing gits ability to detect\n> > patches merged upstream when doing a rebase. That said, how are we supposed to\n> > differentiate between upstream empty patches that have been cherry-picked or\n> > merged, and local branch empty changes that haven't. As humans we can see that\n> > the changelog might be the same, but git has no way to detect that, and if\n> > --allow-empty is specified will just apply any empty patch it finds between the\n> > two branches merge base and the topic branch head. Does anyone have an idea as\n> > to how we should detect such duplication?\n> \n> The changelog might be similar or textually identical, but it is\n> entirely a different matter if it makes sense taken out of the\n> context (i.e. cherry-picked). So I would personally do not bother\n> \"filtering\" about them too much---if you ask for empties, you will\n> get all empties.\n> \nOk, copy that.\nThanks!\nNeil\n"},{"id":"194558","messageId":"7vpq8ctune.fsf@alter.siamese.dyndns.org","threadId":"30916","inReplyTo":"20120703203136.GC10864@hmsreliant.think-freely.org","subject":"Re: [PATCH] add test case for rebase of empty commit","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-07-03T21:13:57Z","receivedAt":"2012-07-03T21:13:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Neil Horman writes:\n\n> On Tue, Jul 03, 2012 at 12:00:27PM -0700, Junio C Hamano wrote:\n>> \n>> The changelog might be similar or textually identical, but it is\n>> entirely a different matter if it makes sense taken out of the\n>> context (i.e. cherry-picked). So I would personally do not bother\n>> \"filtering\" about them too much---if you ask for empties, you will\n>> get all empties.\n>> \n> Ok, copy that.\n\nThat was somewhat unexpected, though ;-) It was 30% tongue-in-cheek\ncomment. People who want to keep the empty commits in the history\nmay want some filtering. As I am not among them, I do not think of\nanything useful (other than \"filter all empty ones away\", that is).\n"},{"id":"194588","messageId":"20120703234048.GA10224@neilslaptop.think-freely.org","threadId":"30916","inReplyTo":"7vpq8ctune.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] add test case for rebase of empty commit","fromName":"Neil Horman","fromEmail":"nhorman@tuxdriver.com","sentAt":"2012-07-03T23:40:48Z","receivedAt":"2012-07-03T23:40:48Z","isPatch":true,"sender":{"key":"nhorman@tuxdriver.com","avatar":"https://avatars.githubusercontent.com/u/1032926?v=4"},"body":"On Tue, Jul 03, 2012 at 02:13:57PM -0700, Junio C Hamano wrote:\n> Neil Horman writes:\n> \n> > On Tue, Jul 03, 2012 at 12:00:27PM -0700, Junio C Hamano wrote:\n> >> \n> >> The changelog might be similar or textually identical, but it is\n> >> entirely a different matter if it makes sense taken out of the\n> >> context (i.e. cherry-picked). So I would personally do not bother\n> >> \"filtering\" about them too much---if you ask for empties, you will\n> >> get all empties.\n> >> \n> > Ok, copy that.\n> \n> That was somewhat unexpected, though ;-) It was 30% tongue-in-cheek\n> comment. People who want to keep the empty commits in the history\n> may want some filtering. As I am not among them, I do not think of\n> anything useful (other than \"filter all empty ones away\", that is).\n> \n> \nI understand what you're saying (for the record, I'm ok with the duplicates, to\nbe fixed up at a later date, as opposed to dropping them all). But the fact\nremains, theres not obvious differentiator, other than some fuzzy search on the\nchangelog we can use to differentiate empty commits. Let me think about it some\nmore, maybe theres some sort of policy specification we can make regarding the\nchangelog that would let us intellegently filter empty commits appropriately.\nBest\nNeil\n"}]}