mackyle/topgit

updating a parent branch tries to update removed branches

Closed this issue · 6 comments

ie, if you have a topgit branch parent, that has this in it's .topdeps:

child1
child2

Then, say someone else upstream removes child2 from the .topdeps. When you try to do a tg-update of your parent branch, merging in their changes from your topgit remote, it will attempt to update child2 against the remote's deps, even though it has been removed from the dependencies in the place you're trying to update to.

This can result in unnecessary commits / merges, and merge conflicts.

Please note that although topgit-0.19.10 was just recently tagged, it does not
attempt to address this so there’s no overwhelming reason to upgrade from 0.19.9.

First question is how was child2 removed from the .topdeps file?

I believe the README was quite specific about this:

IMPORTANT: DO NOT EDIT .topdeps MANUALLY!!! If you do so, you
need to know exactly what you are doing, since this file must stay
in sync with the Git history information, otherwise very bad things
will happen.

It’s only "safe" to remove a line from a .topdeps file when that dependency does not introduce any changes either directly or indirectly (via its own dependencies).

Otherwise all kinds of nasty conflicts and merge issues are likely to ensue.

The tg depend revert (currently vaporware) command will address this at some point by essentially rebuilding the base without the dependencies that one wishes to remove and then applying a patch in the style of the git revert command to the base that effectively removes exactly the changes introduced by the dependencies being removed and nothing else and only then removes lines from the .topdeps file.

If the remote "correctly" removed the child2 line and then updated all its branches, when you fetched (if your branches were also all up-to-date locally) then what should happen is this:

  1. If the remote had used tg annihilate on child2 then your local
    child2 branch would end up being fast-forwarded to that state.
  2. Your local branch parent’s base would end up being fast-forwarded to
    the remote’s revision that includes whatever "patch" was needed to remove
    the effect of the child2 dependency.
  3. Your local branch parent itself would then end up being fast-forwarded
    to the remote’s revision that had already picked up that base change.

There shouldn’t be any excessive merging going on when a dependency line has been removed correctly and the TopGit branches are being kept up-to-date.

(If the remote left the actual child2 branch alone [sort of "virtually" annihilating it] then the update process, even though it would still look at the child2 branch, would see it’s up-to-date and leave it alone locally as well.)

I will take an action item to make sure the tg push command will, at a minimum, warn about pushing TopGit branches that are currently out-of-date – perhaps requiring a special option to allow such a thing.

In light of this additional information, can you provide more information on what exactly was the cause of the "unnecessary commits / merges, and merge conflicts?"

Was something out-of-date that shouldn’t have been or the child2 dependency was not removed entirely correctly?

I would like to understand the exact use case that caused this before adding any tg update options to alter the update process.

TopGit 0.19.11 has been released. tg push will now refuse to push out-of-date TopGit branches unless a special option is given.

Finally had a chance to come back to this, and I have a clear replication scenario. Yes, it involves manually editing the .topdeps (since there's no other way to remove a branch!), but it always merges a "cleanup" commit which undoes the changes introduced by that branch into the top-base, so it's as clean as I can think of making it.

See attached bash script for replication...

I think the best way to resolve this is to change slightly the order in which update handles merges - see this issue for details:

#8

Basically, I think that if we merge the remote branches in before merging in dependencies, we have a chance to "realize" that branches have been removed before "unnecessarily" merging them in.

This is a .sh bash-script renamed to .txt to allow uploading...

branch_removal_conflict.txt

The topgit-0.19.13 release includes initial support for skipping remote-removed dependencies.

That support also provides the beginnings of support needed to be able to add dependency removal support to the tg depend command without it causing lots of problems. There's still a ways to go, but the specific example in the script you provided does work now and there's even a new test for it t5012.

Awesome, good to hear! Thanks!