[TopGit PATCH] t/depend-add-using-export

[TopGit PATCH] t/depend-add-using-export

[Olaf Dabrunz]

When dependencies can be removed as well as added, tg depend add
needs to make sure that the new dependency can bring in changes
from a branch that has previously been removed as a dependency
from the current TopGit branch.

This implementation uses an exported branch set up by tg export,
and merges the new dependency into the commit that corresponds to
the current base. Using the exported branch in the merge has the
advantage that removed dependencies do not appear as parents, and
the merge base selected by git merge does not include changes from
a removed dependency. As a result, these changes can be merged in
again if the new dependency brings in these changes.

The tree of the merge commit is then used to create the next
commit on the TopGit base branch.

Uwe Kleine-König had the idea to use tg export here.

Signed-off-by: Olaf Dabrunz <odabrunz@gmx.net>

---

This is an experimental implementation. It did not get much testing, mainly
because testing ran into a fatal "tg export" bug with my test repository. "tg
export" could be convinced to continue past the error with "bash -x", and that
is how this patch was tested.

This is also my first feature patch for TopGit, so I expect people to have
comments.

Known issues:
    - .top* files in the base should probably be excluded with pretty_tree
    - handle tg export producing zero commits for a dependency


 tg-depend.sh |   80 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 80 insertions(+), 0 deletions(-)

diff --git a/tg-depend.sh b/tg-depend.sh
index 474ccda..4d1572d 100644
--- a/tg-depend.sh
+++ b/tg-depend.sh
@@ -60,6 +60,86 @@ depend_add()
 	grep -F -x -e "$name" "$root_dir/.topdeps" >/dev/null &&
 		die "tg: $current_name already depends on $name"
 
+	tmp_export="_tg_tmp/tg_depend"
+	trap 'git branch -D "$tmp_export" 2>/dev/null || true' EXIT
+
+	# Create a separate history that does not contain parent pointers to
+	# removed dependencies. This makes sure that these dependencies can
+	# come in again if any of them were merged into the new dependency.
+	# (I.e., not having any parents leading to removed deps makes sure that
+	# "git merge new_dep" selects a merge base that does not already
+	# include the removed dependencies.)
+
+	# tg export sets up a branch with the following properties:
+	#   - For each of the exported TopGit branch and it's recursive TopGit
+	#     dependencies, zero, one or two commits are created. If there is
+	#     more than one dependency, one commit is created that contains the
+	#     tree from the tip of the base branch (i.e., the merge of all
+	#     current dependencies). If the TopGit patch branch is non-empty,
+	#     another commit contains the collapsed patch.
+	#   - Only current dependencies from .topdeps are considered; removed
+	#     dependencies are not part of the history of the branch.
+	#   - The .top* files are removed.
+	#
+	# tg export bails if the TopGit branch is not up-to-date.
+	$tg export "$tmp_export" || exit
+
+	# Now checkout the commit representing the base of the current branch
+	# in the export, and use that to merge in the new dep, resulting in the
+	# new base tree.
+	# FIXME: also handle the zero commits case -- does TopGit actually make
+	#        that case possible?
+	if branch_empty "$current_name"; then
+		# There is no patch commit. Use tmp_export directly.
+		git checkout -q "$tmp_export"
+	else
+		# Set $tmp_export to it's parent commit, which has the merged
+		# deps without the current patch on top, and check that out.
+		# FIXME: keep the tree of the HEAD commit with the collapsed
+		# patch / new dep merge resolution and use it in the commit on
+		# the patch branch, instead of running tg update? -- this would
+		# re-use conflict resolutions (and other changes) -- but would
+		# git rerere not kick in anyway?
+		git update-ref "refs/heads/$tmp_export" "$tmp_export^"
+		git checkout -q "$tmp_export"
+	fi
+
+	# merge the new dependency into the temp export
+	if ! git merge "$name"; then
+		# open up a shell to work on the conflict
+		info "You are in a subshell. If you abort the merge,"
+		info "use \`exit 1\` to abort adding the dependency."
+		if ! sh -i </dev/tty; then
+			info "Ok, you aborted the merge. Now, you just need to"
+			info "switch back to some sane branch using \`git checkout\`."
+			exit 3
+		fi
+	fi
+
+	# commit the tree of the complete dependency merge on top of the base branch
+	# git-commit-tree $tree -p $topgitbasebranch -p $newdep
+	new_commit="$(
+		echo "Merge branch '$name' into refs/top-bases/$current_name" |
+		git commit-tree "$tmp_export^{tree}" -p "refs/top-bases/$current_name" -p "$name")"
+
+	git update-ref "refs/top-bases/$current_name" "$new_commit"
+
+	# now update the patch branch
+	git checkout -q "$current_name"
+
+	# TODO: improve .topfiles handling by merging them specially
+	# (not always with topgit's "ours" strategy):
+	#   - when merging deps, which may include topgit branches,
+	#	- .topmsg must not be changed: use "ours", or drop the file in
+	#	                               the base branch altogether
+	#	- .topdeps information from deps is irrelevant to the current
+	#	  branch: use "ours", or drop the file in the base branch
+	#   - when merging the base branch into the patch branch, "ours" is ok,
+	#     for the same reasons as above
+	#   - when merging from collaborator's branches (git pull or push
+	#     from/to a remote), do a real merge ("recursive"): the
+	#     collaborator's changes matter on this branch level (TODO)
+
 	echo "$name" >>"$root_dir/.topdeps"
 	git add -f "$root_dir/.topdeps"
 	git commit -m"New TopGit dependency: $name"
-- 
tg: (9404aa1..) t/depend-add-using-export (depends on: master)

Re: [TopGit PATCH] t/depend-add-using-export

[Bert Wesarg]

On Sat, Oct 9, 2010 at 03:43, Olaf Dabrunz <odabrunz@gmx.net> wrote:
> When dependencies can be removed as well as added, tg depend add
> needs to make sure that the new dependency can bring in changes
> from a branch that has previously been removed as a dependency
> from the current TopGit branch.
>
> This implementation uses an exported branch set up by tg export,
> and merges the new dependency into the commit that corresponds to
> the current base. Using the exported branch in the merge has the
> advantage that removed dependencies do not appear as parents, and
> the merge base selected by git merge does not include changes from
> a removed dependency. As a result, these changes can be merged in
> again if the new dependency brings in these changes.
>
> The tree of the merge commit is then used to create the next
> commit on the TopGit base branch.

I'm really not an expert, but history comes from the commits not from
the trees. Just using an hand made tree and commit this to the base
doesn't change anything, because somewhere in the parent commits is
still the information, that we merged in a branch that we just tried
to remove (i'm speaking for tg depend remove). But maybe I don't
understand what this patch changes. The only idea is, that we don't
use merge commits for the base any more! Is this what this patch does?

Bert

Re: [TopGit PATCH] t/depend-add-using-export

[Olaf Dabrunz]

On 09-Oct-10, Bert Wesarg wrote:
> On Sat, Oct 9, 2010 at 03:43, Olaf Dabrunz <odabrunz@gmx.net> wrote:
> > When dependencies can be removed as well as added, tg depend add
> > needs to make sure that the new dependency can bring in changes
> > from a branch that has previously been removed as a dependency
> > from the current TopGit branch.
> >
> > This implementation uses an exported branch set up by tg export,
> > and merges the new dependency into the commit that corresponds to
> > the current base. Using the exported branch in the merge has the
> > advantage that removed dependencies do not appear as parents, and
> > the merge base selected by git merge does not include changes from
> > a removed dependency. As a result, these changes can be merged in
> > again if the new dependency brings in these changes.
> >
> > The tree of the merge commit is then used to create the next
> > commit on the TopGit base branch.
> 
> I'm really not an expert, but history comes from the commits not from
> the trees. Just using an hand made tree and commit this to the base
> doesn't change anything, because somewhere in the parent commits is
> still the information, that we merged in a branch that we just tried
> to remove (i'm speaking for tg depend remove). But maybe I don't

When the hand made tree is committed (as a hand made merge) on top of
the base, we have a correct merge on top of the base.

This actually should change everything. After such a "tg depend add", we
have either brought in some removed dependency as well, or we did not.
If we didn't, the removed deps in the base's history do not matter for
subsequent merges done by "tg update". But if we did bring in a
previously removed dep, the removed deps in the base's history _would_
matter. But since we already made a successful merge on top of the
base's history with a correct tree, and this merge also brings in the
removed dep through the history from the new dep's side, subsequent "tg
update" merges will use this hand made merge as the merge base, which is
fine for future merges.

> understand what this patch changes. The only idea is, that we don't
> use merge commits for the base any more! Is this what this patch does?

Maybe I addressed this above already. But just to clarify, I left the
following description of the patch in this mail, because it describes
the patch with different words.

What the patch does is the following: it still creates a merge commit on
the base to bring in a new dependency, but this merge commit is crafted
to contain the "correctly" merged tree. So there are three steps:

    - use tg export to create a completely separate history (the "temp
      export branch") which _does not_ reference removed dependencies as
      parents
    - use git merge to merge in the new dependency on top of this
      temp export branch -- as none of the removed dependencies are
      referenced by the temp export branch, this merge is not influenced
      by them and will do the right thing
    - then take the tree (!) from that merge and hand-craft a new merge
      commit with that tree and use the old base and the new dependency
      as parents of this hand-crafted merge commit

Olaf

-- 
Olaf Dabrunz (Olaf.Dabrunz <at> gmx.net)

Re: [TopGit PATCH] t/depend-add-using-export

[Bert Wesarg]

On Sat, Oct 9, 2010 at 12:54, Olaf Dabrunz <Olaf.Dabrunz@gmx.net> wrote:
> When the hand made tree is committed (as a hand made merge) on top of
> the base, we have a correct merge on top of the base.
>
> This actually should change everything. After such a "tg depend add", we
> have either brought in some removed dependency as well, or we did not.
> If we didn't, the removed deps in the base's history do not matter for
> subsequent merges done by "tg update". But if we did bring in a
> previously removed dep, the removed deps in the base's history _would_
> matter. But since we already made a successful merge on top of the
> base's history with a correct tree, and this merge also brings in the
> removed dep through the history from the new dep's side, subsequent "tg
> update" merges will use this hand made merge as the merge base, which is
> fine for future merges.

When I understand this correctly, this hand made merge commit on base,
has also the just removed dep as parent. But the tree does not include
any code from that dep. So git merge-base would select this commit as
the merge base.

Right?

Bert

>
> Olaf
>
> --
> Olaf Dabrunz (Olaf.Dabrunz <at> gmx.net)
>
>

Re: [TopGit PATCH] t/depend-add-using-export

[Bert Wesarg]

[ Just bringing back all Cc's ]

On Sat, Oct 9, 2010 at 14:10, Olaf Dabrunz <Olaf@dabrunz.com> wrote:
> On 09-Oct-10, Bert Wesarg wrote:
>> On Sat, Oct 9, 2010 at 12:54, Olaf Dabrunz <Olaf.Dabrunz@gmx.net> wrote:
>> > [...]
>>
>> When I understand this correctly, this hand made merge commit on base,
>> has also the just removed dep as parent. But the tree does not include
>
> The hand made merge commit on base only has a removed dep as a parent if
> that removed dep is brought in again directly, as the new dep that is
> added here. The only parents of the hand made merge commit are: the tip
> of the current topgit branch's base and the new dependency.
>
>    hand made merge commit on base =                        \
>        git-commit-tree $tree_from_merge_on_export          \
>                        -p $topgit_base_branch -p $new_dep
>
> In general, the new dep may already have merged in a dep that we
> previously removed from our topgit branch. So the previously removed dep
> is brought in indirectly by merging a branch that contains it.
>
> This patch tries to cover both cases. The latter case and requirements
> for tg depend add to cover it were discussed by you and Uwe in this
> thread:
>
> http://lists-archives.org/git/688698-add-list-and-rm-sub-commands-to-tg-depend.html
>
>> any code from that dep. So git merge-base would select this commit as
>
> If the new dep _is_ a previously removed dep, the code of the new dep is
> contained in the hand made merge as well.
>
> If the new dep _brings in_ a previously removed dep, it depends on how
> the previously removed dep was merged into the new dep we are trying to
> merge now. If our new dep merged in the previously removed dep but used
> the "ours" merge strategy to supersede that dep's contents, it does not
> bring in any code from that dep but only brings in that dep as part of
> it's history.
>
>> the merge base.
>
> Yes, as the previously removed dep is now -- directly or indirectly --
> part of the history of the hand made merge on base.
>
> I should really draw graphs. But here is the mail anyway (release
> early...).
>
> Olaf
>
> --
> Olaf Dabrunz (Olaf <at> dabrunz.com)
>
>

Re: [TopGit PATCH] t/depend-add-using-export

[Olaf Dabrunz]

On 09-Oct-10, Bert Wesarg wrote:
> On Sat, Oct 9, 2010 at 12:54, Olaf Dabrunz <Olaf.Dabrunz@gmx.net> wrote:
> > [...]
> 
> When I understand this correctly, this hand made merge commit on base,
> has also the just removed dep as parent. But the tree does not include

The hand made merge commit on base only has a removed dep as a parent if
that removed dep is brought in again directly, as the new dep that is
added here. The only parents of the hand made merge commit are: the tip
of the current topgit branch's base and the new dependency.

    hand made merge commit on base =                        \
        git-commit-tree $tree_from_merge_on_export          \
                        -p $topgit_base_branch -p $new_dep

In general, the new dep may already have merged in a dep that we
previously removed from our topgit branch. So the previously removed dep
is brought in indirectly by merging a branch that contains it.

This patch tries to cover both cases. The latter case and requirements
for tg depend add to cover it were discussed by you and Uwe in this
thread:

http://lists-archives.org/git/688698-add-list-and-rm-sub-commands-to-tg-depend.html

> any code from that dep. So git merge-base would select this commit as

If the new dep _is_ a previously removed dep, the code of the new dep is
contained in the hand made merge as well.

If the new dep _brings in_ a previously removed dep, it depends on how
the previously removed dep was merged into the new dep we are trying to
merge now. If our new dep merged in the previously removed dep but used
the "ours" merge strategy to supersede that dep's contents, it does not
bring in any code from that dep but only brings in that dep as part of
it's history.

> the merge base.

Yes, as the previously removed dep is now -- directly or indirectly --
part of the history of the hand made merge on base.

I should really draw graphs. But here is the mail anyway (release
early...).

Olaf

-- 
Olaf Dabrunz (Olaf <at> dabrunz.com)