Fix "git rebase-update" when multiple branch are stale.

Fix "git rebase-update" when multiple branch are stale.

Fix a bug that left branches tracking a dead branch if both their parent
and grand-parent were left with no changes after a "rebase-update" step.

Given the following initial state:
  $ git map-branches -v
  origin/master
    a
      b
        c *        [ ahead 1 ]

without this patch, a "git rebase-update" on this tree state would
leave the branch "c" as tracking a non-existing branch "a":

  $ git recursive-rebase
  a up-to-date
  b up-to-date
  c up-to-date
  Reparented c to track a (was tracking b)
  Deleted branch b (was 448d1da).
  Deleted branch a (was 448d1da).
  $ git map-branches -v
  {a:GONE}
    c *

with the patch, we record that the branch "c" is tracking must be
updated twice and we end up in a state were "c" is correctly tracking
"origin/master":

  $ git recursive-rebase
  a up-to-date
  b up-to-date
  c up-to-date
  Reparented c to track origin/master (was tracking b)
  Deleted branch b (was 448d1da).
  Deleted branch a (was 448d1da).
  $ git map-branches -v
  origin/master
    c *            [ ahead 1 ]

BUG=456806
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=297792

[sdefresne, 2015-11-27 17:35:27]

sdefresne@chromium.org changed reviewers:
+ iannucci@chromium.org

[sdefresne, 2015-11-27 17:35:28]

Please take a look.

[sdefresne, 2015-11-27 17:36:04]

Description was changed from

==========
Fix "git rebase-update" when multiple branch are stale.

Fix a bug that left branches tracking a dead branch if both their parent
and grand-parent were left with no changes after a "rebase-update" step.

Given the following initial state:
  $ git map-branches -v
  origin/master
    a
      b
        c *        [ ahead 1 ]

without this patch, a "git rebase-update" on the following tree state
would leave the branch "c" as tracking a non-existing branch "b":

  $ git recursive-rebase
  a up-to-date
  b up-to-date
  c up-to-date
  Reparented c to track a (was tracking b)
  Deleted branch b (was 448d1da).
  Deleted branch a (was 448d1da).
  $ git map-branches -v
  {a:GONE}
    c *

with the patch, we move the branch that "c" is tracking twice and we end
up in a state were "c" is correctly tracking "origin/master":

  $ git recursive-rebase
  a up-to-date
  b up-to-date
  c up-to-date
  Reparented c to track origin/master (was tracking b)
  Deleted branch b (was 448d1da).
  Deleted branch a (was 448d1da).
  $ git map-branches -v
  $ git map-branches -v
  origin/master
    c *            [ ahead 1 ]

BUG=None
==========

to

==========
Fix "git rebase-update" when multiple branch are stale.

Fix a bug that left branches tracking a dead branch if both their parent
and grand-parent were left with no changes after a "rebase-update" step.

Given the following initial state:
  $ git map-branches -v
  origin/master
    a
      b
        c *        [ ahead 1 ]

without this patch, a "git rebase-update" on the following tree state
would leave the branch "c" as tracking a non-existing branch "a":

  $ git recursive-rebase
  a up-to-date
  b up-to-date
  c up-to-date
  Reparented c to track a (was tracking b)
  Deleted branch b (was 448d1da).
  Deleted branch a (was 448d1da).
  $ git map-branches -v
  {a:GONE}
    c *

with the patch, we move the branch that "c" is tracking twice and we end
up in a state were "c" is correctly tracking "origin/master":

  $ git recursive-rebase
  a up-to-date
  b up-to-date
  c up-to-date
  Reparented c to track origin/master (was tracking b)
  Deleted branch b (was 448d1da).
  Deleted branch a (was 448d1da).
  $ git map-branches -v
  $ git map-branches -v
  origin/master
    c *            [ ahead 1 ]

BUG=None
==========

[sdefresne, 2015-11-27 17:36:20]

Description was changed from

==========
Fix "git rebase-update" when multiple branch are stale.

