Initial Rebase implementation #964

jamill · 2015-02-17T14:46:21Z

Getting closer to completion

Includes:

Initiate a Rebase (merge strategy)
Abort a Rebase (merge strategy)
Continue a Rebase (merge strategy)
Progress reporting
- Before / After each rebase step
- Checkout progress during each rebase step

Does not include:

Interactive Rebase support, of any kind
Support for rebases not using the merge strategy
Ability to execute rebase "commands"

TODO:

Expand test coverage
- Tests for checkout progress
- Tests for FileConflictStrategy

To Think About:

Ref inputs: I think it would be nice if methods such as merge, rebase could take in a "generic" object or interface that can be peeled to a commit, but still have information about where it came from. For instance, rebase takes 3 "ref" inputs. Instead of adding overloads for each permutation of ref input (commit, string, branch, tag...). I suspect this could be similar to the git_annotated_commit concept in libgit2.

Related issue: #714

nulltoken · 2015-02-22T18:15:42Z

LibGit2Sharp/IRepository.cs

+        /// <summary>
+        /// Access to Rebase functionality.
+        /// </summary>
+        Rebase RebaseOperation { get; }


How do you foresee to use this?

Found it. Not sure I'm a fan of having the Rebase() method in the main namespace and the other methods under RebaseOperation. How about exposing a separate repo.Rebase namespace (à la repo.Diff)?

We could replace repo.Rebase() with something like repo.Rebase.Start(), Initiate(), ...

yes - that seems reasonable. Thinking about the larger picture, cherry-pick and revert both have a similar "sequenced" capability. If we put rebase in its own namespace, then we would we have repo.CherryPick.Start(...), and repo.Revert.Start(...)? These (and Merge) all seem to be similar types of operations, would Merge also move into a repo.Merge.Start(...) type of structure? One option is that we could do this, and then have extension methods to expose these on the repository object directly?

nulltoken · 2015-02-22T18:26:15Z

How does the code behave when

One initiates a new Rebase() whereas one is already started?
One tries to continue (or abort) when no rebase is in progress?

jamill · 2015-02-23T14:22:15Z

How does the code behave when...

The code should throw an exception in these cases. There is already a test case that covers initiating a rebase while one is already started RebaseWhileAlreadyRebasingThrows(). I can add test coverage to assert we throw in the other case as well.

jamill · 2015-02-24T18:27:58Z

