Git development
 help / color / mirror / Atom feed
* [PATCH] Use SHELL_PATH
From: SungHyun Nam @ 2008-07-16  1:31 UTC (permalink / raw)
  To: git


Signed-off-by: SungHyun Nam <goweol@gmail.com>
---
  t/Makefile |    2 +-
  1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/t/Makefile b/t/Makefile
index a778865..0d65ced 100644
--- a/t/Makefile
+++ b/t/Makefile
@@ -26,7 +26,7 @@ clean:
  	$(RM) -r 'trash directory' test-results

  aggregate-results:
-	./aggregate-results.sh test-results/t*-*
+	'$(SHELL_PATH_SQ)' ./aggregate-results.sh test-results/t*-*

  # we can test NO_OPTIMIZE_COMMITS independently of LC_ALL
  full-svn-test:
-- 
1.5.6.3.350.g6c11a

^ permalink raw reply related

* Re: [PATCH] Rename ".dotest/" to ".git/rebase" and ".dotest-merge" to "rebase-merge"
From: Theodore Tso @ 2008-07-16  1:26 UTC (permalink / raw)
  To: Johannes Schindelin
  Cc: René Scharfe, gitster, Stephan Beyer, Joe Fiorini, git,
	Jari Aalto
In-Reply-To: <alpine.DEB.1.00.0807160245440.2841@eeepc-johanness>

On Wed, Jul 16, 2008 at 02:47:33AM +0200, Johannes Schindelin wrote:
> 
> Since the files generated and used during a rebase are never to be
> tracked, they should live in $GIT_DIR.  While at it, avoid the rather
> meaningless term "dotest" to "rebase", and unhide ".dotest-merge".
> 
> This was wished for on the mailing list, but so far unimplemented.

While you have "git am" open, how about adding an "git am --abort"
which nukes the .dotest aka .git/rebase directory, and resets HEAD
back to the original position?

And another nice shortcut would be "git am --forceapply" which applies
the patch, leaving .rej files for the user to resolve by hand.  That
makes it easier for the user to manually run patch while passing a
filename in .dotest aka .git/rebase.

These two additions would make the git-am workflow much smoother.

Regards,

					- Ted

^ permalink raw reply

* Re: [PATCH] Rename ".dotest/" to ".git/rebase" and ".dotest-merge" to "rebase-merge"
From: Junio C Hamano @ 2008-07-16  1:19 UTC (permalink / raw)
  To: Johannes Schindelin
  Cc: René Scharfe, Stephan Beyer, Joe Fiorini, git, Jari Aalto
In-Reply-To: <alpine.DEB.1.00.0807160315020.2841@eeepc-johanness>

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> Of course, you can name it as you want.  But I thought that the name 
> "rebase" applies as well: the patches are rebased from somewhere else...

Somewhere else being my mailbox or gmane newsgroup?

The patch does not apply to my master anyway, sigh...

^ permalink raw reply

* Re: [PATCH] Rename ".dotest/" to ".git/rebase" and ".dotest-merge" to "rebase-merge"
From: Johannes Schindelin @ 2008-07-16  1:15 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: René Scharfe, Stephan Beyer, Joe Fiorini, git, Jari Aalto
In-Reply-To: <7v4p6qzla3.fsf@gitster.siamese.dyndns.org>

Hi,

On Tue, 15 Jul 2008, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > Since the files generated and used during a rebase are never to be 
> > tracked, they should live in $GIT_DIR.  While at it, avoid the rather 
> > meaningless term "dotest" to "rebase", and unhide ".dotest-merge".
> 
> I understand moving away from .dotest/ to .git/something, but I do not 
> follow the logic of making that something to rebase at all.  It is a 
> scratch area for "am" (and applymbox), isn't it?

Of course, you can name it as you want.  But I thought that the name 
"rebase" applies as well: the patches are rebased from somewhere else on 
top of HEAD :-)

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH] Rename ".dotest/" to ".git/rebase" and ".dotest-merge" to "rebase-merge"
From: Junio C Hamano @ 2008-07-16  1:10 UTC (permalink / raw)
  To: Johannes Schindelin
  Cc: René Scharfe, gitster, Stephan Beyer, Joe Fiorini, git,
	Jari Aalto
In-Reply-To: <alpine.DEB.1.00.0807160245440.2841@eeepc-johanness>

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> Since the files generated and used during a rebase are never to be
> tracked, they should live in $GIT_DIR.  While at it, avoid the rather
> meaningless term "dotest" to "rebase", and unhide ".dotest-merge".

I understand moving away from .dotest/ to .git/something, but I do not
follow the logic of making that something to rebase at all.  It is a
scratch area for "am" (and applymbox), isn't it?

^ permalink raw reply

* Re: [PATCH] Rename ".dotest/" to ".git/rebase" and ".dotest-merge" to "rebase-merge"
From: Linus Torvalds @ 2008-07-16  0:57 UTC (permalink / raw)
  To: Johannes Schindelin
  Cc: René Scharfe, gitster, Stephan Beyer, Joe Fiorini, git,
	Jari Aalto
In-Reply-To: <alpine.DEB.1.00.0807160245440.2841@eeepc-johanness>



On Wed, 16 Jul 2008, Johannes Schindelin wrote:
> 
> Since the files generated and used during a rebase are never to be
> tracked, they should live in $GIT_DIR.  While at it, avoid the rather
> meaningless term "dotest" to "rebase", and unhide ".dotest-merge".

Ack. Please make it so. Even _I_ have now taught myself to do "git am" 
instead of using my old "dotest" alias.

		Linus

^ permalink raw reply

* Re: Useful tip about !aliases
From: Kevin Ballard @ 2008-07-16  0:58 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Git Mailing List
In-Reply-To: <alpine.DEB.1.00.0807160250120.2841@eeepc-johanness>

On Jul 15, 2008, at 5:50 PM, Johannes Schindelin wrote:

> Hi,
>
> On Tue, 15 Jul 2008, Kevin Ballard wrote:
>
>> Here's something I discovered recently about !aliases that other  
>> folks might
>> find useful. The canonical form for a shell alias is something like
>>
>> git config alias.foo '!echo bar'
>>
>> where any args given to foo, as in `git foo blah` are passed along  
>> to the
>> shell, so in this case `echo bar blah` would be invoked.
>>
>> Something that I find very useful is the ability to interpolate  
>> arguments into
>> the middle of a command. This doesn't seem possible at first  
>> glance, not
>> without a helper script. But it certainly is possible, with the  
>> help of shell
>> functions:
>>
>> git config alias.reverse '!foo () { args=''; while [[ -n "$*" ]];  
>> do args="$1
>> $args"; shift; done; echo $args; }; foo'
>
> On the gitwiki, you will find a more elegant solution using "sh -c".

I'd forgotten you can do that, but I disagree that it's more elegant.  
It's a bit shorter, certainly, but it also invokes another process.  
Using a shell function doesn't.

-Kevin Ballard

-- 
Kevin Ballard
http://kevin.sb.org
kevin@sb.org
http://www.tildesoft.com

^ permalink raw reply

* Re: Useful tip about !aliases
From: Johannes Schindelin @ 2008-07-16  0:50 UTC (permalink / raw)
  To: Kevin Ballard; +Cc: Git Mailing List
In-Reply-To: <3BD7F543-7CB4-48B6-8D2C-DEA2ADC6EF5F@sb.org>

Hi,

On Tue, 15 Jul 2008, Kevin Ballard wrote:

> Here's something I discovered recently about !aliases that other folks might
> find useful. The canonical form for a shell alias is something like
> 
>  git config alias.foo '!echo bar'
> 
> where any args given to foo, as in `git foo blah` are passed along to the
> shell, so in this case `echo bar blah` would be invoked.
> 
> Something that I find very useful is the ability to interpolate arguments into
> the middle of a command. This doesn't seem possible at first glance, not
> without a helper script. But it certainly is possible, with the help of shell
> functions:
> 
>  git config alias.reverse '!foo () { args=''; while [[ -n "$*" ]]; do args="$1
> $args"; shift; done; echo $args; }; foo'

On the gitwiki, you will find a more elegant solution using "sh -c".

Ciao,
Dscho

^ permalink raw reply

* [PATCH] Rename ".dotest/" to ".git/rebase" and ".dotest-merge" to "rebase-merge"
From: Johannes Schindelin @ 2008-07-16  0:47 UTC (permalink / raw)
  To: René Scharfe, gitster; +Cc: Stephan Beyer, Joe Fiorini, git, Jari Aalto
