lmi
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [lmi] Merge vs. cherry-pick


From: Vadim Zeitlin
Subject: Re: [lmi] Merge vs. cherry-pick
Date: Sun, 18 Oct 2020 20:29:31 +0200

On Sun, 18 Oct 2020 12:37:11 +0000 Greg Chicares <gchicares@sbcglobal.net> 
wrote:

GC> On 2020-10-10 12:27, Vadim Zeitlin wrote:
[...]
GC> >  As usual, I'd have preferred "git merge xanadu/wx-submodules" but if you
GC> > prefer linear history, this works, of course. An alternative to cherry
GC> > picking would be to create a local branch and then rebase it on master, 
but
GC> > this basically boils down to the same thing.
GC> 
GC> After of course doing this:
GC>   git fetch shangri-la/lmi-so
GC> we're discussing several methods:
GC> 
GC>   (1) git merge --ff-only shangri-la/lmi-so
GC> 
GC>   (2) git checkout --track shangri-la/lmi-so
GC>       git rebase origin/master
GC>       git switch master
GC>       git merge --ff-only lmi-so
GC> 
GC>   (3) git cherry-pick ..shangri-la/lmi-so
GC> 
GC> where AIUI (2) is your last sentence quoted above.

 Yes, exactly.

GC> What are the preference
GC> rankings, though? I'm sure you prefer (1) to (3), but is your ranking
GC>   1 > 2 = 3 , or
GC>   1 = 2 > 3 ?

 It's just a plain fact that "2 = 3" (please don't quote me on this out of
context), i.e. this is just objectively true and not subject to ranking
because git-rebase just automates cherry picking all the commits in a
guided process. And, as I indeed prefer (1) to either of them, my ranking
is the first one above (leaving aside the fact that I prefer the solution
(0), which is "git merge --no-ff", to all of them).

GC> And, since I do still prefer linear history, is my ranking
GC>   3 > 2 = 1 , or
GC>   3 = 2 = 1 ?

 Again, you just can't argue for "3 > 2" because they're the same. However
I don't think you can argue for either of them being "> 1" neither because
(1) still results in linear history (due to the use of "--ff-only") and if
it works at all, it's the same as (2) or (3), except with less work. So IMO
your ranking should be almost exactly the same as the one above, i.e.

        1 (if possible) ≥ 2 = 3

with the only small difference is that I think you're rather indifferent
between (1) and (2/3). Still, as I wrote above, if (1) can be done, I don't
understand why wouldn't you do it, it's just less work all around for
exactly the same result.

 The difference with my view is that my true ranking is

        0 >> 1 (if possible) > 2 = 3

and for me "1 > 2" just because doing a fast-forward merge preserves the
existing commit SHA-1 and so allows me to do "git branch -d" locally and
also closes the PR on GitHub automatically. With (2) or (3), I need to
either use "git branch -D" if I'm sure the branch was merged or, if I want
to be careful, do "git rebase" of this branch on top of your changes to
confirm that there are no changes left. And I have to close the GitHub PR
manually. Neither is a big deal, of course, but, once again, if the choice
is between doing extra work and not doing it, I know what I prefer.


GC> Here's what I did today. First of all, I had a few unpushed commits of
GC> my own, so I pushed them in order to start clean. Looking back, I guess
GC> that's exactly what ideally I would not have done, and instead I would
GC> have reset to origin/master, stashed my commits, merged the branch, and
GC> then popped the stash.

 No, sorry, you can't stash the commits. You can only stash uncommitted
changes. If you had already committed them, you could have just kept them
and applied the PR changes after them. Or, alternatively, if you wanted to
do (1), you can do it first and then rebase your existing commits on top of
the results of (1).

GC> Anyway, continuing...
GC> 
GC> /opt/lmi/src/lmi[0]$git checkout --track shangri-la/lmi-so
GC> M       third_party/wxpdfdoc
GC> Branch 'lmi-so' set up to track remote branch 'lmi-so' from 'shangri-la'.
GC> Switched to a new branch 'lmi-so'
GC> cmp: /opt/lmi/free/src/lmi//third_party/wxpdfdoc: Is a directory
GC> 
GC> [That seemed to work, but the 'cmp' error makes me uneasy.]

 I have really no idea where does it come from. Is this some local hook? It
must be, because I don't see where could "/opt/lmi/free" path come from
otherwise...

 OTOH it's normal for third_party/wxpdfdoc to be modified, it hasn't been
updated in this branch. I'm afraid this is another (but, AFAIR, last)
non-obvious and surprising feature of Git submodules: unlike normal files
and directories, Git does _not_ update them automatically when switching
branches. I guess this is done for performance reasons (switching branches
is done often when working with Git and is usually quasi-instantaneous,
while switching to a different submodule commit might take a relatively
long time) and I'm used enough to it by now to find this normal, but I
still understand that it can be surprising.

 If you want to synchronize the state of submodule(s) with the current
