{"thread":{"id":"25613","subject":"Refactoring git-rebase.sh and git-rebase--interactive.sh","startedAt":"2010-11-02T12:33:07Z","lastAt":"2010-11-07T01:57:25Z","messageCount":9,"participants":["Martin von Zweigbergk","Johannes Sixt","Christian Couder","Yann Dirson","Pat Notz"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"154964","messageId":"AANLkTimeWDbJPor9PnKgW5sD7DLjqrm-vTzEtnARvP3M@mail.gmail.com","threadId":"25613","inReplyTo":null,"subject":"Refactoring git-rebase.sh and git-rebase--interactive.sh","fromName":"Martin von Zweigbergk","fromEmail":"martin.von.zweigbergk@gmail.com","sentAt":"2010-11-02T12:33:07Z","receivedAt":"2010-11-02T12:33:07Z","isPatch":false,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"(Resending as plain text. Sorry about the spam to the guys on the CC list.)\n\nHi,\n\nI have now been using Git for something like 18 months, and I think it's\nabout time that I try to contribute.\n\nSo, after adding some features to git-rebase.sh (which I will send\nseparate mails about), I realized I would have to add them to\ngit-rebase--interactive.sh as well. Rather than doing that, I would\nprefer to first extract the common parts of these scripts and add the\nfeatures in only one place. Since this is the first time I do anything\non Git, I will need a lot of advice.\n\nMy main goal is to extract the commonalities in command line parsing and\ninterpretation as well as validation (of command line and repository\nstate, and running the pre-rebase hook).\n\nFirst of all, do you agree that this should be done and is now a good\ntime to do it (I'm thinking mostly about conflicts with other ongoing\nefforts)? While at GitTogether, I talked briefly to Thomas Rast about\ndoing this, and he mentioned that resurrecting the git sequencer might\nbe a better idea. However, I *think* much of what I was thinking about\ndoing involves code that is run before the git sequencer is called. I\nwouldn't mind working on the git sequencer afterwards, unless Christian\nCouder or someone else is currently working on it.\n\nWhile I did put 'refactoring' in the subject line, this is not\nstrictly true for I would like to do. There are currently quite some\nfunctional differences between 'git rebase' and 'git rebase -i' (not\njust the obvious one). While refactoring the code, it would be natural\nto remove some of these differences, because I would guess that most of\nthem are not intentional. The following are some of the differences I\nhave found so far.\n\n1. Several different error messages. For example, 'cannot rebase: you\n   have unstaged changes' versus 'Working tree is dirty'.\n2. Different order of validation of command line and current state. For\n   example, if the working tree is dirty and the upstream is invalid,\n   interactive rebase will report the dirty working tree, while non-\n   interactive rebase will report the invalid upstream.\n3. Different set of supported flags. For example, interactive rebase\n   does not support the '--stat' flag (and rebase.stat configuration)\n   and various versions of '--strategy'.\n\nThe above are just some examples, and it is probably most efficient to\ndecide which behavior to pick in each case while reviewing patches\n(provided, of course, that you think it is worthwhile to extract the\nshared functionality).\n\n\n/Martin\n"},{"id":"154965","messageId":"4CD0081B.5080405@viscovery.net","threadId":"25613","inReplyTo":"AANLkTimeWDbJPor9PnKgW5sD7DLjqrm-vTzEtnARvP3M@mail.gmail.com","subject":"Re: Refactoring git-rebase.sh and git-rebase--interactive.sh","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2010-11-02T12:46:19Z","receivedAt":"2010-11-02T12:46:19Z","isPatch":false,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 11/2/2010 13:33, schrieb Martin von Zweigbergk:\n> There are currently quite some\n> functional differences between 'git rebase' and 'git rebase -i' (not\n> just the obvious one).\n\nDifferences in the option handling is my favorite gripe with git-rebase,\ntoo. I suggest to write a command line processor, git-rebase.sh, that sets\nshell variables from options that it collects from various sources, then\ndispatches to one of git-rebase--interactive.sh, git-rebase--merge.sh, or\ngit-rebase--am.sh (the latter two would be stripped-down copies of the\ncurrent git-rebase.sh).\n\n-- Hannes\n"},{"id":"155018","messageId":"201011030424.33093.chriscool@tuxfamily.org","threadId":"25613","inReplyTo":"AANLkTimeWDbJPor9PnKgW5sD7DLjqrm-vTzEtnARvP3M@mail.gmail.com","subject":"Re: Refactoring git-rebase.sh and git-rebase--interactive.sh","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2010-11-03T03:24:32Z","receivedAt":"2010-11-03T03:24:32Z","isPatch":false,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Tuesday 02 November 2010 13:33:07 Martin von Zweigbergk wrote:\n> (Resending as plain text. Sorry about the spam to the guys on the CC list.)\n> \n> Hi,\n> \n> I have now been using Git for something like 18 months, and I think it's\n> about time that I try to contribute.\n\nGreat!\n\n> So, after adding some features to git-rebase.sh (which I will send\n> separate mails about), I realized I would have to add them to\n> git-rebase--interactive.sh as well. Rather than doing that, I would\n> prefer to first extract the common parts of these scripts and add the\n> features in only one place. Since this is the first time I do anything\n> on Git, I will need a lot of advice.\n> \n> My main goal is to extract the commonalities in command line parsing and\n> interpretation as well as validation (of command line and repository\n> state, and running the pre-rebase hook).\n> \n> First of all, do you agree that this should be done and is now a good\n> time to do it (I'm thinking mostly about conflicts with other ongoing\n> efforts)? While at GitTogether, I talked briefly to Thomas Rast about\n> doing this, and he mentioned that resurrecting the git sequencer might\n> be a better idea. However, I *think* much of what I was thinking about\n> doing involves code that is run before the git sequencer is called. I\n> wouldn't mind working on the git sequencer afterwards, unless Christian\n> Couder or someone else is currently working on it.\n\nNow that GTAC (http://www.gtac.biz) is over, I plan to work on options \n--continue, --abort and --skip for git cherry-pick/revert. After that I hope \nto be able to refactor the code so that in the end common code is used by \ncherry-pick/revert and rebase.\n\nAnd I agree that what you want to do does not conflict with my plan. On the \ncontrary it might help in the end. Go for it!\n\nThanks,\nChristian.\n"},{"id":"155043","messageId":"AANLkTin2hC8fk-5tAwVTjT=6AivQU2k+YcHczS6WgpZu@mail.gmail.com","threadId":"25613","inReplyTo":"201011030424.33093.chriscool@tuxfamily.org","subject":"Re: Refactoring git-rebase.sh and git-rebase--interactive.sh","fromName":"Martin von Zweigbergk","fromEmail":"martin.von.zweigbergk@gmail.com","sentAt":"2010-11-03T12:22:02Z","receivedAt":"2010-11-03T12:22:02Z","isPatch":false,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"Perfect! I liked Johannes's proposal and started working on it\nyesterday. It's looking good so far.\n\n/Martin\n"},{"id":"155214","messageId":"20101104211556.GB8911@home.lan","threadId":"25613","inReplyTo":"201011030424.33093.chriscool@tuxfamily.org","subject":"Re: Refactoring git-rebase.sh and git-rebase--interactive.sh","fromName":"Yann Dirson","fromEmail":"ydirson@free.fr","sentAt":"2010-11-04T21:15:56Z","receivedAt":"2010-11-04T21:15:56Z","isPatch":false,"sender":{"key":"ydirson@free.fr","avatar":null},"body":"Hi Christian,\n\nOn Wed, Nov 03, 2010 at 04:24:32AM +0100, Christian Couder wrote:\n> Now that GTAC (http://www.gtac.biz) is over, I plan to work on options \n> --continue, --abort and --skip for git cherry-pick/revert. After that I hope \n> to be able to refactor the code so that in the end common code is used by \n> cherry-pick/revert and rebase.\n\nSounds like \"sequencer is coming back\", great news :)\n\nI don't know if you would like the idea enough, but something I often\nthink would be good to have (and which could be useful for cherry-pick\nand other commands in need of a sequencer), would be more flexibility.\nThe thing I find myself lacking most often, is the possibility to\nchange my mind on an already-edited commit (ie, go back after\n--continue), the alternatives I can see today being:\n\n- keeping a note on what to do on next pass (but may be more work in case\n  of conflicts with further commits)\n- fast-forward --continue'ing to keep curent changes and add new ones in\n  next pass (same restriction)\n- --abort'ing the rebase and starting it again, possibly fetching the\n  changes from previous run via HEAD's reflog (not very handy either)\n- checkout back to where you want to re-amend and cherry-pick those you\n  already passed, essentially redoing an interactive rebase by hand\n\nIf we could go back to previous commit, while keeping changes done to\nthe current one (say, --previous), or reverting to the original one\n(say, --revert).  In the same way, continuing until another\npreviously-unforeseen commit without the need to edit the todo file\nwould be nice to have (eg. --next).\n\nWhile I'm at it, another somewhat loosely option I have thought of\nwould be to seed the todo file with \"edit\" commands instead of \"pick\",\nto make it possible to validate a series of patches one by one before\nsending.  That could be generalized for running a test script\nautomatically, that is inserting \"x whatever\" between all pick's - and\nmy 1st idea would boil down to inserting arg-less \"edit\" or \"x false\"\ninstead.  Maybe some --stepcmd=<command> flag ?\n\n-- \nYann\n"},{"id":"155222","messageId":"AANLkTi=cK0M2icUzTa2c4HUkNfSAGEmeY1LNds_AcRPk@mail.gmail.com","threadId":"25613","inReplyTo":"20101104211556.GB8911@home.lan","subject":"Re: Refactoring git-rebase.sh and git-rebase--interactive.sh","fromName":"Pat Notz","fromEmail":"patnotz@gmail.com","sentAt":"2010-11-04T21:49:42Z","receivedAt":"2010-11-04T21:49:42Z","isPatch":false,"sender":{"key":"patnotz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45364?v=4"},"body":"On Thu, Nov 4, 2010 at 3:15 PM, Yann Dirson <ydirson@free.fr> wrote:\n> Hi Christian,\n>\n> On Wed, Nov 03, 2010 at 04:24:32AM +0100, Christian Couder wrote:\n>> Now that GTAC (http://www.gtac.biz) is over, I plan to work on options\n>> --continue, --abort and --skip for git cherry-pick/revert. After that I hope\n>> to be able to refactor the code so that in the end common code is used by\n>> cherry-pick/revert and rebase.\n>\n> Sounds like \"sequencer is coming back\", great news :)\n>\n> I don't know if you would like the idea enough, but something I often\n> think would be good to have (and which could be useful for cherry-pick\n> and other commands in need of a sequencer), would be more flexibility.\n> The thing I find myself lacking most often, is the possibility to\n> change my mind on an already-edited commit (ie, go back after\n> --continue), the alternatives I can see today being:\n>\n> - keeping a note on what to do on next pass (but may be more work in case\n>  of conflicts with further commits)\n> - fast-forward --continue'ing to keep curent changes and add new ones in\n>  next pass (same restriction)\n> - --abort'ing the rebase and starting it again, possibly fetching the\n>  changes from previous run via HEAD's reflog (not very handy either)\n> - checkout back to where you want to re-amend and cherry-pick those you\n>  already passed, essentially redoing an interactive rebase by hand\n>\n> If we could go back to previous commit, while keeping changes done to\n> the current one (say, --previous), or reverting to the original one\n> (say, --revert).  In the same way, continuing until another\n> previously-unforeseen commit without the need to edit the todo file\n> would be nice to have (eg. --next).\n\nI've long wanted something like --next.  That would save me a lot of\nmulti-pass rebases.\n\n> While I'm at it, another somewhat loosely option I have thought of\n> would be to seed the todo file with \"edit\" commands instead of \"pick\",\n> to make it possible to validate a series of patches one by one before\n> sending.  That could be generalized for running a test script\n> automatically, that is inserting \"x whatever\" between all pick's - and\n> my 1st idea would boil down to inserting arg-less \"edit\" or \"x false\"\n> instead.  Maybe some --stepcmd=<command> flag ?\n\nYes, --stepcmd would be *really* useful for testing a patch sequence\nor adding forgotten signed-off by's.\n\n> --\n> Yann\n> --\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at  http://vger.kernel.org/majordomo-info.html\n>\n"},{"id":"155254","messageId":"AANLkTi=tQFN-LckpMyKj=NN9ZSmb=DWC=ztLE_e666Xj@mail.gmail.com","threadId":"25613","inReplyTo":"20101104211556.GB8911@home.lan","subject":"Re: Refactoring git-rebase.sh and git-rebase--interactive.sh","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2010-11-05T08:58:17Z","receivedAt":"2010-11-05T08:58:17Z","isPatch":false,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"Hi Yann,\n\nOn Thu, Nov 4, 2010 at 10:15 PM, Yann Dirson <ydirson@free.fr> wrote:\n> Hi Christian,\n>\n> On Wed, Nov 03, 2010 at 04:24:32AM +0100, Christian Couder wrote:\n>> Now that GTAC (http://www.gtac.biz) is over, I plan to work on options\n>> --continue, --abort and --skip for git cherry-pick/revert. After that I hope\n>> to be able to refactor the code so that in the end common code is used by\n>> cherry-pick/revert and rebase.\n>\n> Sounds like \"sequencer is coming back\", great news :)\n\nYeah, but don't expect something soon because I am quite busy these days.\n\n> I don't know if you would like the idea enough, but something I often\n> think would be good to have (and which could be useful for cherry-pick\n> and other commands in need of a sequencer), would be more flexibility.\n> The thing I find myself lacking most often, is the possibility to\n> change my mind on an already-edited commit (ie, go back after\n> --continue), the alternatives I can see today being:\n>\n> - keeping a note on what to do on next pass (but may be more work in case\n>  of conflicts with further commits)\n> - fast-forward --continue'ing to keep curent changes and add new ones in\n>  next pass (same restriction)\n> - --abort'ing the rebase and starting it again, possibly fetching the\n>  changes from previous run via HEAD's reflog (not very handy either)\n> - checkout back to where you want to re-amend and cherry-pick those you\n>  already passed, essentially redoing an interactive rebase by hand\n>\n> If we could go back to previous commit, while keeping changes done to\n> the current one (say, --previous), or reverting to the original one\n> (say, --revert).  In the same way, continuing until another\n> previously-unforeseen commit without the need to edit the todo file\n> would be nice to have (eg. --next).\n>\n> While I'm at it, another somewhat loosely option I have thought of\n> would be to seed the todo file with \"edit\" commands instead of \"pick\",\n> to make it possible to validate a series of patches one by one before\n> sending.  That could be generalized for running a test script\n> automatically, that is inserting \"x whatever\" between all pick's - and\n> my 1st idea would boil down to inserting arg-less \"edit\" or \"x false\"\n> instead.  Maybe some --stepcmd=<command> flag ?\n\nIf you want them, feel free to work on those options without waiting\nfor a sequencer.\n\nThanks,\nChristian.\n"},{"id":"155277","messageId":"AANLkTinSb09oZHN8br5goeoG2b+Cgra88E1qeLqi-Y=z@mail.gmail.com","threadId":"25613","inReplyTo":"AANLkTimeWDbJPor9PnKgW5sD7DLjqrm-vTzEtnARvP3M@mail.gmail.com","subject":"Re: Refactoring git-rebase.sh and git-rebase--interactive.sh","fromName":"Martin von Zweigbergk","fromEmail":"martin.von.zweigbergk@gmail.com","sentAt":"2010-11-06T02:03:12Z","receivedAt":"2010-11-06T02:03:12Z","isPatch":false,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"I have now removed all the command line processing from\ngit-rebase--interactive.sh. I think the next step will be to align the\norder of the validation steps between the two script files. When that is\ndone, I will remove one step at a time from git-rebase--interactive.sh\nwhile simultaneously moving down the call to it from git-rebase.sh.\n\nTo avoid going off in the wrong direction, I would appreciate your input\non the order of the validation steps. I was thinking about having them\nin this order:\n1. Check command line\n2. a. If --continue, --skip or --abort requested, rebase-apply/ or\n      rebase-merge/ must exist. (What if -i is also passed and\n\t  rebase-apply/ exists?)\n   b. Otherwise, rebase-apply/ or rebase-merge/ must not exist\n3. Check that references, such as upstream and onto, are valid\n4. Check that working tree is clean\n5. Run pre-rebase hook\n\nWhat do you think?\n\nrebase -i currently checks that GIT_COMMITTER_IDENT is set. Should that\nbe checked and if so, should it only be checked if the rebase is\ninteractive?\n\n/Martin\n"},{"id":"155308","messageId":"AANLkTikbSU7LMnrT7uz5SSd8_x_cK_dHiVN37McJe+AT@mail.gmail.com","threadId":"25613","inReplyTo":"AANLkTinSb09oZHN8br5goeoG2b+Cgra88E1qeLqi-Y=z@mail.gmail.com","subject":"Re: Refactoring git-rebase.sh and git-rebase--interactive.sh","fromName":"Martin von Zweigbergk","fromEmail":"martin.von.zweigbergk@gmail.com","sentAt":"2010-11-07T01:57:25Z","receivedAt":"2010-11-07T01:57:25Z","isPatch":false,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"> 2. a. If --continue, --skip or --abort requested, rebase-apply/ or\n>      rebase-merge/ must exist. (What if -i is also passed and\n>          rebase-apply/ exists?)\n>   b. Otherwise, rebase-apply/ or rebase-merge/ must not exist\n\nThis actually hints at a more general question, namely whether command\nline or saved options should be used when continuing a rebase. For\nexample, if an interactive rebase has been started with a certain merge\nstrategy, should that merge strategy be used throughout the rebase, or\nshould whatever is passed on the command line when continuing be used\ninstead? Does it depend on which option we are talking about?\n\nAs far as I can understand from the code, it seems like non-interactive\nrebase currently does not store e.g. the merge strategy, but allows it\nto be passed on the command line together with '--continue' (but only if\npassed before). Interactive rebase, OTOH, does store the option when the\nrebase is initated and does not allow it to be overriden on the command\nline. I have not tested either of them, so I may very well be wrong.\n\nWhatever the current behavior is, how do you think it *should* behave?\n"}]}