git checkout without arguments from branch saves branch dump as master

[cgunther]

I just hit this, didn't get a chance to dig in, but documenting it a bit for future self.

If you're on branch foo and run git checkout (no arguments, meant to run git checkout . to discard changes in my working directory), git leaves you on branch foo, but the hooks seem to cause branchbot to dump the state of the current database (branch) as master, which effectively just blew away your master dump, replacing it with your feature dump.

I'll have to come back and test this a bit more to confirm, but assuming that's reproducible, I'd be curious if we can catch this and do something better, either not dumping at all (you're not really changing branches), or at least not dumping to master.

Edit: Should also note the feature branch didn't have any commits yet, so it's head commit would have matched master at that point. Perhaps if there had been a commit already, that deviation in head commit would have caused it to dump the feature DB named after the feature, rather than after master.

[jimryan]

@cgunther Nice catch. This is definitely a bug. I don't think branchbot should do anything if the previous and new HEADs are the same ref (e.g. checked out the current branch, or just git checkout). What do you think?

This is severe enough that I think branchbot should actually refuse to do anything in that scenario, but as a temporary workaround, you can update the conditional in your post-checkout hook to this:

if [ "$3" = "1" ] && [ $1 != $2 ]
then
  # branchbot...
fi

That will only run branchbot when the old and new HEAD refs are different.

[cgunther]

In both of your examples, I agree because you're not actually changing branches, however I don't think that's the extent of possibilities for both heads matching. Imagine this scenario, starting on master:

$ git checkout -b foo
# Dumps master

# Do something manipulating data without committing code changes

$ git checkout master

The heads match, so we can assert no code changed, however we can't guarantee no data changed. In a perfect world, we'd almost need a "head" reference from the database. Certainly an obscure case, but theoretically possible.

Maybe this is a scenario we prompt for what to do, assuming prompting is possible within a hook?

[jimryan]

Ah great point.

Firstly, we need to identify what went wrong in your initial example, because we do have some safeguards around dumping/restoring when the source/destination branches are the same.

I'm guessing that your foo branch shared its HEAD with master. Is that right? If so, branchbot would currently (incorrectly) identify master as the source branch when running git checkout on the foo branch.

If that's accurate, the problem is that branchbot is not very good at identifying the source branch name. There's no easy way to find it (that I'm aware of). We kind of hack it by saying "show me all the branches with the current ref as its HEAD that isn't the branch we just came from", but that's still error-prone, as you saw. Example:

# on master

$ git checkout -b foo
# dumps to master

$ git checkout -b bar
# dumps to foo and master

$ git checkout foo
# dumps to master and bar, restores foo

The problem there is that master, foo, and bar all have the same ref as their HEAD.

This behavior is probably also not desired/wrong. What we really want to do is always dump to the source branch name. Not all branches that have the same HEAD as the source ref.

I don't know how we get there, but assuming all of the above is accurate, and that is in fact the issue you ran into - we really should find a way to identify the source branch name. Otherwise, we should prompt the user as you said (e.g. "Which branch did you just come from? [list out branches with HEADs matching the source ref])

[cgunther]

Yup, check out a new branch foo, mess with the DB without committing anything, then do something to trigger branchbot.

If you don't mind depending on git output, git branch shows an asterisk next to the current branch:

$ git branch
  foo
* bar
  master

On old capistrano deploys, we used to grab the branch name for staging like:

`git branch` =~ /\* (\S+)\s/m
puts $1
=> "bar"

We conditionally set it as the deploy branch as I'd guess it may not mark a branch if you're in detached head mode, where you have an individual commit checked out (or maybe a tag too, or maybe even in the midst of an interactive rebase).

[jimryan]

Yeah there's a few good ways to find the current branch, and I think our current code is reliable in doing that.

However, it's more difficult to find the name of the branch the user is coming from. The post-checkout hook is given the hash for the old and new branch HEADs, but no names, and is invoked after the branch is already checked out. So it's on us to derive the source branch name from the source branch's HEAD (likely impossible - there's simply not enough info).

We really need something like a pre-checkout hook and to then store some state while the branch is changed, but that doesn't exist.

Another option might be to completely change how branchbot is used, and instead of relying on git hooks, rely on a CLI directly. So branchbot would proxy commands to git, but give us an opportunity to do stuff at any point (e.g. branchbot checkout could grab the current branch name, run a git checkout, then grab the new branch name, and do its thing). But that's quite heavy-handed.

Barring another option that I'm just not aware of, I think our only remaining option is to ask the user to select the appropriate branch when the source branch is ambiguous. Assuming we can prompt mid-git-hook.

[cgunther]

Ah, right, it's post-checkout, so I guess that means by the time this runs, the current branch is the target, not the source. Mentally, since branchbot hasn't returned my terminal yet, I assume the branch hasn't actually changed, but that's likely wrong.

This was a once-in-seven-years issue, so certainly no urgency, just wanted to document it while it was fresh.

[jimryan]

so I guess that means by the time this runs, the current branch is the target, not the source.

Exactly. Not an easy problem to solve unfortunately, but definitely worth solving.

I actually run into this all the time and I see branchbot dumping the database multiple times because the source ref is the HEAD for multiple branches. But it hasn't (yet) caused a major issue for me. But it certainly could.

I'm also never overly concerned because TimeMachine takes frequent backups of my branch backups, so I can always restore them if needed (and have done that on multiple occasions). But not everyone has that safety net.

Let's leave this open as a bug (which I think is accurate), and we'll likely tackle as a prompt if that's possible from a hook.