In-Reply-To: <487D1B3D.70500@lsrfire.ath.cx>

[-- Attachment #1: Type: TEXT/PLAIN, Size: 18180 bytes --]


Since the files generated and used during a rebase are never to be
tracked, they should live in $GIT_DIR.  While at it, avoid the rather
meaningless term "dotest" to "rebase", and unhide ".dotest-merge".

This was wished for on the mailing list, but so far unimplemented.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---

	On Tue, 15 Jul 2008, René Scharfe wrote:

	> Stephan Beyer schrieb:
	> > git-rebase (without -i/-m) generates a directory called 
	> > ".dotest/" to save temporary stuff like the commits you want to 
	> > rebase.
	> 
	> In February it was discussed to move .dotest below $GIT_DIR.  
	> There was even a patch (to rename it to .git-dotest).  I suspect the 
	> upcoming version 1.6.0 is a good opportunity to finally remove this 
	> wart.

	I kind of expected this to be a good opportunity to increase the 
	track record for a certain GSoC project, but here you have it.

 Documentation/SubmittingPatches        |    2 +-
 Documentation/git-am.txt               |    4 +-
 Documentation/git-rebase.txt           |    2 +-
 Documentation/user-manual.txt          |    2 +-
 contrib/emacs/git.el                   |    8 +++---
 git-am.sh                              |    6 ++--
 git-completion.bash                    |   20 +++++++-------
 git-quiltimport.sh                     |    2 +-
 git-rebase--interactive.sh             |    2 +-
 git-rebase.sh                          |   46 ++++++++++++++++----------------
 t/t3401-rebase-partial.sh              |    4 +-
 t/t3404-rebase-interactive.sh          |    8 +++---
 t/t3407-rebase-abort.sh                |    4 +-
 t/t4150-am.sh                          |   22 +++++++-------
 t/t9106-git-svn-commit-diff-clobber.sh |    2 +-
 15 files changed, 67 insertions(+), 67 deletions(-)

diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches
index b116475..fdfa536 100644
--- a/Documentation/SubmittingPatches
+++ b/Documentation/SubmittingPatches
@@ -301,7 +301,7 @@ If it does not apply correctly, there can be various reasons.
   patch appropriately.
 
 * Your MUA corrupted your patch; "am" would complain that
-  the patch does not apply.  Look at .dotest/ subdirectory and
+  the patch does not apply.  Look at .git/rebase/ subdirectory and
   see what 'patch' file contains and check for the common
   corruption patterns mentioned above.
 
diff --git a/Documentation/git-am.txt b/Documentation/git-am.txt
index 3863eeb..5b800d3 100644
--- a/Documentation/git-am.txt
+++ b/Documentation/git-am.txt
@@ -140,9 +140,9 @@ aborts in the middle,.  You can recover from this in one of two ways:
   the index file to bring it in a state that the patch should
   have produced.  Then run the command with '--resolved' option.
 
-The command refuses to process new mailboxes while `.dotest`
+The command refuses to process new mailboxes while `.git/rebase`
 directory exists, so if you decide to start over from scratch,
-run `rm -f -r .dotest` before running the command with mailbox
+run `rm -f -r .git/rebase` before running the command with mailbox
 names.
 
 
diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index f3459c7..c645073 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -38,7 +38,7 @@ It is possible that a merge failure will prevent this process from being
 completely automatic.  You will have to resolve any such merge failure
 and run `git rebase --continue`.  Another option is to bypass the commit
 that caused the merge failure with `git rebase --skip`.  To restore the
-original <branch> and remove the .dotest working files, use the command
+original <branch> and remove the .git/rebase working files, use the command
 `git rebase --abort` instead.
 
 Assume the following history exists and the current branch is "topic":
diff --git a/Documentation/user-manual.txt b/Documentation/user-manual.txt
index 01c1af6..94c9a58 100644
--- a/Documentation/user-manual.txt
+++ b/Documentation/user-manual.txt
@@ -2431,7 +2431,7 @@ $ git rebase origin
 -------------------------------------------------
 
 This will remove each of your commits from mywork, temporarily saving
-them as patches (in a directory named ".dotest"), update mywork to
+them as patches (in a directory named ".git/rebase"), update mywork to
 point at the latest version of origin, then apply each of the saved
 patches to the new mywork.  The result will look like:
 
diff --git a/contrib/emacs/git.el b/contrib/emacs/git.el
index 4fa853f..43b059b 100644
--- a/contrib/emacs/git.el
+++ b/contrib/emacs/git.el
@@ -1252,8 +1252,8 @@ Return the list of files that haven't been handled."
        "\n")
       (when subject (insert subject "\n\n"))
       (cond (msg (insert msg "\n"))
-            ((file-readable-p ".dotest/msg")
-             (insert-file-contents ".dotest/msg"))
+            ((file-readable-p ".git/rebase/msg")
+             (insert-file-contents ".git/rebase/msg"))
             ((file-readable-p ".git/MERGE_MSG")
              (insert-file-contents ".git/MERGE_MSG")))
       ; delete empty lines at end
@@ -1272,9 +1272,9 @@ Return the list of files that haven't been handled."
           (coding-system (git-get-commits-coding-system))
           author-name author-email subject date)
       (when (eq 0 (buffer-size buffer))
-        (when (file-readable-p ".dotest/info")
+        (when (file-readable-p ".git/rebase/info")
           (with-temp-buffer
-            (insert-file-contents ".dotest/info")
+            (insert-file-contents ".git/rebase/info")
             (goto-char (point-min))
             (when (re-search-forward "^Author: \\(.*\\)\nEmail: \\(.*\\)$" nil t)
               (setq author-name (match-string 1))
diff --git a/git-am.sh b/git-am.sh
index fe53608..3a11f8b 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -118,7 +118,7 @@ It does not apply to blobs recorded in its index."
 }
 
 prec=4
-dotest=".dotest"
+dotest="$GIT_DIR/rebase"
 sign= utf8=t keep= skip= interactive= resolved= binary= rebasing=
 resolvemsg= resume=
 git_apply_opt=
@@ -194,7 +194,7 @@ then
 		false
 		;;
 	esac ||
-	die "previous dotest directory $dotest still exists but mbox given."
+	die "previous rebase directory $dotest still exists but mbox given."
 	resume=yes
 else
 	# Make sure we are not given --skip nor --resolved
@@ -324,7 +324,7 @@ do
 			<"$dotest"/info >/dev/null &&
 			go_next && continue
 
-		test -s $dotest/patch || {
+		test -s "$dotest/patch" || {
 			echo "Patch is empty.  Was it split wrong?"
 			stop_here $this
 		}
diff --git a/git-completion.bash b/git-completion.bash
index 04e2ef5..8c8af4f 100755
--- a/git-completion.bash
+++ b/git-completion.bash
@@ -68,26 +68,26 @@ __git_ps1 ()
 	if [ -n "$g" ]; then
 		local r
 		local b
-		if [ -d "$g/../.dotest" ]
+		if [ -d "$g/rebase" ]
 		then
-			if test -f "$g/../.dotest/rebasing"
+			if test -f "$g/rebase/rebasing"
 			then
 				r="|REBASE"
-			elif test -f "$g/../.dotest/applying"
+			elif test -f "$g/rebase/applying"
 			then
 				r="|AM"
 			else
 				r="|AM/REBASE"
 			fi
 			b="$(git symbolic-ref HEAD 2>/dev/null)"
-		elif [ -f "$g/.dotest-merge/interactive" ]
+		elif [ -f "$g/rebase-merge/interactive" ]
 		then
 			r="|REBASE-i"
-			b="$(cat "$g/.dotest-merge/head-name")"
-		elif [ -d "$g/.dotest-merge" ]
+			b="$(cat "$g/rebase-merge/head-name")"
+		elif [ -d "$g/rebase-merge" ]
 		then
 			r="|REBASE-m"
-			b="$(cat "$g/.dotest-merge/head-name")"
+			b="$(cat "$g/rebase-merge/head-name")"
 		elif [ -f "$g/MERGE_HEAD" ]
 		then
 			r="|MERGING"
@@ -455,8 +455,8 @@ __git_whitespacelist="nowarn warn error error-all strip"
 
 _git_am ()
 {
-	local cur="${COMP_WORDS[COMP_CWORD]}"
-	if [ -d .dotest ]; then
+	local cur="${COMP_WORDS[COMP_CWORD]}" dir="$(__gitdir)"
+	if [ -d "$dir"/rebase ]; then
 		__gitcomp "--skip --resolved"
 		return
 	fi
@@ -864,7 +864,7 @@ _git_push ()
 _git_rebase ()
 {
 	local cur="${COMP_WORDS[COMP_CWORD]}" dir="$(__gitdir)"
-	if [ -d .dotest ] || [ -d "$dir"/.dotest-merge ]; then
+	if [ -d "$dir"/rebase ] || [ -d "$dir"/rebase-merge ]; then
 		__gitcomp "--continue --skip --abort"
 		return
 	fi
diff --git a/git-quiltimport.sh b/git-quiltimport.sh
index 7cd8f71..d1efa1d 100755
--- a/git-quiltimport.sh
+++ b/git-quiltimport.sh
@@ -53,7 +53,7 @@ if ! [ -d "$QUILT_PATCHES" ] ; then
 fi
 
 # Temporary directories
-tmp_dir=.dotest
+tmp_dir="$GIT_DIR"/rebase
 tmp_msg="$tmp_dir/msg"
 tmp_patch="$tmp_dir/patch"
 tmp_info="$tmp_dir/info"
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 297b646..dd01a45 100755
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -17,7 +17,7 @@ OPTIONS_SPEC=
 . git-sh-setup
 require_work_tree
 
-DOTEST="$GIT_DIR/.dotest-merge"
+DOTEST="$GIT_DIR/rebase-merge"
 TODO="$DOTEST"/git-rebase-todo
 DONE="$DOTEST"/done
 MSG="$DOTEST"/message
diff --git a/git-rebase.sh b/git-rebase.sh
index bf0e7a9..74ff873 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -14,7 +14,7 @@ It is possible that a merge failure will prevent this process from being
 completely automatic.  You will have to resolve any such merge failure
 and run git rebase --continue.  Another option is to bypass the commit
 that caused the merge failure with git rebase --skip.  To restore the
-original <branch> and remove the .dotest working files, use the command
+original <branch> and remove the .git/rebase working files, use the command
 git rebase --abort instead.
 
 Note that if <branch> is not specified on the command line, the
@@ -42,7 +42,7 @@ To restore the original branch and stop rebasing run \"git rebase --abort\".
 unset newbase
 strategy=recursive
 do_merge=
-dotest=$GIT_DIR/.dotest-merge
+dotest="$GIT_DIR"/rebase-merge
 prec=4
 verbose=
 git_am_opt=
@@ -150,7 +150,7 @@ while test $# != 0
 do
 	case "$1" in
 	--continue)
-		test -d "$dotest" -o -d .dotest ||
+		test -d "$dotest" -o -d "$GIT_DIR"/rebase ||
 			die "No rebase in progress?"
 
 		git diff-files --quiet --ignore-submodules || {
@@ -173,15 +173,15 @@ do
 			finish_rb_merge
 			exit
 		fi
-		head_name=$(cat .dotest/head-name) &&
-		onto=$(cat .dotest/onto) &&
-		orig_head=$(cat .dotest/orig-head) &&
+		head_name=$(cat "$GIT_DIR"/rebase/head-name) &&
+		onto=$(cat "$GIT_DIR"/rebase/onto) &&
+		orig_head=$(cat "$GIT_DIR"/rebase/orig-head) &&
 		git am --resolved --3way --resolvemsg="$RESOLVEMSG" &&
 		move_to_original_branch
 		exit
 		;;
 	--skip)
-		test -d "$dotest" -o -d .dotest ||
+		test -d "$dotest" -o -d "$GIT_DIR"/rebase ||
 			die "No rebase in progress?"
 
 		git reset --hard HEAD || exit $?
@@ -200,19 +200,19 @@ do
 			done
 			finish_rb_merge
 			exit
-		elif ! test -d .dotest
+		elif ! test -d "$GIT_DIR"/rebase
 		then
 			die "No rebase in progress?"
 		fi
-		head_name=$(cat .dotest/head-name) &&
-		onto=$(cat .dotest/onto) &&
-		orig_head=$(cat .dotest/orig-head) &&
+		head_name=$(cat "$GIT_DIR"/rebase/head-name) &&
+		onto=$(cat "$GIT_DIR"/rebase/onto) &&
+		orig_head=$(cat "$GIT_DIR"/rebase/orig-head) &&
 		git am -3 --skip --resolvemsg="$RESOLVEMSG" &&
 		move_to_original_branch
 		exit
 		;;
 	--abort)
-		test -d "$dotest" -o -d .dotest ||
+		test -d "$dotest" -o -d "$GIT_DIR"/rebase ||
 			die "No rebase in progress?"
 
 		git rerere clear
@@ -220,7 +220,7 @@ do
 		then
 			move_to_original_branch
 		else
-			dotest=.dotest
+			dotest="$GIT_DIR"/rebase
 			move_to_original_branch
 		fi
 		git reset --hard $(cat "$dotest/orig-head")
@@ -268,24 +268,24 @@ do
 	shift
 done
 
-# Make sure we do not have .dotest
+# Make sure we do not have $GIT_DIR/rebase
 if test -z "$do_merge"
 then
-	if mkdir .dotest
+	if mkdir "$GIT_DIR"/rebase
 	then
-		rmdir .dotest
+		rmdir "$GIT_DIR"/rebase
 	else
 		echo >&2 '
-It seems that I cannot create a .dotest directory, and I wonder if you
+It seems that I cannot create a .git/rebase directory, and I wonder if you
 are in the middle of patch application or another rebase.  If that is not
-the case, please rm -fr .dotest and run me again.  I am stopping in case
+the case, please rm -fr .git/rebase and run me again.  I am stopping in case
 you still have something valuable there.'
 		exit 1
 	fi
 else
 	if test -d "$dotest"
 	then
-		die "previous dotest directory $dotest still exists." \
+		die "previous rebase directory $dotest still exists." \
 			'try git-rebase < --continue | --abort >'
 	fi
 fi
@@ -399,10 +399,10 @@ then
 	git am $git_am_opt --rebasing --resolvemsg="$RESOLVEMSG" &&
 	move_to_original_branch
 	ret=$?
-	test 0 != $ret -a -d .dotest &&
-		echo $head_name > .dotest/head-name &&
-		echo $onto > .dotest/onto &&
-		echo $orig_head > .dotest/orig-head
+	test 0 != $ret -a -d "$GIT_DIR"/rebase &&
+		echo $head_name > "$GIT_DIR"/rebase/head-name &&
+		echo $onto > "$GIT_DIR"/rebase/onto &&
+		echo $orig_head > "$GIT_DIR"/rebase/orig-head
 	exit $ret
 fi
 
diff --git a/t/t3401-rebase-partial.sh b/t/t3401-rebase-partial.sh
index 4934a4e..36d9a2a 100755
--- a/t/t3401-rebase-partial.sh
+++ b/t/t3401-rebase-partial.sh
@@ -50,12 +50,12 @@ test_debug \
 
 test_expect_success \
     'rebase topic branch against new master and check git-am did not get halted' \
-    'git-rebase master && test ! -d .dotest'
+    'git-rebase master && test ! -d .git/rebase'
 
 test_expect_success \
 	'rebase --merge topic branch that was partially merged upstream' \
 	'git-checkout -f my-topic-branch-merge &&
 	 git-rebase --merge master-merge &&
-	 test ! -d .git/.dotest-merge'
+	 test ! -d .git/rebase-merge'
 
 test_done
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 1c80148..d27554e 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -159,19 +159,19 @@ test_expect_success 'stop on conflicting pick' '
 	git tag new-branch1 &&
 	test_must_fail git rebase -i master &&
 	test "$(git rev-parse HEAD~3)" = "$(git rev-parse master)" &&
-	test_cmp expect .git/.dotest-merge/patch &&
+	test_cmp expect .git/rebase-merge/patch &&
 	test_cmp expect2 file1 &&
 	test "$(git-diff --name-status |
 		sed -n -e "/^U/s/^U[^a-z]*//p")" = file1 &&
-	test 4 = $(grep -v "^#" < .git/.dotest-merge/done | wc -l) &&
-	test 0 = $(grep -c "^[^#]" < .git/.dotest-merge/git-rebase-todo)
+	test 4 = $(grep -v "^#" < .git/rebase-merge/done | wc -l) &&
+	test 0 = $(grep -c "^[^#]" < .git/rebase-merge/git-rebase-todo)
 '
 
 test_expect_success 'abort' '
 	git rebase --abort &&
 	test $(git rev-parse new-branch1) = $(git rev-parse HEAD) &&
 	test "$(git symbolic-ref -q HEAD)" = "refs/heads/branch1" &&
-	! test -d .git/.dotest-merge
+	! test -d .git/rebase-merge
 '
 
 test_expect_success 'retain authorship' '
diff --git a/t/t3407-rebase-abort.sh b/t/t3407-rebase-abort.sh
index 1777ffe..12c8804 100755
--- a/t/t3407-rebase-abort.sh
+++ b/t/t3407-rebase-abort.sh
@@ -74,7 +74,7 @@ testrebase() {
 	'
 }
 
-testrebase "" .dotest
-testrebase " --merge" .git/.dotest-merge
+testrebase "" .git/rebase
+testrebase " --merge" .git/rebase-merge
 
 test_done
diff --git a/t/t4150-am.sh b/t/t4150-am.sh
index bc98260..5cbd5ef 100755
--- a/t/t4150-am.sh
+++ b/t/t4150-am.sh
@@ -102,7 +102,7 @@ test_expect_success 'am applies patch correctly' '
 	git checkout first &&
 	test_tick &&
 	git am <patch1 &&
-	! test -d .dotest &&
+	! test -d .git/rebase &&
 	test -z "$(git diff second)" &&
 	test "$(git rev-parse second)" = "$(git rev-parse HEAD)" &&
 	test "$(git rev-parse second^)" = "$(git rev-parse HEAD^)"
@@ -123,7 +123,7 @@ test_expect_success 'am changes committer and keeps author' '
 	test_tick &&
 	git checkout first &&
 	git am patch2 &&
-	! test -d .dotest &&
+	! test -d .git/rebase &&
 	test "$(git rev-parse master^^)" = "$(git rev-parse HEAD^^)" &&
 	test -z "$(git diff master..HEAD)" &&
 	test -z "$(git diff master^..HEAD^)" &&
@@ -163,7 +163,7 @@ test_expect_success 'am without --keep removes Re: and [PATCH] stuff' '
 test_expect_success 'am --keep really keeps the subject' '
 	git checkout HEAD^ &&
 	git am --keep patch4 &&
-	! test -d .dotest &&
+	! test -d .git/rebase &&
 	git-cat-file commit HEAD |
 		grep -q -F "Re: Re: Re: [PATCH 1/5 v2] third"
 '
@@ -176,19 +176,19 @@ test_expect_success 'am -3 falls back to 3-way merge' '
 	test_tick &&
 	git commit -m "copied stuff" &&
 	git am -3 lorem-move.patch &&
-	! test -d .dotest &&
+	! test -d .git/rebase &&
 	test -z "$(git diff lorem)"
 '
 
 test_expect_success 'am pauses on conflict' '
 	git checkout lorem2^^ &&
 	! git am lorem-move.patch &&
-	test -d .dotest
+	test -d .git/rebase
 '
 
 test_expect_success 'am --skip works' '
 	git am --skip &&
-	! test -d .dotest &&
+	! test -d .git/rebase &&
 	test -z "$(git diff lorem2^^ -- file)" &&
 	test goodbye = "$(cat another)"
 '
@@ -196,31 +196,31 @@ test_expect_success 'am --skip works' '
 test_expect_success 'am --resolved works' '
 	git checkout lorem2^^ &&
 	! git am lorem-move.patch &&
-	test -d .dotest &&
+	test -d .git/rebase &&
 	echo resolved >>file &&
 	git add file &&
 	git am --resolved &&
-	! test -d .dotest &&
+	! test -d .git/rebase &&
 	test goodbye = "$(cat another)"
 '
 
 test_expect_success 'am takes patches from a Pine mailbox' '
 	git checkout first &&
 	cat pine patch1 | git am &&
-	! test -d .dotest &&
+	! test -d .git/rebase &&
 	test -z "$(git diff master^..HEAD)"
 '
 
 test_expect_success 'am fails on mail without patch' '
 	! git am <failmail &&
-	rm -r .dotest/
+	rm -r .git/rebase/
 '
 
 test_expect_success 'am fails on empty patch' '
 	echo "---" >>failmail &&
 	! git am <failmail &&
 	git am --skip &&
-	! test -d .dotest
+	! test -d .git/rebase
 '
 
 test_expect_success 'am works from stdin in subdirectory' '
diff --git a/t/t9106-git-svn-commit-diff-clobber.sh b/t/t9106-git-svn-commit-diff-clobber.sh
index 58a3a7b..27a65e0 100755
--- a/t/t9106-git-svn-commit-diff-clobber.sh
+++ b/t/t9106-git-svn-commit-diff-clobber.sh
@@ -87,7 +87,7 @@ test_expect_success 'multiple dcommit from git-svn will not clobber svn' "
 	"
 
 
-test_expect_success 'check that rebase really failed' 'test -d .dotest'
+test_expect_success 'check that rebase really failed' 'test -d .git/rebase'
 
 test_expect_success 'resolve, continue the rebase and dcommit' "
 	echo clobber and I really mean it > file &&
-- 
1.5.6.2.449.g342381.dirty

^ permalink raw reply related

* Useful tip about !aliases
From: Kevin Ballard @ 2008-07-16  0:46 UTC (permalink / raw)
  To: Git Mailing List

Here's something I discovered recently about !aliases that other folks  
might find useful. The canonical form for a shell alias is something  
like

   git config alias.foo '!echo bar'

where any args given to foo, as in `git foo blah` are passed along to  
the shell, so in this case `echo bar blah` would be invoked.

Something that I find very useful is the ability to interpolate  
arguments into the middle of a command. This doesn't seem possible at  
first glance, not without a helper script. But it certainly is  
possible, with the help of shell functions:

   git config alias.reverse '!foo () { args=''; while [[ -n "$*" ]];  
do args="$1 $args"; shift; done; echo $args; }; foo'

Now if you invoke `git foo one two three` you'll get the response  
"three two one".

Here's another example. This one I particularly like. I call it 'send- 
patches', because what it does is it takes a single hash and creates  
patches out of all commits since that hash, invokes send-mail on them,  
and deletes them. It's a rather quick way of sending off patches. And  
if you pass any extra arguments, they're given to git-send-email. The  
most useful part is it adjusts the patch prefix to contain the name of  
the repository itself, so your recipient knows exactly what your patch  
is for.

   git config --global alias.send-patches '!foo () { rev="$1"; shift;  
git send-email $(git format-patch -o .mbox --no-prefix --subject- 
prefix="$(printf "PATCH: %s" $(basename $(cd "$(git rev-parse --show- 
cdup)" && pwd)))" $rev) "$@"; rm -rf .mbox; }; foo'

-Kevin Ballard

-- 
Kevin Ballard
http://kevin.sb.org
kevin@sb.org
http://www.tildesoft.com

^ permalink raw reply

* Re: [PATCH 2/2] git-gui: Allow "Stage Line" to stage adjacent changes independently
From: Shawn O. Pearce @ 2008-07-16  0:35 UTC (permalink / raw)
  Cc: Johannes Sixt, Junio C Hamano, git
In-Reply-To: <7vy742zul7.fsf@gitster.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> wrote:
> Johannes Sixt <johannes.sixt@telecom.at> writes:
> 
> > Consider this hunk:
> >
> >   @@ -10,4 +10,4 @@
> >    context before
> >   -old 1
> >   -old 2
> >   +new 1
> >   +new 2
> >    context after
> >
> > [Nomenclature: to "stage change 2" means to stage lines "-old 1" and
> > "+new 1", in any order; likewise for "unstage" and "change 2".]
> 
> You lost me.
...
> and try explaining what you are doing again, pretty please?

Me too.  I really appreciate the effort spent to improve this
feature, but I really didn't follow the commit message at all.  :-|

-- 
Shawn.

^ permalink raw reply

* Re: [PATCH] bash completion: Fix the . -> .. revision range completion
From: Shawn O. Pearce @ 2008-07-15 23:38 UTC (permalink / raw)
  To: Andreas Ericsson; +Cc: Linus Torvalds, Junio C Hamano, Petr Baudis, git
In-Reply-To: <487C5A2D.3000707@op5.se>

Andreas Ericsson <ae@op5.se> wrote:
> I beat you to it ;-) This works just fine for me regardless of whether
> or not I have a colon in COMP_WORDBREAKS.
...
> Subject: git-completion.bash: Handle "rev:path" completion properly
...
> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index d268e6f..e138022 100755
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -293,7 +293,11 @@ __git_complete_file ()
> 		*)
> 			ls="$ref"
> 			;;
> -	    esac
> +		esac
> +		# When completing something like 'rev:path', bash behaves
> +		# differently whether or not COMP_WORDBREAKS contains a
> +		# colon or not. This lets it handle both cases
> +		test "${COMP_WORDBREAKS//:}" = "$COMP_WORDBREAKS" && pfx="$ref:$pfx"
> 		COMPREPLY=($(compgen -P "$pfx" \
> 			-W "$(git --git-dir="$(__gitdir)" ls-tree "$ls" \
> 				| sed '/^100... blob /s,^.*	,,

Yea, I did more or less the same thing in my patch, but I also
handled this fix in git-fetch and git-push.  The : is also used
there in a refspec and we support completion the right side of the
: in both cases (and yes, on git-push that can be slow as we do
network IO, possibly over SSH).

So I'm in favor of my patch over yours, but only because of
the fetch and push fixes as well.

-- 
Shawn.

^ permalink raw reply

* Re: Closing the merge window for 1.6.0
From: Johannes Schindelin @ 2008-07-15 22:45 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Dmitry Potapov, Petr Baudis, Nicolas Pitre, Gerrit Pape, git
In-Reply-To: <7vd4lezske.fsf@gitster.siamese.dyndns.org>

Hi,

On Tue, 15 Jul 2008, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > Having said that, I do not have the resources to test and fix 
> > everything that may arise from Debian being seemingly unable to update 
> > to Git 1.5.
> 
> Heh, what happent to your earlier "a few minutes for Junio to change and 
> commit"?

That was meant for the integration of the patch that makes the 
backwards-incompatible patch.

Not for the necessary forward-compatible changes.

Ciao,
Dscho

^ permalink raw reply

* Re: Closing the merge window for 1.6.0
From: Junio C Hamano @ 2008-07-15 22:33 UTC (permalink / raw)
  To: Johannes Schindelin
  Cc: Dmitry Potapov, Petr Baudis, Nicolas Pitre, Gerrit Pape, git
In-Reply-To: <alpine.DEB.1.00.0807160005540.2990@eeepc-johanness>

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> On Tue, 15 Jul 2008, Junio C Hamano wrote:
>
>> What troubles me the most is that you seem to be forgetting that we are 
>> using git to manage our codebase.
>
> I don't.  I have vivid memories of updating an ancient git repository of 
> Git itself, which had some almost forgotten changes in it.  That was in 
> the bad old days, when the version number did not even have a "1" in it.
>
> It could not even fetch the current git.git.
>
> I do _not_ want that to happen to anybody else, _even if_ we leave 1.4.4.4 
> Behind as if it was an American Child.

My reference to "git" was about "forking is easy".  We seem to have to
agree that talking is even cheaper, though ;-)

> Having said that, I do not have the resources to test and fix everything 
> that may arise from Debian being seemingly unable to update to Git 1.5.  

Heh, what happent to your earlier "a few minutes for Junio to change and
commit"?

^ permalink raw reply

* Re: [RFC/PATCH 0/2] Enhance performance of blame -C -C
From: Junio C Hamano @ 2008-07-15 22:25 UTC (permalink / raw)
  To: Alexander Gavrilov; +Cc: git
In-Reply-To: <200807160158.34994.angavrilov@gmail.com>

Alexander Gavrilov <angavrilov@gmail.com> writes:

> This pair of patches aims at increasing performance of copy detection in
> blame by avoiding unnecessary comparisons. Note that since I'm new to
> this code, I might have misunderstood something.
>
> There are two cases than I aim to fix:
>
> 1) Copy detection is done by comparing all outstanding chunks of the
> target file to all blobs in the parent. After that, chunks with suitable
> matches are split, and comparison is repeated again, until there are no
> new matches. The trouble is, chunks that didn't match the first time,
> and weren't split, are compared against the same set of blobs again and
> again. I add a flag to track that.
>
>   On my test case it decreased blame -C -C time from over 10min to
>   ~6min; 4min with -C80.
>
> 2) Chunks are split only if the match scores above a certain
> threshold. I understand that a split of an entry cannot score more than
> the entry itself. Thus, it is pointless to even try doing costly
> comparisons for small entries.
>
>   (Time goes down to 4min; 2min with -C80)

