Hi Alex and Junio
On 11/05/2024 12:46, Alejandro Colomar wrote:
Hi Phillip, Junio,
On Fri, May 10, 2024 at 10:03:31AM GMT, Junio C Hamano wrote:
Phillip Wood <phillip.wood123@xxxxxxxxx> 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 would be tempting if cherry-pick didn't have a "--strategy" option.
I think we need something more generic to cope with that.
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.
My hope was that the changes required to create a couple of new trees
that are then used instead of the original commits when a pathspec is
given would be fairly localized.
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.
Best Wishes
Phillip
