On Fri, Apr 2, 2021 at 8:04 PM Elijah Newren <newren@xxxxxxxxx> wrote:
>
> On Fri, Apr 2, 2021 at 6:36 PM Jerry Zhang <jerry@xxxxxxxxxx> wrote:
> >
> > I'm creating a script/tool that will be able to cherry-pick
> > multiple commits from a single branch, rebase them onto a
> > base commit, and push those references to a remote.
> >
> > Ex. with a branch like "origin/master -> A -> B -> C"
> > The tool will create "master -> A", "master -> B",
> > "master -> C" and either make local branches or
> > push them to a remote. This can be useful since code
> > review tools like github use branches as the basis
> > for pull requests.
>
> Not sure I understand the "master -> A", "master -> B" syntax. What
> do you mean here?
Ah yeah my syntax wasn't super clear here.
I mean a branch "dev" pointing to commit "C", which is on top of "B",
which is on top of "A", which is on top of "master".
My tool would fake "cherry-pick" each of A, B, and C on top of master.
>
> > A key feature here is that the above happens without
> > any changes to the user's working directory or cache.
> > This is important since those operations will add
> > time and generate build churn. We use these steps
> > for synthesizing a "cherry-pick" of B to master.
> >
> > 1. cp .git/index index.temp
> > 2. set GIT_INDEX_FILE=index.temp
> > 3. git reset master -- . (git read-tree also works here, but is a bit slower)
> > 4. git format-patch --full-index B~..B
> > 5. git apply --cached B.patch
> > 6. git write-tree
> > 7. git commit-tree {output of 6} -p master -m "message"
> > 8. either `git symbolic-ref` to make a branch or `git push` to remote
>
> Yeah, folks have resorted to various variants of this kind of thing in
> the past. It is a clever way to handle some basic cases, but it does
> often fall short. It's unfortunate that cherry-pick and rebase cannot
> yet just provide this functionality (more on that below).
>
> It may also interest you that rebase has two different backends, one
> built on am (which in turn is built on format-patch + apply), and one
> built on the merge machinery (which the am --3way also uses when it
> needs to). We deprecated the format-patch + apply backend in part
> because it sometimes results in misapplied patches; see the "Context"
> subsection of the "BEHAVIORAL DIFFERENCES" section of the git-rebase
> manpage. However, the am version would at least handle basic renames,
> which I believe might cause problems for a direct format-patch + apply
> invocation like yours (I'll also discuss this more below).
Thanks -- I was able to repro a case where am machinery applied a patch
incorrectly but 3way applied it correctly. This actually brings up
another point,
because am doesn't report errors when applying a patch incorrectly in this
case, we don't end up falling back to 3way. There also is no user flag
to force 3way, so the user can't do anything to ensure the correct
application here. Maybe it would be better for --3way to directly invoke
the 3way merge rather than causing it to fallback? (Junio might also
have some input here).
>
> > I'm looking to improve the git apply step in #5.
> > Currently we can't use --cached in combination with
> > --3way, which limits some of the usefulness of this method.
> > There are many diffs that will block applying a patch
> > that a 3 way merge can resolve without conflicts. Even
> > in the case where there are real conflicts, performing
> > a 3 way merge will allow us to show the user the lines
> > where the conflict occurred.
> >
> > With the above in mind, I've created a small patch that
> > implements the behavior I'd like. Rather than disallow
> > the cached and 3way flags to be combined, we allow them,
> > but write any conflicts directly to the cached file. Since
> > we're unable to modify the working directory, it seems
> > reasonable in this case to not actually present the user
> > with any options to resolve conflicts. Instead, a script
> > or tool using this command can diff the temporary cache
> > to get the source of the conflict.
>
> Looks like you're focusing on content conflicts. What about path
> conflicts? For example, apply's --3way just uses a per-file
> ll_merge() call, meaning it won't handle renames, so your method would
> also often get spurious modify/delete conflicts when renames occur.
> How does your plan to just "cache" conflicts work with these
> modify/delete files? Will users just look for conflict markers and
> ignore the fact that both modified newfile and modified oldfile are
> present? I'm also curious how e.g. directory/file conflicts would be
> handled by your scheme; these seem somewhat problematic to me from
> your description.
>
> > Happy to address any feedback. After I address any major
> > changes I will add new tests for this path.
>
> Don't know the timeframe you're looking at, but I'm looking to modify
> cherry-pick and rebase to be able to operate on branches that are not
> checked out, or in bare repositories. The blocker to that
That functionality would be great. I initially did look at what it would
take to modify sequencer to get what I wanted, but I quickly realized
it would be a big refactor.
> traditionally has been that the merge machinery required a working
> directory. The good news is that I wrote a new merge backend that
> doesn't require a working directory. The bad news is I'm still trying
> to get that new merge backend through the review process, and no
> current release of git has a complete version of that new backend yet.
> Further, the changes to cherry-pick and rebase have not yet been
> started. There were some decisions to make too, such as how to handle
> the case with conflicts -- just report them and tell the user to retry
> with a checkout? Provide some kind of basic information about the
> conflicts? What'd be useful to you?
After thinking some more I'd generally agree with comments to leave
the conflicts at higher stages rather than check in the conflict markers.
This should result in less issues with path conflicts as well (or at least
be similar to 3way by itself). This is probably ok, because the conflict
markers can always be generated from the higher stage files (git diff or
git checkout -m -- .), but the reverse isn't true.
Overall 90% of the functionality comes from being able to do the 3-way
at all, since it's able to handle more cases correctly. Having *any* output
to tell the user why their operation failed would just be a bonus.
I'd envision flags similar to these, for cherry-pick
--cached : Do not touch the working tree to apply conflict markers.
Instead conflicts are left at a higher order in the cache.
--cached-parent : Checkout the index to the given commit, then
apply the cherry-pick with the given commit as a parent. Print out
the new commit. Warning: index will be left unsynchronized with
HEAD after this operation. Intended to be used with a temporary index
rather than the main one.
In the end I'm not sure how to still accomplish the desired functionality
without using a temporary index -- this would always result in
desyncing the user's index / working dir afterwards. Maybe error or
warn if the user isn't using a temporary index?