Ideas for both patches sound very sane.  Will take a deeper look later.

Thanks.

^ permalink raw reply

* Re: [PATCH] cherry: cache patch-ids to avoid repeating work
From: Junio C Hamano @ 2008-07-15 22:14 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Geoffrey Irving, git@vger.kernel.org
In-Reply-To: <alpine.DEB.1.00.0807152255020.2990@eeepc-johanness>

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> Okay, it seems like I never have time to review this, so I'll just 
> take a few minutes to comment on some aspects:
>
>> @@ -1094,6 +1104,8 @@ int cmd_cherry(int argc, const char **argv,
>> const char *prefix)
>>  	const char *limit = NULL;
>>  	int verbose = 0;
>> 
>> +	git_config(git_cherry_config, NULL);
>> +
>>  	if (argc > 1 && !strcmp(argv[1], "-v")) {
>>  		verbose = 1;
>>  		argc--;
>
> Is this really purely for cherry, and not at all for "log --cherry-pick"?  
> Maybe it should be "cache.patchIds" to begin with.

What other things would we want caches for?

As a general rule, I'd prefer keeping these unproven new features opt
in (i.e. default to false unless explicitly asked for).

>> +union cached_sha1_map_header {
>> +	struct {
>> +		char signature[4]; /* CS1M */
>> +		uint32_t version;
>> +		uint32_t count;
>> +		uint32_t size;
>> +		uint32_t pad; /* pad to 20 bytes */
>> +	} u;
>> +	/* pad header out to 40 bytes.  As a consistency
>> +	 * check, pad.value stores the sha1 of pad.key. */
>> +	struct cached_sha1_entry pad;
>
> Why does it have to be a union?

Hmm.  I think you are right.

	struct cached_sha1_map_header {
        	char signature[4];
                uint32_t version;
                uint32_t count;
                uint32_t size;
                uint32_t unused;
		unsigned char csum[20];
	};

would equally be good, as long as we assume the struct is naturally
packed.  I do agree with you that it may not worth checking only the
header, though. 

>> +static const char *signature = "CS1M";
>
> Carrie Scr*ws 1 Man?

No Idea ;-)

>> +	cache->mmapped = 0;
>> +	cache->dirty = 1;
>
> Is it already dirty?  I don't think so.

This flag is more about "do we need to write it back to file", and when it
starts out without reading from an existing file, we always need to as
long as the table contains something at the end of the processing.

You could instead check (!cache->mmapped && cache->count) for that, I
guess.

>> +	cache->entries = calloc(size, sizeof(struct cached_sha1_entry));
>> +	if (!cache->entries) {
>> +		warning("failed to allocate empty map of size %"PRIu32" for %s",
>> +			size, git_path(cache->filename));
>
> xcalloc() to the rescue.

This is purely optional cache and we would want to degrade to operate
without it if any of these fails.  xcalloc() won't let you do so.

> Really, I think that these checks should be _made_ unnecessary, by 
> restricting the size of the cache.  IMO Caching more than 2^10 patch ids 
> (completely made up on the spot) is probably even detrimental, and it 
> might be better to just scratch them all and start with a new cache then.

Probably.  Or fall back on uncached operation.

>> +static int init_cached_sha1_map(struct cached_sha1_map *cache)
>> +{
>>
>> [...]
>>
>> +	SHA1_Init(&ctx);
>> +	SHA1_Update(&ctx, header.pad.key, 20);
>> +	SHA1_Final(header.pad.key, &ctx); /* reuse pad.key to store its sha1 */
>> +	if (hashcmp(header.pad.key, header.pad.value)) {
>> +		warning("%s header has invalid sha1", filename);
>> +		goto empty;
>> +	}
>
> I do not think that it is worth checking that.  If you do not trust your 
> hard disk, you might just as well jump out the window.
>
> Checking just takes too much time.

This is only checking the header, so it won't take much time, but I tend
to doubt the value of this.

>> +	/* mmap entire file so that file / memory blocks are aligned */
>> +	map_size = sizeof(struct cached_sha1_entry) * (cache->size + 1);
>> +	cache->entries = mmap(NULL, map_size,
>> +		PROT_READ | PROT_WRITE, MAP_PRIVATE, fd, 0);
>
> AFAIR there were _serious_ performance issues with mmap() on non-Linux 
> platforms.  I chose pread() in my original implementation for a reason.

That is not a reason to punish users on platforms with working mmap(2) ;-).

^ permalink raw reply

* Re: Closing the merge window for 1.6.0
From: Johannes Schindelin @ 2008-07-15 22:10 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Dmitry Potapov, Petr Baudis, Nicolas Pitre, Gerrit Pape, git
In-Reply-To: <7v3amb0ymg.fsf@gitster.siamese.dyndns.org>

Hi,

On Tue, 15 Jul 2008, Junio C Hamano wrote:

> What troubles me the most is that you seem to be forgetting that we are 
> using git to manage our codebase.

I don't.  I have vivid memories of updating an ancient git repository of 
Git itself, which had some almost forgotten changes in it.  That was in 
the bad old days, when the version number did not even have a "1" in it.

It could not even fetch the current git.git.

I do _not_ want that to happen to anybody else, _even if_ we leave 1.4.4.4 
Behind as if it was an American Child.

Having said that, I do not have the resources to test and fix everything 
that may arise from Debian being seemingly unable to update to Git 1.5.  
So I agree completely that the ball is in Debian's half, and if they let 
it rot, it is sad, but I cannot help it.

Ciao,
Dscho

^ permalink raw reply

* [RFC/PATCH 1/2 (No Wrap)] Avoid rescanning unchanged entries in search for copies.
From: Alexander Gavrilov @ 2008-07-15 22:05 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano
In-Reply-To: <200807160159.56228.angavrilov@gmail.com>

Repeatedly comparing the same entry against the same set
of blobs in search for copies is quite pointless. This
huge waste of effort can be avoided using a flag in
the blame_entry structure.

Signed-off-by: Alexander Gavrilov <angavrilov@gmail.com>
---

	I'm terribly sorry, word wrap was turned on. I disabled it permanently.

	-- Alexander

 builtin-blame.c |   21 +++++++++++++++++++--
 1 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/builtin-blame.c b/builtin-blame.c
index b451f6c..a6cf6b6 100644
--- a/builtin-blame.c
+++ b/builtin-blame.c
@@ -161,6 +161,10 @@ struct blame_entry {
 	 */
 	char guilty;
 
+	/* true if the entry has been scanned for copies in the current parent
+	 */
+	char scanned;
+
 	/* the line number of the first line of this group in the
 	 * suspect's file; internally all line numbers are 0 based.
 	 */
@@ -1048,12 +1052,12 @@ static struct blame_list *setup_blame_list(struct scoreboard *sb,
 	struct blame_list *blame_list = NULL;
 
 	for (e = sb->ent, num_ents = 0; e; e = e->next)
-		if (!e->guilty && same_suspect(e->suspect, target))
+		if (!e->scanned && !e->guilty && same_suspect(e->suspect, target))
 			num_ents++;
 	if (num_ents) {
 		blame_list = xcalloc(num_ents, sizeof(struct blame_list));
 		for (e = sb->ent, i = 0; e; e = e->next)
-			if (!e->guilty && same_suspect(e->suspect, target))
+			if (!e->scanned && !e->guilty && same_suspect(e->suspect, target))
 				blame_list[i++].ent = e;
 	}
 	*num_ents_p = num_ents;
@@ -1061,6 +1065,16 @@ static struct blame_list *setup_blame_list(struct scoreboard *sb,
 }
 
 /*
+ * Reset the scanned status on all entries.
+ */
+static void reset_scanned_flag(struct scoreboard *sb)
+{
+	struct blame_entry *e;
+	for (e = sb->ent; e; e = e->next)
+		e->scanned = 0;
+}
+
+/*
  * For lines target is suspected for, see if we can find code movement
  * across file boundary from the parent commit.  porigin is the path
  * in the parent we already tried.
@@ -1152,6 +1166,8 @@ static int find_copy_in_parent(struct scoreboard *sb,
 				split_blame(sb, split, blame_list[j].ent);
 				made_progress = 1;
 			}
+			else
+				blame_list[j].ent->scanned = 1;
 			decref_split(split);
 		}
 		free(blame_list);
@@ -1164,6 +1180,7 @@ static int find_copy_in_parent(struct scoreboard *sb,
 			break;
 		}
 	}
+	reset_scanned_flag(sb);
 	diff_flush(&diff_opts);
 	diff_tree_release_paths(&diff_opts);
 	return retval;
-- 
1.5.6.2.39.gd084

^ permalink raw reply related

* [RFC/PATCH 2/2] Do not try to detect move/copy for entries below threshold.
From: Alexander Gavrilov @ 2008-07-15 22:00 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano
In-Reply-To: <200807160159.56228.angavrilov@gmail.com>

Splits for such entries are rejected anyway, so there is no
point even trying to compute them.

Signed-off-by: Alexander Gavrilov <angavrilov@gmail.com>
---
 builtin-blame.c |   16 +++++++++++-----
 1 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/builtin-blame.c b/builtin-blame.c
index a6cf6b6..01453e2 100644
--- a/builtin-blame.c
+++ b/builtin-blame.c
@@ -1020,7 +1020,8 @@ static int find_move_in_parent(struct scoreboard *sb,
 	while (made_progress) {
 		made_progress = 0;
 		for (e = sb->ent; e; e = e->next) {
-			if (e->guilty || !same_suspect(e->suspect, target))
+			if (e->guilty || !same_suspect(e->suspect, target) ||
+			    blame_move_score >= ent_score(sb, e))
 				continue;
 			find_copy_in_blob(sb, e, parent, split, &file_p);
 			if (split[1].suspect &&
@@ -1045,6 +1046,7 @@ struct blame_list {
  */
 static struct blame_list *setup_blame_list(struct scoreboard *sb,
 					   struct origin *target,
+					   int min_score,
 					   int *num_ents_p)
 {
 	struct blame_entry *e;
@@ -1052,12 +1054,16 @@ static struct blame_list *setup_blame_list(struct scoreboard *sb,
 	struct blame_list *blame_list = NULL;
 
 	for (e = sb->ent, num_ents = 0; e; e = e->next)
-		if (!e->scanned && !e->guilty && same_suspect(e->suspect, target))
+		if (!e->scanned && !e->guilty &&
+		    same_suspect(e->suspect, target) &&
+		    ent_score(sb, e) > min_score)
 			num_ents++;
 	if (num_ents) {
 		blame_list = xcalloc(num_ents, sizeof(struct blame_list));
 		for (e = sb->ent, i = 0; e; e = e->next)
-			if (!e->scanned && !e->guilty && same_suspect(e->suspect, target))
+			if (!e->scanned && !e->guilty &&
+			    same_suspect(e->suspect, target) &&
+			    ent_score(sb, e) > min_score)
 				blame_list[i++].ent = e;
 	}
 	*num_ents_p = num_ents;
@@ -1092,7 +1098,7 @@ static int find_copy_in_parent(struct scoreboard *sb,
 	struct blame_list *blame_list;
 	int num_ents;
 
-	blame_list = setup_blame_list(sb, target, &num_ents);
+	blame_list = setup_blame_list(sb, target, blame_copy_score, &num_ents);
 	if (!blame_list)
 		return 1; /* nothing remains for this target */
 
@@ -1174,7 +1180,7 @@ static int find_copy_in_parent(struct scoreboard *sb,
 
 		if (!made_progress)
 			break;
-		blame_list = setup_blame_list(sb, target, &num_ents);
+		blame_list = setup_blame_list(sb, target, blame_copy_score, &num_ents);
 		if (!blame_list) {
 			retval = 1;
 			break;
-- 
1.5.6.2.39.gd084

^ permalink raw reply related

* [RFC/PATCH 1/2] Avoid rescanning unchanged entries in search for copies.
From: Alexander Gavrilov @ 2008-07-15 21:59 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano
In-Reply-To: <200807160158.34994.angavrilov@gmail.com>

Repeatedly comparing the same entry against the same set
of blobs in search for copies is quite pointless. This
huge waste of effort can be avoided using a flag in
the blame_entry structure.

Signed-off-by: Alexander Gavrilov <angavrilov@gmail.com>
---
 builtin-blame.c |   21 +++++++++++++++++++--
 1 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/builtin-blame.c b/builtin-blame.c
index b451f6c..a6cf6b6 100644
--- a/builtin-blame.c
+++ b/builtin-blame.c
@@ -161,6 +161,10 @@ struct blame_entry {
 	 */
 	char guilty;
 
+	/* true if the entry has been scanned for copies in the current parent
+	 */
+	char scanned;
+
 	/* the line number of the first line of this group in the
 	 * suspect's file; internally all line numbers are 0 based.
 	 */
@@ -1048,12 +1052,12 @@ static struct blame_list *setup_blame_list(struct 
scoreboard *sb,
 	struct blame_list *blame_list = NULL;
 
 	for (e = sb->ent, num_ents = 0; e; e = e->next)
-		if (!e->guilty && same_suspect(e->suspect, target))
+		if (!e->scanned && !e->guilty && same_suspect(e->suspect, target))
 			num_ents++;
 	if (num_ents) {
 		blame_list = xcalloc(num_ents, sizeof(struct blame_list));
 		for (e = sb->ent, i = 0; e; e = e->next)
-			if (!e->guilty && same_suspect(e->suspect, target))
+			if (!e->scanned && !e->guilty && same_suspect(e->suspect, target))
 				blame_list[i++].ent = e;
 	}
 	*num_ents_p = num_ents;
@@ -1061,6 +1065,16 @@ static struct blame_list *setup_blame_list(struct 
scoreboard *sb,
 }
 
 /*
+ * Reset the scanned status on all entries.
+ */
+static void reset_scanned_flag(struct scoreboard *sb)
+{
+	struct blame_entry *e;
+	for (e = sb->ent; e; e = e->next)
+		e->scanned = 0;
+}
+
+/*
  * For lines target is suspected for, see if we can find code movement
  * across file boundary from the parent commit.  porigin is the path
  * in the parent we already tried.
@@ -1152,6 +1166,8 @@ static int find_copy_in_parent(struct scoreboard *sb,
 				split_blame(sb, split, blame_list[j].ent);
 				made_progress = 1;
 			}
+			else
+				blame_list[j].ent->scanned = 1;
 			decref_split(split);
 		}
 		free(blame_list);
@@ -1164,6 +1180,7 @@ static int find_copy_in_parent(struct scoreboard *sb,
 			break;
 		}
 	}
+	reset_scanned_flag(sb);
 	diff_flush(&diff_opts);
 	diff_tree_release_paths(&diff_opts);
 	return retval;
-- 
1.5.6.2.39.gd084

^ permalink raw reply related

* [RFC/PATCH 0/2] Enhance performance of blame -C -C
From: Alexander Gavrilov @ 2008-07-15 21:58 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano

This pair of patches aims at increasing performance of copy detection in blame
by avoiding unnecessary comparisons. Note that since I'm new to this code, I
might have misunderstood something.

There are two cases than I aim to fix:

1) Copy detection is done by comparing all outstanding chunks of the target file
  to all blobs in the parent. After that, chunks with suitable matches are split, and
  comparison is repeated again, until there are no new matches. The trouble is,
  chunks that didn't match the first time, and weren't split, are compared against
  the same set of blobs again and again. I add a flag to track that.

  On my test case it decreased blame -C -C time from over 10min to ~6min; 4min with -C80.

2) Chunks are split only if the match scores above a certain threshold. I understand
  that a split of an entry cannot score more than the entry itself. Thus, it is pointless
  to even try doing costly comparisons for small entries.

  (Time goes down to 4min; 2min with -C80)



Alexander Gavrilov (2):
      Avoid rescanning unchanged entries in search for copies.
      Do not try to detect move/copy for entries below threshold.


 builtin-blame.c |   33 ++++++++++++++++++++++++++++-----
 1 files changed, 28 insertions(+), 5 deletions(-)



diff --git a/builtin-blame.c b/builtin-blame.c
index b451f6c..01453e2 100644
--- a/builtin-blame.c
+++ b/builtin-blame.c
@@ -161,6 +161,10 @@ struct blame_entry {
 	 */
 	char guilty;
 
+	/* true if the entry has been scanned for copies in the current parent
+	 */
+	char scanned;
+
 	/* the line number of the first line of this group in the
 	 * suspect's file; internally all line numbers are 0 based.
 	 */
@@ -1016,7 +1020,8 @@ static int find_move_in_parent(struct scoreboard *sb,
 	while (made_progress) {
 		made_progress = 0;
 		for (e = sb->ent; e; e = e->next) {
-			if (e->guilty || !same_suspect(e->suspect, target))
+			if (e->guilty || !same_suspect(e->suspect, target) ||
+			    blame_move_score >= ent_score(sb, e))
 				continue;
 			find_copy_in_blob(sb, e, parent, split, &file_p);
 			if (split[1].suspect &&
@@ -1041,6 +1046,7 @@ struct blame_list {
  */
 static struct blame_list *setup_blame_list(struct scoreboard *sb,
 					   struct origin *target,
+					   int min_score,
 					   int *num_ents_p)
 {
 	struct blame_entry *e;
@@ -1048,12 +1054,16 @@ static struct blame_list *setup_blame_list(struct scoreboard *sb,
 	struct blame_list *blame_list = NULL;
 
 	for (e = sb->ent, num_ents = 0; e; e = e->next)
-		if (!e->guilty && same_suspect(e->suspect, target))
+		if (!e->scanned && !e->guilty &&
+		    same_suspect(e->suspect, target) &&
+		    ent_score(sb, e) > min_score)
 			num_ents++;
 	if (num_ents) {
 		blame_list = xcalloc(num_ents, sizeof(struct blame_list));
 		for (e = sb->ent, i = 0; e; e = e->next)
-			if (!e->guilty && same_suspect(e->suspect, target))
+			if (!e->scanned && !e->guilty &&
+			    same_suspect(e->suspect, target) &&
+			    ent_score(sb, e) > min_score)
 				blame_list[i++].ent = e;
 	}
 	*num_ents_p = num_ents;
@@ -1061,6 +1071,16 @@ static struct blame_list *setup_blame_list(struct scoreboard *sb,
 }
 
 /*
+ * Reset the scanned status on all entries.
+ */
+static void reset_scanned_flag(struct scoreboard *sb)
+{
+	struct blame_entry *e;
+	for (e = sb->ent; e; e = e->next)
+		e->scanned = 0;
+}
+
+/*
  * For lines target is suspected for, see if we can find code movement
  * across file boundary from the parent commit.  porigin is the path
  * in the parent we already tried.
@@ -1078,7 +1098,7 @@ static int find_copy_in_parent(struct scoreboard *sb,
 	struct blame_list *blame_list;
 	int num_ents;
 
-	blame_list = setup_blame_list(sb, target, &num_ents);
+	blame_list = setup_blame_list(sb, target, blame_copy_score, &num_ents);
 	if (!blame_list)
 		return 1; /* nothing remains for this target */
 
@@ -1152,18 +1172,21 @@ static int find_copy_in_parent(struct scoreboard *sb,
 				split_blame(sb, split, blame_list[j].ent);
 				made_progress = 1;
 			}
+			else
+				blame_list[j].ent->scanned = 1;
 			decref_split(split);
 		}
 		free(blame_list);
 
 		if (!made_progress)
 			break;
-		blame_list = setup_blame_list(sb, target, &num_ents);
+		blame_list = setup_blame_list(sb, target, blame_copy_score, &num_ents);
 		if (!blame_list) {
 			retval = 1;
 			break;
 		}
 	}
+	reset_scanned_flag(sb);
 	diff_flush(&diff_opts);
 	diff_tree_release_paths(&diff_opts);
 	return retval;

^ permalink raw reply related

* MSysGit Stability
From: Joe Fiorini @ 2008-07-15 21:58 UTC (permalink / raw)
  To: git

Hey all,

I'm going to be working with a small dev shop in the near future and
we would like to use Git for our project.  Two of us are on Macs, but
the other is on Windows.  I know the options are MSysGit or Git
through cygwin.  I'm curious which is better to use and if MSysGit is
even stable yet.  Does anyone have experience running Git on Windows?
Any experiences you can share?  Is MSysGit ready yet or should we
wait?

Thanks all!
Joe

-- 
joe fiorini
http://www.faithfulgeek.org
// freelancing & knowledge sharing

^ permalink raw reply

* Re: [PATCH] cherry: cache patch-ids to avoid repeating work
From: Johannes Schindelin @ 2008-07-15 21:52 UTC (permalink / raw)
  To: Geoffrey Irving; +Cc: Junio C Hamano, git@vger.kernel.org
In-Reply-To: <7f9d599f0807150957o78d46204x280668c763fba2bf@mail.gmail.com>

Hi,

Okay, it seems like I never have time to review this, so I'll just 
take a few minutes to comment on some aspects:

On Tue, 15 Jul 2008, Geoffrey Irving wrote:

> +static int git_cherry_config(const char *var, const char *value, void *cb)
> +{
> +	if (!strcmp(var, "cherry.cachepatchids")) {
> +		cache_patch_ids = git_config_bool(var, value);
> +		return 0;
> +	}
> +
> +	return 0;
> +}
> +
>  static const char cherry_usage[] =
>  "git-cherry [-v] <upstream> [<head>] [<limit>]";
>  int cmd_cherry(int argc, const char **argv, const char *prefix)
> @@ -1094,6 +1104,8 @@ int cmd_cherry(int argc, const char **argv,
> const char *prefix)
>  	const char *limit = NULL;
>  	int verbose = 0;
> 
> +	git_config(git_cherry_config, NULL);
> +
>  	if (argc > 1 && !strcmp(argv[1], "-v")) {
>  		verbose = 1;
>  		argc--;

Is this really purely for cherry, and not at all for "log --cherry-pick"?  
Maybe it should be "cache.patchIds" to begin with.

> diff --git a/cached-sha1-map.c b/cached-sha1-map.c
> new file mode 100644
> index 0000000..9cf7252
> --- /dev/null
> +++ b/cached-sha1-map.c
> @@ -0,0 +1,293 @@
> +#include "cached-sha1-map.h"
> +
> +union cached_sha1_map_header {
> +	struct {
> +		char signature[4]; /* CS1M */
> +		uint32_t version;
> +		uint32_t count;
> +		uint32_t size;
> +		uint32_t pad; /* pad to 20 bytes */
> +	} u;
> +	/* pad header out to 40 bytes.  As a consistency
> +	 * check, pad.value stores the sha1 of pad.key. */
> +	struct cached_sha1_entry pad;

Why does it have to be a union?

> +};
> +
> +static const char *signature = "CS1M";

Carrie Scr*ws 1 Man?

> +static int init_empty_map(struct cached_sha1_map *cache, uint32_t size)
> +{
> +	cache->count = 0;
> +	cache->size = size;

We seem to call this "alloc" (almost) everywhere else.

> +	cache->initialized = 1;

Maybe we do not need that: when size is != 0, it was initialized.

> +	cache->mmapped = 0;
> +	cache->dirty = 1;

Is it already dirty?  I don't think so.

> +	cache->entries = calloc(size, sizeof(struct cached_sha1_entry));
> +	if (!cache->entries) {
> +		warning("failed to allocate empty map of size %"PRIu32" for %s",
> +			size, git_path(cache->filename));

xcalloc() to the rescue.

> +static int grow_map(struct cached_sha1_map *cache)
> +{
> +	struct cached_sha1_map new_cache;
> +	uint32_t i;
> +
> +	if (cache->size * 2 == 0) {
> +		warning("%s overflowed, so resetting to empty",
> +			git_path(cache->filename));

IMHO we can safely ignore that case: If that is true, we have seen at 
least 2^32 objects.  However, each object takes more than 4 bytes, so that 
is a literal impossibility.

I'd rather not bother with this case.

> +	/* allocate cache with twice the size */
> +	new_cache.filename = cache->filename;
> +	if (init_empty_map(&new_cache, cache->size * 2)) {

Really, I think that these checks should be _made_ unnecessary, by 
restricting the size of the cache.  IMO Caching more than 2^10 patch ids 
(completely made up on the spot) is probably even detrimental, and it 
might be better to just scratch them all and start with a new cache then.

Besides, the file would have a substantial size by then.

> +static int init_cached_sha1_map(struct cached_sha1_map *cache)
> +{
>
> [...]
>
> +	SHA1_Init(&ctx);
> +	SHA1_Update(&ctx, header.pad.key, 20);
> +	SHA1_Final(header.pad.key, &ctx); /* reuse pad.key to store its sha1 */
> +	if (hashcmp(header.pad.key, header.pad.value)) {
> +		warning("%s header has invalid sha1", filename);
> +		goto empty;
> +	}

I do not think that it is worth checking that.  If you do not trust your 
hard disk, you might just as well jump out the window.

Checking just takes too much time.

> +	/* mmap entire file so that file / memory blocks are aligned */
> +	map_size = sizeof(struct cached_sha1_entry) * (cache->size + 1);
> +	cache->entries = mmap(NULL, map_size,
> +		PROT_READ | PROT_WRITE, MAP_PRIVATE, fd, 0);

AFAIR there were _serious_ performance issues with mmap() on non-Linux 
platforms.  I chose pread() in my original implementation for a reason.

> +static int32_t find_helper(struct cached_sha1_map *cache,
> +	const unsigned char *key)
> +{
> +	int32_t i, mask, full;
> +
> +	mask = cache->size - 1;
> +	i = get_hash_index(key) & mask;
> +	full = (i-1) & mask;
> +
> +	for (; ; i = (i+1) & mask) {

Wow, that is ugly.

> +struct cached_sha1_map {
> +	const char *filename; /* relative to GIT_DIR */

Why does the map need to know its name?  The index does not.

> +static struct diff_options default_options;
> +#define IGNORED_DIFF_OPTS (DIFF_OPT_HAS_CHANGES | DIFF_OPT_CHECK_FAILED)
> 
>  static int commit_patch_id(struct commit *commit, struct diff_options *options,
>  		    unsigned char *sha1)
>  {
> +	int use_cache = 0;
> +	int ret;
> +
> +	/* only cache if diff options are defaults */
> +	if (cache_patch_ids) {
> +		default_options.found_changes = options->found_changes;
> +		default_options.flags = (options->flags & IGNORED_DIFF_OPTS)
> +			| (default_options.flags & ~IGNORED_DIFF_OPTS);
> +		use_cache = !memcmp(options, &default_options,
> +				    sizeof(struct diff_options));
> +	}

Hmm.

I'd rather set "revs.diff" late, and unset "cache_patch_ids" if it is set.  
IOW let the rev_opt parser decide.

Unfortunately, I do not have time to look into your patch in more detail, 
even if I like the idea.

Ciao,
Dscho

^ permalink raw reply

* Re: git-mergetool + WinMerge
From: Steffen Prohaska @ 2008-07-15 21:51 UTC (permalink / raw)
  To: chongyc; +Cc: git
In-Reply-To: <59f9aa2d-75f2-493d-8f83-1a5fcb24a889@34g2000hsh.googlegroups.com>


On Jul 15, 2008, at 4:55 AM, chongyc wrote:

> I am going to merge conflict by using git-mergetool of msysgit(for
> windows) and Winmerge.
>
> what shall i do?

If you have already read the documentation of "git mergetool", see

http://www.kernel.org/pub/software/scm/git/docs/git-mergetool.html

what is your specific question?

	Steffen

^ permalink raw reply

* Re: [PATCH 2/2] git-gui: Allow "Stage Line" to stage adjacent changes independently
From: Junio C Hamano @ 2008-07-15 21:49 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: Shawn O. Pearce, git
In-Reply-To: <1216156261-9687-2-git-send-email-johannes.sixt@telecom.at>

Johannes Sixt <johannes.sixt@telecom.at> writes:

> Consider this hunk:
>
>   @@ -10,4 +10,4 @@
>    context before
>   -old 1
>   -old 2
>   +new 1
>   +new 2
>    context after
>
> [Nomenclature: to "stage change 2" means to stage lines "-old 1" and
> "+new 1", in any order; likewise for "unstage" and "change 2".]

You lost me.

Do you mean to say that you always interpret the above hunk as:

   @@ -10,4 +10,4 @@
    context before
   -old 1
   +new 1
   -old 2
   +new 2
    context after

and call "replace 'old 1' with 'new 1'" as "change 1", "replace 'old
2' with 'new 2'" as "change 2"?

If it is what you are doing, it does not make much sense to me.  "new 1"
may correspond to "old 1" and "old 2" while "new 2" may be an independent
addition.  E.g.

   @@ -10,4 +10,4 @@
    context before
   -#define add(x,y) \
   - (x) + (y)
   +#define add(x,y) ((x)+(y))
   +#define sub(x,y) ((x)-(y))
    context after

I might want to pick bugfix of add() definition without using the new
definition of sub().

Please call

	"-old 1" - change #1
        "-old 2" - change #2
        "+new 1" - change #3
        "+new 2" - change #4

and try explaining what you are doing again, pretty please?

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox