Page MenuHomePhabricator

Make "arc land" great again

Authored by epriestley on Oct 28 2015, 2:13 AM.
Referenced Files
Unknown Object (File)
Tue, Aug 9, 1:04 PM
Unknown Object (File)
Wed, Aug 3, 7:07 AM
Unknown Object (File)
Wed, Aug 3, 7:07 AM
Unknown Object (File)
Wed, Aug 3, 7:06 AM
Unknown Object (File)
Wed, Aug 3, 7:06 AM
Unknown Object (File)
Sat, Jul 23, 10:06 PM
Unknown Object (File)
Fri, Jul 22, 8:27 AM
Unknown Object (File)
Wed, Jul 20, 10:37 PM
"Mountain of Wealth" token, awarded by joshuaspence."Love" token, awarded by tycho.tatitscheff."Baby Tequila" token, awarded by jhurwitz."Love" token, awarded by nipunn."Yellow Medal" token, awarded by avivey.



Ref T3855. Fixes T9537. Fixes T8620. Fixes T4333.

This declares bankruptcy and replaces the entire arc land workflow under Git. These are the notable changes:

  • (T3855) You can now land from a branch to itself.
  • (T3855) We now try to restore the original state very aggressively after any failure, instead of dumping you into the middle of a mess.
  • (T9537) You can now land without a local branch.
  • ([not actually] T9543) We'll now ignore the local branch if it just happens to be named the same thing as the remote branch but doesn't actually track it.
  • (T8620) You can now land from a detached HEAD.
  • (T4333) We now preserve the author and author date of whatever you land.

This may need some followup work. In particular:

  • The signal handler (that tries to put you in a better place if you ^C in the middle of things) causes ^C to work awkwardly in prompts. This might not be worth it.
  • Errors/instructions on push/merge issues might need work.
  • I dropped support for --delete-remote and --update-with-blah-blah because I think these flags aren't worth their complexity.
  • I've simplified the update/merge algorithm a bit. It may need some complexity added back in.
  • I probably missed a few things because this covers like 200 unique, creative workflows.
  • Users might need more guidance on the workflows that drop them in the middle of nowhere if they manage to reach them more often than I think.
Test Plan
  • Used arc land to land like at least 15,000 different kinds of changes.
  • Landed normally.
  • Landed from a branch onto itself.
  • Landed from a detached head.
  • Landed nothing.
  • Landed with no local branch.
  • Landed onto made-up branches.
  • Landed with bad targets.
  • ^C'd things in the middle.

Diff Detail

rARC Arcanist
Lint Not Applicable
Tests Not Applicable

Event Timeline

chad edited edge metadata.
This revision is now accepted and ready to land.Oct 28 2015, 2:27 AM
  • Hold the SIGINT stuff for now. I'm not sure we need it, and there may be better approaches even if we do.
  • Do upstream branch comparison a little better?
  • Work around potential Windows escaping issues for now.
This revision was automatically updated to reflect the committed changes.

Would you be in favor of Mercurial workflow attempting to mimic this new Git workflow or are there too many differences between how changes are managed (I really only know super simple workflows in Git, not entirely sure what detached HEAD means)? The new workflow I think would be beneficial is "landing branch onto self" - developers frequently forget to either create a bookmark to begin with or deactivate/switch off master so it doesn't get ahead of remote. Yesterday I started fooling around with D14356 to see if I could translate Git to Mercurial but didn't get far enough to know if I should stop.

Yes, but I'd like to wait for this stuff to stabilize a bit first. T9661 made this way more complex and recent activity on T3855 suggests I may have broken a bunch of stuff.

This is also an awkward mishmash of git-specific and land-general code right now. My ideal pathway forward is:

  • I kind of poke this enough so that it stabilizes.
  • Some time in the future when we have infinite time we write a nice test suite.
  • We cover the behavior in lots of lovely tests, then refactor all the stuff into engines.

Depending on how badly broken this actually is maybe my hand is about to get forced on doing the test suite now, though.