[TopGit PATCH] tg-create.sh: Introduce --add option to add a dependency.

[TopGit PATCH] tg-create.sh: Introduce --add option to add a dependency.

[Jan Nieuwenhuizen]

This implements 

    tg create --add DEP

to add dependency DEP to an already existing topgit branch.

The bad thing is that this does not play well with tg undepend;
it won't work to re-add a previously removed dependency.  This
--add is implemented as a merge, and all merge commits are
already present; it is only that lateron they are reverted.

Any ideas on how to fix that?

Signed-off-by: Jan Nieuwenhuizen <janneke@gnu.org>
---
 README       |    6 +++++-
 tg-create.sh |   44 ++++++++++++++++++++++++++++++++++++--------
 2 files changed, 41 insertions(+), 9 deletions(-)

diff --git a/README b/README
index 096b9ec..a9957f2 100644
--- a/README
+++ b/README
@@ -215,6 +215,9 @@ tg create
 	it will detect that you are on a topic branch base ref and
 	resume the topic branch creation operation.
 
+	With the --add option, all given arguments are added as
+	dependencies to current topic branch.
+
 tg delete
 ~~~~~~~~~
 	Remove a TopGit-controlled topic branch of given name
@@ -333,7 +336,8 @@ tg export
 tg undepend
 ~~~~~~~~~~~
 	Update the current topic branch by removing the given
-	branch (required argument) from the list of dependencies.
+	branch (required argument) from the list of dependencies
+	and reverting all its commits.
 
 tg update
 ~~~~~~~~~
diff --git a/tg-create.sh b/tg-create.sh
index 939af33..c5bc5fb 100644
--- a/tg-create.sh
+++ b/tg-create.sh
@@ -3,9 +3,11 @@
 # (c) Petr Baudis <pasky@suse.cz>  2008
 # GPLv2
 
+add= # Set to 1 when adding dependencies to existing topgit branch
 deps= # List of dependent branches
 restarted= # Set to 1 if we are picking up in the middle of base setup
 merge= # List of branches to be merged; subset of $deps
+merged= # List branches actually merged
 name=
 
 
@@ -14,8 +16,12 @@ name=
 while [ -n "$1" ]; do
 	arg="$1"; shift
 	case "$arg" in
+	--add)
+		add=1
+		name=$(git symbolic-ref HEAD | cut -b 12-)
+		;;
 	-*)
-		echo "Usage: tg create NAME [DEPS...]" >&2
+		echo "Usage: tg create --add|NAME [DEPENDENCY]..." >&2
 		exit 1;;
 	*)
 		if [ -z "$name" ]; then
@@ -50,12 +56,22 @@ fi
 
 [ -n "$merge" -o -n "$restarted" ] || merge="$deps "
 
+if [ -z "$add" ]; then
+	! git rev-parse --verify "$name" >/dev/null 2>&1 \
+		|| die "branch '$name' already exists"
+else
+	dupes=$(grep -E "^${merge// /|}/\$" .topdeps | tr '\n' ' ')
+	[ -z "$dupes" ] || die "already depend on: $dupes"
+	deps=$(echo "$merge" | cat .topdeps - | tr '\n' ' ' | sed -e 's/ \+$//')
+	merged="$merge"
+	merge="$name $merge"
+fi
+
+
 for d in $deps; do
 	git rev-parse --verify "$d" >/dev/null 2>&1 ||
 		die "unknown branch dependency '$d'"
 done
-! git rev-parse --verify "$name" >/dev/null 2>&1 ||
-	die "branch '$name' already exists"
 
 # Clean up any stale stuff
 rm -f "$git_dir/top-name" "$git_dir/top-deps" "$git_dir/top-merge"
@@ -96,7 +112,13 @@ done
 
 ## Set up the topic branch
 
-git update-ref "refs/top-bases/$name" "HEAD" ""
+if [ -z "$add" ]; then
+	git update-ref "refs/top-bases/$name" "HEAD" ""
+else
+	#[ -n "$add" ] && git -D "$name"
+	git branch -D save/"$name" || :
+	git branch -m "$name" save/$name
+fi
 git checkout -b "$name"
 
 echo "$deps" | sed 's/ /\n/g' >"$root_dir/.topdeps"
@@ -104,7 +126,7 @@ git add "$root_dir/.topdeps"
 
 author="$(git var GIT_AUTHOR_IDENT)"
 author_addr="${author%> *}>"
