Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

git up does a silly thing if a feature branch is rebased on master but not force-pushed yet. #64

Open
fj128 opened this issue Sep 27, 2017 · 3 comments

Comments

@fj128
Copy link

fj128 commented Sep 27, 2017

If I work on my personal feature branch that I periodically rebase on master and force-push into origin, and I accidentally do git up after rebasing but before force-pushing, the result is weird and confusing: git up rebases local feature on remote feature, which produces rebased copies of all new commits in master.

image

Maybe it could be possible to try and detect such situation and at least warn that something weird is going on? For example, check if there are local/remote branches that we will be leaving behind after rebase?

Or, I don't know, if that would bring a lot of complications and slow down operation for everyone, while addressing a relatively unusual workflow, oh well, at least if someone else stumbles upon it, they'll see this issue here.

This shell script reproduces the problem:

#!/bin/sh
set -euxo pipefail

rm -rf remote/ local/
git init remote
echo a >> remote/a.txt # this file will be modified on remote
echo b >> remote/b.txt # this file will be modified on local
git -C remote add a.txt b.txt
git -C remote commit -am "0"
echo a >> remote/a.txt
git -C remote commit -am "1"

git clone remote local
git -C local checkout -b feature
echo b >> local/b.txt
git -C local commit -am "feature"
git -C local push --set-upstream origin feature

echo a >> remote/a.txt
git -C remote commit -am "2"
echo a >> remote/a.txt
git -C remote commit -am "3"
echo a >> remote/a.txt
git -C remote commit -am "4"
git -C local up # OK, updates master
git -C local rebase master

git -C local up # rebases local/feature on remote/feature, producing rebased versions of commits 2-3-4
@msiemens
Copy link
Owner

msiemens commented Oct 9, 2017

Hey @fj128,

first thanks for the awesome reproduction script and the effort you put into bug reports!

Now regarding the issue, the the trouble is that for PyGitUp this might be a perfectly valid situation. Before running git up the second time, the commit graph looks like this:

screenshot 2017-10-09 22 52 05

For PyGitUp it's just a situation where the local feature branch is a couple of commits ahead of its remote counterpart. You mentioned, we could

check if there are local/remote branches that we will be leaving behind after rebase?

How exactly would that look like? How would we differentiate this from a legal situation?

@fj128
Copy link
Author

fj128 commented Aug 22, 2018

So I stepped on the same rake today, and it's worse than purely accidentally doing git up, it's practically inevitable when I've rebased the local branch, did some work, and now want to check if there are new commits on master... I have to override a lot of muscle memory to type git fetch instead.

What I had in mind was to compute the set difference: set(git branch --merged feature) - set(git branch --merged origin/feature), when we are rebasing feature onto origin/feature, excluding feature itself of course, that is, all branches that will no longer be contained in the rebased branch. In my case it will contain master , and I think that in general if it's non-empty it means that something weird is going on.

A valid situation similar to this might be if you do

git branch tmp_before_rebase
git rebase master

But in this case you're typing the rebase command yourself, you generally wouldn't do git up instead or want git up to silently do that for you.

So it would be nice if git up at the very least printed a warning about all abandoned branches, but even better if it asked for confirmation (that is, required a --force flag I guess).

I don't know if this should be made default or enabled via a configuration option. On one hand, I really don't think that switching it on by default would break a lot of other people's workflows. On the other hand, the check will add a not-imperceptible delay, especially on larger repositories (and this applies to merely displaying warning too).

I also don't know what's the best way of computing that difference, I was surprised that apparently you can't specify both --merged and --unmerged to git branch, maybe a more efficient way would be to do an equivalent of

git rev-list feature ^origin/feature | xargs -L 1 git branch --points-at

because this would take time proportional to the number of rebased commits, not reachable branches in the whole repository. Or maybe there's a way to tell git what we want directly. Bonus points for also being able to list all would-be-abandoned tags at the same time, because we probably want a warning/confirmation for those as well (though I personally would be completely satisfied with it working for branches for now).

@msiemens
Copy link
Owner

Sorry for not following up on this for so long! I currently don't have the capacity to look deeper into this issue (as I'm focusing on finishing up my master's degree), but I'm more than happy to accept contributions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants