Page MenuHomePhabricator

Support for landing without local tracking branch
Closed, ResolvedPublic

Description

The typical git workflow for some members of my team is as follows:

  1. Exist in detached HEAD state most of the time
  2. When it's time to make changes, git checkout -b feature_branch origin/master and then commit
  3. Post a code review
  4. Address changes from the code review
  5. Squash (via rebase) all commits on the feature branch into a single commit that describes the change
  6. Push the commit directly to the remote tracking branch

As we've been trying out Phabricator, the people that like to use this workflow have been sad that they can't use arc land, because it seems to always want to merge the local feature branch into a local branch that is tracking a remote branch, before pushing to the remote. They don't like having the interstitial local branch, and would much prefer to just push the feature commit directly to the remote.

These users would love it if Arcanist supported this workflow, so that they could take advantage of its existing ability to squash commits and update the commit message, without requiring intermediate local branches.

Revisions and Commits

Event Timeline

edibiase updated the task description. (Show Details)
edibiase added a project: Arcanist.
edibiase added a subscriber: edibiase.

I think this is about the same as T9082, does that sound right? (Is that a report from another team member on your team? This workflow seems completely unorthodox to me, although maybe it is less bizarre than I imagine.)

Oh, are these different? They all seem like a similar workflow.

Yeah, they're a bit different. There's some nuance here, but basically T8620 is talking about landing from a detached HEAD, and T9082 is talking about landing without creating or interacting with a local target branch (usually "master"). This is potentially talking about both, I think, although more on the T9082 side.

The "land from detached head" part is reasonable and straightforward (if you already got yourself into that state, I'm comfortable assuming that you know what you're doing), just niche and hard to prioritize and entangled with a bunch of Mercurial stuff that should probably happen at the same time (T3855).

There's a similar case which is landing from a dirty local master to itself, which we should also make just work, probably, although this might be a mistake / novice error somewhat often.

The T9082 aspect of this (hide all the work and don't generate or interact with a local version of the target branch) I'm less sold on because I don't really understand what the cost of having a local "master" is beyond one extra line of text in git branch.

This may all sort of be moot since there might be a relatively unambiguous approach to T9082 and this which uses a rule like: if the target does not exist locally, but does exist in the remote, use a separate workflow.

One issue is that I'm not sure where users are expecting to end up afterward. If we leave them on their landed local branch this workflow seems worse than landing to a local branch since cleaning the branch up later seems like more work to me than ignoring a local master that you don't care about too much. So I guess we leave them on the remote master with a detached HEAD. This just seems like a lot of extra work in service of an arbitrary goal rather than like we're solving a real problem.

Thanks, as always, for your prompt and thoughtful responses!

I think this is about the same as T9082, does that sound right? (Is that a report from another team member on your team? This workflow seems completely unorthodox to me, although maybe it is less bizarre than I imagine.)

T9082 isn't from someone on my team, but it does sound pretty much like what the workflow is for a lot of my teammates.

Yeah, they're a bit different. There's some nuance here, but basically T8620 is talking about landing from a detached HEAD, and T9082 is talking about landing without creating or interacting with a local target branch (usually "master"). This is potentially talking about both, I think, although more on the T9082 side.

My team typically uses local feature branches, so I don't think they'd need the ability to land from a detached HEAD.

The T9082 aspect of this (hide all the work and don't generate or interact with a local version of the target branch) I'm less sold on because I don't really understand what the cost of having a local "master" is beyond one extra line of text in git branch.

We tend to have many release branches active on the remote, and it's very easy to create feature branches that track the appropriate remote release branch, e.g.:

git checkout -b feature1234 origin/releaseXYZ

Then, during ongoing development, feature1234 can easily be updated just by running git pull.

With local target branches, however, creating a new feature branch becomes more complicated:

  1. Create a local release branch that tracks the remote release branch
  2. Update the local release branch so that it's up-to-date with the remote release branch
  3. Check out a local feature branch that tracks the local release branch

Updating the local feature branch with changes from the remote release branch also seems more complicated, as users have to either:

  • update the local release branch and then pull those changes into the local feature branch; or
  • pull the changes from the remote branch directly in to the feature branch.

The first approach takes two steps, but has the advantage of leaving the local feature branch ahead of the local release branch by only the commits that are involved in implementing the feature.

The second approach takes only one step to update, which is nice. It does, however, leave the feature branch in the somewhat odd position of being more up-to-date with the remote than the local release branch—which is the branch that is actually tracking the remote. Additionally, this approach imposes the additional cognitive overhead of having to remember which remote release branch to pull from.

So, for these reasons, the folks I've talked to believe that using local feature branches that track remote release branches is considerably simpler and easier than interposing a local release branch, and, as a result, they're very reluctant to switch to using local release branches.

This may all sort of be moot since there might be a relatively unambiguous approach to T9082 and this which uses a rule like: if the target does not exist locally, but does exist in the remote, use a separate workflow.

One issue is that I'm not sure where users are expecting to end up afterward. If we leave them on their landed local branch this workflow seems worse than landing to a local branch since cleaning the branch up later seems like more work to me than ignoring a local master that you don't care about too much. So I guess we leave them on the remote master with a detached HEAD. This just seems like a lot of extra work in service of an arbitrary goal rather than like we're solving a real problem.

The script we use today stages all of the changes from the feature branch as a single commit on a temporary local branch, pushes that branch to the tracked branch on the remote, and then deletes the temporary local branch, leaving the user with a detached HEAD.

Given that, I think the fundamental feature request here is for arc land to target a temporary local branch that is used just for staging and pushing the final commit, rather than targeting an existing local branch. Other than that, the land workflow is pretty much exactly in line with the workflow that most members of my team use today.

Given that, I think the fundamental feature request here is for arc land to target a temporary local branch that is used just for staging and pushing the final commit, rather than targeting an existing local branch.

My primary concern with implementing this as the default/only behavior is that we won't have anywhere to put the user when we're done that makes sense to an inexperienced / novice Git user. Our options are either a detached HEAD or (sometimes) a possibly out-of-date local branch which won't contain their change.

I think novice users are often unfamiliar or at least not comfortable with the concept of a detached HEAD (it is normally difficult to end up in this state, and I don't think I've ever read a Git tutorial covering detached HEADs in the intro material), and I know they generally aren't familiar with using commands like git reflog to restore previous states if they make a mistake. Because they aren't yet comfortable with git reflog and there's no easy way to discover that it exists, they can reasonably conclude that changes which are recoverable from the reflog have been permanently destroyed. For inexperienced users using git by following any of the widely available novice user guides, I worry the reaction to ending up on a detached HEAD or on a local branch without their changes will be "git / arc destroyed my work forever".

Generally, if a user is used to using pull + merge/rebase + push, I want to do something similar in arc so that if there's an error (or they typed the wrong command by mistake) they have a better chance of understanding what has happened and how to proceed (or fix/undo their error if they got something wrong).

This also interacts with the current, undesirable behavior of arc land of stopping mid-workflow instead of restoring the original state when it encounters an error. T3855 discusses this a bit. Basically, this behavior made "sense" as the least-bad behavior in an environment at Facebook riddled with "Facebook problems", but seems like a major misstep in the world at large. I plan to change this and make the behavior "restore original state on error", but until this gets fixed the "confusing for novice users" problem is worse because if we hit an error partway through, we potentially dump them into a detached head that doesn't correspond to any meaningful, concrete artifact. Although experienced users can navigate this situation easily, it could be particularly confusing and alarming for novice users.

Basically:

  • I want to resolve T3855 -- at least the "restore original state on error" part, if not the whole thing -- before touching any of this, since it impacts behaviors here and gives us much more flexibility to take an unusual path between wherever we start and wherever we end up if we know users aren't going to ever see the path or be dropped into the middle of it.
  • Once T3855 is cleared, I belive T8620 (landing from detached head) is totally straightforward, although not relevant to this request.
  • After T3855, I strongly suspect we can use presence of the local branch as a reasonable heuristic for choosing a "this is a normal user following a standard workflow" pathway vs "this is a sophisticated user following a more advanced workflow" pathway and just skip the local branch if it doesn't exist without making the default flow where it does exist any more weird (that is, any more divergent from what the user would have done manually).

Then I think everyone's needs will be met and arc land should be a little less confusing overall.

There are also some requests somewhere for --remote-onto that I don't want to implement if we can help it, but they might be able to fold into this: basically making --onto become --remote-onto and just skipping the local branch if it doesn't exist.

I'm sure we'll still run into some cases with multiple remotes and users who want to land from local A into B and then push to remote C on origin D, where both A and B track other branches in different remotes, but I think this will give us reasonable/desirable behaviors across a wider range of inputs at least.

My primary concern with implementing this as the default/only behavior is that we won't have anywhere to put the user when we're done that makes sense to an inexperienced / novice Git user.

Oh, totally—I didn't mean to suggest that this be the default or only behavior, and I apologize if it came off that way! I think the current behavior is totally reasonable, and is definitely friendlier to the novice Git user. It makes total sense to have the current workflow be the default and then provide this workflow as an alternative for more advanced users.


Basically:

  • I want to resolve T3855 -- at least the "restore original state on error" part, if not the whole thing -- before touching any of this, since it impacts behaviors here and gives us much more flexibility to take an unusual path between wherever we start and wherever we end up if we know users aren't going to ever see the path or be dropped into the middle of it.
  • Once T3855 is cleared, I belive T8620 (landing from detached head) is totally straightforward, although not relevant to this request.
  • After T3855, I strongly suspect we can use presence of the local branch as a reasonable heuristic for choosing a "this is a normal user following a standard workflow" pathway vs "this is a sophisticated user following a more advanced workflow" pathway and just skip the local branch if it doesn't exist without making the default flow where it does exist any more weird (that is, any more divergent from what the user would have done manually).

Then I think everyone's needs will be met and arc land should be a little less confusing overall.

This sounds great! It'll be awesome to have the original state restored when errors occur, and I agree that the presence of the local target branch is a solid heuristic for determining which workflow to use. Since we have users who like to have local target branches as well as those who don't, having Arcanist make the decision and do the right thing will work very well for us.

Thanks again for being so responsive and for putting so much thought into this.

After D14356, we ignore the local branch (if it exists) until we've pushed.

Then we try to guess if you want to be put on it, or put somewhere else, or left on a detached HEAD. We will probably get this right some of the time.