| Submitter | Adam Simpkins |
|---|---|
| Date | May 30, 2017, 7:47 p.m. |
| Message ID | <3988f1c49f0d86d6e0c8.1496173635@devbig125.prn1.facebook.com> |
| Download | mbox | patch |
| Permalink | /patch/21074/ |
| State | Accepted |
| Headers | show |
Comments
Adam Simpkins <simpkins@fb.com> writes: > # HG changeset patch > # User Adam Simpkins <simpkins@fb.com> > # Date 1496173480 25200 > # Tue May 30 12:44:40 2017 -0700 > # Node ID 3988f1c49f0d86d6e0c8a7bbfaf738d70fddf916 > # Parent 78c8966c02759755235a6566e0c557b82ff9cb0a > pull: fix pull --rebase with a --tool argument > > The code previously stripped off the --rebase argument when invoking the > original pull() command code, but did not strip off --tool, causing the command > to incorrectly fail. > > This fixes the code to also strip off --tool when invoking pull, and then pass > this argument in when doing the rebase. Ah, good catch; queued! > However, the way the rebase code invokes merge does seem to result in rather > confusing behavior: the "internal:other" tool preserves the local changes, > while "internal:local" preserves the changes pulled from the remote. Isn't this the case with all rebases (it switches the sides)?
Adam Simpkins <simpkins@fb.com> writes: > # HG changeset patch > # User Adam Simpkins <simpkins@fb.com> > # Date 1496173480 25200 > # Tue May 30 12:44:40 2017 -0700 > # Node ID 3988f1c49f0d86d6e0c8a7bbfaf738d70fddf916 > # Parent 78c8966c02759755235a6566e0c557b82ff9cb0a > pull: fix pull --rebase with a --tool argument > > The code previously stripped off the --rebase argument when invoking the > original pull() command code, but did not strip off --tool, causing the command > to incorrectly fail. > > This fixes the code to also strip off --tool when invoking pull, and then pass > this argument in when doing the rebase. > > However, the way the rebase code invokes merge does seem to result in rather > confusing behavior: the "internal:other" tool preserves the local changes, > while "internal:local" preserves the changes pulled from the remote. > > diff --git a/remotenames.py b/remotenames.py > --- a/remotenames.py > +++ b/remotenames.py > @@ -880,8 +880,9 @@ > if dest: > # Let `pull` do its thing without `rebase.py->pullrebase()` > del opts['rebase'] > + tool = opts.pop('tool', '') > ret = orig(ui, repo, source, **opts) > - return ret or rebasemodule.rebase(ui, repo, dest=dest) > + return ret or rebasemodule.rebase(ui, repo, dest=dest, tool=tool) > else: > return orig(ui, repo, source, **opts) > > diff --git a/tests/test-pull-rebase.t b/tests/test-pull-rebase.t > --- a/tests/test-pull-rebase.t > +++ b/tests/test-pull-rebase.t > @@ -151,3 +151,62 @@ > $ hg -q pull > $ hg log -l 1 --template="{desc} {remotenames}\n" > between_pull default/bookmarkonremote default/default > + > +Test pull with --rebase and --tool > + $ cd ../remoterepo > + $ hg up bookmarkonremote -q > + $ echo remotechanges > editedbyboth > + $ hg add editedbyboth > + $ mkcommit remotecommit > + $ cd ../localrepo > + $ hg book -t default/bookmarkonremote -r default/bookmarkonremote tracking2 > + $ hg update tracking2 -q > + $ echo localchanges > editedbyboth > + $ hg add editedbyboth > + $ mkcommit somelocalchanges > + $ printdag > + @ somelocalchanges | tracking2 | > + | > + o between_pull | | default/bookmarkonremote > + | > + o foo | | > + | > + | o localcommit | bmnottracking tracking | > + | | > + | o untrackedremotecommit | | > + | | > + o | trackedremotecommit | | > + |/ > + o root | | > + > + $ hg pull --rebase --tool internal:union > + pulling from $TESTTMP/remoterepo Forgot a (glob) here: $ hg locate | sed 's-\\-/-g' | > xargs "$check_code" --warnings --per-file=0 || false + tests/test-pull-rebase.t:183: + > pulling from $TESTTMP/remoterepo + use (glob) to match Windows paths too + [1] test-check-code-hg.t says "hi" ;-) (fixed in-flight)
Patch
diff --git a/remotenames.py b/remotenames.py --- a/remotenames.py +++ b/remotenames.py @@ -880,8 +880,9 @@ if dest: # Let `pull` do its thing without `rebase.py->pullrebase()` del opts['rebase'] + tool = opts.pop('tool', '') ret = orig(ui, repo, source, **opts) - return ret or rebasemodule.rebase(ui, repo, dest=dest) + return ret or rebasemodule.rebase(ui, repo, dest=dest, tool=tool) else: return orig(ui, repo, source, **opts) diff --git a/tests/test-pull-rebase.t b/tests/test-pull-rebase.t --- a/tests/test-pull-rebase.t +++ b/tests/test-pull-rebase.t @@ -151,3 +151,62 @@ $ hg -q pull $ hg log -l 1 --template="{desc} {remotenames}\n" between_pull default/bookmarkonremote default/default + +Test pull with --rebase and --tool + $ cd ../remoterepo + $ hg up bookmarkonremote -q + $ echo remotechanges > editedbyboth + $ hg add editedbyboth + $ mkcommit remotecommit + $ cd ../localrepo + $ hg book -t default/bookmarkonremote -r default/bookmarkonremote tracking2 + $ hg update tracking2 -q + $ echo localchanges > editedbyboth + $ hg add editedbyboth + $ mkcommit somelocalchanges + $ printdag + @ somelocalchanges | tracking2 | + | + o between_pull | | default/bookmarkonremote + | + o foo | | + | + | o localcommit | bmnottracking tracking | + | | + | o untrackedremotecommit | | + | | + o | trackedremotecommit | | + |/ + o root | | + + $ hg pull --rebase --tool internal:union + pulling from $TESTTMP/remoterepo + searching for changes + adding changesets + adding manifests + adding file changes + added 1 changesets with 2 changes to 2 files (+1 heads) + (run 'hg heads .' to see heads, 'hg merge' to merge) + rebasing 6:1d01e32a0efb "somelocalchanges" (tracking2) + merging editedbyboth + saved backup bundle to $TESTTMP/localrepo/.hg/strip-backup/*-backup.hg (glob) + $ printdag + @ somelocalchanges | tracking2 | + | + o remotecommit | | default/bookmarkonremote + | + o between_pull | | + | + o foo | | + | + | o localcommit | bmnottracking tracking | + | | + | o untrackedremotecommit | | + | | + o | trackedremotecommit | | + |/ + o root | | + + $ cat editedbyboth + remotechanges + localchanges