-{
+[ -z "$add" ] && {
 	echo "From: $author_addr"
 	! header="$(git config topgit.to)" || echo "To: $header"
 	! header="$(git config topgit.cc)" || echo "Cc: $header"
@@ -120,7 +142,13 @@ EOT
 } >"$root_dir/.topmsg"
 git add "$root_dir/.topmsg"
 
+if [ -z "$add" ]; then
+	info "Topic branch $name set up. Please fill .topmsg now and make initial commit."
+	info "To abort: git rm -f .top* && git checkout ${deps%% *} && tg delete $name"
+else
+	git commit -am "Add dependency: $merged"
+fi
 
-
-info "Topic branch $name set up. Please fill .topmsg now and make initial commit."
-info "To abort: git rm -f .top* && git checkout ${deps%% *} && tg delete $name"
+# Local Variables:
+# sh-basic-offset:8
+# End:
-- 
1.6.0.rc0.44.g67270


-- 
Jan Nieuwenhuizen <janneke@gnu.org> | GNU LilyPond - The music typesetter
http://www.xs4all.nl/~jantien       | http://www.lilypond.org

Re: [TopGit PATCH] tg-create.sh: Introduce --add option to add a dependency.

[Jonathan Nieder]

Hi,

Jan Nieuwenhuizen wrote:

> The bad thing is that this does not play well with tg undepend;
> it won't work to re-add a previously removed dependency.  This
> --add is implemented as a merge, and all merge commits are
> already present; it is only that lateron they are reverted.
> 
> Any ideas on how to fix that?

Interesting - I had imagined changing dependencies working in an
entirely different way.

Let's say your history is

    B -- (some mess) -- P

where P is your current branch head, and B is the current top base, a
merge of all your current dependencies.  Now you change your
dependencies drastically, and you end up with a new top base B'.  Then
ideally you want your new branch head P' to have the same tree as if you
had run "git rebase --onto B' B P", but of course you want to preserve
the history, so you could

	$ git checkout -b P' P
	$ git rebase --onto B' B
	$ git checkout P
	$ git merge --no-ff --no-commit B'   (*)
	$ git read-tree -u P'
	$ git commit
	$ git branch -D P'

or something like that.  The line marked with a (*) doesn't work as it
should in current git as far as I remember, but it would be a simple
change.  The point is to achieve the result

    B -- (some mess) -- old P -- P
                                /
                              B'

with the diff from B to old P being approximately the same as the diff
from B' to P, even if we just dropped some dependencies.

The main problem I see with this story is that if B' is just B with some
new changes added this is overly complicated.  In other words, in the
simple case of moving from (say) a patch based on master to a patch
based on next, one would probably prefer

    B -- (some mess) -- old P -- P
     \                         /
      ---------------------- B'

and similarly for moving from depending on master+(one additional patch)
to next+(that same patch).

Hope this helps, and sorry I don't have something more constructive to
say.  Thanks for starting this going.

Jonathan

Re: [TopGit PATCH] tg-create.sh: Introduce --add option to add a dependency.

[Jan Nieuwenhuizen]

On wo, 2008-08-13 at 11:20 -0500, Jonathan Nieder wrote:

Hi,

> Interesting - I had imagined changing dependencies working in an
> entirely different way.

Thanks!  This is quite interesting.  A few questions

> 
> 	$ git checkout -b P' P
> 	$ git rebase --onto B' B

.. is using rebase a robust solution?  We should provide a way to
recover after user intervention here?

> 	$ git checkout P
> 	$ git merge --no-ff --no-commit B'   (*)

Do you remember in what area the problem is here, that would make it a 
lot easier for me to look.

> 	$ git read-tree -u P'

Ouch, I'm feeling so git-unitiated here; what is read-tree doing 
differently from merge?  Isn't here a -m missing?

> The main problem I see with this story is that if B' is just B with some
> new changes added this is overly complicated.

Yes, that's my main gripe.  One of the use cases I'm looking at is
our ooo-build master branch; which includes ~300 topic branches.

Removing or [re-]adding one dependency using this rebase-by-merging 
approch would take ~7 minutes on my machine.

I'm now also looking at a .topundeps file, to support
the re-adding of a depenency using the cherry-pick approach...

Greetings,
Janneke. 

-- 
Jan Nieuwenhuizen <janneke@gnu.org> | GNU LilyPond - The music typesetter
http://www.xs4all.nl/~jantien       | http://www.lilypond.org

Re: [TopGit PATCH] tg-create.sh: Introduce --add option to add a dependency.

[Jonathan Nieder]

Hi,

I think my last message missed your real point.  Let P be the old topic
branch tip, B the old base, P' the new tip, and B' the new base.  There
are four questions to answer:

1) How should the tree of B' be determined?
2) What should the parents of B' be?
3) How should the tree of P' be determined from P, B, and B'?
4) What should the parents of P' be?

I gave my answers to 2, 3, and 4, but I think the problem you ran into
is already there in question 1.  It comes from the basic model of how
TopGit works, not some design decision you made, I think.  Suppose I

	Make a topic branch t/foo depending on master.
	Change the dependency of t/foo to the older version maint.
	Make a new topic branch t/bar depending on t/foo and master.

Because in TopGit we do not ever rewrite history on the t/foo branch,
master is a parent of t/foo even after the dependencies change.  So
merging t/foo and master gives just t/foo, and the base for the new
topic branch does not incorporate the changes from maint to master at
all.

But note that this has nothing to do with how the second step takes
place - the problem occurs as long as we are not throwing away history
and we are using merges to calculate the topic branch bases.

We would really like in the third step to calculate a three-way merge of
t/foo with master, with maint as merge base.  The merge-recursive magic
(making use of the history between maint and t/foo) is just a recipe for
trouble here, and we know better than the automatically chosen merge
base (master).

So:

	foo_deps=$(git show t/foo:.topdeps)
	merge_bases=$(git merge-base --all master $foo_deps)
	git merge-resolve $merge_bases -- master t/foo

should give the right effect in this case (even if t/foo has multiple
dependencies).  This example fails to take into account the case where
master and t/foo are completely independent, but that is easily fixed.
But if t/foo depends on other topic branches in turn, it is not so easy
to fix - I am not sure what to do then.  I am tempted to suggest some
insanity like making the dependency graph into history with grafts...

So all this trouble is there when we try to come up with the topic base
for a new topic, as long as it is possible /in some way/ to weaken
dependencies.

Ok, onto your questions about 2, 3, and 4:

> > 	$ git checkout -b P' P
> > 	$ git rebase --onto B' B
> 
> .. is using rebase a robust solution?  We should provide a way to
> recover after user intervention here?

It is the temporary P' branch that is being rebased, so we can recover
by checking out P and throwing away the P' branch.  I suggested rebase
because there is UI for skipping a change, etc. but the more I think
about it, the more I think it would make sense to just

	...
	$ git checkout -b P' B'
	$ {
	>	echo Subject: temporary commit &&
	>	echo --- &&
	>	git diff -M B P
	> } | git am -3
	...

or, if we make B a parent of B',

	$ git checkout P
	$ git merge B'

which might be preferrable.

> > 	$ git checkout P
> > 	$ git merge --no-ff --no-commit B'   (*)
> 
> Do you remember in what area the problem is here, that would make it a 
> lot easier for me to look.

I tried this out, and it seems here I was worrying needlessly.  I was
probably thinking of the following, which might or might not be
intentional:

	$ mkdir newrepo && cd newrepo
	$ git init
	$ git remote add upstream ../oldrepo
	$ git fetch upstream
	$ git merge --no-ff --no-commit upstream/master

We asked for no commit, but because we are on a branch yet to be born,
the merge makes a commit anyway.

> > 	$ git read-tree -u P'
> 
> Ouch, I'm feeling so git-unitiated here; what is read-tree doing 
> differently from merge?  Isn't here a -m missing?

Why?  We want to just blindly take the tree from P' and using it.  The
point is to make setting the contents of the new branch tip and its
parentage separate decisions.

> > The main problem I see with this story is that if B' is just B with some
> > new changes added this is overly complicated.
> 
> Yes, that's my main gripe.  One of the use cases I'm looking at is
> our ooo-build master branch; which includes ~300 topic branches.
> 
> Removing or [re-]adding one dependency using this rebase-by-merging 
> approch would take ~7 minutes on my machine.

Can you be more precise here?  What user action causes topgit to do
so much work (adding one dependency to what topic)?  What other
approach avoids all this work?
 
> I'm now also looking at a .topundeps file, to support
> the re-adding of a depenency using the cherry-pick approach...

Does it address the situation I mention at the top of this file?

Thanks for clarifying, and sorry I missed your point before.  I'll take
a look at your patch now.

Regards,
Jonathan

Re: [TopGit PATCH] tg-create.sh: Introduce --add option to add a dependency.

[Jan Nieuwenhuizen]

On vr, 2008-08-15 at 10:25 -0500, Jonathan Nieder wrote:

Hi,

> 1) How should the tree of B' be determined?
> 2) What should the parents of B' be?
> 3) How should the tree of P' be determined from P, B, and B'?
> 4) What should the parents of P' be?

> So all this trouble is there when we try to come up with the topic 
> base
> for a new topic, as long as it is possible /in some way/ to weaken
> dependencies.

Which I have just been avoiding...  Thanks, that clarifies things for
me.

> or, if we make B a parent of B',
> 
> 	$ git checkout P
> 	$ git merge B'
> 
> which might be preferrable.

> I tried this out, and it seems here I was worrying needlessly.

Ok, good to know.

> > Ouch, I'm feeling so git-unitiated here; what is read-tree doing 
> > differently from merge?  Isn't here a -m missing?
> 
> Why?  We want to just blindly take the tree from P' and using it.  The
> point is to make setting the contents of the new branch tip and its
> parentage separate decisions.

Hmm, for one, "git read-tree -u SHA" does not work for me.

> > Removing or [re-]adding one dependency using this rebase-by-merging 
> > approch would take ~7 minutes on my machine.
> 
> Can you be more precise here?  What user action causes topgit to do
> so much work (adding one dependency to what topic)?  What other
> approach avoids all this work?

Good question, I was thinking a bit careless here.  Creating the master
topgit branch, which depends on ~300 single-topic topgit branches, like
so

    tg create t/master $(git branch | grep -vE '/|patched|pristine')

takes ~7 minutes; which does not really surprise me.

I realise now that if we make a strict *adding* of dependencies a
special case, we can just checkout our old B, and add (merge) the
additional dependencies on top of that.

However, when *removing* a dependency, I see no easy way to do that cheaply.

Greetings,
Janneke

-- 
Jan Nieuwenhuizen <janneke@gnu.org> | GNU LilyPond - The music typesetter
http://www.xs4all.nl/~jantien       | http://www.lilypond.org