[PATCH] Warnings before rebasing -i published history

[PATCH] Warnings before rebasing -i published history

[Lucien Kong]

"git rebase -i" can be very dangerous if used on an already published
history. This code detects that one is rewriting a commit that is an
ancestor of a remote-tracking branch, and warns the user through the
editor.

Signed-off-by: Lucien Kong <Lucien.Kong@ensimag.imag.fr>
Signed-off-by: Valentin Duperray <Valentin.Duperray@ensimag.imag.fr>
Signed-off-by: Franck Jonas <Franck.Jonas@ensimag.imag.fr>
Signed-off-by: Thomas Nguy <Thomas.Nguy@ensimag.imag.fr>
Signed-off-by: Huynh Khoi Nguyen Nguyen <Huynh-Khoi-Nguyen.Nguyen@ensimag.imag.fr>
Signed-off-by: Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>
---
For now, the code only warns the user; he can't revert back to his
original state. We agree with everyone that he should be given a
way to abort the rebase.

 git-rebase--interactive.sh    |   17 +++++++++++++++++
 t/t3404-rebase-interactive.sh |   26 ++++++++++++++++++++++++++
 2 files changed, 43 insertions(+), 0 deletions(-)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 0c19b7c..67b5faf 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -684,6 +684,20 @@ rearrange_squash () {
 	rm -f "$1.sq" "$1.rearranged"
 }
 
+# Add a warning notification at the end of each pick or fixup/squash
+# line of the todo list, providing the picking commit is already
+# published.
+warn_published () {
+	cat "$1" | while read -r command sha1 message
+	do
+		test -n "$sha1" || break
+		if test -n "$(git branch -r --contains "$sha1")"
+		then
+			printf "%s\n" "$(sed -e "/"$sha1"/ s|$| [Published]|" "$1")" >"$1"
+		fi
+	done
+}
+
 case "$action" in
 continue)
 	# do we have anything to commit?
@@ -857,6 +871,7 @@ fi
 
 test -s "$todo" || echo noop >> "$todo"
 test -n "$autosquash" && rearrange_squash "$todo"
+warn_published "$todo"
 cat >> "$todo" << EOF
 
 # Rebase $shortrevisions onto $shortonto
@@ -869,6 +884,8 @@ cat >> "$todo" << EOF
 #  f, fixup = like "squash", but discard this commit's log message
 #  x, exec = run command (the rest of the line) using shell
 #
+# Warning: [Published] means that the commit has already been published
+#
 # These lines can be re-ordered; they are executed from top to bottom.
 #
 # If you remove a line here THAT COMMIT WILL BE LOST.
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 025c1c6..f7c31c1 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -755,4 +755,30 @@ test_expect_success 'rebase-i history with funny messages' '
 	test_cmp expect actual
 '
 
+test_expect_success 'warn before rewriting published history' '
+	test_when_finished "rm -rf git.git git" &&
+	git init git.git &&
+	git clone git &&
+	(
+		cd git &&
+		test_commit one_commit main.txt one_commit &&
+		test_commit two_commit main.txt two_commit &&
+		test_commit three_commit main.txt three_commit &&
+		git push --all &&
+		test_commit four_commit main.txt four_commit &&
+		FAKE_LINES="1 2 3" &&
+		export FAKE_LINES &&
+		tmp=$(git rebase -i HEAD~3 | sed -n 2,4p) &&
+		echo "$tmp" >actual &&
+		tmp=$(git cherry --abbrev=7 HEAD~3 | sed -e 's/+[[:space:]]//g') &&
+		two_sha1=$(echo "$tmp" | sed -n 1p) &&
+		three_sha1=$(echo "$tmp" | sed -n 2p) &&
+		four_sha1=$(echo "$tmp" | sed -n 3p) &&
+		echo "pick $two_sha1 two_commit [Published]" >expected &&
+		echo "pick $three_sha1 three_commit [Published]" >>expected &&
+		echo "pick $four_sha1 four_commit" >>expected &&
+		test_cmp expected actual
+	)
+'
+
 test_done
-- 
1.7.8

Re: [PATCH] Warnings before rebasing -i published history

[Matthieu Moy]

Lucien Kong <Lucien.Kong@ensimag.imag.fr> writes:

> For now, the code only warns the user; he can't revert back to his
> original state. We agree with everyone that he should be given a
> way to abort the rebase.

He already has: empty the todo-list in the editor and you're done.

> +warn_published () {
> +	cat "$1" | while read -r command sha1 message
> +	do
> +		test -n "$sha1" || break
> +		if test -n "$(git branch -r --contains "$sha1")"
> +		then
> +			printf "%s\n" "$(sed -e "/"$sha1"/ s|$| [Published]|" "$1")" >"$1"
> +		fi
> +	done

Aren't you reading and writing from the same file here? Sounds
dangerous.

This appends [Published] to commits that are reachable from a
remote-tracking branch, but I'm wondering if actually showing the remote
branch name wouldn't be better, e.g.

pick <id> commit one
# Commits above this line appear in origin/master
pick <id> commit two
pick <id> commit three
# Commits above this line appear in origin/next
pick <id> most recent commit

>  test -s "$todo" || echo noop >> "$todo"
>  test -n "$autosquash" && rearrange_squash "$todo"
> +warn_published "$todo"

That should be configurable.

> @@ -869,6 +884,8 @@ cat >> "$todo" << EOF
>  #  f, fixup = like "squash", but discard this commit's log message
>  #  x, exec = run command (the rest of the line) using shell
>  #
> +# Warning: [Published] means that the commit has already been published

It's a pity to show the message unconditionally. If you go this way,
skip the message if you didn't display [Published].

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: [PATCH] Warnings before rebasing -i published history

[konglu]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=";"; format=flowed	DelSp=Yes, Size: 352 bytes --]


Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> a écrit :

>>  test -s "$todo" || echo noop >> "$todo"
>>  test -n "$autosquash" && rearrange_squash "$todo"
>> +warn_published "$todo"
>
> That should be configurable.

Do you mean that it should be controlled by a key config (maybe a new
advice.*) in the config file ? Or through an option ?

Re: [PATCH] Warnings before rebasing -i published history

[Matthieu Moy]

konglu@minatec.inpg.fr writes:

> Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> a écrit :
>
>>>  test -s "$todo" || echo noop >> "$todo"
>>>  test -n "$autosquash" && rearrange_squash "$todo"
>>> +warn_published "$todo"
>>
>> That should be configurable.
>
> Do you mean that it should be controlled by a key config (maybe a new
> advice.*) in the config file ? Or through an option ?

That should be a config key, I don't think a user wants to type
--no-published-warnings each time he doesn't want warnings.

Current usage of advice.* is a bit different as it usually talks about
git's output on the terminal, but I think added comments in the rebase
todolist would be an acceptable use of advice.*.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: [PATCH] Warnings before rebasing -i published history

[Junio C Hamano]

konglu@minatec.inpg.fr writes:

> Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> a écrit :
>
>>>  test -s "$todo" || echo noop >> "$todo"
>>>  test -n "$autosquash" && rearrange_squash "$todo"
>>> +warn_published "$todo"
>>
>> That should be configurable.
>
> Do you mean that it should be controlled by a key config (maybe a new
> advice.*) in the config file ? Or through an option ?

Probably "[rebase] checkremoterefs" in .git/config; advice.*
variables default to true by convention, and it is not suitable for
a misleading "feature" like this one.

Re: [PATCH] Warnings before rebasing -i published history

[Junio C Hamano]

Lucien Kong <Lucien.Kong@ensimag.imag.fr> writes:

> +# Add a warning notification at the end of each pick or fixup/squash
> +# line of the todo list, providing the picking commit is already
> +# published.
> +warn_published () {
> +	cat "$1" | while read -r command sha1 message

Make it a habit to question yourself whenever you cat a single file
and immediately pipe it to elsewhere, i.e.

	cat "$1" | anything

because 99% of the time you are much better off writing

	anything <"$1"

instead.

> +	do
> +		test -n "$sha1" || break
> +		if test -n "$(git branch -r --contains "$sha1")"
> +		then
> +			printf "%s\n" "$(sed -e "/"$sha1"/ s|$| [Published]|" "$1")" >"$1"
> +		fi
> +	done

What's inside $() looks like it wants to say something like

	sed -e "/ $sha1 /s/$/ [Published]/" "$1"

but it has a few fishy double-quotes that makes it unclear why $sha1
wants to be outside the quotes.

Why does it need 'printf "%s" $()' in the first place?  Wouldn't

	sed ... >"$1"

sufficient?  You let cat read "$1", sed read "$1" and then the loop
overwrite "$1", which looks very fishy.

The logic is merely _guessing_ that the commit could have been
published, no?  The particular remote repository the test happens to
find may not be for consumption by other people.

I am afraid that doing this would send users a wrong message that is
unnecessarily alarming, especially the marker says "Published" as if
it were a confirmed fact.

In short, I am not unsympathetic to the motivation, but I find the
resulting user experience (mostly the wording) questionable, and I
am not impressed by the implementation very much.

Re: [PATCH] Warnings before rebasing -i published history

[konglu]

Junio C Hamano <gitster@pobox.com> a écrit :

>> +	do
>> +		test -n "$sha1" || break
>> +		if test -n "$(git branch -r --contains "$sha1")"
>> +		then
>> +			printf "%s\n" "$(sed -e "/"$sha1"/ s|$| [Published]|" "$1")" >"$1"
>> +		fi
>> +	done
>
> What's inside $() looks like it wants to say something like
>
> 	sed -e "/ $sha1 /s/$/ [Published]/" "$1"
>
> but it has a few fishy double-quotes that makes it unclear why $sha1
> wants to be outside the quotes.
>
> Why does it need 'printf "%s" $()' in the first place?  Wouldn't
>
> 	sed ... >"$1"
> sufficient?

The fact is that the input of sed is "$1" itself. An redirecting the output
of sed to the same input doesn't seem to work.

> The logic is merely _guessing_ that the commit could have been
> published, no?  The particular remote repository the test happens to
> find may not be for consumption by other people.
>
> I am afraid that doing this would send users a wrong message that is
> unnecessarily alarming, especially the marker says "Published" as if
> it were a confirmed fact.

True. It would be annoying for people that rewrite consciously published
history. From this point of view, a better idea would be to show the
remote branch name, as Matthieu suggested, so that the user could know
which branch is involved.

Re: [PATCH] Warnings before rebasing -i published history

[Matthieu Moy]

konglu@minatec.inpg.fr writes:

> The fact is that the input of sed is "$1" itself. An redirecting the output
> of sed to the same input doesn't seem to work.

No, reading and writing to the same file usually doesn't work (unless
you have a very good reason to do it). But using printf seems weird. It
just forces the shell to hold the whole output of sed in memory to be
able to pass it to printf.

Isn't a more conventional way to do that

sed -e '...' "$1" >"$1".new
mv "$1".new "$1"

?

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: [PATCH] Warnings before rebasing -i published history

[Tomas Carnecky]

On Fri, 08 Jun 2012 10:52:04 +0200, Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> wrote:
> Isn't a more conventional way to do that
> 
> sed -e '...' "$1" >"$1".new
> mv "$1".new "$1"
> 
> ?

Is sed -i not portable or what is the reason not to use it?

Re: [PATCH] Warnings before rebasing -i published history

[Matthieu Moy]

