mackyle / topgit

TopGit - A different patch queue manager
https://repo.or.cz/topgit/pro.git
Other
67 stars 7 forks source link

updating a parent branch tries to update removed branches #6

Closed pmolodo closed 3 years ago

pmolodo commented 6 years ago

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.

mackyle commented 6 years ago

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.

mackyle commented 6 years ago

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.

pmolodo commented 6 years ago

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:

https://github.com/mackyle/topgit/issues/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.

pmolodo commented 6 years ago

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

branch_removal_conflict.txt

mackyle commented 3 years ago

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.

pmolodo commented 3 years ago

Awesome, good to hear! Thanks!