branch, you need to run "git submodule update" on them. But this means, of
course, that you'll also need to re-do it again when switching back to the
original branch -- and so usually you wouldn't do it if you planned to not
have the new branch checked out for long, as in this case.

GC> /opt/lmi/src/lmi[0]$git status
GC> On branch lmi-so
GC> Your branch is up to date with 'shangri-la/lmi-so'.
GC> 
GC> Changes not staged for commit:
GC>   (use "git add <file>..." to update what will be committed)
GC>   (use "git restore <file>..." to discard changes in working directory)
GC>         modified:   third_party/wxpdfdoc (new commits)
GC> 
GC> [Oh, no..."(new commits)" again.]

 This is not necessarily a problem (and isn't in this case). I don't know
if it's a good advice, but some people prefer to use an alias for
git-status or set status.submoduleSummary to ignore submodules in its
output by default.

GC> /opt/lmi/src/lmi[0]$git rebase origin/master 
GC> First, rewinding head to replay your work on top of it...
GC> cmp: /opt/lmi/free/src/lmi//third_party/wxpdfdoc: Is a directory

[I still have no idea where does this come from]

GC> Applying: Fix using of LMI_SO macros for functions
GC> Applying: Fix the SO attribute for the class forward delaration
GC> Applying: Don't use disable-auto-import linker flag for the native Linux 
build
GC> Applying: Enable lmi building with SO attributes for the native CI build
GC> 
GC> /opt/lmi/src/lmi[0]$git switch master
GC> Switched to branch 'master'
GC> Your branch is up to date with 'origin/master'.
GC> 
GC> /opt/lmi/src/lmi[0]$git merge --ff-only lmi-so
GC> Updating a8cb8e8d..0f314d35
GC> Fast-forward
GC> 
GC> [Those last few steps seemed to work.]

 Yes, everything seems fine.

GC> Are those steps, which are (2) above, ideal from your POV?
GC>  - ideal, because they preserve each commit message (but
GC>      doesn't the cherry-pick technique do that as well?); or
GC>  - not ideal, because they change the SHA1s?

 Not quite ideal because SHA-1s change, but good enough, thanks.

GC> Either way, if you rebase or I do, the SHA1s change; but do
GC> you prefer to rebase shangri-la, because then the altered
GC> SHA1s are "yours" in some sense, rather than have me rebase
GC> my tracking branch, because then the SHA1s are "mine"?

 If we really want to split hairs, yes, "my" SHA-1s are slightly better
because then I (force-)update the PR myself and it will still be closed
automatically. But it's not a big difference. Using (0) or (1) (when it's
possible) would be better, but otherwise it doesn't matter much.

GC> Anyway, I wanted to get this done this morning before turning
GC> to other things, and the messages above:
GC>   cmp: /opt/lmi/free/src/lmi//third_party/wxpdfdoc: Is a directory
GC>   modified:   third_party/wxpdfdoc (new commits)
GC> scared me off using method (2), so I used (3) instead:
[...]
GC> and now `git status` is clean and third_party/wxpdfdoc doesn't
GC> seem to have any weird problems,

 However the end result is the same as before.

GC> Maybe you've already explained, and I just haven't understood, but what
GC> drawback does this method have from your POV?

 None, compared to (2). I still think that (2) is slightly preferable from
your point of view because e.g. it's easier to cancel a rebase if anything
goes wrong, but it's up to you to decide whether it really is or not, of
course.

GC> Is it just that now we have equivalent commits, but with different
GC> SHA1s? Can you resolve that by running some easy command (I can't guess
GC> what it would be) that brings everything back into alignment? Or have I
GC> created some irresolvable problem on your side?

 No, nothing like this.

GC> BTW, the wxpdfdoc submodule looks good:
GC> 
GC> /opt/lmi/src/lmi[0]$git submodule status third_party/wxpdfdoc
GC>  1839b231c5138edd40b752272631e2f1d5311456 third_party/wxpdfdoc 
(v0.9.8-13-g1839b23)
GC> 
GC> after method (3). Have you any idea why method (2) reintroduced the
GC>   modified:   third_party/wxpdfdoc (new commits)
GC> issue?

 I hope to have explained this above, but please let me know if I haven't
answered your questions there.

 OTOH I'm still very curious about the "cmp" error message. I'm 99% certain
that it comes from something specific to your local setup, but I don't
really know what could it be and how could you have done it without being
aware of it. Please let me know if you can find where does it come from.

 Thanks,
VZ

Attachment: pgp73E_lToP4o.pgp
Description: PGP signature


reply via email to

[Prev in Thread] Current Thread [Next in Thread]