Tomas Carnecky <tomas.carnecky@gmail.com> writes:

> On Fri, 08 Jun 2012 10:52:04 +0200, Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> wrote:
>> Isn't a more conventional way to do that
>> 
>> sed -e '...' "$1" >"$1".new
>> mv "$1".new "$1"
>> 
>> ?
>
> Is sed -i not portable ...

It's a GNU extension, yes.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: [PATCH] Warnings before rebasing -i published history

[Junio C Hamano]

Tomas Carnecky <tomas.carnecky@gmail.com> writes:

> On Fri, 08 Jun 2012 10:52:04 +0200, Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> wrote:
>> Isn't a more conventional way to do that
>> 
>> sed -e '...' "$1" >"$1".new
>> mv "$1".new "$1"
>> 
>> ?
>
> Is sed -i not portable or what is the reason not to use it?

It is a GNUism so it is not portable (check POSIX.1 before asking),
and that is the reason to avoid it.

[PATCHv2] Warnings before rebasing -i published history

[Lucien Kong]

"git rebase -i" can be very dangerous if used on an already published
history. This code detects that one is rewriting a commit that is an
ancestor of a remote-tracking branch, and warns the user through the
editor. This feature is controlled by a new config key
rebase.checkremoterefs.

Signed-off-by: Lucien Kong <Lucien.Kong@ensimag.imag.fr>
Signed-off-by: Valentin Duperray <Valentin.Duperray@ensimag.imag.fr>
Signed-off-by: Franck Jonas <Franck.Jonas@ensimag.imag.fr>
Signed-off-by: Thomas Nguy <Thomas.Nguy@ensimag.imag.fr>
Signed-off-by: Huynh Khoi Nguyen Nguyen <Huynh-Khoi-Nguyen.Nguyen@ensimag.imag.fr>
Signed-off-by: Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>
---
 Documentation/config.txt      |    5 +++++
 Documentation/git-rebase.txt  |    5 +++++
 git-rebase--interactive.sh    |   22 ++++++++++++++++++++++
 git-rebase.sh                 |    2 ++
 t/lib-rebase.sh               |   11 +++++++++++
 t/t3404-rebase-interactive.sh |   29 +++++++++++++++++++++++++++++
 6 files changed, 74 insertions(+), 0 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 915cb5a..e47f6e0 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1753,6 +1753,11 @@ rebase.stat::
 rebase.autosquash::
 	If set to true enable '--autosquash' option by default.
 
+rebase.checkremoterefs::
+	If it is set to 'true', git rebase -i will show after each
+	commit ancestor of a remote-tracking branch the name of these
+	branches through the editor.
+
 receive.autogc::
 	By default, git-receive-pack will run "git-gc --auto" after
 	receiving data from git-push and updating refs.  You can stop
diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index 147fa1a..b68a80b 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -208,6 +208,11 @@ rebase.stat::
 rebase.autosquash::
 	If set to true enable '--autosquash' option by default.
 
+rebase.checkremoterefs::
+	If it is set to 'true', git rebase -i will show after each
+	commit ancestor of a remote-tracking branch the name of these
+	branches through the editor.
+
 OPTIONS
 -------
 <newbase>::
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 0c19b7c..ad6f8a7 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -684,6 +684,27 @@ rearrange_squash () {
 	rm -f "$1.sq" "$1.rearranged"
 }
 
+# Add the name the branches after each pick, fixup or squash commit that
+# is an ancestor of a remote-tracking branch.
+add_remoterefs () {
+	while read -r command sha1 message
+	do
+		printf '%s\n' "$command $sha1 $message"
+		git branch -r --contains "$sha1" >"$1.branch"
+		if test -s "$1.branch"
+		then
+			printf "# Commit above this line appear in:"
+			while read -r branch
+			do
+				printf " $branch"
+			done <"$1.branch"
+			printf '\n'
+		fi
+	done >"$1.published" <"$1"
+	cat "$1.published" >"$1"
+	rm -f "$1.published" "$1.branch"
+}
+
 case "$action" in
 continue)
 	# do we have anything to commit?
@@ -857,6 +878,7 @@ fi
 
 test -s "$todo" || echo noop >> "$todo"
 test -n "$autosquash" && rearrange_squash "$todo"
+test -n "$checkremoterefs" && add_remoterefs "$todo"
 cat >> "$todo" << EOF
 
 # Rebase $shortrevisions onto $shortonto
diff --git a/git-rebase.sh b/git-rebase.sh
index e616737..f8675b5 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -99,7 +99,9 @@ action=
 preserve_merges=
 autosquash=
 keep_empty=
+checkremoterefs=
 test "$(git config --bool rebase.autosquash)" = "true" && autosquash=t
+test "$(git config --bool rebase.checkremoterefs)" = "true" && checkremoterefs=t
 
 read_basic_state () {
 	head_name=$(cat "$state_dir"/head-name) &&
diff --git a/t/lib-rebase.sh b/t/lib-rebase.sh
index 6ccf797..4906867 100644
--- a/t/lib-rebase.sh
+++ b/t/lib-rebase.sh
@@ -65,3 +65,14 @@ EOF
 	test_set_editor "$(pwd)/fake-editor.sh"
 	chmod a+x fake-editor.sh
 }
+
+
+set_copy_editor () {
+	echo "#!$SHELL_PATH" >editor.sh
+	cat >> editor.sh <<\EOF
+cat "$1" >"$TODO_DUMP"
+EOF
+
+	test_set_editor "$(pwd)/editor.sh"
+	chmod a+x editor.sh
+}
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 025c1c6..f99c0c2 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -755,4 +755,33 @@ test_expect_success 'rebase-i history with funny messages' '
 	test_cmp expect actual
 '
 
+test_expect_success 'warn before rewriting published history' '
+	test_when_finished "rm -rf git.git git" &&
+	git init git.git &&
+	git clone git &&
+	(
+		cd git &&
+		git config rebase.checkremoterefs true &&
+		test_commit one_commit main.txt one_commit &&
+		test_commit two_commit main.txt two_commit &&
+		test_commit three_commit main.txt three_commit &&
+		git push --all &&
+		test_commit four_commit main.txt four_commit &&
+		set_copy_editor &&
+		TODO_DUMP=actual EDITOR=./editor.sh git rebase -i HEAD~3 &&
+		tmp=$(cat actual | sed -n 1,5p) &&
+		echo "$tmp" >actual &&
+		tmp=$(git cherry --abbrev=7 HEAD~3 | sed -e 's/+[[:space:]]//g') &&
+		two_sha1=$(echo "$tmp" | sed -n 1p) &&
+		three_sha1=$(echo "$tmp" | sed -n 2p) &&
+		four_sha1=$(echo "$tmp" | sed -n 3p) &&
+		echo "pick $two_sha1 two_commit" >expected &&
+		echo "# Commit above this line appear in: origin/master" >>expected &&
+		echo "pick $three_sha1 three_commit" >>expected &&
+		echo "# Commit above this line appear in: origin/master" >>expected &&
+		echo "pick $four_sha1 four_commit" >>expected &&
+		test_cmp expected actual
+	)
+'
+
 test_done
-- 
1.7.8

Re: [PATCHv2] Warnings before rebasing -i published history

[Matthieu Moy]

Lucien Kong <Lucien.Kong@ensimag.imag.fr> writes:

> "git rebase -i" can be very dangerous if used on an already published
> history. This code detects that one is rewriting a commit that is an
> ancestor of a remote-tracking branch, and warns the user through the
> editor. This feature is controlled by a new config key
> rebase.checkremoterefs.

For the lazy, you could provide an example of result in the commit
message. People don't want to review how the patch is written if they
disagree with the result.

> +		two_sha1=$(echo "$tmp" | sed -n 1p) &&
> +		three_sha1=$(echo "$tmp" | sed -n 2p) &&
> +		four_sha1=$(echo "$tmp" | sed -n 3p) &&

IIRC, the test suite was made to give reproducible sha1, so you
shouldn't need these.

> +		echo "pick $two_sha1 two_commit" >expected &&
> +		echo "# Commit above this line appear in: origin/master" >>expected &&
> +		echo "pick $three_sha1 three_commit" >>expected &&
> +		echo "# Commit above this line appear in: origin/master" >>expected &&
> +		echo "pick $four_sha1 four_commit" >>expected &&
> +		test_cmp expected actual

You don't want to repeat "Commit above this line" for each commit. What
I meant in my previous suggestion was

pick foo
pick bar
# Commits above this line appear in origin/master
pick boz

i.e. just show where the remote points once.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: [PATCHv2] Warnings before rebasing -i published history

[konglu]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=";"; format=flowed	DelSp=Yes, Size: 683 bytes --]


Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> a écrit :

> Lucien Kong <Lucien.Kong@ensimag.imag.fr> writes:
>
>> "git rebase -i" can be very dangerous if used on an already published
>> history. This code detects that one is rewriting a commit that is an
>> ancestor of a remote-tracking branch, and warns the user through the
>> editor. This feature is controlled by a new config key
>> rebase.checkremoterefs.
>
> For the lazy, you could provide an example of result in the commit
> message. People don't want to review how the patch is written if they
> disagree with the result.

Right, but wouldn't it be better to put the example in the doc
git-rebase.txt ?

Re: [PATCHv2] Warnings before rebasing -i published history

[Matthieu Moy]

konglu@minatec.inpg.fr writes:

> Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> a écrit :
>
>> Lucien Kong <Lucien.Kong@ensimag.imag.fr> writes:
>>
>>> "git rebase -i" can be very dangerous if used on an already published
>>> history. This code detects that one is rewriting a commit that is an
>>> ancestor of a remote-tracking branch, and warns the user through the
>>> editor. This feature is controlled by a new config key
>>> rebase.checkremoterefs.
>>
>> For the lazy, you could provide an example of result in the commit
>> message. People don't want to review how the patch is written if they
>> disagree with the result.
>
> Right, but wouldn't it be better to put the example in the doc
> git-rebase.txt ?

Why not. In any case, it should be easy to find for reviewers (I had to
dig a little to find the expected value in the tests).

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

branch --contains is unbearably slow [Re: [PATCHv2] Warnings before rebasing -i published history]

[Thomas Rast]

[+Cc Junio who wrote branch --contains, and Peff who sped up tag
--contains in ffc4b801.]

Lucien Kong <Lucien.Kong@ensimag.imag.fr> writes:

> "git rebase -i" can be very dangerous if used on an already published
> history. This code detects that one is rewriting a commit that is an
> ancestor of a remote-tracking branch, and warns the user through the
> editor. This feature is controlled by a new config key
> rebase.checkremoterefs.
[...]
> +# Add the name the branches after each pick, fixup or squash commit that
> +# is an ancestor of a remote-tracking branch.
> +add_remoterefs () {
> +	while read -r command sha1 message
> +	do
> +		printf '%s\n' "$command $sha1 $message"
> +		git branch -r --contains "$sha1" >"$1.branch"
[...]
> +	done >"$1.published" <"$1"
> +	cat "$1.published" >"$1"
> +	rm -f "$1.published" "$1.branch"
> +}

While I like the idea, I think it unfortunately needs some changes in
'git branch --contains'.  That command is unbelievably slow on a
repository with many remote branches, like my git.git:

  $ g remote -v | wc -l  # note that each appears twice, for fetch/push
  28
  $ git branch -r | wc -l
  364

  $ time git branch -r --contains origin/next
    origin/next

  real    0m32.060s
  user    0m31.895s
  sys     0m0.036s

I think an upper bound for the runtime of any 'git branch --contains'
should be generating the *complete* topology like this:

  $ time git log --graph --oneline --all >/dev/null

  real    0m2.637s
  user    0m2.246s
  sys     0m0.364s

It should also be possible to generate the --contains output for several
commits at the same time.  Otherwise the feature will be too painfully
slow for all but the simplest rebases.  Currently the startup time for
'rebase -i' to show an editor is near-instantaneous for me; adding N*2s
would be too much on most of my topics, where I tend to gather a handful
of fixups and improvements before the next 'rebase -i' round.

-- 
Thomas Rast
trast@{inf,student}.ethz.ch

Re: branch --contains is unbearably slow [Re: [PATCHv2] Warnings before rebasing -i published history]

[Junio C Hamano]

Thomas Rast <trast@student.ethz.ch> writes:

>   $ time git branch -r --contains origin/next
>     origin/next
>
>   real    0m32.060s
>   user    0m31.895s
>   sys     0m0.036s
>
> I think an upper bound for the runtime of any 'git branch --contains'
> should be generating the *complete* topology like this:
>
>   $ time git log --graph --oneline --all >/dev/null
>
>   real    0m2.637s
>   user    0m2.246s
>   sys     0m0.364s

Hrm, there must be something I am doing wrong.

    $ time git log --graph --oneline --all >/dev/null

    real    0m3.896s
    user    0m3.476s
    sys     0m0.416s

Ok, so my disk is slower than yours, perhaps.  But

    $ time git branch -r --contains next
      github2/next
      gph/next
      ko/next
      repo/next

    real    0m3.853s
    user    0m3.804s
    sys     0m0.048s

Re: branch --contains is unbearably slow [Re: [PATCHv2] Warnings before rebasing -i published history]

[Thomas Rast]

Junio C Hamano <gitster@pobox.com> writes:

> Thomas Rast <trast@student.ethz.ch> writes:
>
>>   $ time git branch -r --contains origin/next
>>     origin/next
>>
>>   real    0m32.060s
>>
>> I think an upper bound for the runtime of any 'git branch --contains'
>> should be generating the *complete* topology like this:
>>
>>   $ time git log --graph --oneline --all >/dev/null
>>
>>   real    0m2.637s
>
> Hrm, there must be something I am doing wrong.
>
>     $ time git log --graph --oneline --all >/dev/null
>
>     real    0m3.896s
>
> Ok, so my disk is slower than yours, perhaps.

That was all hot cache, so that shouldn't be an issue.

> But
>
>     $ time git branch -r --contains next
>       github2/next
>       gph/next
>       ko/next
>       repo/next
>
>     real    0m3.853s

I suspect most of your branches are in some way closely related to next,
while I have a lot of cruft fetched from all sorts of people that fell
behind over the years.  Can you try

  git for-each-ref refs/remotes/ |
  while read sha tp ref; do
    printf "%8d %8d %s\n" $(
      git rev-list --left-right --count $ref...next
    ) $ref
  done

For me the output is as listed below (I ran it with origin/next instead
of next).  Note that for almost all of them, a naive merge-base walk
(going back from both until a common commit is found) has to walk
thousands of commits.