I moved the entry point for rebase into the Rebase namespace.
I added tests to assert the behavior when attempting to abort, continue a rebase when one is not in progress. This is currently returning a NotFoundException (as it can't find the rebase operation). This is a bit weird, we could return a specificly typed exception here.

rcorre · 2015-03-09T15:31:44Z

LibGit2Sharp/Rebase.cs

+        /// <param name="committer"></param>
+        /// <param name="options"></param>
+        /// <returns>true if completed successfully, false if conflicts encountered.</returns>
+        public virtual RebaseResult Start(Branch branch, Branch upstream, Branch onto, Signature committer, RebaseOptions options)


In the long run, I think it would be more flexible to take a commit rather than a branch, at least for the onto argument. I often use rebase -i some_commit_id to clean up before a push. This could be added as an overload later though.

We probably don't want to take a bare commit here - these things end up influencing the messages that we produce in the reflog and the markers in the conflict files. Do we have the equivalent of an AnnotatedCommit in libgit2sharp?

Sorry, to expand on that a little bit more: I mean we probably shouldn't only take a commit id (but we should perhaps allow it). Although a Branch is going to be convertible to a commit ID at some point, if you let libgit2 handle that conversion it will keep around the information of where that commit ID came from (as an "annotated commit"). Without that your conflict markers will look like:

<<<< ab1dc99f... change on one side ===== ab1cdde common ancestor ===== change on the other side >>>>> 9acca3a

Which is quite hard to read (I think) compared to having the branch names / etc.

Could it just take GitAnnotatedCommitHandles as arguments? The branch args are getting converted into these anyways.

Thanks @rcorre - GitAnnotatedCommitHandle is a lower level concept between lg2# and lg2 - it does not get exposed on the lg2# API. I agree with the idea that the API should be more flexible in the inputs it accepts (this is one of the items I had called out in my To Think About section in the PR description).

nulltoken · 2015-04-07T15:03:35Z

Rebased this PR. Ah, the irony...

nulltoken · 2015-04-10T08:39:08Z

LibGit2Sharp/Core/Proxy.cs

+
+            using (ThreadAffinity())
+            {
+                int result = NativeMethods.git_rebase_abort(rebase);


Shouldn't we Ensure here?

We now Ensure the result - thanks for pointing this out!

jamill · 2015-04-13T13:50:50Z

@Therzok I have some changes for moving more of the code into using the using construct - will push it up shortly

Therzok · 2015-04-13T14:12:18Z

❤️

Therzok · 2015-04-13T18:08:28Z

LibGit2Sharp.Tests/RebaseFixture.cs

+
+                // Verify the chain of commits that resulted from the rebase.
+                Commit expectedParent = expectedOntoCommit;
+                foreach(CompletedRebaseStepInfo stepInfo in PostRebaseResults)


Nit: missing spaaaaace.

nulltoken · 2015-05-04T20:53:11Z

LibGit2Sharp/RebaseStepInfo.cs

+        /// <summary>
+        /// Command to execute, if any.
+        /// </summary>
+        public virtual string Exec { get; private set; }


Similarly do we need to expose this yet?

jamill · 2015-06-18T17:11:00Z

@nulltoken tests are green, rebased on top of vNext.

jamill · 2015-06-22T18:57:07Z

@nulltoken - anything else here? 😄

nulltoken · 2015-06-22T19:00:48Z

❓

/cc @Therzok @dahlbyk

Therzok · 2015-06-22T19:03:51Z

LGTM! 🚀 🚢 🇮🇹

Initial Rebase implementation

nulltoken · 2015-06-23T16:07:48Z

@jamill 💯 ‼️

whoisj · 2015-06-23T16:26:48Z

@jamill 💯 ‼️

Agreed. This is awesome - I am super excited to finally have rebase support. Oh the wonderful things we can bring to the world with this. 😃

cortex-pg · 2016-03-30T13:52:29Z

Is it possible in git2sharp to rebase individual commits such as in this example http://blog.dennisrobinson.name/reorder-commits-with-git/ ? I am trying to do this in order to squash them into one commit which can then be cherrypicked.

dahlbyk · 2016-03-30T14:31:06Z

If you just need to squash, you'll have an easier time with:

Soft reset (leave index intact) to merge base.
Commit.
Rebase/cherry-pick.

cortex-pg · 2016-03-30T15:16:09Z

Thanks for that, this is actually what I have done at the moment. However there can be additional commits mixed in that I don't want to merge which was why I was looking at rebase. Alternatively if I could cherrypick multiple commits it would avoid the need to merge them at all but it seems Git2Sharp only supports picking 1 at a time.

ethomson · 2016-03-30T17:38:38Z

Alternatively if I could cherrypick multiple commits it would avoid the need to merge them at all but it seems Git2Sharp only supports picking 1 at a time.

I'm not sure that I understand what you mean. You can only ever cherry-pick one at a time - git (for example) will simply apply them in serial. (It doesn't create some strange multi-patch and try to apply them all at once).

So you should be able to just call cherry-pick in a loop...

dahlbyk · 2016-03-30T19:48:25Z

Maybe referring to cherry-pick -n, which will apply multiple in serial but leaves all changes staged but uncommitted?

cortex-pg · 2016-03-31T08:48:50Z

Thanks for that, I was considering using a loop but I can do
git cherry-pick 51959c1 fc5f375 --no-commit or
git cherry-pick 51959c1 --no-commit
git cherry-pick fc5f375 --no-commit and these work as expected.

However if I use
_repository.CherryPick(commit, author, new CherryPickOptions() { CommitOnSuccess = false }); on each of these commits the 2nd call throws LibGit2Sharp.CheckoutConflictException 1 uncommitted change would be overwritten by merge

jamill self-assigned this Feb 17, 2015

jamill added this to the v0.22 milestone Feb 17, 2015

jamill added the in-progress label Feb 17, 2015

jamill force-pushed the jamill/rebase branch from 573bddf to 36657cb Compare February 20, 2015 03:12

nulltoken reviewed Feb 22, 2015
View reviewed changes

rcorre reviewed Mar 9, 2015
View reviewed changes

jamill force-pushed the jamill/rebase branch 2 times, most recently from 03a04d8 to 339a9c7 Compare March 12, 2015 18:41

nulltoken force-pushed the jamill/rebase branch from 10dfa4c to d81c08f Compare April 7, 2015 15:02

nulltoken reviewed Apr 10, 2015
View reviewed changes

jamill force-pushed the jamill/rebase branch from 161e5ac to 368a0a8 Compare April 13, 2015 17:17

Therzok reviewed Apr 13, 2015
View reviewed changes

jamill force-pushed the jamill/rebase branch from 833af6a to fdc62f8 Compare April 28, 2015 22:20

jamill mentioned this pull request Apr 30, 2015

Streaming Filter Support #1030

Merged

nulltoken reviewed May 4, 2015
View reviewed changes

jamill force-pushed the jamill/rebase branch from 8a8ecdb to 146aba9 Compare May 14, 2015 02:41

jamill force-pushed the jamill/rebase branch 3 times, most recently from 9a55a2c to 3a03ef3 Compare May 29, 2015 20:57

jamill force-pushed the jamill/rebase branch from 3a03ef3 to 5607562 Compare June 18, 2015 16:25

Implement Rebase operation

3382549

jamill force-pushed the jamill/rebase branch from 37a3cf1 to 3382549 Compare June 22, 2015 14:58

dahlbyk mentioned this pull request Jun 23, 2015

Perform Final Review of the API before v1.0 Locks It #1103

Open

nulltoken added a commit that referenced this pull request Jun 23, 2015

Merge pull request #964 from libgit2/jamill/rebase

87dccce

Initial Rebase implementation

nulltoken merged commit 87dccce into vNext Jun 23, 2015

nulltoken deleted the jamill/rebase branch June 23, 2015 16:07

dahlbyk mentioned this pull request Jun 23, 2015

Unify Tense #1112

Open

carlosmn mentioned this pull request Mar 7, 2016

Introduce rebase support #714

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Initial Rebase implementation #964

Initial Rebase implementation #964

jamill commented Feb 17, 2015

nulltoken Feb 22, 2015

nulltoken Feb 22, 2015

jamill Feb 23, 2015

nulltoken commented Feb 22, 2015

jamill commented Feb 23, 2015

jamill commented Feb 24, 2015

rcorre Mar 9, 2015

ethomson Mar 9, 2015

ethomson Mar 9, 2015

rcorre Mar 9, 2015

jamill Mar 9, 2015

nulltoken commented Apr 7, 2015

nulltoken Apr 10, 2015

jamill Apr 13, 2015

jamill commented Apr 13, 2015

Therzok commented Apr 13, 2015

Therzok Apr 13, 2015

nulltoken May 4, 2015

jamill commented Jun 18, 2015

jamill commented Jun 22, 2015

nulltoken commented Jun 22, 2015

Therzok commented Jun 22, 2015

nulltoken commented Jun 23, 2015

whoisj commented Jun 23, 2015

cortex-pg commented Mar 30, 2016

dahlbyk commented Mar 30, 2016

cortex-pg commented Mar 30, 2016

ethomson commented Mar 30, 2016

dahlbyk commented Mar 30, 2016

cortex-pg commented Mar 31, 2016

Initial Rebase implementation #964

Initial Rebase implementation #964

Conversation

jamill commented Feb 17, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nulltoken commented Feb 22, 2015

jamill commented Feb 23, 2015

jamill commented Feb 24, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nulltoken commented Apr 7, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jamill commented Apr 13, 2015

Therzok commented Apr 13, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jamill commented Jun 18, 2015

jamill commented Jun 22, 2015

nulltoken commented Jun 22, 2015

Therzok commented Jun 22, 2015

nulltoken commented Jun 23, 2015

whoisj commented Jun 23, 2015

cortex-pg commented Mar 30, 2016

dahlbyk commented Mar 30, 2016

cortex-pg commented Mar 30, 2016

ethomson commented Mar 30, 2016

dahlbyk commented Mar 30, 2016

cortex-pg commented Mar 31, 2016