Fix a bug that left branches tracking a dead branch if both their parent
and grand-parent were left with no changes after a "rebase-update" step.

Given the following initial state:
  $ git map-branches -v
  origin/master
    a
      b
        c *        [ ahead 1 ]

without this patch, a "git rebase-update" on the following tree state
would leave the branch "c" as tracking a non-existing branch "a":

  $ git recursive-rebase
  a up-to-date
  b up-to-date
  c up-to-date
  Reparented c to track a (was tracking b)
  Deleted branch b (was 448d1da).
  Deleted branch a (was 448d1da).
  $ git map-branches -v
  {a:GONE}
    c *

with the patch, we move the branch that "c" is tracking twice and we end
up in a state were "c" is correctly tracking "origin/master":

  $ git recursive-rebase
  a up-to-date
  b up-to-date
  c up-to-date
  Reparented c to track origin/master (was tracking b)
  Deleted branch b (was 448d1da).
  Deleted branch a (was 448d1da).
  $ git map-branches -v
  $ git map-branches -v
  origin/master
    c *            [ ahead 1 ]

BUG=None
==========

to

==========
Fix "git rebase-update" when multiple branch are stale.

Fix a bug that left branches tracking a dead branch if both their parent
and grand-parent were left with no changes after a "rebase-update" step.

Given the following initial state:
  $ git map-branches -v
  origin/master
    a
      b
        c *        [ ahead 1 ]

without this patch, a "git rebase-update" on the following tree state
would leave the branch "c" as tracking a non-existing branch "a":

  $ git recursive-rebase
  a up-to-date
  b up-to-date
  c up-to-date
  Reparented c to track a (was tracking b)
  Deleted branch b (was 448d1da).
  Deleted branch a (was 448d1da).
  $ git map-branches -v
  {a:GONE}
    c *

with the patch, we move the branch that "c" is tracking twice and we end
up in a state were "c" is correctly tracking "origin/master":

  $ git recursive-rebase
  a up-to-date
  b up-to-date
  c up-to-date
  Reparented c to track origin/master (was tracking b)
  Deleted branch b (was 448d1da).
  Deleted branch a (was 448d1da).
  $ git map-branches -v
  origin/master
    c *            [ ahead 1 ]

BUG=None
==========

[sdefresne, 2015-11-27 17:39:40]

Description was changed from

==========
Fix "git rebase-update" when multiple branch are stale.

Fix a bug that left branches tracking a dead branch if both their parent
and grand-parent were left with no changes after a "rebase-update" step.

Given the following initial state:
  $ git map-branches -v
  origin/master
    a
      b
        c *        [ ahead 1 ]

without this patch, a "git rebase-update" on the following tree state
would leave the branch "c" as tracking a non-existing branch "a":

  $ git recursive-rebase
  a up-to-date
  b up-to-date
  c up-to-date
  Reparented c to track a (was tracking b)
  Deleted branch b (was 448d1da).
  Deleted branch a (was 448d1da).
  $ git map-branches -v
  {a:GONE}
    c *

with the patch, we move the branch that "c" is tracking twice and we end
up in a state were "c" is correctly tracking "origin/master":

  $ git recursive-rebase
  a up-to-date
  b up-to-date
  c up-to-date
  Reparented c to track origin/master (was tracking b)
  Deleted branch b (was 448d1da).
  Deleted branch a (was 448d1da).
  $ git map-branches -v
  origin/master
    c *            [ ahead 1 ]

BUG=None
==========

to

==========
Fix "git rebase-update" when multiple branch are stale.

Fix a bug that left branches tracking a dead branch if both their parent
and grand-parent were left with no changes after a "rebase-update" step.

Given the following initial state:
  $ git map-branches -v
  origin/master
    a
      b
        c *        [ ahead 1 ]

without this patch, a "git rebase-update" on this tree state would
leave the branch "c" as tracking a non-existing branch "a":

  $ git recursive-rebase
  a up-to-date
  b up-to-date
  c up-to-date
  Reparented c to track a (was tracking b)
  Deleted branch b (was 448d1da).
  Deleted branch a (was 448d1da).
  $ git map-branches -v
  {a:GONE}
    c *

