search

msiemens / PyGitUp

A nicer `git pull`
MIT License
528 stars 43 forks source link

Add --delete-deleted option to delete branches which have been deleted from the remote #41

Open johnddx opened 8 years ago

johnddx commented 8 years ago

Whenever this error occurs running git-up:

hotfix-write-process-output-to-console error: remote branch doesn't exist

There is an opportunity to clean up the local branches to reflect similar clean-up performed on the remote. --delete-deleted as an option name seems somewhat reflective of the intent. -d is probably suitable for short.

If -d is specified, a local branch should be deleted if all of the following are true:

Note: This is a heuristic. It is possible to assign an uplink to a branch without ever actually using it. This would mean the remote branch wasn't actually deleted, it was never created to begin with. However, as long as the branch is contained in another branch, it should still be safe to delete the local reference.

johnddx commented 8 years ago

Closed by mistake.

msiemens commented 8 years ago

If I remember correctly, this is what the git-up.fetch.prune option should do (set it via git config git-up.fetch.prune true). It's described in the README:

git-up.fetch.prune [*true*|false]: If set to true, PyGitUp will append the --pruneoption to git fetch and thus remove any remote tracking branches which no longer exist on the remote (see git fetch –help).

Could you check if it does what you want?

msiemens commented 8 years ago

@johnddx Have you tried using the setting git-up.fetch.prune as described in my previous comment?

johnddx commented 8 years ago

I'm concerned that it would delete branches that are not contained by another branch. (the remote deletion could have been a de-publishing act, rather than an indication that the branch is no longer needed). I might have to try it.

johnddx commented 8 years ago

Okay yes, there are actually several problems with "git fetch --prune":

1. "git fetch --prune" will remove branches that are not merged. If you push a branch, and a colleague deletes it on your behalf from the server to unpublish it, then when you "fetch --prune" your local branch will be deleted, even though it has not been merged. 2. "git fetch --prune" will only delete the branch the first time it notices the tracking branch was deleted. So if you run "git fetch" and wind up deleting several tracking branches in the process, a subsequent run of "git fetch --prune" will remove none of the local branches.

1 would not be a problem if only it confirmed the branch was contained in another branch first. That is somewhat arbitrary, so maybe it should be configurable which branches it should be contained in (i.e. default to master and develop).

2 is a problem because it pulls the rug out from under the feature. I run git fetch frequently, just to sample the state on the origin. I'm not going to make --prune the default, because of issue 1. I would have been happy if I could fetch, review the deleted branches, then prune. But that's not supported.

Basically, I think git fetch --prune is itself an incomplete feature, so I wouldn't use it for this problem as-is.

johnddx commented 8 years ago

I struck through most of my previous comment, because it was misguided.

The real problem with "git fetch --prune" is that it will only delete remote refs. It will never delete the local branch associated with the remote ref. As such, git-up.fetch.prune should not even be an option, it should just be the only behavior. And the feature requested in this issue can really only be implemented at the layer of git-up.

msiemens commented 8 years ago

The real problem with "git fetch --prune" is that it will only delete remote refs.

Ah, I see. So the locak tracking branch is kept while removing remotes/origin/....

As such, git-up.fetch.prune should not even be an option, it should just be the only behavior.

Unfortunately we can't change this without breaking compatibiltiy with the original aanand/git-up which PyGitUp is port of.

And the feature requested in this issue can really only be implemented at the layer of git-up.

I'm still undecided whether this should be a command line flag or rather a git config option like git-up.rebase.delete-deleted. What do you think about the latter?

msiemens commented 8 years ago

Also I think we only should remove the branch if it already has been merged, right?

johnddx commented 8 years ago

As such, git-up.fetch.prune should not even be an option, it should just be the only behavior.

Unfortunately we can't change this without breaking compatibiltiy with the original aanand/git-up which PyGitUp is port of.

It doesn't hurt to have prune as an option, although I imagine the newly requested behavior would depend on it being enabled. Maybe that means the new option would imply the old option.

And the feature requested in this issue can really only be implemented at the layer of git-up.