I'll get rid of all this cruft now, but this command doesn't scale in
the direction I'm abusing it :-)

    1027    29494 refs/remotes/alt-git/html
       0     2349 refs/remotes/alt-git/maint
     921    29494 refs/remotes/alt-git/man
       0     2245 refs/remotes/alt-git/master
      27     2191 refs/remotes/alt-git/next
      80     2130 refs/remotes/alt-git/pu
     922    29494 refs/remotes/alt-git/todo
       1     6232 refs/remotes/avar/Makefile-add-CC-to-TRACK_CFLAGS
     121     6615 refs/remotes/avar/a2c5c4b-broke-tracking-info-test
      92     4508 refs/remotes/avar/ab/i18n
     164     5961 refs/remotes/avar/ab/i18n-WIP
      65     6280 refs/remotes/avar/ab/i18n-add-translations
      43     2430 refs/remotes/avar/ab/i18n-all
      78     6275 refs/remotes/avar/ab/i18n-all-continue
      78     6275 refs/remotes/avar/ab/i18n-all-continue-squash
      78     5098 refs/remotes/avar/ab/i18n-c-_-only
      93     6240 refs/remotes/avar/ab/i18n-continue
      93     6248 refs/remotes/avar/ab/i18n-continue-hi.po
      99     6269 refs/remotes/avar/ab/i18n-continue-more
      94     6240 refs/remotes/avar/ab/i18n-continue-with-hindi
     170     5114 refs/remotes/avar/ab/i18n-docs
     161     6069 refs/remotes/avar/ab/i18n-even-more
      92     6232 refs/remotes/avar/ab/i18n-for-junio
      98     6228 refs/remotes/avar/ab/i18n-for-junio-with-docs
     162     6069 refs/remotes/avar/ab/i18n-german
      63     6280 refs/remotes/avar/ab/i18n-gettextize
     140     6191 refs/remotes/avar/ab/i18n-in-pu
     160     6123 refs/remotes/avar/ab/i18n-libcharset
       1     4463 refs/remotes/avar/ab/i18n-missing-Q_-pot-target
       7     4463 refs/remotes/avar/ab/i18n-more-gettextize
       9     3008 refs/remotes/avar/ab/i18n-po
       2     5880 refs/remotes/avar/ab/i18n-prereqs
     159     6191 refs/remotes/avar/ab/i18n-rebase-out-bugs
     160     6191 refs/remotes/avar/ab/i18n-rewrite
     161     6191 refs/remotes/avar/ab/i18n-rewrite-with-po-line-numbers
      54     3977 refs/remotes/avar/ab/i18n-sh-only
       1     2397 refs/remotes/avar/ab/i18n-slimmed
     161     5880 refs/remotes/avar/ab/i18n-squashed
     140     6228 refs/remotes/avar/ab/i18n-v2
       8     6123 refs/remotes/avar/ab/icase-directory
       8     6123 refs/remotes/avar/ab/icase-directory-v2
       4     3804 refs/remotes/avar/ab/jk/tag-contains
       1     5782 refs/remotes/avar/add-HTTPS_PROXY-env-variable-to-docs
     134     6338 refs/remotes/avar/add-fallback-bsd-printf-and-revert-107880a
       2     5092 refs/remotes/avar/add-missing-const
       1     5114 refs/remotes/avar/add-platform-specific-tweaks
     127     6615 refs/remotes/avar/another-test-dont-skip-but-prereq
     127     6615 refs/remotes/avar/another-test-dont-skip-but-prereq-v2
       6     7501 refs/remotes/avar/avar/cvsserver-pserver-auth-support
      26     2591 refs/remotes/avar/avar/fe-fixes
       2     2430 refs/remotes/avar/avar/fixup-gettext-poison-breakage
       1     5098 refs/remotes/avar/avar/git-cherry-example
       1     7776 refs/remotes/avar/avar/git-commit-allow-empty-message
       1     7689 refs/remotes/avar/avar/git-commit-synopsis-no-allow-empty
       0     7909 refs/remotes/avar/avar/git-config.d
       1     7576 refs/remotes/avar/avar/git-svn-no-yoda
       3     5092 refs/remotes/avar/avar/gitweb-513-warnings
       5     3829 refs/remotes/avar/avar/jk/tag-contains
       1     7351 refs/remotes/avar/avar/makefile-remove-INSTLIBDIR
       3     5203 refs/remotes/avar/avar/minor-patches-to-t-t5400-send-pack.sh
       3     2430 refs/remotes/avar/avar/nuke-pointless-unsigned-comparisons
       1     2430 refs/remotes/avar/avar/pull-rebase-config
       1     3504 refs/remotes/avar/avar/tag-contains-skew
       6     7576 refs/remotes/avar/avar/test-tap
       2     7491 refs/remotes/avar/avar/usr-bin-env-not-usr-bin-perl
       3     6743 refs/remotes/avar/better-test-prereq-handling
      66     6454 refs/remotes/avar/blah-poison
       1     7016 refs/remotes/avar/builtin-fetch-errors-to-stderr
       2     6191 refs/remotes/avar/bump-perl-to-5.8
       1     6781 refs/remotes/avar/clang
       2     7165 refs/remotes/avar/cvs-pass
      69     6454 refs/remotes/avar/debug-gettext-poison
     143     6338 refs/remotes/avar/disable-gettext-by-default-in-releases
     217     6773 refs/remotes/avar/dont-be-quiet-under-tap
      50     6607 refs/remotes/avar/dont-run-todo-in-t0000-basic
     164     6533 refs/remotes/avar/dont-run-todo-in-t0000-basic-v2
     102     6439 refs/remotes/avar/dont-run-todo-in-t0000-basic-v3
     134     6615 refs/remotes/avar/fix-multiple-prereq-bug
     134     6615 refs/remotes/avar/fix-multiple-prereq-bug-v2
       1     4508 refs/remotes/avar/fix-poison-for-head-is-now-at
     192     6894 refs/remotes/avar/forgot-old-git-gettext-v5
     192     6894 refs/remotes/avar/forgot-old-git-gettext-v6
      49     6800 refs/remotes/avar/forgot-old-git-gettext-v7-ontop-of-pu-fixup-RHEL-error
       1     6692 refs/remotes/avar/friendlier-git-reset-mixed-deprecated-message
       1     6692 refs/remotes/avar/friendlier-git-reset-mixed-deprecated-message-v2
      49     6800 refs/remotes/avar/gettext
     129     6296 refs/remotes/avar/gettext-pu
     128     6270 refs/remotes/avar/gettext-remove-old-sanity-test
     154     6270 refs/remotes/avar/gettextize-git-in-german
     127     6279 refs/remotes/avar/gettextize-git-mainporcelain
     153     6270 refs/remotes/avar/gettextize-git-mainporcelain-even-more
     127     6270 refs/remotes/avar/gettextize-git-mainporcelain-more
     144     6338 refs/remotes/avar/gettextize-git-mainporcelain-v2
     144     6338 refs/remotes/avar/gettextize-git-mainporcelain-v3
     143     6338 refs/remotes/avar/gettextize-git-mainporcelain-with-perl
       1     7501 refs/remotes/avar/git-am-allow-leading-whitespace
       3     6714 refs/remotes/avar/git-am-ignore-whitespace-before-patches
      25     6746 refs/remotes/avar/git-am-ignore-whitespace-before-patches-v2
       1     7587 refs/remotes/avar/git-config.d-glob
       1     7909 refs/remotes/avar/git-config.d-hack
       6     6815 refs/remotes/avar/git-cover
       7     6815 refs/remotes/avar/git-cover-2
       7     6815 refs/remotes/avar/git-cover-3
     142     6591 refs/remotes/avar/git-cvsimport-alarm-on-while
       1     6228 refs/remotes/avar/git-send-email-use-catfile
      98     6711 refs/remotes/avar/git-smoke
      99     6711 refs/remotes/avar/git-smoke-v2
      99     6711 refs/remotes/avar/git-smoke-v3
      99     6664 refs/remotes/avar/git-smoke-v4
      99     6664 refs/remotes/avar/git-smoke-v5
     102     6664 refs/remotes/avar/git-smoke-v6
       1     7501 refs/remotes/avar/git-submodule-toplevel
       1     7501 refs/remotes/avar/gitignore-patches
       1     5114 refs/remotes/avar/gitignore-test-mktemp
       1     6837 refs/remotes/avar/gnu-regex-lib
     909    29494 refs/remotes/avar/html
       1     3911 refs/remotes/avar/i18n-synopsis
       1     6743 refs/remotes/avar/imap-send-clang-warnings
     151     6591 refs/remotes/avar/in-progress-dont-depend-on-perl
     142     6591 refs/remotes/avar/internal-error-regression-test
     145     6553 refs/remotes/avar/internal-error-regression-test-v2
     121     6615 refs/remotes/avar/lib-git-svn-use-PERL_PATH
       0     8073 refs/remotes/avar/maint
       1     6069 refs/remotes/avar/make-autoconf-failure-warning
       1     5961 refs/remotes/avar/makefile-move-platform-specific-tweaks-above-lib_h-and-lib_objs
     815    29494 refs/remotes/avar/man
      93     4508 refs/remotes/avar/master
      78     6731 refs/remotes/avar/merge-dir-to-symlink-todo-test
     127     6342 refs/remotes/avar/minor-gettext-infrastructure-fixes
     142     6591 refs/remotes/avar/mkdir-test-results-on-smoking
       1     2430 refs/remotes/avar/more-grep-cpus
     185     6925 refs/remotes/avar/my-next
     127     6615 refs/remotes/avar/my-pu
       1     6123 refs/remotes/avar/netbsd-S_IFREG-warning
       1     6123 refs/remotes/avar/netbsd-S_IFREG-warning-v2
      44     6607 refs/remotes/avar/next
       1     6781 refs/remotes/avar/perl-makefile-install-base
     150     6610 refs/remotes/avar/perl-makefile-install-base-v2-onto-pu
     148     4952 refs/remotes/avar/pu
     121     6615 refs/remotes/avar/run-partial-expensive-git-notes-test-everywhere
     131     6615 refs/remotes/avar/run-partial-expensive-git-notes-test-everywhere-v2
       1     6765 refs/remotes/avar/run-tests-as-root
       4     6743 refs/remotes/avar/run-tests-as-root-v2
      16     6128 refs/remotes/avar/send-email-perl-cleanup
      16     6128 refs/remotes/avar/send-email-perl-cleanup-v2
      99     6439 refs/remotes/avar/squash-compat-regex-warnings-sent-to-gawk-maintainer
     186     6925 refs/remotes/avar/submodule-add-f-on-add
      86     7119 refs/remotes/avar/sun-studio-12-builtin-notes
      99     6439 refs/remotes/avar/svn-fe-test-x
      27     6796 refs/remotes/avar/t/doc-config-extraction
     161     6533 refs/remotes/avar/t3507-cherry-pick-conflict-sh-error
       1     6069 refs/remotes/avar/t5560-http-backend-noserver-export-fix
       1     5092 refs/remotes/avar/t7500-commit.sh-use-test_cmp-instead-of-test
     139     6228 refs/remotes/avar/temp
      99     6232 refs/remotes/avar/temp-swedish
     160     6533 refs/remotes/avar/test-fixes
     134     6615 refs/remotes/avar/test-intra-progress-using-tap-subtests
     136     6615 refs/remotes/avar/test-intra-progress-using-tap-subtests-v2
       1     6894 refs/remotes/avar/test-lib-no-test-results-under-harness
     121     6615 refs/remotes/avar/test-lib-no-test-results-under-harness-v2
     126     6338 refs/remotes/avar/test-lib-use-subshell-instead-of-chdir-back
     119     7202 refs/remotes/avar/test-tap-onto-next
     170     6997 refs/remotes/avar/test-tap-onto-next-v4
     170     6997 refs/remotes/avar/test-tap-onto-next-v5
     171     6997 refs/remotes/avar/test-tap-onto-next-v6
     171     6997 refs/remotes/avar/test-tap-onto-next-v7
     193     6925 refs/remotes/avar/test-tap-readme-docs-v1
     193     6925 refs/remotes/avar/test-tap-readme-docs-v2
     217     6773 refs/remotes/avar/tests-remove-no-python
       1     6228 refs/remotes/avar/tests-use-test-cmp-instead-of-diff-1
     712    29494 refs/remotes/avar/todo
       2     7347 refs/remotes/avar/topic/gettext
       9     7347 refs/remotes/avar/topic/gettext-rewrote-translations-out-of-infrastructure-work
       2     7342 refs/remotes/avar/topic/git-gettext
       7     7347 refs/remotes/avar/topic/git-gettext-squashed
       2     7250 refs/remotes/avar/topic/git-gettext-v10
       2     7250 refs/remotes/avar/topic/git-gettext-v11
       2     7347 refs/remotes/avar/topic/git-gettext-v6
       2     7347 refs/remotes/avar/topic/git-gettext-v7
       2     7342 refs/remotes/avar/topic/git-gettext-v8
       2     7342 refs/remotes/avar/topic/git-gettext-v9
       1     6123 refs/remotes/avar/turn-test-expect-code-into-a-function
       1     6123 refs/remotes/avar/turn-test-expect-code-into-a-function-v2
       1     6123 refs/remotes/avar/turn-test-expect-code-into-a-function-v3
      47     6607 refs/remotes/avar/update-fallback-regex-engine
      47     6607 refs/remotes/avar/update-fallback-regex-engine-v2
      49     6607 refs/remotes/avar/update-fallback-regex-engine-v3
      45     6607 refs/remotes/avar/update-index-after-running-pre-commit-hook
       1     6553 refs/remotes/avar/use-builtin-h-for-builtin-commands
       1     6280 refs/remotes/avar/use-builtin-h-for-builtin-commands-v2
     173     5092 refs/remotes/avar/wip/ab/i18n
     151     6591 refs/remotes/avar/yet-another-test-dont-skip-but-prereq
     150     6610 refs/remotes/avar/your-vsnprintf-is-broken
      15    11483 refs/remotes/bigfiles/master
      65     7016 refs/remotes/byang/diff-range
      52     7256 refs/remotes/byang/diff-range-split
      29     6815 refs/remotes/byang/line-log
      19     6815 refs/remotes/byang/line-log-rebasing
       1     6932 refs/remotes/byang/master
      43     6692 refs/remotes/byang/mc-detect
      48     6692 refs/remotes/byang/mc-detect-2
      24     6932 refs/remotes/byang/parent-rewrite
      26     6925 refs/remotes/byang/parent-rewrite-rebase
      29     6815 refs/remotes/byang/parent-rewrite-submmiting
      26     7256 refs/remotes/byang/playaround
      19     6815 refs/remotes/byang/rebasing-p2
      20     6815 refs/remotes/byang/rebasing-p3
      18     6692 refs/remotes/byang/rebasing-p4
      14     6932 refs/remotes/byang/round2
      14     6894 refs/remotes/byang/round2-fix
      19     6692 refs/remotes/byang/series-1
      49     6692 refs/remotes/byang/series-2
      13     7016 refs/remotes/byang/submitting
       4     1048 refs/remotes/de-po/maint
       1      596 refs/remotes/de-po/master
       2    11826 refs/remotes/dscho/2gb
     130    11252 refs/remotes/dscho/add-e
     127    15429 refs/remotes/dscho/after-1.5.5
     206    29494 refs/remotes/dscho/blog
       4    18539 refs/remotes/dscho/branchnewworkdir
    1008    17197 refs/remotes/dscho/builtin-fast-export
     974    17250 refs/remotes/dscho/builtin-merge-one-file
      57    15850 refs/remotes/dscho/builtin-remote
      59    15219 refs/remotes/dscho/color-words
     971    17250 refs/remotes/dscho/cover-letter
    2512     6301 refs/remotes/dscho/devel
       2    16599 refs/remotes/dscho/diff-no-stdout
       4     9756 refs/remotes/dscho/diff-submodule-summary
     131    11252 refs/remotes/dscho/diffopts
     158     9715 refs/remotes/dscho/enhance_git_diff_for_submodules
    1040    16827 refs/remotes/dscho/execv-builtin
      74    15799 refs/remotes/dscho/fast-make-doc
     143    10261 refs/remotes/dscho/grafts
      20    12474 refs/remotes/dscho/hardlinks
       1    17820 refs/remotes/dscho/hpux
      42    10104 refs/remotes/dscho/junio/next
     143     9736 refs/remotes/dscho/log-rewrap
       7    29494 refs/remotes/dscho/logos
       2    16377 refs/remotes/dscho/lstat
     139    15090 refs/remotes/dscho/mailsplit
       3     7794 refs/remotes/dscho/master
     272    11983 refs/remotes/dscho/mob
      28    13909 refs/remotes/dscho/my-master
     252    12054 refs/remotes/dscho/my-next
     679    29494 refs/remotes/dscho/my-todo
     158    12336 refs/remotes/dscho/name-objects
     291    12547 refs/remotes/dscho/notes
     106    14892 refs/remotes/dscho/nul-fix
      28    14636 refs/remotes/dscho/parseopt
      43    12424 refs/remotes/dscho/patience
      77    11536 refs/remotes/dscho/percent-branch
       4    29494 refs/remotes/dscho/presentations
     973    17259 refs/remotes/dscho/push-mirror
     118    12170 refs/remotes/dscho/rebase-early
     137    12170 refs/remotes/dscho/rebase-i-p
     107    12177 refs/remotes/dscho/rebase-i-submodule
      32     9715 refs/remotes/dscho/remote-hg
     140    15022 refs/remotes/dscho/sanitize-submodules
     111    10334 refs/remotes/dscho/shortlog-toy
     148    12006 refs/remotes/dscho/submodule
     150     9715 refs/remotes/dscho/submodule-summary
     256     8329 refs/remotes/dscho/tmp
       8    22811 refs/remotes/dscho/utf8
     118    12177 refs/remotes/dscho/valgrind
    2132     6925 refs/remotes/dscho/work/msvc-fixes
       0     2249 refs/remotes/mhagger/master
      51     2249 refs/remotes/mhagger/ref-api-D
      15     2638 refs/remotes/mhagger/refname-full
       2     3600 refs/remotes/mhagger/refperf
       0      218 refs/remotes/origin/HEAD
    1100    29494 refs/remotes/origin/html
       0      861 refs/remotes/origin/maint
     980    29494 refs/remotes/origin/man
       0      218 refs/remotes/origin/master
       0        0 refs/remotes/origin/next
     100      200 refs/remotes/origin/pu
    1012    29494 refs/remotes/origin/todo
       1     1845 refs/remotes/peff/git2-attrs
       1     1225 refs/remotes/peff/jk/blame-tree
       2     1225 refs/remotes/peff/jk/builtin-attr
      14     1225 refs/remotes/peff/jk/bundle-fetch
       1     1225 refs/remotes/peff/jk/byte-diff
       7     1225 refs/remotes/peff/jk/cache-patch-id
       1     1225 refs/remotes/peff/jk/child-cleanup
       1     1225 refs/remotes/peff/jk/commits-notes-wip
       1     1225 refs/remotes/peff/jk/config-sources
       0     1225 refs/remotes/peff/jk/dash-tests
       1     1225 refs/remotes/peff/jk/describe
       2     1225 refs/remotes/peff/jk/diff-highlight
       1     1224 refs/remotes/peff/jk/dumpstat
       3     1225 refs/remotes/peff/jk/envirostat
       3     1225 refs/remotes/peff/jk/fast-commit-list
       1     1225 refs/remotes/peff/jk/fetch-tracking
       4     1225 refs/remotes/peff/jk/follow-multiple-wip
       1     1225 refs/remotes/peff/jk/format-patch-fix
       9     1225 refs/remotes/peff/jk/generations
       1     1225 refs/remotes/peff/jk/leading-symlinks
       1     4203 refs/remotes/peff/jk/maint-ignore-have
       5     1225 refs/remotes/peff/jk/metadata-cache
       1     1225 refs/remotes/peff/jk/pager
       1     1225 refs/remotes/peff/jk/pager-subcommand
       1     1225 refs/remotes/peff/jk/pretty-nul
       1     1225 refs/remotes/peff/jk/read-tree-content-merge
       1     1225 refs/remotes/peff/jk/reflog-rewind
       0     1225 refs/remotes/peff/jk/remote-helper-disconnect
       4     1225 refs/remotes/peff/jk/rename-progress
       1     1225 refs/remotes/peff/jk/repack-alternates
       1     1225 refs/remotes/peff/jk/sockstats
       3     1225 refs/remotes/peff/jk/stash-apply-p
       0     1225 refs/remotes/peff/jk/stream-filter
       1     1225 refs/remotes/peff/jk/stupid-git-tricks
       2     1225 refs/remotes/peff/jk/unreachable-archive-loosen
     106    29494 refs/remotes/peff/meta
     217     1218 refs/remotes/peff/private
       1    13904 refs/remotes/sbeyer/extern
       6    12486 refs/remotes/sbeyer/leaks
     465    10238 refs/remotes/sbeyer/master
     454    10238 refs/remotes/sbeyer/mob
     454    10238 refs/remotes/sbeyer/seq-builtin-dev
      24    10238 refs/remotes/sbeyer/seq-builtin-rfc
       7    14109 refs/remotes/sbeyer/seq-proto-rfc
       0    10146 refs/remotes/spearce/maint
       0     9760 refs/remotes/spearce/master
     145     9719 refs/remotes/spearce/next
     189     9714 refs/remotes/spearce/pu
     520    29494 refs/remotes/spearce/todo
     131      368 refs/remotes/tgummerer/index-v5
       0     1099 refs/remotes/tgummerer/maint
      65      368 refs/remotes/tgummerer/master
       0      678 refs/remotes/tgummerer/next
      91      737 refs/remotes/tgummerer/pu
     118      368 refs/remotes/tgummerer/pythonprototype
     986    29494 refs/remotes/tgummerer/todo
      15       52 refs/remotes/trast-github/line-log-WIP
      33     1598 refs/remotes/trast-github/line-log-cleanup
       0     1776 refs/remotes/trast-github/maint
       0     1686 refs/remotes/trast-github/master
     204     1216 refs/remotes/trast-github/next
      61     1590 refs/remotes/trast-github/pu
     184     1257 refs/remotes/trast-github/t/dangerous-linus-style-xdl-hash
      11     1224 refs/remotes/trast-github/t/perf-gather-post-1.7.10
       1     1224 refs/remotes/trast-github/t/proper-xdl-speedup
     944    29494 refs/remotes/trast-github/todo
       1      524 refs/remotes/trast-github/tr/darwin-xdl-fast-hash
       1     1139 refs/remotes/trast-github/tr/inotify-POC
       1     1224 refs/remotes/trast-github/tr/xdiff-fast-hash
       1    10853 refs/remotes/trast/LOCAL/completion
      16     9520 refs/remotes/trast/js/rebase-i-p
       2    10317 refs/remotes/trast/lh/traverse-gitlinks-on-master
      33     1598 refs/remotes/trast/line-log-cleanup
       7    12377 refs/remotes/trast/master
     178     7157 refs/remotes/trast/next
     509    29494 refs/remotes/trast/notes/full
     415    29494 refs/remotes/trast/notes/terse
       6     6815 refs/remotes/trast/t/color-porcelain-message-output
      25     6796 refs/remotes/trast/t/doc-config-extraction
       3     6046 refs/remotes/trast/t/doc-config-extraction-v2
     243     5264 refs/remotes/trast/t/gfb-both-submodule-features
     240     5264 refs/remotes/trast/t/gfb-remap-submodules
       2     5331 refs/remotes/trast/t/gfb-split-submodule
      32     2191 refs/remotes/trast/t/perf-framework
      38     2191 refs/remotes/trast/t/sha1_file-parallel-with-perf-framework
     264     2295 refs/remotes/trast/tr/alternate-grep-userdiff-parallel
       1     1224 refs/remotes/trast/tr/xdiff-fast-hash

-- 
Thomas Rast
trast@{inf,student}.ethz.ch

Re: branch --contains is unbearably slow [Re: [PATCHv2] Warnings before rebasing -i published history]

[Thomas Rast]

Thomas Rast <trast@student.ethz.ch> writes:

>> But
>>
>>     $ time git branch -r --contains next
>>       github2/next
>>       gph/next
>>       ko/next
>>       repo/next
>>
>>     real    0m3.853s
[...]
> I'll get rid of all this cruft now, but this command doesn't scale in
> the direction I'm abusing it :-)

BTW, my original point still stands: at nearly 4s per invocation of
'branch -r --contains', the loop as written by Lucien

> +add_remoterefs () {
> +	while read -r command sha1 message
> +	do
> +		printf '%s\n' "$command $sha1 $message"
> +		git branch -r --contains "$sha1" >"$1.branch"
[...]
> +	done >"$1.published" <"$1"
> +	cat "$1.published" >"$1"
> +	rm -f "$1.published" "$1.branch"
> +}

is unusable in your repository, too.  Which is a real pity, it's a nice
idea.

-- 
Thomas Rast
trast@{inf,student}.ethz.ch

Re: branch --contains is unbearably slow [Re: [PATCHv2] Warnings before rebasing -i published history]

[Junio C Hamano]

Thomas Rast <trast@student.ethz.ch> writes:

> I suspect most of your branches are in some way closely related to next,
> while I have a lot of cruft fetched from all sorts of people that fell
> behind over the years.  Can you try
>
>   git for-each-ref refs/remotes/ |
>   while read sha tp ref; do
>     printf "%8d %8d %s\n" $(
>       git rev-list --left-right --count $ref...next
>     ) $ref
>   done
>
> For me the output is as listed below (I ran it with origin/next instead
> of next).  Note that for almost all of them, a naive merge-base walk
> (going back from both until a common commit is found) has to walk
> thousands of commits.

I too seem to have a lot of cruft I'd better get rid of.  Thanks for
reminding ;-).

$ git for-each-ref refs/remotes/ |
  while read sha tp ref; do
    printf "%8d %8d %s\n" $(
      git rev-list --left-right --count $ref...next
    ) $ref
  done
     159     6180 refs/remotes/ab-i18n/i18n
      65     6284 refs/remotes/ab-i18n/i18n-add-translations
      63     6279 refs/remotes/ab-i18n/i18n-all
      93     6244 refs/remotes/ab-i18n/i18n-all-continue
      78     6279 refs/remotes/ab-i18n/i18n-all-continue-squash
      93     6244 refs/remotes/ab-i18n/i18n-continue
      94     6244 refs/remotes/ab-i18n/i18n-continue-with-hindi
      92     6236 refs/remotes/ab-i18n/i18n-for-junio
      98     6236 refs/remotes/ab-i18n/i18n-for-junio-with-docs
      63     6284 refs/remotes/ab-i18n/i18n-gettextize
     140     6195 refs/remotes/ab-i18n/i18n-in-pu
     160     6195 refs/remotes/ab-i18n/i18n-rewrite
     161     6195 refs/remotes/ab-i18n/i18n-rewrite-with-po-line-numbers
     140     6232 refs/remotes/ab-i18n/i18n-v2
       0    16335 refs/remotes/gfi/HEAD
       0    17824 refs/remotes/gfi/maint
       0    16335 refs/remotes/gfi/master
      31    20748 refs/remotes/gfi/sp/pack4
       7    16320 refs/remotes/gfi/sp/sendpack-sideband
       3    20518 refs/remotes/gfi/sp/splitpack
       2    16325 refs/remotes/gfi/sp/unixsock
       0    28459 refs/remotes/git-gui/maint
       0    28446 refs/remotes/git-gui/master
       3    28459 refs/remotes/git-gui/pu
       9    29498 refs/remotes/git-gui/todo
       0    15622 refs/remotes/git-p4/git-p4
       0      752 refs/remotes/git-svn/HEAD
       0     3782 refs/remotes/git-svn/instaweb
       0      752 refs/remotes/git-svn/master
       0      753 refs/remotes/git-svn/origin
       0    13207 refs/remotes/git-svn/spearce/origin
       0     6744 refs/remotes/git-svn/webrick
       0      865 refs/remotes/github2/maint
       0      222 refs/remotes/github2/master
       0        4 refs/remotes/github2/next
     100      204 refs/remotes/github2/pu
     981    29498 refs/remotes/github2/todo
       0      865 refs/remotes/gph/maint
       0      222 refs/remotes/gph/master
       0        4 refs/remotes/gph/next
     100      204 refs/remotes/gph/pu
     871    29498 refs/remotes/gph/todo
       8    16603 refs/remotes/jbf-um/HEAD
       3    17804 refs/remotes/jbf-um/better-whitespace-checks
      12    18132 refs/remotes/jbf-um/docwork-concepts
       1    18010 refs/remotes/jbf-um/docwork-design
       1    17849 refs/remotes/jbf-um/docwork-detached
       1    18010 refs/remotes/jbf-um/docwork-foreign-scms
       1    17883 refs/remotes/jbf-um/docwork-recovery
       0    17825 refs/remotes/jbf-um/maint
      10    17867 refs/remotes/jbf-um/maint-history
       8    16603 refs/remotes/jbf-um/master
       1    17804 refs/remotes/jbf-um/messages
       1    19546 refs/remotes/jbf-um/parallel-diff
       7    20685 refs/remotes/jbf-um/recovery
       1    21989 refs/remotes/jbf-um/tutorial-2
      48     3056 refs/remotes/ko-private/jch
       0     3656 refs/remotes/ko-private/maint
       5    22527 refs/remotes/ko-private/maint-1.4.4
       0    16075 refs/remotes/ko-private/maint-1.5.4
       0    15296 refs/remotes/ko-private/maint-1.5.5
       0    14555 refs/remotes/ko-private/maint-1.5.6
       0    11606 refs/remotes/ko-private/maint-1.6.2
       0     9484 refs/remotes/ko-private/maint-1.6.5
       0     8014 refs/remotes/ko-private/maint-1.7.0
       0     7333 refs/remotes/ko-private/maint-1.7.1
       0     6657 refs/remotes/ko-private/maint-1.7.2
       0     6616 refs/remotes/ko-private/master
     175     3056 refs/remotes/ko-private/next
       0     3171 refs/remotes/ko-private/origin
      66     8872 refs/remotes/ko-private/private-jch
      99     3027 refs/remotes/ko-private/pu
       0     8016 refs/remotes/ko-private/snap
       0    11942 refs/remotes/ko-private/test
       0      865 refs/remotes/ko/maint
       0      222 refs/remotes/ko/master
       0        4 refs/remotes/ko/next
     100      204 refs/remotes/ko/pu
       0     1188 refs/remotes/l10n/master
       9    14815 refs/remotes/lea/master
       0    14877 refs/remotes/lea/origin
       0    18037 refs/remotes/mergetool/master
       0    18037 refs/remotes/mergetool/mergetool
     918    13101 refs/remotes/mingw/master
     862    15327 refs/remotes/mingw/master-borked
       0    18173 refs/remotes/pasky.web/master
     825    18172 refs/remotes/pasky.web/mob
       4    18173 refs/remotes/pasky.web/next
       8    18173 refs/remotes/pasky.web/pu
       0    28975 refs/remotes/paulus/HEAD
       6    29067 refs/remotes/paulus/asdf
       0    28989 refs/remotes/paulus/dev
      17    29331 refs/remotes/paulus/lines
       0    28975 refs/remotes/paulus/master
      32    29321 refs/remotes/paulus/new
    1100    29498 refs/remotes/repo/html
       0      865 refs/remotes/repo/maint
     980    29498 refs/remotes/repo/man
       0      222 refs/remotes/repo/master
       0        4 refs/remotes/repo/next
     100      204 refs/remotes/repo/pu
     894    29498 refs/remotes/repo/todo
       0    13638 refs/remotes/spearce.git/maint
       0    13207 refs/remotes/spearce.git/master
     149    13176 refs/remotes/spearce.git/next
     381    29498 refs/remotes/spearce.git/todo
       0    28540 refs/remotes/thoyts/maint
       0    28511 refs/remotes/thoyts/master
       0    28527 refs/remotes/thoyts/pu
       9    29498 refs/remotes/thoyts/todo
       0     8920 refs/remotes/trast-doc/HEAD
       0     8920 refs/remotes/trast-doc/for-next
       0     8920 refs/remotes/trast-doc/master
       0     8917 refs/remotes/trast-doc/pu
:

[PATCHv3 1/2] Warnings before rebasing -i published history

[Lucien Kong]

"git rebase -i" can be very dangerous if used on an already published
history. This code detects that one is rewriting a commit that is an
ancestor of a remote-tracking branch, and warns the user through the
editor. This feature is controlled by a new config key
rebase.checkremoterefs.

Here is an example of the behaviour of this feature:

   pick <id> commit pushed_one
   # Commits above this line appear in origin/master
   pick <id> commit pushed_two
   pick <id> commit pushed_three
   # Commits above this line appear in origin/next
   pick <id> commit local

Signed-off-by: Lucien Kong <Lucien.Kong@ensimag.imag.fr>
Signed-off-by: Valentin Duperray <Valentin.Duperray@ensimag.imag.fr>
Signed-off-by: Franck Jonas <Franck.Jonas@ensimag.imag.fr>
Signed-off-by: Thomas Nguy <Thomas.Nguy@ensimag.imag.fr>
Signed-off-by: Huynh Khoi Nguyen Nguyen <Huynh-Khoi-Nguyen.Nguyen@ensimag.imag.fr>
Signed-off-by: Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>
---
The remote points are now only shown once. The behavior of
git branch --contains is the same than in v2 (not optimized).

 Documentation/config.txt      |    5 +++++
 Documentation/git-rebase.txt  |    5 +++++
 git-rebase--interactive.sh    |   32 ++++++++++++++++++++++++++++++++
 git-rebase.sh                 |    2 ++
 t/lib-rebase.sh               |   11 +++++++++++
 t/t3404-rebase-interactive.sh |   27 +++++++++++++++++++++++++++
 6 files changed, 82 insertions(+), 0 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 915cb5a..e47f6e0 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1753,6 +1753,11 @@ rebase.stat::
 rebase.autosquash::
 	If set to true enable '--autosquash' option by default.
 
+rebase.checkremoterefs::
+	If it is set to 'true', git rebase -i will show after each
+	commit ancestor of a remote-tracking branch the name of these
+	branches through the editor.
+
 receive.autogc::
 	By default, git-receive-pack will run "git-gc --auto" after
 	receiving data from git-push and updating refs.  You can stop
diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index 147fa1a..b68a80b 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -208,6 +208,11 @@ rebase.stat::
 rebase.autosquash::
 	If set to true enable '--autosquash' option by default.
 
+rebase.checkremoterefs::
+	If it is set to 'true', git rebase -i will show after each
+	commit ancestor of a remote-tracking branch the name of these
+	branches through the editor.
+
 OPTIONS
 -------
 <newbase>::
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 0c19b7c..7e4ae16 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -684,6 +684,37 @@ rearrange_squash () {
 	rm -f "$1.sq" "$1.rearranged"
 }
 
+# Add the name of the branches after each pick, fixup or squash commit that
+# is an ancestor of a remote-tracking branch.
+add_remoterefs () {
+	to_print=
+	branches_name=
+	while read -r command sha1 message
+	do
+		git branch -r --contains "$sha1" >"$1.branch"
+		if test -s "$1.branch"
+		then
+			while read -r branch
+			do
+				branches_name="$branches_name $branch"
+			done <"$1.branch"
+		fi
+
+		if test "$to_print" != "$branches_name"
+		then
+			if test -n "$to_print"
+			then
+				printf '%s\n' "# Commits above this line appear in:$to_print"
+			fi
+			to_print=$branches_name
+		fi
+		branches_name=
+		printf '%s\n' "$command $sha1 $message"
+	done >"$1.published" <"$1"
+	cat "$1.published" >"$1"
+	rm -f "$1.published" "$1.branch"
+}
+
 case "$action" in
 continue)
 	# do we have anything to commit?
@@ -857,6 +888,7 @@ fi
 
 test -s "$todo" || echo noop >> "$todo"
 test -n "$autosquash" && rearrange_squash "$todo"
+test -n "$checkremoterefs" && add_remoterefs "$todo"
 cat >> "$todo" << EOF
 
 # Rebase $shortrevisions onto $shortonto
diff --git a/git-rebase.sh b/git-rebase.sh
index e616737..f8675b5 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -99,7 +99,9 @@ action=
 preserve_merges=
 autosquash=
 keep_empty=
+checkremoterefs=
 test "$(git config --bool rebase.autosquash)" = "true" && autosquash=t
+test "$(git config --bool rebase.checkremoterefs)" = "true" && checkremoterefs=t
 
 read_basic_state () {
 	head_name=$(cat "$state_dir"/head-name) &&
diff --git a/t/lib-rebase.sh b/t/lib-rebase.sh
index 6ccf797..4906867 100644
--- a/t/lib-rebase.sh
+++ b/t/lib-rebase.sh
@@ -65,3 +65,14 @@ EOF
 	test_set_editor "$(pwd)/fake-editor.sh"
 	chmod a+x fake-editor.sh
 }
+
+
+set_copy_editor () {
+	echo "#!$SHELL_PATH" >editor.sh
+	cat >> editor.sh <<\EOF
+cat "$1" >"$TODO_DUMP"
+EOF
+
+	test_set_editor "$(pwd)/editor.sh"
+	chmod a+x editor.sh
+}
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 025c1c6..72934a5 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -755,4 +755,31 @@ test_expect_success 'rebase-i history with funny messages' '
 	test_cmp expect actual
 '
 
+test_expect_success 'warn before rewriting published history' '
+	test_when_finished "rm -rf git.git git" &&
+	git init git.git &&
+	git clone git &&
+	(
+		cd git &&
+		git config rebase.checkremoterefs true &&
+		test_commit one_commit main.txt one_commit &&
+		test_commit two_commit main.txt two_commit &&
+		test_commit three_commit main.txt three_commit &&
+		git push --all &&
+		test_commit four_commit main.txt four_commit &&
+		set_copy_editor &&
+		TODO_DUMP=actual EDITOR=./editor.sh git rebase -i HEAD~3 &&
+		tmp=$(cat actual | sed -n 1,5p) &&
+		echo "$tmp" >actual &&
+		two_sha1=$(git rev-parse --short HEAD~2) &&
+		three_sha1=$(git rev-parse --short HEAD~1) &&
+		four_sha1=$(git rev-parse --short HEAD) &&
+		echo "pick $two_sha1 two_commit" >expected &&
+		echo "pick $three_sha1 three_commit" >>expected &&
+		echo "# Commits above this line appear in: origin/master" >>expected &&
+		echo "pick $four_sha1 four_commit" >>expected &&
+		test_cmp expected actual
+	)
+'
+
 test_done
-- 
1.7.8

[PATCHv3 2/2] Warnings before amending published history

[Lucien Kong]

This code detects that one is rewriting a commit that is an ancestor
of a remote-tracking branch with "git commit --amend", and warns the
user through the editor.

Signed-off-by: Lucien Kong <Lucien.Kong@ensimag.imag.fr>
Signed-off-by: Valentin Duperray <Valentin.Duperray@ensimag.imag.fr>
Signed-off-by: Franck Jonas <Franck.Jonas@ensimag.imag.fr>
Signed-off-by: Thomas Nguy <Thomas.Nguy@ensimag.imag.fr>
Signed-off-by: Huynh Khoi Nguyen Nguyen <Huynh-Khoi-Nguyen.Nguyen@ensimag.imag.fr>
Signed-off-by: Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>
---
At this point, this feature is not controlled by the config key
rebase.checkremoterefs. Also, the code can only detects the commits
that were published on the same current branch.

 builtin/commit.c              |   82 ++++++++++++++++++++++++++
 sha1_name.c                   |   95 +++++++-----------------------
 sha1_name.h                   |  130 +++++++++++++++++++++++++++++++++++++++++
 t/t3404-rebase-interactive.sh |   65 ++++++++++++++++++++
 4 files changed, 298 insertions(+), 74 deletions(-)
 create mode 100644 sha1_name.h

diff --git a/builtin/commit.c b/builtin/commit.c
index f43eaaf..53fe120 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -28,6 +28,7 @@
 #include "submodule.h"
 #include "gpg-interface.h"
 #include "column.h"
+#include "sha1_name.h"
 
 static const char * const builtin_commit_usage[] = {
 	"git commit [options] [--] <filepattern>...",
@@ -584,6 +585,83 @@ static char *cut_ident_timestamp_part(char *string)
 	return ket;
 }
 