with the patch, we record that the branch "c" is tracking must be
updated twice and we end up in a state were "c" is correctly tracking
"origin/master":

  $ git recursive-rebase
  a up-to-date
  b up-to-date
  c up-to-date
  Reparented c to track origin/master (was tracking b)
  Deleted branch b (was 448d1da).
  Deleted branch a (was 448d1da).
  $ git map-branches -v
  origin/master
    c *            [ ahead 1 ]

BUG=None
==========

[iannucci, 2015-11-30 20:59:16]

On 2015/11/27 at 17:35:28, sdefresne wrote:
> Please take a look.

I think this may fix https://code.google.com/p/chromium/issues/detail?id=456806
?

[iannucci, 2015-12-01 02:46:22]

Thanks for working on this!

I think this looks really good. Would you mind adding a test for it here:
https://chromium.googlesource.com/chromium/tools/depot_tools/+/master/tests/g...
?

I think you should be able to add a regression test for this case pretty easily,
and it would add confidence to the code here. Let me know if you have issues
getting the tests working (they should work pretty well from linux/mac... from
windows it's a bit hairy, but possible).

https://chromiumcodereview.appspot.com/1482753002/diff/20001/git_rebase_updat...
File git_rebase_update.py (right):

https://chromiumcodereview.appspot.com/1482753002/diff/20001/git_rebase_updat...
git_rebase_update.py:116: if not reparents.get(down):
`if down not in reparents:`

? or am I misunderstanding this line?

[sdefresne, 2015-12-01 08:14:12]

Description was changed from

==========
Fix "git rebase-update" when multiple branch are stale.

Fix a bug that left branches tracking a dead branch if both their parent
and grand-parent were left with no changes after a "rebase-update" step.

Given the following initial state:
  $ git map-branches -v
  origin/master
    a
      b
        c *        [ ahead 1 ]

without this patch, a "git rebase-update" on this tree state would
leave the branch "c" as tracking a non-existing branch "a":

  $ git recursive-rebase
  a up-to-date
  b up-to-date
  c up-to-date
  Reparented c to track a (was tracking b)
  Deleted branch b (was 448d1da).
  Deleted branch a (was 448d1da).
  $ git map-branches -v
  {a:GONE}
    c *

with the patch, we record that the branch "c" is tracking must be
updated twice and we end up in a state were "c" is correctly tracking
"origin/master":

  $ git recursive-rebase
  a up-to-date
  b up-to-date
  c up-to-date
  Reparented c to track origin/master (was tracking b)
  Deleted branch b (was 448d1da).
  Deleted branch a (was 448d1da).
  $ git map-branches -v
  origin/master
    c *            [ ahead 1 ]

BUG=None
==========

to

==========
Fix "git rebase-update" when multiple branch are stale.

Fix a bug that left branches tracking a dead branch if both their parent
and grand-parent were left with no changes after a "rebase-update" step.

Given the following initial state:
  $ git map-branches -v
  origin/master
    a
      b
        c *        [ ahead 1 ]

without this patch, a "git rebase-update" on this tree state would
leave the branch "c" as tracking a non-existing branch "a":

  $ git recursive-rebase
  a up-to-date
  b up-to-date
  c up-to-date
  Reparented c to track a (was tracking b)
  Deleted branch b (was 448d1da).
  Deleted branch a (was 448d1da).
  $ git map-branches -v
  {a:GONE}
    c *

with the patch, we record that the branch "c" is tracking must be
updated twice and we end up in a state were "c" is correctly tracking
"origin/master":

  $ git recursive-rebase
  a up-to-date
  b up-to-date
  c up-to-date
  Reparented c to track origin/master (was tracking b)
  Deleted branch b (was 448d1da).
  Deleted branch a (was 448d1da).
  $ git map-branches -v
  origin/master
    c *            [ ahead 1 ]

BUG=456806
==========

[sdefresne, 2015-12-01 09:09:16]

I've added one unit test. Here is the log for the test passing:

$ tests/git_rebase_update_test.py 
..Switched to branch foobar.
Switched to branch int1_foobar.
Switched to branch int2_foobar.
Switched to branch sub_foobar.
Switched to branch sub_K.
Switched to branch old_branch.
Switched to branch empty_branch.
Switched to branch empty_branch2.
.Switched to branch foobar.
.
----------------------------------------------------------------------
Ran 4 tests in 23.698s

OK
Name                  Stmts   Miss  Cover   Missing
---------------------------------------------------
git_new_branch           28      0   100%   
git_rebase_update       174      0   100%   
git_rename_branch        29      0   100%   
git_reparent_branch      45      0   100%   
---------------------------------------------------
TOTAL                   276      0   100%   

BTW, I see that the "Switched to branch ${branch}." are due to git_new_branch.py
using sys.stderr.write instead of sys.stdout.write when outputting this message.
Do you think it would be worth changing to sys.stdout instead (as it is not an
error message).

https://chromiumcodereview.appspot.com/1482753002/diff/20001/git_rebase_updat...
File git_rebase_update.py (right):

https://chromiumcodereview.appspot.com/1482753002/diff/20001/git_rebase_updat...
git_rebase_update.py:116: if not reparents.get(down):
On 2015/12/01 at 02:46:22, iannucci wrote:
> `if down not in reparents:`
> 
> ? or am I misunderstanding this line?

This is equivalent, ack.

[sdefresne, 2015-12-01 09:12:15]

Patchset #3 (id:40001) has been deleted

[sdefresne, 2015-12-02 10:29:43]

Please take another look.

[iannucci, 2015-12-02 18:08:51]

lgtm

https://codereview.chromium.org/1482753002/diff/60001/tests/git_rebase_update...
File tests/git_rebase_update_test.py (right):

https://codereview.chromium.org/1482753002/diff/60001/tests/git_rebase_update...
tests/git_rebase_update_test.py:71: def testRebaseUpdate(self):
I've created a monster (unit test)!

I think I'm going to have to break this up later... it's getting pretty
silly-long :)

