pygit2 icon indicating copy to clipboard operation
pygit2 copied to clipboard

[git-cherry-pick recipe] The example in docs produces a misleading result

Open webknjaz opened this issue 5 years ago • 9 comments

I was following https://www.pygit2.org/recipes/git-cherry-pick.html#cherry-picking-a-commit-without-a-working-copy and it seemed to work at first but then I noticed that it pulls in extra commits that I've never wanted.

I think that this example is incomplete.

My src (https://github.com/sanitizers/patchback-github-app/blob/92934c6/patchback/event_handlers.py#L66-L133) should produce a branch with one cherry-picked commit but instead gets a whole bunch of earlier commits too: https://github.com/webknjaz/backport-test-repo/pull/13/commits.

webknjaz avatar Sep 10 '20 15:09 webknjaz

To make my problem more illustrative here's the state I have

(a) * <- master
    |
(x) *
    |
(y) * * (b) <- backport-branch
    |/
(z) *

And so when I want to cherry-pick (a) into "backport-branch", I need to get a resulting commit with (a') that would be a direct child of (b) as a branch (that will later be used in a PR). I'm getting something like

                * (a') <- branch-for-backport-pr
                |
master -> (a) * * (x')
              | |
          (x) * * (y')
              | |
          (y) * * (b) <- backport-branch
              |/
          (z) *

and this ^ is not what I want. I expect to produce

(a) * <- master
    |
(x) *
    | * (a') <- branch-for-backport-pr
(y) * * (b) <- backport-branch
    |/
(z) *

P.S. (a) is sometimes a merge commit so in some cases I want to emulate cherry-pick -x -m1 but it is currently broken even for a case when (a) is a regular commit.

webknjaz avatar Sep 10 '20 16:09 webknjaz

@jdavid do you have any idea what's missing here? Is the recipe even correct?

webknjaz avatar Sep 10 '20 16:09 webknjaz

cc @rmoehn

webknjaz avatar Sep 10 '20 16:09 webknjaz

@webknjaz, sorry, I can't help you with this. I wrote the recipe more than five years ago and I haven't used pygit2 since.

rmoehn avatar Sep 11 '20 01:09 rmoehn

The 2nd part of the recipe looks bad.

I think to support this use case first we need to wrap git_cherrypick_commit (see https://libgit2.org/libgit2/#v1.0.1/group/cherrypick/git_cherrypick_commit)

jdavid avatar Sep 12 '20 08:09 jdavid

Yeah, I thought so. I would've sent a PR but I'm not sure how to implement it...

webknjaz avatar Sep 12 '20 08:09 webknjaz

It would be a lot like Repository_cherrypick https://github.com/libgit2/pygit2/blob/master/src/repository.c#L674 but with more arguments, and it would return an index object like merge_trees does https://github.com/libgit2/pygit2/blob/master/pygit2/repository.py#L791

If you want to do a PR you can choose to write it in C or with cffi, whatever you're more comfortable with. Above you've 2 examples, cherrypick is done in C and merge_trees is done with cffi.

jdavid avatar Sep 13 '20 14:09 jdavid

Thanks for the pointers! I'll see if I can find time for this.

webknjaz avatar Sep 13 '20 16:09 webknjaz