+static char *read_line_from_git_path(const char *filename)
+{
+	struct strbuf buf = STRBUF_INIT;
+	FILE *fp = fopen(git_path("%s", filename), "r");
+	if (!fp) {
+		strbuf_release(&buf);
+		return NULL;
+	}
+	strbuf_getline(&buf, fp, '\n');
+	if (!fclose(fp))
+		return strbuf_detach(&buf, NULL);
+	else
+		return NULL;
+}
+
+static int insert_first_line_file(char *to_write, char *file_to_modify)
+{
+	int c;
+	FILE *tmp = tmpfile();
+	FILE *file = fopen(file_to_modify, "r+");
+	if (!file || !tmp)
+		return 0;
+
+	while ((c = fgetc(file)) != EOF)
+		fputc(c, tmp);
+
+	rewind(file);
+	rewind(tmp);
+	fputs(to_write, file);
+	while ((c = fgetc(tmp)) != EOF)
+		fputc(c, file);
+
+	fclose(tmp);
+	fclose(file);
+	return 1;
+}
+
+static int amend_warn_published(void)
+{
+	char *head_path = read_line_from_git_path("HEAD");
+	char *last_commit_sha1;
+	char remote_path[PATH_MAX] = "refs/remotes/origin/";
+	char *remote_branch;
+	unsigned char nth_ancestor_remote_sha1[20];
+	int i = 0;
+
+	if (!head_path)
+		return 0;
+
+	strtok(head_path, " ");
+	last_commit_sha1 = read_line_from_git_path(strtok(NULL, ""));
+	if (!last_commit_sha1)
+		return 0;
+
+	remote_branch = read_line_from_git_path("HEAD");
+	if (!remote_branch)
+		return 0;
+
+	strtok(remote_branch, "/");
+	strtok(NULL, "/");
+	strcat(remote_path, strtok(NULL, ""));
+
+	while (!get_nth_ancestor(remote_path, 40, nth_ancestor_remote_sha1, i)) {
+		if (!strcmp(sha1_to_hex(nth_ancestor_remote_sha1), last_commit_sha1)) {
+			insert_first_line_file("# The commit to reword is already published.\n\n",
+					git_path(commit_editmsg));
+			break;
+		}
+		i++;
+	}
+
+	free(head_path);
+	free(last_commit_sha1);
+	free(remote_branch);
+	return 1;
+}
+
 static int prepare_to_commit(const char *index_file, const char *prefix,
 			     struct commit *current_head,
 			     struct wt_status *s,
@@ -840,6 +918,10 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
 		const char *env[2] = { NULL };
 		env[0] =  index;
 		snprintf(index, sizeof(index), "GIT_INDEX_FILE=%s", index_file);
+
+		if (amend)
+			amend_warn_published();
+
 		if (launch_editor(git_path(commit_editmsg), NULL, env)) {
 			fprintf(stderr,
 			_("Please supply the message using either -m or -F option.\n"));
diff --git a/sha1_name.c b/sha1_name.c
index c633113..14a0e96 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -1,4 +1,5 @@
 #include "cache.h"
+#include "sha1_name.h"
 #include "tag.h"
 #include "commit.h"
 #include "tree.h"
@@ -7,9 +8,7 @@
 #include "refs.h"
 #include "remote.h"
 
-static int get_sha1_oneline(const char *, unsigned char *, struct commit_list *);
-
-static int find_short_object_filename(int len, const char *name, unsigned char *sha1)
+int find_short_object_filename(int len, const char *name, unsigned char *sha1)
 {
 	struct alternate_object_database *alt;
 	char hex[40];
@@ -56,7 +55,7 @@ static int find_short_object_filename(int len, const char *name, unsigned char *
 	return found;
 }
 
-static int match_sha(unsigned len, const unsigned char *a, const unsigned char *b)
+int match_sha(unsigned len, const unsigned char *a, const unsigned char *b)
 {
 	do {
 		if (*a != *b)
@@ -71,7 +70,7 @@ static int match_sha(unsigned len, const unsigned char *a, const unsigned char *
 	return 1;
 }
 
-static int find_short_packed_object(int len, const unsigned char *match, unsigned char *sha1)
+int find_short_packed_object(int len, const unsigned char *match, unsigned char *sha1)
 {
 	struct packed_git *p;
 	const unsigned char *found_sha1 = NULL;
@@ -130,10 +129,7 @@ static int find_short_packed_object(int len, const unsigned char *match, unsigne
 	return found;
 }
 
-#define SHORT_NAME_NOT_FOUND (-1)
-#define SHORT_NAME_AMBIGUOUS (-2)
-
-static int find_unique_short_object(int len, char *canonical,
+int find_unique_short_object(int len, char *canonical,
 				    unsigned char *res, unsigned char *sha1)
 {
 	int has_unpacked, has_packed;
@@ -157,7 +153,7 @@ static int find_unique_short_object(int len, char *canonical,
 	return 0;
 }
 
-static int get_short_sha1(const char *name, int len, unsigned char *sha1,
+int get_short_sha1(const char *name, int len, unsigned char *sha1,
 			  int quietly)
 {
 	int i, status;
@@ -216,7 +212,7 @@ const char *find_unique_abbrev(const unsigned char *sha1, int len)
 	return hex;
 }
 
-static int ambiguous_path(const char *path, int len)
+int ambiguous_path(const char *path, int len)
 {
 	int slash = 1;
 	int cnt;
@@ -241,7 +237,7 @@ static int ambiguous_path(const char *path, int len)
 	return slash;
 }
 
-static inline int upstream_mark(const char *string, int len)
+inline int upstream_mark(const char *string, int len)
 {
 	const char *suffix[] = { "@{upstream}", "@{u}" };
 	int i;
@@ -255,9 +251,7 @@ static inline int upstream_mark(const char *string, int len)
 	return 0;
 }
 
-static int get_sha1_1(const char *name, int len, unsigned char *sha1);
-
-static int get_sha1_basic(const char *str, int len, unsigned char *sha1)
+int get_sha1_basic(const char *str, int len, unsigned char *sha1)
 {
 	static const char *warn_msg = "refname '%.*s' is ambiguous.";
 	char *real_ref = NULL;
@@ -358,7 +352,7 @@ static int get_sha1_basic(const char *str, int len, unsigned char *sha1)
 	return 0;
 }
 
-static int get_parent(const char *name, int len,
+int get_parent(const char *name, int len,
 		      unsigned char *result, int idx)
 {
 	unsigned char sha1[20];
@@ -388,7 +382,7 @@ static int get_parent(const char *name, int len,
 	return -1;
 }
 
-static int get_nth_ancestor(const char *name, int len,
+int get_nth_ancestor(const char *name, int len,
 			    unsigned char *result, int generation)
 {
 	unsigned char sha1[20];
@@ -436,7 +430,7 @@ struct object *peel_to_type(const char *name, int namelen,
 	}
 }
 
-static int peel_onion(const char *name, int len, unsigned char *sha1)
+int peel_onion(const char *name, int len, unsigned char *sha1)
 {
 	unsigned char outer[20];
 	const char *sp;
@@ -522,7 +516,7 @@ static int peel_onion(const char *name, int len, unsigned char *sha1)
 	return 0;
 }
 
-static int get_describe_name(const char *name, int len, unsigned char *sha1)
+int get_describe_name(const char *name, int len, unsigned char *sha1)
 {
 	const char *cp;
 
@@ -542,7 +536,7 @@ static int get_describe_name(const char *name, int len, unsigned char *sha1)
 	return -1;
 }
 
-static int get_sha1_1(const char *name, int len, unsigned char *sha1)
+int get_sha1_1(const char *name, int len, unsigned char *sha1)
 {
 	int ret, has_suffix;
 	const char *cp;
@@ -590,17 +584,7 @@ static int get_sha1_1(const char *name, int len, unsigned char *sha1)
 	return get_short_sha1(name, len, sha1, 0);
 }
 
-/*
- * This interprets names like ':/Initial revision of "git"' by searching
- * through history and returning the first commit whose message starts
- * the given regular expression.
- *
- * For future extension, ':/!' is reserved. If you want to match a message
- * beginning with a '!', you have to repeat the exclamation mark.
- */
-#define ONELINE_SEEN (1u<<20)
-
-static int handle_one_ref(const char *path,
+int handle_one_ref(const char *path,
 		const unsigned char *sha1, int flag, void *cb_data)
 {
 	struct commit_list **list = cb_data;
@@ -618,7 +602,7 @@ static int handle_one_ref(const char *path,
 	return 0;
 }
 
-static int get_sha1_oneline(const char *prefix, unsigned char *sha1,
+int get_sha1_oneline(const char *prefix, unsigned char *sha1,
 			    struct commit_list *list)
 {
 	struct commit_list *backup = NULL, *l;
@@ -675,12 +659,7 @@ static int get_sha1_oneline(const char *prefix, unsigned char *sha1,
 	return found ? 0 : -1;
 }
 
-struct grab_nth_branch_switch_cbdata {
-	long cnt, alloc;
-	struct strbuf *buf;
-};
-
-static int grab_nth_branch_switch(unsigned char *osha1, unsigned char *nsha1,
+int grab_nth_branch_switch(unsigned char *osha1, unsigned char *nsha1,
 				  const char *email, unsigned long timestamp, int tz,
 				  const char *message, void *cb_data)
 {
@@ -704,11 +683,7 @@ static int grab_nth_branch_switch(unsigned char *osha1, unsigned char *nsha1,
 	return 0;
 }
 
-/*
- * Parse @{-N} syntax, return the number of characters parsed
- * if successful; otherwise signal an error with negative value.
- */
-static int interpret_nth_prior_checkout(const char *name, struct strbuf *buf)
+int interpret_nth_prior_checkout(const char *name, struct strbuf *buf)
 {
 	long nth;
 	int i, retval;
@@ -794,27 +769,6 @@ int get_sha1_mb(const char *name, unsigned char *sha1)
 	return st;
 }
 
-/*
- * This reads short-hand syntax that not only evaluates to a commit
- * object name, but also can act as if the end user spelled the name
- * of the branch from the command line.
- *
- * - "@{-N}" finds the name of the Nth previous branch we were on, and
- *   places the name of the branch in the given buf and returns the
- *   number of characters parsed if successful.
- *
- * - "<branch>@{upstream}" finds the name of the other ref that
- *   <branch> is configured to merge with (missing <branch> defaults
- *   to the current branch), and places the name of the branch in the
- *   given buf and returns the number of characters parsed if
- *   successful.
- *
- * If the input is not of the accepted format, it returns a negative
- * number to signal an error.
- *
- * If the input was ok but there are not N branch switches in the
- * reflog, it returns 0.
- */
 int interpret_branch_name(const char *name, struct strbuf *buf)
 {
 	char *cp;
@@ -898,18 +852,13 @@ int strbuf_check_branch_ref(struct strbuf *sb, const char *name)
 	return check_refname_format(sb->buf, 0);
 }
 
-/*
- * This is like "get_sha1_basic()", except it allows "sha1 expressions",
- * notably "xyz^" for "parent of xyz"
- */
 int get_sha1(const char *name, unsigned char *sha1)
 {
 	struct object_context unused;
 	return get_sha1_with_context(name, sha1, &unused);
 }
 
-/* Must be called only when object_name:filename doesn't exist. */
-static void diagnose_invalid_sha1_path(const char *prefix,
+void diagnose_invalid_sha1_path(const char *prefix,
 				       const char *filename,
 				       const unsigned char *tree_sha1,
 				       const char *object_name)
@@ -946,8 +895,7 @@ static void diagnose_invalid_sha1_path(const char *prefix,
 	}
 }
 
-/* Must be called only when :stage:filename doesn't exist. */
-static void diagnose_invalid_index_path(int stage,
+void diagnose_invalid_index_path(int stage,
 					const char *prefix,
 					const char *filename)
 {
@@ -1003,7 +951,6 @@ static void diagnose_invalid_index_path(int stage,
 	free(fullname);
 }
 
-
 int get_sha1_with_mode_1(const char *name, unsigned char *sha1, unsigned *mode,
 			 int only_to_die, const char *prefix)
 {
@@ -1014,7 +961,7 @@ int get_sha1_with_mode_1(const char *name, unsigned char *sha1, unsigned *mode,
 	return ret;
 }
 
-static char *resolve_relative_path(const char *rel)
+char *resolve_relative_path(const char *rel)
 {
 	if (prefixcmp(rel, "./") && prefixcmp(rel, "../"))
 		return NULL;
diff --git a/sha1_name.h b/sha1_name.h
new file mode 100644
index 0000000..bdbcecd
--- /dev/null
+++ b/sha1_name.h
@@ -0,0 +1,130 @@
+#ifndef SHA1_NAME_H
+#define SHA1_NAME_H
+
+#include "commit.h"
+
+int find_short_object_filename(int len, const char *name, unsigned char *sha1);
+
+int match_sha(unsigned len, const unsigned char *a, const unsigned char *b);
+
+int find_short_packed_object(int len, const unsigned char *match, unsigned char *sha1);
+
+#define SHORT_NAME_NOT_FOUND (-1)
+#define SHORT_NAME_AMBIGUOUS (-2)
+
+int find_unique_short_object(int len, char *canonical,
+				    unsigned char *res, unsigned char *sha1);
+
+int get_short_sha1(const char *name, int len, unsigned char *sha1,
+			  int quietly);
+
+const char *find_unique_abbrev(const unsigned char *sha1, int len);
+
+int ambiguous_path(const char *path, int len);
+
+inline int upstream_mark(const char *string, int len);
+
+int get_sha1_basic(const char *str, int len, unsigned char *sha1);
+
+int get_parent(const char *name, int len,
+		      unsigned char *result, int idx);
+
+int get_nth_ancestor(const char *name, int len,
+			    unsigned char *result, int generation);
+
+struct object *peel_to_type(const char *name, int namelen,
+			    struct object *o, enum object_type expected_type);
+
+int peel_onion(const char *name, int len, unsigned char *sha1);
+
+int get_describe_name(const char *name, int len, unsigned char *sha1);
+
+int get_sha1_1(const char *name, int len, unsigned char *sha1);
+
+/*
+ * This interprets names like ':/Initial revision of "git"' by searching
+ * through history and returning the first commit whose message starts
+ * the given regular expression.
+ *
+ * For future extension, ':/!' is reserved. If you want to match a message
+ * beginning with a '!', you have to repeat the exclamation mark.
+ */
+#define ONELINE_SEEN (1u<<20)
+
+int handle_one_ref(const char *path,
+		const unsigned char *sha1, int flag, void *cb_data);
+
+int get_sha1_oneline(const char *prefix, unsigned char *sha1,
+			    struct commit_list *list);
+
+struct grab_nth_branch_switch_cbdata {
+	long cnt, alloc;
+	struct strbuf *buf;
+};
+
+int grab_nth_branch_switch(unsigned char *osha1, unsigned char *nsha1,
+				  const char *email, unsigned long timestamp, int tz,
+				  const char *message, void *cb_data);
+
+/*
+ * Parse @{-N} syntax, return the number of characters parsed
+ * if successful; otherwise signal an error with negative value.
+ */
+int interpret_nth_prior_checkout(const char *name, struct strbuf *buf);
+
+int get_sha1_mb(const char *name, unsigned char *sha1);
+
+/*
+ * This reads short-hand syntax that not only evaluates to a commit
+ * object name, but also can act as if the end user spelled the name
+ * of the branch from the command line.
+ *
+ * - "@{-N}" finds the name of the Nth previous branch we were on, and
+ *   places the name of the branch in the given buf and returns the
+ *   number of characters parsed if successful.
+ *
+ * - "<branch>@{upstream}" finds the name of the other ref that
+ *   <branch> is configured to merge with (missing <branch> defaults
+ *   to the current branch), and places the name of the branch in the
+ *   given buf and returns the number of characters parsed if
+ *   successful.
+ *
+ * If the input is not of the accepted format, it returns a negative
+ * number to signal an error.
+ *
+ * If the input was ok but there are not N branch switches in the
+ * reflog, it returns 0.
+ */
+int interpret_branch_name(const char *name, struct strbuf *buf);
+
+int strbuf_branchname(struct strbuf *sb, const char *name);
+
+int strbuf_check_branch_ref(struct strbuf *sb, const char *name);
+
+/*
+ * This is like "get_sha1_basic()", except it allows "sha1 expressions",
+ * notably "xyz^" for "parent of xyz"
+ */
+int get_sha1(const char *name, unsigned char *sha1);
+
+/* Must be called only when object_name:filename doesn't exist. */
+void diagnose_invalid_sha1_path(const char *prefix,
+				       const char *filename,
+				       const unsigned char *tree_sha1,
+				       const char *object_name);
+
+/* Must be called only when :stage:filename doesn't exist. */
+void diagnose_invalid_index_path(int stage,
+					const char *prefix,
+					const char *filename);
+
+int get_sha1_with_mode_1(const char *name, unsigned char *sha1, unsigned *mode,
+			 int only_to_die, const char *prefix);
+
+char *resolve_relative_path(const char *rel);
+
+int get_sha1_with_context_1(const char *name, unsigned char *sha1,
+			    struct object_context *oc,
+			    int only_to_die, const char *prefix);
+
+#endif
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 72934a5..fec448b 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -782,4 +782,69 @@ test_expect_success 'warn before rewriting published history' '
 	)
 '
 
+test_expect_success 'warn before rewriting published history: one user' '
+	test_when_finished "rm -rf git.git git" &&
+	git init git.git &&
+	git clone git &&
+	(
+		cd git &&
+		git config rebase.checkremoterefs true &&
+		test_commit one_commit main.txt one_commit &&
+		git push --all
+		set_copy_editor &&
+		TODO_DUMP=actual EDITOR=./editor.sh git commit --amend &&
+		tmp=$(cat actual | sed -n 1p) &&
+		echo "$tmp" >actual &&
+		echo "# The commit to reword is already published." >expected &&
+		test_cmp expected actual
+	)
+'
+
+test_expect_success 'warn before rewriting published history: several users' '
+	test_when_finished "rm -rf git.git git1 git2" &&
+	git init --bare --share git.git &&
+	git clone git.git git1 &&
+	git clone git.git git2 &&
+	(
+		cd git1 &&
+		test_commit one_commit main.txt one_commit &&
+		git push --all
+	) &&
+	(
+		cd git2 &&
+		git pull &&
+		test_commit two_commit main.txt two_commit &&
+		git push --all
+	) &&
+	(
+		cd git1 &&
+		git config rebase.checkremoterefs true &&
+		set_copy_editor &&
+		TODO_DUMP=actual EDITOR=./editor.sh git commit --amend &&
+		tmp=$(cat actual | sed -n 1p) &&
+		echo "$tmp" >actual &&
+		echo "# The commit to reword is already published." >expected &&
+		test_cmp expected actual
+	)
+'
+
+test_expect_success 'warn before rewriting published history: shared branch' '
+	test_when_finished "rm -rf git.git git" &&
+	git init git.git &&
+	git clone git &&
+	(
+		cd git &&
+		git config rebase.checkremoterefs true &&
+		git checkout -b sharedbranch &&
+		test_commit one_commit main.txt one_commit &&
+		git push --set-upstream origin sharedbranch &&
+		set_copy_editor &&
+		TODO_DUMP=actual EDITOR=./editor.sh git commit --amend &&
+		tmp=$(cat actual | sed -n 1p) &&
+		echo "$tmp" >actual &&
+		echo "# The commit to reword is already published." >expected &&
+		test_cmp expected actual
+	)
+'
+
 test_done
-- 
1.7.8

Re: [PATCHv3 2/2] Warnings before amending published history

[Matthieu Moy]

Lucien Kong <Lucien.Kong@ensimag.imag.fr> writes:

>  builtin/commit.c              |   82 ++++++++++++++++++++++++++
>  sha1_name.c                   |   95 +++++++-----------------------
>  sha1_name.h                   |  130 +++++++++++++++++++++++++++++++++++++++++

I'm surprised that you need such a big patch. Basically, you're making
all static functions in sha1_name.c public. If you really need such
intrusive change, then you should at least explain why in the commit
message, and most preferably split the patch into one refactoring patch
to expose the functions and one to use them.

But I suspect what you're looking for is already in cache.h.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: [PATCHv3 2/2] Warnings before amending published history

[Junio C Hamano]

Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:

> Lucien Kong <Lucien.Kong@ensimag.imag.fr> writes:
>
>>  builtin/commit.c              |   82 ++++++++++++++++++++++++++
>>  sha1_name.c                   |   95 +++++++-----------------------
>>  sha1_name.h                   |  130 +++++++++++++++++++++++++++++++++++++++++
>
> I'm surprised that you need such a big patch. Basically, you're making
> all static functions in sha1_name.c public. If you really need such
> intrusive change, then you should at least explain why in the commit
> message, and most preferably split the patch into one refactoring patch
> to expose the functions and one to use them.
>
> But I suspect what you're looking for is already in cache.h.

I do not think I am going to take this nor the rebase patch that
bases its decision on the hardcoded "Does the commit appear in the
history of refs/remotes/*anything*?" logic.

At least, there should be "Here is a list of the branches I promised
others that I am not to going to rewind." even if you are going to
make its default value to be "for-each-ref refs/remotes/".  It is
too inflexible to be useful otherwise.  Not only in the contributor
and integrator workflow, but a simple "Alice asks Bob to pull from
her Github repository" will be hurt on the "I fixed up the issues
you raised. Could you please take another look" round.  Besides, I
won't be able to amend things outside 'next' but are in 'pu' ;-).

The logic in the patch in this thread to check each ref~$n is not
even worth commenting on, but as to the logic in the other "rebase"
one, I think it is wasteful to ask "what are the refs that can reach
this commit?" when what you really want to know is "is there any ref
among this set that can reach this commit?" (the former needs to
keep a lot more state).  It should be something like looking at the
output of:

	git rev-list <list commits you are going to touch here> \
		--not <list tips of refs you have published>

and make sure all the commits you are going to touch appear in the
result.  Any missing one is reachable from the refs you have
published and you may not want to rebase.

It may be an interesting thought experiment to see if you can take
advantage of the inherent ancestry relationship among the list of
commits you are going to touch. The later commits that will be
replayed in a rebase are very likely to be children of earlier one,
so in theory, if you can identify the set of topologically earliest
commits that will be replayed, you only need to check them, and if
you can cheaply come up with that set of earliest commits, the above
rev-list may become cheaper.

Re: [PATCHv3 1/2] Warnings before rebasing -i published history

[Nguy Thomas]

Le 11/06/2012 23:56, Lucien Kong a écrit :
> +# Add the name of the branches after each pick, fixup or squash commit that
> +# is an ancestor of a remote-tracking branch.
> +add_remoterefs () {
> +	to_print=
> +	branches_name=
> +	while read -r command sha1 message
> +	do
> +		git branch -r --contains "$sha1">"$1.branch"
> +		if test -s "$1.branch"
> +		then
> +			while read -r branch
> +			do
> +				branches_name="$branches_name $branch"
> +			done<"$1.branch"
> +		fi
> +
> +		if test "$to_print" != "$branches_name"
> +		then
> +			if test -n "$to_print"
> +			then
> +				printf '%s\n' "# Commits above this line appear in:$to_print"
> +			fi
> +			to_print=$branches_name
> +		fi
> +		branches_name=
> +		printf '%s\n' "$command $sha1 $message"
> +	done>"$1.published"<"$1"
> +	cat "$1.published">"$1"
> +	rm -f "$1.published" "$1.branch"
> +}
>
Hum, this function doesn't consider the last commit.
It will be fixed.