[iannucci, 2015-12-02 18:10:29]

On 2015/12/02 at 18:08:51, iannucci wrote:
> lgtm
> 
>
https://codereview.chromium.org/1482753002/diff/60001/tests/git_rebase_update...
> File tests/git_rebase_update_test.py (right):
> 
>
https://codereview.chromium.org/1482753002/diff/60001/tests/git_rebase_update...
> tests/git_rebase_update_test.py:71: def testRebaseUpdate(self):
> I've created a monster (unit test)!
> 
> I think I'm going to have to break this up later... it's getting pretty
silly-long :)

Oh, and the stderr->stdout change is probably worth doing, but I suspect it MAY
not work how you expect it to. The filedescriptor routing is kinda weird in
these tests. I wouldn't spend more than a couple minutes on it.

[sdefresne, 2015-12-02 18:12:03]

The CQ bit was checked by sdefresne@chromium.org

[commit-bot: I haz the power, 2015-12-02 18:12:08]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1482753002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1482753002/60001

[sdefresne, 2015-12-02 18:12:54]

Thank you for the review. I've not touched the sys.stderr/sys.stdout thing.

https://codereview.chromium.org/1482753002/diff/60001/tests/git_rebase_update...
File tests/git_rebase_update_test.py (right):

https://codereview.chromium.org/1482753002/diff/60001/tests/git_rebase_update...
tests/git_rebase_update_test.py:71: def testRebaseUpdate(self):
On 2015/12/02 at 18:08:51, iannucci wrote:
> I've created a monster (unit test)!
> 
> I think I'm going to have to break this up later... it's getting pretty
silly-long :)

:-) BTW, those tests are not independent (i.e. running just "testRebaseUpdate"
fails), but this pre-existed my CL:

