git-cherry-pick(1) -- path

> I'd be interested in being able to partially cherry-pick commits, based
> on a path filter (similar to how many other git(1) commands behave with
> -- path).
>
> Do you have any opinions on that?

> Alejandro Colomar <alx@kernel.org> writes:
> 
> > I'd be interested in being able to partially cherry-pick commits, based
> > on a path filter (similar to how many other git(1) commands behave with
> > -- path).
> >
> > Do you have any opinions on that?
> 
> Opinions?  
> 
> Personally it is a "Meh" for me, because I would instead do
> 
>     $ git format-patch --stdout -1 $ThatCommit -- $ThisPath |
>       git am -3

> which would be mostly sufficient for my purpose, but I do not see
> any fundamental reason why it should not exist.
> 
> It is already well known that future merges from the same branch
> will have to deal with duplicated changes brought in by picking
> commits with "git cherry-pick".  Having a pathspec-limited variant
> of "git cherry-pick" would not make the world a worse place.

> Hi Junio,
> 
> On Thu, May 09, 2024 at 06:15:24PM GMT, Junio C Hamano wrote:
>> Alejandro Colomar <alx@kernel.org> writes:
>>
>>> I'd be interested in being able to partially cherry-pick commits, based
>>> on a path filter (similar to how many other git(1) commands behave with
>>> -- path).
>>>
>>> Do you have any opinions on that?

>>      $ git format-patch --stdout -1 $ThatCommit -- $ThisPath |
>>        git am -3
> 
> Hmmm, I hadn't thought of that; very interesting!
> 
> Although I have some concerns with git-am(1); basically that it's almost
> clueless when there's a conflict.

>>>      $ git format-patch --stdout -1 $ThatCommit -- $ThisPath |
>>>        git am -3
>> Hmmm, I hadn't thought of that; very interesting!
>> Although I have some concerns with git-am(1); basically that it's
>> almost
>> clueless when there's a conflict.
>
> "git am -3" should be fine here as you're guaranteed to have the
> necessary blobs available to create conflicts - this is what "git
> rebase --apply" does.

> Phillip Wood <phillip.wood123@gmail.com> writes:
> 
> >>>      $ git format-patch --stdout -1 $ThatCommit -- $ThisPath |
> >>>        git am -3
> >> Hmmm, I hadn't thought of that; very interesting!
> >> Although I have some concerns with git-am(1); basically that it's
> >> almost
> >> clueless when there's a conflict.
> >
> > "git am -3" should be fine here as you're guaranteed to have the
> > necessary blobs available to create conflicts - this is what "git
> > rebase --apply" does.
> 
> Good thing to point out.  "am -3" is just as good for this purpose
> and "almost clueless" is a derogatory comment that requires an
> apology ;-)

> > As far as the implementation goes I haven't thought too deeply but I
> > suspect we'd want to create a couple of trees based on the commit we
> > want to cherry-pick and its parent filtered by the pathspec and use
> > those in the tree-way merge with HEAD.
> 
> If we were to use the ort machinery, it may not be too bad to use
> the pathspec only at the final writeout phase.  That is, perform a
> full cherry-pick in the in-core index, reset all the entries in the
> in-core index back to HEAD that are outside the given pathspec, and
> then write out the result to the working tree.  That way, an old
> change that was made to paths at the original location can be cherry
> picked to a much newer tree after these paths have been moved to a
> different location.  Doing the same with the recursive machinery
> would be missier but perhaps the more recent merge-tree that uses
> the ort machinery to work purely in-core might be a good way to go.

> Hi Phillip, Junio,
> 
> On Fri, May 10, 2024 at 10:03:31AM GMT, Junio C Hamano wrote:
>> Phillip Wood <phillip.wood123@gmail.com> writes:
>>
>>>>>       $ git format-patch --stdout -1 $ThatCommit -- $ThisPath |
>>>>>         git am -3
>>>> Hmmm, I hadn't thought of that; very interesting!
>>>> Although I have some concerns with git-am(1); basically that it's
>>>> almost
>>>> clueless when there's a conflict.
>>>
>>> "git am -3" should be fine here as you're guaranteed to have the
>>> necessary blobs available to create conflicts - this is what "git
>>> rebase --apply" does.
>>
>> Good thing to point out.  "am -3" is just as good for this purpose
>> and "almost clueless" is a derogatory comment that requires an
>> apology ;-)
> 
> Huh, I am quite surprised by `git am -3`.  I've tried it just now, and
> it's amazing.  I certainly must apologize.  :-)
> 
> I tried it yesterday, but the patches were from a different repo, and
> not available locally, so it really didn't do anything.  But now I tried
> it within the same repo, and it's really nice!
> 
>>> As far as the implementation goes I haven't thought too deeply but I
>>> suspect we'd want to create a couple of trees based on the commit we
>>> want to cherry-pick and its parent filtered by the pathspec and use
>>> those in the tree-way merge with HEAD.
>>
>> If we were to use the ort machinery, it may not be too bad to use
>> the pathspec only at the final writeout phase.

>>  That is, perform a
>> full cherry-pick in the in-core index, reset all the entries in the
>> in-core index back to HEAD that are outside the given pathspec, and
>> then write out the result to the working tree.  That way, an old
>> change that was made to paths at the original location can be cherry
>> picked to a much newer tree after these paths have been moved to a
>> different location.  Doing the same with the recursive machinery
>> would be missier but perhaps the more recent merge-tree that uses
>> the ort machinery to work purely in-core might be a good way to go.

> I didn't understand the last part well, but I guess I may do when I
> start researching into it.  :)
> 
> Apart from <builtin/revert.c>, do you recommend I look into any
> particular files?

> sequencer.c. If we go for the "write new trees and use those in the merge"
> approach then we'd need to change do_pick_commit() to create the trees and
> we'd probably want to change do_recursive_merge() to take trees rather than
> commits. We'd also need to add a new pathspec member to struct replay_opts
> to pass the pathspec around.

> Hi Phillip,
> 
> On Sat, May 11, 2024 at 04:01:18PM GMT, Phillip Wood wrote:
> > sequencer.c. If we go for the "write new trees and use those in the merge"
> > approach then we'd need to change do_pick_commit() to create the trees and
> > we'd probably want to change do_recursive_merge() to take trees rather than
> > commits. We'd also need to add a new pathspec member to struct replay_opts
> > to pass the pathspec around.
> 
> I've been thinking this evening that since
> `git format-patch ... | git am -3` works so well, and since the behavior
> of cherry-pick -- path isn't so obvious (we're discussing different
> strategies), maybe we should just not do it.  I fell in love with am -3.