Details

Reviewers

Maniphest Tasks

T9661: arc land doesn't return to local tracking branch if it has a different name from the remote branch it's tracking

Commits

rARCc844669326ed: After pushing at the end of "arc land", cascade the origin through all local…

Summary

Fixes T9661. Users can construct arbitrarily long chains from the remote, like:

(remote) origin/master -> (local) cascade-a -> (local) cascade-b -> (local) cascade-c -> (local) cascade-d

When a user lands "cascade-d" onto "origin/master", we should pull A, B and C if they aren't ahead of the remote.

If a user lands "cascade-d" onto itself, we should pull A, B, and C if they aren't ahead of the remote, then reset D to the remote.

We also find this chain if the last component of it is connected by the local branch having the same name as the remote branch (typical for "master") instead of an actual connection through tracking brnaches.

Test Plan

See comment below.

Diff Detail

Repository

rARC Arcanist

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

epriestley updated this revision to Diff 34671.Oct 28 2015, 7:37 PM

epriestley retitled this revision from to After pushing at the end of "arc land", cascade the origin through all local tracking branches.

epriestley updated this object.

epriestley edited the test plan for this revision. (Show Details)

epriestley added a reviewer: chad.

epriestley added a task: T9661: arc land doesn't return to local tracking branch if it has a different name from the remote branch it's tracking.

Conceivably we could actually put test coverage on this, but I don't really see a way to spend less than like a day writing it and then a lot of time per-test, and this "one hour" change has already spiraled into almost two days of work.

Here are the cases I tested:

With master -> cas-a -> cas-b -> cas-c -> cas-d, make a change on cas-d, land it. A B C get updated, D gets deleted, we end up on cas-c. This generally seems correct/reasonable.

...
 UPDATE  Local "cas-c" tracks target remote "origin/master", checking out and pulling changes.
 PULL  Checking out and pulling "master".
 PULL  Checking out and pulling "cas-a".
 PULL  Checking out and pulling "cas-b".
 PULL  Checking out and pulling "cas-c".
Cleaning up branch "cas-d"...
(Use `git checkout -b cas-d ff4b83fc4b7e1962e747788a1f66acce0538af4e` if you want it back.)
 DONE  Landed changes.

With that state, I dirtied cas-a and then landed cas-c:

...
 UPDATE  Local "cas-a" is ahead of remote "origin/master". Checking out "cas-b" but not pulling changes.
 CHECKOUT  Checking out "cas-b".
Cleaning up branch "cas-c"...
(Use `git checkout -b cas-c c3ba86f7553e26f9dd234facb8669eaede9fd3c1` if you want it back.)
 DONE  Landed changes.

This ends up on cas-b with cas-a unpulled.

I did the same thing again and hit the "empty changes" state since I'd dirtied cas-a the same way I dirtied cas-b:

...
Exception
Local "cas-b" is already exactly the same as "origin/master". This usually means these changes have already landed.

Then I hit an (expected) merge failure:

...
Exception
Local "cas-b" does not merge cleanly into "origin/master". Merge or rebase local changes so they can merge cleanly.

I rebased onto origin/master and landed:

...
 UPDATE  Local "cas-a" is ahead of remote "origin/master". Checking out "cas-a" but not pulling changes.
 CHECKOUT  Checking out "cas-a".
Cleaning up branch "cas-b"...
(Use `git checkout -b cas-b e814c3f4318ab4adde969daf2f8a3eff0d9e7114` if you want it back.)
 DONE  Landed changes.

Ended up on cas-a in the expected state.

Now I tried to land that and got an expected merge failure. I rebased and eventually landed and got a clean update by branch name equivalence:

...
 UPDATE  Local "master" tracks target remote "origin/master", checking out and pulling changes.
 PULL  Checking out and pulling "master".
Cleaning up branch "cas-a"...
(Use `git checkout -b cas-a 1ac7245a9c99de93a5160c5fe1a004a4910c4034` if you want it back.)
 DONE  Landed changes.

Then I landed that and got a reset-to-HEAD result:

...
 RESET  Local "master" landed into remote "origin/master", resetting local branch to remote state.
 DONE  Landed changes.

This isn't totally exhaustive because so many workflows are possible, but I think it covers most of the new behavior.

Improvements for "empty commit" and merge conflict error messages.

epriestley edited the test plan for this revision. (Show Details)Oct 28 2015, 8:12 PM

edibiase added a subscriber: edibiase.Oct 28 2015, 8:15 PM

Minor error message wordsmithing.

Fix missing words in comment.

Fix logInfo() -> writeInfo() call when restoring branch after arc land <other>.

epriestley mentioned this in T9661: arc land doesn't return to local tracking branch if it has a different name from the remote branch it's tracking.Oct 28 2015, 8:28 PM

chad accepted this revision.Oct 28 2015, 8:45 PM

chad edited edge metadata.

This revision is now accepted and ready to land.Oct 28 2015, 8:45 PM

Closed by commit rARCc844669326ed: After pushing at the end of "arc land", cascade the origin through all local… (authored by epriestley, committed by epriestley). · Explain WhyOct 28 2015, 9:01 PM

This revision was automatically updated to reflect the committed changes.

After pushing at the end of "arc land", cascade the origin through all local tracking branches
ClosedPublic
Actions

Details

Diff Detail

Event Timeline

Revision Contents
Changeset List

Diff 34680

src/land/ArcanistGitLandEngine.php

src/repository/api/ArcanistGitUpstreamPath.php

After pushing at the end of "arc land", cascade the origin through all local tracking branchesClosedPublicActions

Details

Diff Detail

Event Timeline

Revision ContentsChangeset List

Diff 34680

src/land/ArcanistGitLandEngine.php

src/repository/api/ArcanistGitUpstreamPath.php

After pushing at the end of "arc land", cascade the origin through all local tracking branches
ClosedPublic
Actions

Revision Contents
Changeset List