$ tests/git_rebase_update_test.py GitRebaseUpdateTest.testRebaseUpdate
Switched to branch foobar.
Switched to branch int1_foobar.
Switched to branch int2_foobar.
Switched to branch sub_foobar.
Switched to branch sub_K.
Switched to branch old_branch.
F
======================================================================
FAIL: testRebaseUpdate (__main__.GitRebaseUpdateTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "tests/git_rebase_update_test.py", line 125, in testRebaseUpdate
    self.assertIn('Cannot rebase-update', output)
AssertionError: 'Cannot rebase-update' not found in ''

----------------------------------------------------------------------
Ran 1 test in 2.072s

FAILED (failures=1)
Name                  Stmts   Miss  Cover   Missing
---------------------------------------------------
git_new_branch           28      7    75%   37-40, 46-49
git_rebase_update       174    152    13%   34-43, 48-82, 87-137, 141-215,
238-320
git_rename_branch        29     22    24%   17-47
git_reparent_branch      45     37    18%   19-74
---------------------------------------------------
TOTAL                   276    218    21%   
FATAL: not at required 100.000000% coverage.

[commit-bot: I haz the power, 2015-12-02 18:13:55]

Description was changed from

==========
Fix "git rebase-update" when multiple branch are stale.

Fix a bug that left branches tracking a dead branch if both their parent
and grand-parent were left with no changes after a "rebase-update" step.

Given the following initial state:
  $ git map-branches -v
  origin/master
    a
      b
        c *        [ ahead 1 ]

without this patch, a "git rebase-update" on this tree state would
leave the branch "c" as tracking a non-existing branch "a":

  $ git recursive-rebase
  a up-to-date
  b up-to-date
  c up-to-date
  Reparented c to track a (was tracking b)
  Deleted branch b (was 448d1da).
  Deleted branch a (was 448d1da).
  $ git map-branches -v
  {a:GONE}
    c *

with the patch, we record that the branch "c" is tracking must be
updated twice and we end up in a state were "c" is correctly tracking
"origin/master":

  $ git recursive-rebase
  a up-to-date
  b up-to-date
  c up-to-date
  Reparented c to track origin/master (was tracking b)
  Deleted branch b (was 448d1da).
  Deleted branch a (was 448d1da).
  $ git map-branches -v
  origin/master
    c *            [ ahead 1 ]

BUG=456806
==========

to

==========
Fix "git rebase-update" when multiple branch are stale.

Fix a bug that left branches tracking a dead branch if both their parent
and grand-parent were left with no changes after a "rebase-update" step.

Given the following initial state:
  $ git map-branches -v
  origin/master
    a
      b
        c *        [ ahead 1 ]

without this patch, a "git rebase-update" on this tree state would
leave the branch "c" as tracking a non-existing branch "a":

  $ git recursive-rebase
  a up-to-date
  b up-to-date
  c up-to-date
  Reparented c to track a (was tracking b)
  Deleted branch b (was 448d1da).
  Deleted branch a (was 448d1da).
  $ git map-branches -v
  {a:GONE}
    c *

with the patch, we record that the branch "c" is tracking must be
updated twice and we end up in a state were "c" is correctly tracking
"origin/master":

  $ git recursive-rebase
  a up-to-date
  b up-to-date
  c up-to-date
  Reparented c to track origin/master (was tracking b)
  Deleted branch b (was 448d1da).
  Deleted branch a (was 448d1da).
  $ git map-branches -v
  origin/master
    c *            [ ahead 1 ]

BUG=456806

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=297792
==========

[commit-bot: I haz the power, 2015-12-02 18:13:56]

Committed patchset #3 (id:60001) as
http://src.chromium.org/viewvc/chrome?view=rev&revision=297792

[iannucci, 2015-12-02 18:22:21]

Blah, good to know... :/

On Wed, Dec 2, 2015, 10:13 commit-bot@chromium.org via
codereview.chromium.org <reply@chromiumcodereview-hr.appspotmail.com> wrote:

> Committed patchset #3 (id:60001) as
> http://src.chromium.org/viewvc/chrome?view=rev&revision=297792
>
> https://codereview.chromium.org/1482753002/
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.