I'm still undecided whether this should be a command line flag or rather a git config option like git-up.rebase.delete-deleted . What do you think about the latter?

I didn't actually know about --prune when I made the original request. I'm wondering if we can build on that term here, e.g. "prune local" or "prune merged". --help can clarify that it would prune local merged branches and that it's safe to use all the time.

I think having an option like git-up.prune-local is fine, but it's the kind of thing someone might be paranoid about and prefer to add to the command-line. That's not me, though, so if you made it config-only I'd be happy. Well, presuming all the deleted branches were logged so that I would have something to audit.

In fact, it seems that the reflog for deleted branches is removed along with the branch. I.e. it won't be possible to do 'git reflog ' after the branch is deleted, so it might be a good idea to do this to force a recent reflog entry for HEAD:

git reset --soft <branch-to-delete>
git reset --soft HEAD{1}

This guarantees that any deleted branch has a recent entry in the reflog for HEAD. Except that is not a transactional sequence, so it would be better to:

$save=`git rev-parse HEAD`
# insert verification of success here...
git reset --soft <branch-to-delete>
git reset $save

Also I think we only should remove the branch if it already has been merged, right?

Definitely only delete a merged branch, that is one of the three conditions I listed in the original request.

msiemens commented 8 years ago

I'm wondering if we can build on that term here, e.g. "prune local" or "prune merged". --help can clarify that it would prune local merged branches and that it's safe to use all the time.

prune-local sounds good to me. Makes the intention quite clear: pruning local branches.

I think having an option like git-up.prune-local is fine, but it's the kind of thing someone might be paranoid about and prefer to add to the command-line. That's not me, though, so if you made it config-only I'd be happy. Well, presuming all the deleted branches were logged so that I would have something to audit.

Re config vs. arg: Actually we have some kind of inconsistency here: Except for no-fetch all configuration is done via git-config. I'd prefer using git-config for prune-local to not increase the discrepancy.

Re logging: We could use the same mechanism that's used for the other status output:

$ git up
Fetching origin
some-branch     rebasing
another-branch  deleting

In fact, it seems that the reflog for deleted branches is removed along with the branch. I.e. it won't be possible to do 'git reflog ' after the branch is deleted

Interesting, how did you come to this conclusion? According to the internet (http://blag.ahax.de/post/421939327/recovering-a-deleted-branch-using-git-reflog) and my own (admittedly quick) test git actually should keep the reflog for deleted branches.

Also, would you like to draw up a PR with that feature?

johnddx commented 8 years ago

Interesting, how did you come to this conclusion? According to the internet (http://blag.ahax.de/post/421939327/recovering-a-deleted-branch-using-git-reflog) and my own (admittedly quick) test git actually should keep the reflog for deleted branches.

What we're seeing, both in our experiments and on that web page, is that the reflog for HEAD is always around. If you were to run that same experiment in the blog, but then type 'git reflog gamelan', it would complain that 'gamelan' does not exist, because it was deleted.

'gamelan' happens to exist in the HEAD reflog because it was recently checked out. If it had been a year since anyone had checked out the 'gamelan' branch, and then the branch was deleted, then there would likely be no reflog pointing at that branch anymore, and no way to get it back. So the two reset commands I offered above were specifically geared toward guaranteeing that there is a recent reflog entry in HEAD for the soon-to-be-deleted branch.

We're only supposed to delete branches which are merged into other branches, but in the unlikely event we do the wrong thing, or that the user panics and realizes they really needed to know what merged commit the deleted branch was pointing to, they'll always be able to recover that information... for a time.

Also, would you like to draw up a PR with that feature?

I haven't worked with python before, but I suppose I can give it a try.

msiemens commented 8 years ago

So the two reset commands I offered above were specifically geared toward guaranteeing that there is a recent reflog entry in HEAD for the soon-to-be-deleted branch.

Ah, okay. I guess being more user-friendly than git definitely won't hurt :)

I haven't worked with python before, but I suppose I can give it a try.

Sounds great! Feel free to comment here (or create a WIP pull request and comment there) if you run into problems, I'd be glad to help.