Git development
 help / color / mirror / Atom feed
* Re: git send-email prompting too much
From: Jay Soffian @ 2009-04-05  2:13 UTC (permalink / raw)
  To: Bruce Stephens; +Cc: Dan McGee, git
In-Reply-To: <80hc148hso.fsf@tiny.isode.net>

On Sat, Apr 4, 2009 at 1:12 PM, Bruce Stephens <bruce.stephens@isode.com> wrote:
> Dan McGee <dpmcgee@gmail.com> writes:
>
>> I'm guessing this is related to commit
>> 6e1825186bd052fc1f77b7c8c9a31fbb9a67d90c but I haven't bisected yet.
>> Having to hit enter 10 times ad the Message-ID prompt seemed a bit odd
>> to me. Has anyone else seen this behavior?
>
> Yes.  Apparently one uses \C-d now.

Wait. Did hitting just enter work in the past? Because I tested this
before I made this commit and at least on my machine, hitting enter
just presented the prompt again. So I retained that behavior.

I looked over the Term::ReadLine code and I don't see how just hitting
return ever worked. But if you tell me it did, I believe that and I
can restore that behavior.

Out of curiosity, what is your OS and version, perl version, and what
does does perl -e 'use Term::ReadLine; print
"$Term::ReadLine::VERSION\n"' return?

Thanks,

j.

^ permalink raw reply

* Re: [PATCH v2 1/2] add --html-path to get the location of installed HTML docs
From: Markus Heidelberg @ 2009-04-05  2:15 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Shawn O. Pearce, git
In-Reply-To: <200904050335.43453.markus.heidelberg@web.de>

This can be used in GUIs to open installed HTML documentation in the
browser.

Signed-off-by: Markus Heidelberg <markus.heidelberg@web.de>
---

I've overseen a compiler warning due to a missing prototype. I've
removed git_html_path() and call system_path(GIT_HTML_PATH) directly
now. This wrapper is not necessary, initially I just followed the way of
how --exec-path calls git_exec_path().

 Documentation/git.txt                  |    6 +++++-
 Makefile                               |    1 +
 contrib/completion/git-completion.bash |    1 +
 git.c                                  |    5 ++++-
 perl/Git.pm                            |   12 +++++++++++-
 5 files changed, 22 insertions(+), 3 deletions(-)

diff --git a/Documentation/git.txt b/Documentation/git.txt
index 7513c57..2ce5e6b 100644
--- a/Documentation/git.txt
+++ b/Documentation/git.txt
@@ -9,7 +9,7 @@ git - the stupid content tracker
 SYNOPSIS
 --------
 [verse]
-'git' [--version] [--exec-path[=GIT_EXEC_PATH]]
+'git' [--version] [--exec-path[=GIT_EXEC_PATH]] [--html-path]
     [-p|--paginate|--no-pager]
     [--bare] [--git-dir=GIT_DIR] [--work-tree=GIT_WORK_TREE]
     [--help] COMMAND [ARGS]
@@ -178,6 +178,10 @@ help ...`.
 	environment variable. If no path is given, 'git' will print
 	the current setting and then exit.
 
+--html-path::
+	Print the path to wherever your git HTML documentation is installed
+	and exit.
+
 -p::
 --paginate::
 	Pipe all output into 'less' (or if set, $PAGER).
diff --git a/Makefile b/Makefile
index 7867eac..bcf7cbb 100644
--- a/Makefile
+++ b/Makefile
@@ -1191,6 +1191,7 @@ strip: $(PROGRAMS) git$X
 
 git.o: git.c common-cmds.h GIT-CFLAGS
 	$(QUIET_CC)$(CC) -DGIT_VERSION='"$(GIT_VERSION)"' \
+		'-DGIT_HTML_PATH="$(htmldir_SQ)"' \
 		$(ALL_CFLAGS) -c $(filter %.c,$^)
 
 git$X: git.o $(BUILTIN_OBJS) $(GITLIBS)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index e72ce24..fdc5a24 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1870,6 +1870,7 @@ _git ()
 			--bare
 			--version
 			--exec-path
+			--html-path
 			--work-tree=
 			--help
 			"
diff --git a/git.c b/git.c
index c2b181e..ff72e22 100644
--- a/git.c
+++ b/git.c
@@ -5,7 +5,7 @@
 #include "run-command.h"
 
 const char git_usage_string[] =
-	"git [--version] [--exec-path[=GIT_EXEC_PATH]] [-p|--paginate|--no-pager] [--bare] [--git-dir=GIT_DIR] [--work-tree=GIT_WORK_TREE] [--help] COMMAND [ARGS]";
+	"git [--version] [--exec-path[=GIT_EXEC_PATH]] [--html-path] [-p|--paginate|--no-pager] [--bare] [--git-dir=GIT_DIR] [--work-tree=GIT_WORK_TREE] [--help] COMMAND [ARGS]";
 
 const char git_more_info_string[] =
 	"See 'git help COMMAND' for more information on a specific command.";
@@ -75,6 +75,9 @@ static int handle_options(const char*** argv, int* argc, int* envchanged)
 				puts(git_exec_path());
 				exit(0);
 			}
+		} else if (!strcmp(cmd, "--html-path")) {
+			puts(system_path(GIT_HTML_PATH));
+			exit(0);
 		} else if (!strcmp(cmd, "-p") || !strcmp(cmd, "--paginate")) {
 			use_pager = 1;
 		} else if (!strcmp(cmd, "--no-pager")) {
diff --git a/perl/Git.pm b/perl/Git.pm
index 7d7f2b1..291ff5b 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -56,7 +56,7 @@ require Exporter;
 @EXPORT_OK = qw(command command_oneline command_noisy
                 command_output_pipe command_input_pipe command_close_pipe
                 command_bidi_pipe command_close_bidi_pipe
-                version exec_path hash_object git_cmd_try
+                version exec_path html_path hash_object git_cmd_try
                 remote_refs
                 temp_acquire temp_release temp_reset temp_path);
 
@@ -492,6 +492,16 @@ C<git --exec-path>). Useful mostly only internally.
 sub exec_path { command_oneline('--exec-path') }
 
 
+=item html_path ()
+
+Return path to the Git html documentation (the same as
+C<git --html-path>). Useful mostly only internally.
+
+=cut
+
+sub html_path { command_oneline('--html-path') }
+
+
 =item repo_path ()
 
 Return path to the git repository. Must be called on a repository instance.
-- 
1.6.2.2.449.g1d936

^ permalink raw reply related

* Re: git send-email prompting too much
From: Jay Soffian @ 2009-04-05  2:49 UTC (permalink / raw)
  To: Bruce Stephens; +Cc: Dan McGee, git
In-Reply-To: <76718490904041913s2c769022t92ca194263e29eb0@mail.gmail.com>

On Sat, Apr 4, 2009 at 10:13 PM, Jay Soffian <jaysoffian@gmail.com> wrote:
> Wait. Did hitting just enter work in the past? Because I tested this
> before I made this commit and at least on my machine, hitting enter
> just presented the prompt again. So I retained that behavior.

Argh. I should read my own commit message. I claim:

    2) "Who should the emails be sent to?". Previously this prompt passed a
    second argument ("") to $term->readline() which was ignored. I believe
    the intent was to allow the user to just hit return. Now the user
    can do so, or type ctrl-d.

    3) "Message-ID to be used as In-Reply-To for the first email?".
    Previously this prompt passed a second argument (effectively undef) to
    $term->readline() which was ignored. I believe the intent was the same
    as for (2), to allow the user to just hit return. Now the user can do
    so, or type ctrl-d.

Clearly I broke something. Will send a patch shortly. :-(

j.

^ permalink raw reply

* Re: git-{diff,merge} refactor round 2
From: Markus Heidelberg @ 2009-04-05  2:58 UTC (permalink / raw)
  To: David Aguilar; +Cc: gitster, charles, git
In-Reply-To: <1238590514-41893-1-git-send-email-davvid@gmail.com>

David Aguilar, 01.04.2009:
> Here's the 2nd round of refactoring.

I just noticed that mergetool.<mergetool>.path doesn't work anymore.
git grep mergetool.*path only hits one line in git-difftool--helper.sh
Neither does it seem to work with difftool, but I'm gonna go to bed now.

Markus

^ permalink raw reply

* [PATCH] send-email: ensure quoted addresses are rfc2047 encoded
From: Jay Soffian @ 2009-04-05  3:22 UTC (permalink / raw)
  To: git; +Cc: Jay Soffian

sanitize_address assumes that quoted addresses (e.g., "first last"
<first.last@example.com) do not need rfc2047 encoding, but this is
not always the case.

For example, various places in send-email extract addresses using
parse_address_line. parse_address_line returns the addresses already
quoted (e.g., "first last" <first.last@example.com), but not rfc2047
encoded.

This patch makes sanitize_address stricter about what needs rfc2047
encoding and adds a test demonstrating where I noticed the problem.
---
 git-send-email.perl   |    3 ++-
 t/t9001-send-email.sh |   13 +++++++++++++
 2 files changed, 15 insertions(+), 1 deletions(-)

diff --git a/git-send-email.perl b/git-send-email.perl
index fc153f9..6bbdfec 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -776,12 +776,13 @@ sub sanitize_address
 	}
 
 	# if recipient_name is already quoted, do nothing
-	if ($recipient_name =~ /^(".*"|=\?utf-8\?q\?.*\?=)$/) {
+	if ($recipient_name =~ /^("[[:ascii:]]*"|=\?utf-8\?q\?.*\?=)$/) {
 		return $recipient;
 	}
 
 	# rfc2047 is needed if a non-ascii char is included
 	if ($recipient_name =~ /[^[:ascii:]]/) {
+		$recipient_name =~ s/^"(.*)"$/$1/;
 		$recipient_name = quote_rfc2047($recipient_name);
 	}
 
diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index 84238f7..192b97b 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -505,6 +505,19 @@ test_expect_success 'confirm doesnt loop forever' '
 	test $ret = "0"
 '
 
+test_expect_success 'utf8 Cc is rfc2047 encoded' '
+	clean_fake_sendmail &&
+	rm -fr outdir &&
+	git format-patch -1 -o outdir --cc="àéìöú <utf8@example.com>" &&
+	git send-email \
+	--from="Example <nobody@example.com>" \
+	--to=nobody@example.com \
+	--smtp-server="$(pwd)/fake.sendmail" \
+	outdir/*.patch &&
+	grep "^Cc:" msgtxt1 |
+	grep "=?utf-8?q?=C3=A0=C3=A9=C3=AC=C3=B6=C3=BA?= <utf8@example.com>"
+'
+
 test_expect_success '--compose adds MIME for utf8 body' '
 	clean_fake_sendmail &&
 	(echo "#!$SHELL_PATH" &&
-- 
1.6.2.1.427.g15408

^ permalink raw reply related

* Re: [PATCH] send-email: ensure quoted addresses are rfc2047 encoded
From: Jay Soffian @ 2009-04-05  3:23 UTC (permalink / raw)
  To: git
In-Reply-To: <1238901726-47026-1-git-send-email-jaysoffian@gmail.com>

On Sat, Apr 4, 2009 at 11:22 PM, Jay Soffian <jaysoffian@gmail.com> wrote:
> sanitize_address assumes that quoted addresses (e.g., "first last"
> <first.last@example.com) do not need rfc2047 encoding, but this is
> not always the case.

OOOOOOOPPPS. Ignore.

j.

^ permalink raw reply

* [PATCH] send-email: fix nasty bug in ask() function
From: Jay Soffian @ 2009-04-05  3:23 UTC (permalink / raw)
  To: git; +Cc: Jay Soffian, Bruce Stephens, Dan McGee, Junio C Hamano
In-Reply-To: <449c10960904041002s22124b74k8440af216b1de9ee@mail.gmail.com>

Commit 6e18251 (send-email: refactor and ensure prompting doesn't loop
forever) introduced an ask function, which unfortunately had a nasty
bug. This caused it not to accept anything but the default reply to the
"Who should the emails appear to be from?" prompt, and nothing but
ctrl-d to the "Who should the emails be sent to?" and "Message-ID to be
used as In-Reply-To for the first email?" prompts.

This commit corrects the issues and adds a test to confirm the fix.

Signed-off-by: Jay Soffian <jaysoffian@gmail.com>
---
On Sat, Apr 4, 2009 at 1:02 PM, Dan McGee <dpmcgee@gmail.com> wrote:
> I'm guessing this is related to commit
> 6e1825186bd052fc1f77b7c8c9a31fbb9a67d90c but I haven't bisected yet.
> Having to hit enter 10 times ad the Message-ID prompt seemed a bit odd
> to me. Has anyone else seen this behavior?
>
> dmcgee@galway ~/projects/git (working)
> $ git send-email 000*
> 0001-git-repack-use-non-dashed-update-server-info.patch
> 0002-pack-objects-report-actual-number-of-threads-to-be.patch
> Who should the emails appear to be from? [Dan McGee <dpmcgee@gmail.com>]
> Emails will be sent from: Dan McGee <dpmcgee@gmail.com>
> Message-ID to be used as In-Reply-To for the first email?
> Message-ID to be used as In-Reply-To for the first email?

I really apologize for this breakage. This patch should fix the issue.
I'm quite surprised that there wasn't already a test for the prompting,
but shame on me for not double-checking before refactoring.

I'm also super confused why the issue is occuring. You can see from the
patch below that by default the ask() function attempted to match the
user's input against the empty regex //, which should match anything:

$ perl -e 'use strict; my $resp="something"; my $re=""; print "true\n" if $resp =~ /$re/'
true
$ perl -e 'use strict; my $resp=""; my $re=""; print "true\n" if $resp =~ /$re/'
true

Any yet while my demonstration above works as I expect, for some reason
what is basically the same code (AFAICT) in send-email does not match. I
thought I knew my perl fairly well, but maybe some perl guru can see
what's going wrong.

 git-send-email.perl   |    4 ++--
 t/t9001-send-email.sh |   13 +++++++++++++
 2 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/git-send-email.perl b/git-send-email.perl
index 6bbdfec..172b53c 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -608,7 +608,7 @@ EOT
 
 sub ask {
 	my ($prompt, %arg) = @_;
-	my $valid_re = $arg{valid_re} || ""; # "" matches anything
+	my $valid_re = $arg{valid_re};
 	my $default = $arg{default};
 	my $resp;
 	my $i = 0;
@@ -624,7 +624,7 @@ sub ask {
 		if ($resp eq '' and defined $default) {
 			return $default;
 		}
-		if ($resp =~ /$valid_re/) {
+		if (!defined $valid_re or $resp =~ /$valid_re/) {
 			return $resp;
 		}
 	}
diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index 192b97b..3c90c4f 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -130,6 +130,19 @@ test_expect_success 'Show all headers' '
 	test_cmp expected-show-all-headers actual-show-all-headers
 '
 
+test_expect_success 'Prompting works' '
+	clean_fake_sendmail &&
+	(echo "Example <from@example.com>"
+	 echo "to@example.com"
+	 echo ""
+	) | GIT_SEND_EMAIL_NOTTY=1 git send-email \
+		--smtp-server="$(pwd)/fake.sendmail" \
+		$patches \
+		2>errors &&
+		grep "^From: Example <from@example.com>$" msgtxt1 &&
+		grep "^To: to@example.com$" msgtxt1
+'
+
 z8=zzzzzzzz
 z64=$z8$z8$z8$z8$z8$z8$z8$z8
 z512=$z64$z64$z64$z64$z64$z64$z64$z64
-- 
1.6.2.2.405.g6d8cc4

^ permalink raw reply related

* Re: [PATCH] send-email: fix nasty bug in ask() function
From: Jay Soffian @ 2009-04-05  3:31 UTC (permalink / raw)
  To: git; +Cc: Bruce Stephens, Dan McGee, Junio C Hamano
In-Reply-To: <1238901801-47109-1-git-send-email-jaysoffian@gmail.com>

> I'm also super confused why the issue is occuring. You can see from the
> patch below that by default the ask() function attempted to match the
> user's input against the empty regex //, which should match anything:
>
> $ perl -e 'use strict; my $resp="something"; my $re=""; print "true\n" if $resp =~ /$re/'
> true
> $ perl -e 'use strict; my $resp=""; my $re=""; print "true\n" if $resp =~ /$re/'
> true
>
> Any yet while my demonstration above works as I expect, for some reason
> what is basically the same code (AFAICT) in send-email does not match. I
> thought I knew my perl fairly well, but maybe some perl guru can see
> what's going wrong.

Ah, found it in perlreref:

   If 'pattern' is an empty string, the last successfully matched
   regex is used.

Grumble.

j.

^ permalink raw reply

* Re: git-{diff,merge} refactor round 2
From: David Aguilar @ 2009-04-05  3:34 UTC (permalink / raw)
  To: Markus Heidelberg; +Cc: gitster, charles, git
In-Reply-To: <200904050458.17708.markus.heidelberg@web.de>

On  0, Markus Heidelberg <markus.heidelberg@web.de> wrote:
> David Aguilar, 01.04.2009:
> > Here's the 2nd round of refactoring.
> 
> I just noticed that mergetool.<mergetool>.path doesn't work anymore.
> git grep mergetool.*path only hits one line in git-difftool--helper.sh
> Neither does it seem to work with difftool, but I'm gonna go to bed now.
> 
> Markus
> 

Oops.  Well, I have one final patch that removes the last bit of
redundant code.  It also fixed this problem so I'll go ahead
and send it (it's based on top of da/difftool mentioned in
pu).

Since the test cases didn't catch that breakage I added a test
for it. 

Look for a patch called:

mergetool--lib: consolidate the last redundant bits in {diff,merge}tool


-- 

	David

^ permalink raw reply

* Re: [PATCH 7/7] user-manual: simplify the user configuration
From: Jay Soffian @ 2009-04-05  3:36 UTC (permalink / raw)
  To: Felipe Contreras; +Cc: git, Junio C Hamano
In-Reply-To: <1238837909-3060-8-git-send-email-felipe.contreras@gmail.com>

On Sat, Apr 4, 2009 at 5:38 AM, Felipe Contreras
<felipe.contreras@gmail.com> wrote:
> This is shorter, avoids the burder to think about the format of the
> configuration file, and git config is already used in other places in
> the manual.
> [...]
>  ------------------------------------------------
> -[user]
> -       name = Your Name Comes Here
> -       email = you@yourdomain.example.com
> +$ git config --global user.name "Your Name Comes Here"
> +$ git config --global user.email you@yourdomain.example.com
>  ------------------------------------------------

This has come up before, and the idea was to hint to the reader that
.gitconfig is something they can edit themselves.

j.

^ permalink raw reply

* [PATCH] mergetool--lib: consolidate the last redundant bits in {diff,merge}tool
From: David Aguilar @ 2009-04-05  3:45 UTC (permalink / raw)
  To: gitster; +Cc: markus.heidelberg, charles, git, David Aguilar

This adds get_merge_tool, get_merge_tool_path, get_merge_tool_cmd
and uses them in git-mergetool and git-difftool--helper.

Signed-off-by: David Aguilar <davvid@gmail.com>
---
 Documentation/git-mergetool--lib.txt |   22 ++++---
 git-difftool--helper.sh              |   80 ++------------------------
 git-mergetool--lib.sh                |  106 +++++++++++++++++++++++++++++++--
 git-mergetool.sh                     |   65 ++------------------
 t/t7800-difftool.sh                  |    8 +++
 5 files changed, 132 insertions(+), 149 deletions(-)

diff --git a/Documentation/git-mergetool--lib.txt b/Documentation/git-mergetool--lib.txt
index ffda66b..3d57031 100644
--- a/Documentation/git-mergetool--lib.txt
+++ b/Documentation/git-mergetool--lib.txt
@@ -26,18 +26,22 @@ functions.  'diff' and 'merge' are valid values.
 
 FUNCTIONS
 ---------
-get_merge_tool_path::
-	returns the `merge_tool_path` for a `merge_tool`.
+get_merge_tool::
+	returns a merge tool
 
-run_mergetool::
-	launches a merge tool given the tool name and a true/false
-	flag to indicate whether a merge base is present
+get_merge_tool_cmd::
+	returns the custom command for a merge tool.
 
-valid_tool::
-	tests whether a merge tool is setup correctly.
+get_merge_tool_path::
+	returns the custom path for a merge tool.
 
-get_custom_cmd::
-	given a merge tool, returns the custom command for that tool.
+run_merge_tool::
+	launches a merge tool given the tool name and a true/false
+	flag to indicate whether a merge base is present.
+	'$merge_tool', '$merge_tool_path', and for custom commands,
+	'$merge_tool_cmd', must be defined prior to calling
+	run_merge_tool.  Additionally, '$MERGED', '$LOCAL', '$REMOTE',
+	and '$BASE' must be defined for use by the merge tool.
 
 Author
 ------
diff --git a/git-difftool--helper.sh b/git-difftool--helper.sh
index 602cd4f..b450036 100755
--- a/git-difftool--helper.sh
+++ b/git-difftool--helper.sh
@@ -20,8 +20,7 @@ should_prompt () {
 	fi
 }
 
-# This function prepares temporary files and launches the appropriate
-# merge tool.
+# Sets up shell variables and runs a merge tool
 launch_merge_tool () {
 	# Merged is the filename as it appears in the work tree
 	# Local is the contents of a/filename
@@ -41,85 +40,16 @@ launch_merge_tool () {
 	fi
 
 	# Run the appropriate merge tool command
-	run_mergetool "$merge_tool";
+	run_merge_tool "$merge_tool"
 }
 
 # Allow GIT_DIFF_TOOL and GIT_MERGE_TOOL to provide default values
 test -n "$GIT_MERGE_TOOL" && merge_tool="$GIT_MERGE_TOOL"
 test -n "$GIT_DIFF_TOOL" && merge_tool="$GIT_DIFF_TOOL"
 
-# If merge tool was not specified then use the diff.tool
-# configuration variable.  If that's invalid then reset merge_tool.
-# Fallback to merge.tool.
-if test -z "$merge_tool"; then
-	merge_tool=$(git config diff.tool)
-	test -z "$merge_tool" &&
-	merge_tool=$(git config merge.tool)
-	if test -n "$merge_tool" && ! valid_tool "$merge_tool"; then
-		echo >&2 "git config option diff.tool set to unknown tool: $merge_tool"
-		echo >&2 "Resetting to default..."
-		unset merge_tool
-	fi
-fi
-
-# Try to guess an appropriate merge tool if no tool has been set.
-if test -z "$merge_tool"; then
-	# We have a $DISPLAY so try some common UNIX merge tools
-	if test -n "$DISPLAY"; then
-		# If gnome then prefer meld, otherwise, prefer kdiff3 or kompare
-		if test -n "$GNOME_DESKTOP_SESSION_ID" ; then
-			merge_tool_candidates="meld kdiff3 kompare tkdiff xxdiff gvimdiff diffuse"
-		else
-			merge_tool_candidates="kdiff3 kompare tkdiff xxdiff meld gvimdiff diffuse"
-		fi
-	fi
-	if echo "${VISUAL:-$EDITOR}" | grep 'emacs' > /dev/null 2>&1; then
-		# $EDITOR is emacs so add emerge as a candidate
-		merge_tool_candidates="$merge_tool_candidates emerge opendiff vimdiff"
-	elif echo "${VISUAL:-$EDITOR}" | grep 'vim' > /dev/null 2>&1; then
-		# $EDITOR is vim so add vimdiff as a candidate
-		merge_tool_candidates="$merge_tool_candidates vimdiff opendiff emerge"
-	else
-		merge_tool_candidates="$merge_tool_candidates opendiff emerge vimdiff"
-	fi
-	echo "merge tool candidates: $merge_tool_candidates"
-
-	# Loop over each candidate and stop when a valid merge tool is found.
-	for i in $merge_tool_candidates
-	do
-		merge_tool_path="$(get_merge_tool_path "$i")"
-		if type "$merge_tool_path" > /dev/null 2>&1; then
-			merge_tool=$i
-			break
-		fi
-	done
-
-	if test -z "$merge_tool" ; then
-		echo "No known merge resolution program available."
-		exit 1
-	fi
-
-	merge_tool_cmd=$(get_custom_cmd "$merge_tool")
-else
-	# A merge tool has been set, so verify that it's valid.
-	if ! valid_tool "$merge_tool"; then
-		echo >&2 "Unknown merge tool $merge_tool"
-		exit 1
-	fi
-
-	merge_tool_cmd=$(get_custom_cmd "$merge_tool")
-	merge_tool_path=$(git config difftool."$1".path)
-	test -z "$merge_tool_path" &&
-	merge_tool_path=$(git config mergetool."$1".path)
-	merge_tool_path="$(get_merge_tool_path "$merge_tool" \
-	                                       "$merge_tool_path")"
-
-	if test -z "$merge_tool_cmd" && ! type "$merge_tool_path" > /dev/null 2>&1; then
-		echo "The merge tool $merge_tool is not available as '$merge_tool_path'"
-		exit 1
-	fi
-fi
-
+merge_tool=$(get_merge_tool "$merge_tool") || exit
+merge_tool_cmd="$(get_merge_tool_cmd "$merge_tool")"
+merge_tool_path="$(get_merge_tool_path "$merge_tool")" || exit
 
 # Launch the merge tool on each path provided by 'git diff'
 while test $# -gt 6
diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
index b8566b2..dcd4516 100644
--- a/git-mergetool--lib.sh
+++ b/git-mergetool--lib.sh
@@ -7,9 +7,10 @@ merge_mode() {
 	test $TOOL_MODE = "merge"
 }
 
-get_merge_tool_path () {
-	path="$1"
-	if test -z "$2"; then
+translate_merge_tool_path () {
+	if test -n "$2"; then
+		echo "$2"
+	else
 		case "$1" in
 		emerge)
 			path=emacs
@@ -18,8 +19,8 @@ get_merge_tool_path () {
 			path="$1"
 			;;
 		esac
+		echo "$path"
 	fi
-	echo "$path"
 }
 
 check_unchanged () {
@@ -51,14 +52,14 @@ valid_tool () {
 		fi
 		;; # happy
 	*)
-		if test -z "$(get_custom_cmd "$1")"; then
+		if test -z "$(get_merge_tool_cmd "$1")"; then
 			return 1
 		fi
 		;;
 	esac
 }
 
-get_custom_cmd () {
+get_merge_tool_cmd () {
 	diff_mode &&
 	custom_cmd="$(git config difftool.$1.cmd)"
 	test -z "$custom_cmd" &&
@@ -67,7 +68,7 @@ get_custom_cmd () {
 	echo "$custom_cmd"
 }
 
-run_mergetool () {
+run_merge_tool () {
 	base_present="$2"
 	if diff_mode; then
 		base_present="false"
@@ -235,3 +236,94 @@ run_mergetool () {
 	esac
 	return $status
 }
+
+guess_merge_tool () {
+	if diff_mode; then
+		kompare="kompare"
+	fi
+	if test -n "$DISPLAY"; then
+		if test -n "$GNOME_DESKTOP_SESSION_ID" ; then
+			tools="meld kdiff3 $kompare tkdiff"
+			tools="$tools xxdiff gvimdiff diffuse"
+		else
+			tools="kdiff3 $kompare tkdiff xxdiff"
+			tools="$tools meld gvimdiff diffuse"
+		fi
+	fi
+	if echo "${VISUAL:-$EDITOR}" | grep emacs > /dev/null 2>&1; then
+		# $EDITOR is emacs so add emerge as a candidate
+		tools="$tools emerge opendiff vimdiff"
+	elif echo "${VISUAL:-$EDITOR}" | grep vim > /dev/null 2>&1; then
+		# $EDITOR is vim so add vimdiff as a candidate
+		tools="$tools vimdiff opendiff emerge"
+	else
+		tools="$tools opendiff emerge vimdiff"
+	fi
+	echo >&2 "merge tool candidates: $tools"
+
+	# Loop over each candidate and stop when a valid merge tool is found.
+	for i in $tools
+	do
+		merge_tool_path="$(translate_merge_tool_path "$i")"
+		if type "$merge_tool_path" > /dev/null 2>&1; then
+			merge_tool="$i"
+			break
+		fi
+	done
+
+	if test -z "$merge_tool" ; then
+		echo >&2 "No known merge resolution program available."
+		return 1
+	fi
+	echo "$merge_tool"
+}
+
+get_configured_merge_tool () {
+	# Diff mode first tries diff.tool and falls back to merge.tool.
+	# Merge mode only checks merge.tool
+	if diff_mode; then
+		tool=$(git config diff.tool)
+	fi
+	if test -z "$tool"; then
+		tool=$(git config merge.tool)
+	fi
+	if test -n "$merge_tool" && ! valid_tool "$merge_tool"; then
+		echo >&2 "git config option $TOOL_MODE.tool set to unknown tool: $merge_tool"
+		echo >&2 "Resetting to default..."
+		return 1
+	fi
+	echo "$tool"
+}
+
+get_merge_tool_path () {
+	# A merge tool has been set, so verify that it's valid.
+	if ! valid_tool "$merge_tool"; then
+		echo >&2 "Unknown merge tool $merge_tool"
+		exit 1
+	fi
+	if diff_mode; then
+		merge_tool_path=$(git config difftool."$merge_tool".path)
+	fi
+	if test -z "$merge_tool_path"; then
+		merge_tool_path=$(git config mergetool."$merge_tool".path)
+	fi
+	merge_tool_path="$(translate_merge_tool_path "$merge_tool" "$merge_tool_path")"
+	if test -z "$merge_tool_cmd" && ! type "$merge_tool_path" > /dev/null 2>&1; then
+		echo >&2 "The $TOOL_MODE tool $merge_tool is not available as '$merge_tool_path'"
+		exit 1
+	fi
+	echo "$merge_tool_path"
+}
+
+get_merge_tool () {
+	merge_tool="$1"
+	# Check if a merge tool has been configured
+	if test -z "$merge_tool"; then
+		merge_tool=$(get_configured_merge_tool)
+	fi
+	# Try to guess an appropriate merge tool if no tool has been set.
+	if test -z "$merge_tool"; then
+		merge_tool=$(guess_merge_tool) || exit
+	fi
+	echo "$merge_tool"
+}
diff --git a/git-mergetool.sh b/git-mergetool.sh
index 298b0a1..efa31a2 100755
--- a/git-mergetool.sh
+++ b/git-mergetool.sh
@@ -178,7 +178,7 @@ merge_file () {
     base_present &&
     present=true
 
-    if ! run_mergetool "$merge_tool" "$present"; then
+    if ! run_merge_tool "$merge_tool" "$present"; then
 	echo "merge of $MERGED failed" 1>&2
 	mv -- "$BACKUP" "$MERGED"
 
@@ -254,63 +254,12 @@ prompt_after_failed_merge() {
     done
 }
 
-if test -z "$merge_tool"; then
-    merge_tool=$(git config merge.tool)
-    if test -n "$merge_tool" && ! valid_tool "$merge_tool"; then
-	    echo >&2 "git config option merge.tool set to unknown tool: $merge_tool"
-	    echo >&2 "Resetting to default..."
-	    unset merge_tool
-    fi
-fi
-
-if test -z "$merge_tool" ; then
-    if test -n "$DISPLAY"; then
-        if test -n "$GNOME_DESKTOP_SESSION_ID" ; then
-            merge_tool_candidates="meld kdiff3 tkdiff xxdiff gvimdiff diffuse"
-        else
-            merge_tool_candidates="kdiff3 tkdiff xxdiff meld gvimdiff diffuse"
-        fi
-    fi
-    if echo "${VISUAL:-$EDITOR}" | grep 'emacs' > /dev/null 2>&1; then
-        merge_tool_candidates="$merge_tool_candidates emerge opendiff vimdiff"
-    elif echo "${VISUAL:-$EDITOR}" | grep 'vim' > /dev/null 2>&1; then
-        merge_tool_candidates="$merge_tool_candidates vimdiff opendiff emerge"
-    else
-        merge_tool_candidates="$merge_tool_candidates opendiff emerge vimdiff"
-    fi
-    echo "merge tool candidates: $merge_tool_candidates"
-    for i in $merge_tool_candidates; do
-        merge_tool_path="$(get_merge_tool_path "$i" "$merge_tool_path")"
-        if type "$merge_tool_path" > /dev/null 2>&1; then
-            merge_tool=$i
-            break
-        fi
-    done
-    if test -z "$merge_tool" ; then
-	echo "No known merge resolution program available."
-	exit 1
-    fi
-    merge_tool_cmd="$(git config mergetool."$merge_tool".cmd)"
-else
-    if ! valid_tool "$merge_tool"; then
-        echo >&2 "Unknown merge_tool $merge_tool"
-        exit 1
-    fi
-
-    merge_tool_cmd="$(git config mergetool.$merge_tool.cmd)"
-    merge_tool_path="$(get_merge_tool_path "$merge_tool")"
-    merge_keep_backup="$(git config --bool merge.keepBackup || echo true)"
-    merge_keep_temporaries="$(git config --bool mergetool.keepTemporaries || echo false)"
-
-    if test -z "$merge_tool_cmd" && ! type "$merge_tool_path" > /dev/null 2>&1; then
-        echo "The merge tool $merge_tool is not available as '$merge_tool_path'"
-        exit 1
-    fi
-
-    if ! test -z "$merge_tool_cmd"; then
-        merge_tool_trust_exit_code="$(git config --bool mergetool.$merge_tool.trustExitCode || echo false)"
-    fi
-fi
+merge_tool=$(get_merge_tool "$merge_tool") || exit
+merge_tool_cmd="$(get_merge_tool_cmd "$merge_tool")"
+merge_tool_path="$(get_merge_tool_path "$merge_tool")" || exit
+merge_keep_backup="$(git config --bool merge.keepBackup || echo true)"
+merge_keep_temporaries="$(git config --bool mergetool.keepTemporaries || echo false)"
+merge_tool_trust_exit_code="$(git config --bool mergetool."$merge_tool".trustExitCode || echo false)"
 
 last_status=0
 rollup_status=0
diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh
index 33d07e6..cbfbe87 100755
--- a/t/t7800-difftool.sh
+++ b/t/t7800-difftool.sh
@@ -200,4 +200,12 @@ test_expect_success 'difftool + mergetool config variables' '
 	restore_test_defaults
 '
 
+test_expect_success 'difftool.<tool>.path' '
+	git config difftool.tkdiff.path echo &&
+	diff=$(git difftool -y -t tkdiff branch) &&
+	git config --unset difftool.tkdiff.path &&
+	lines=$(echo "$diff" | grep file | wc -l) &&
+	test "$lines" = 1
+'
+
 test_done
-- 
1.6.2.1.469.gdffc1

^ permalink raw reply related

* Re: Performance issue: initial git clone causes massive repack
From: Nicolas Sebrecht @ 2009-04-05  3:54 UTC (permalink / raw)
  To: Robin H. Johnson; +Cc: Git Mailing List
In-Reply-To: <20090405T001239Z@curie.orbis-terrarum.net>

On Sat, Apr 04, 2009 at 05:37:53PM -0700, Robin H. Johnson wrote:

> That causes incredibly bloat unfortunately.
> 
> I'll summarize why here for the git mailing list. Most our developers
> have the entire tree checked out, and in informal surveys, would like to
> continue to do so. There are ~13500 packages right now 

Each developer doesn't work on so many packages, right ? From my point
of view, checkin'out the entire tree is the wrong way on how to do
things.

Also, you could keep an entire tree repo assuming it's _not_
"fetch-able".

> For each package, the .git directory, assuming in a single pack,
> consumes at least 36 inodes.  Tail-packing is limited to Reiserfs3 and
> JFS, and isn't widely used other than that, so assuming 4KiB inodes,
> that's an overhead of at least 144KiB per package. Multiple by the
> number of packages, and we get an overhead of 2GiB, before we've added
> ANY content.

> Without tail packing, the Gentoo tree is presently around 520MiB (you
> can fit it into ~190MiB with tail packing). This means that
> repo-per-package would have an overhead in the range of 400%.

Don't know about the business for Gentoo, but HDD is cheap. Also, I'd
like to know how much space you will gain with the CVS to Git migration.
How bigger is a CVS repo against a Git one ?

One repo per category could be a good compromise assuming one seperate
branch per ebuild, then.

> Additionally, there's a lot of commonality between ebuilds and packages,
> and having repo-per-package means that the compression algorithms can't
> make use of it - dictionary algorithms are effective at compression for
> a reason.

Please, no. We are in the long term issues. Compression will be
efficient. It's all about the content of the files and dictionary
algorithms certainly will do a good job over the ebuilds revisions.

-- 
Nicolas Sebrecht

^ permalink raw reply

* [PATCH v2] mergetool--lib: add new merge tool TortoiseMerge
From: David Aguilar @ 2009-04-05  4:00 UTC (permalink / raw)
  To: gitster; +Cc: markus.heidelberg, charles, git

From: Markus Heidelberg <markus.heidelberg@web.de>

TortoiseMerge comes with TortoiseSVN or TortoiseGit for Windows. It can
only be used as a merge tool with an existing base file. It cannot be
used without a base nor as a diff tool.

The documentation only mentions the slash '/' as command line option
prefix, which refused to work, but the parser also accepts the dash '-'

See http://code.google.com/p/msysgit/issues/detail?id=226

Signed-off-by: Markus Heidelberg <markus.heidelberg@web.de>
---

This is Markus' patch rebased on top of my latest refactoring
patch.  It didn't apply cleanly so I figured I'd resolve the
merge myself.

 git-mergetool--lib.sh |   15 ++++++++++++++-
 1 files changed, 14 insertions(+), 1 deletions(-)

diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
index dcd4516..268485a 100644
--- a/git-mergetool--lib.sh
+++ b/git-mergetool--lib.sh
@@ -46,10 +46,13 @@ check_unchanged () {
 
 valid_tool () {
 	case "$1" in
-	kdiff3 | kompare | tkdiff | xxdiff | meld | opendiff | emerge | vimdiff | gvimdiff | ecmerge | diffuse)
+	kdiff3 | kompare | tkdiff | xxdiff | meld | opendiff | emerge | vimdiff | gvimdiff | ecmerge | diffuse | tortoisemerge)
 		if test "$1" = "kompare" && ! diff_mode; then
 			return 1
 		fi
+		if test "$1" = "tortoisemerge" && ! merge_mode; then
+			return 1
+		fi
 		;; # happy
 	*)
 		if test -z "$(get_merge_tool_cmd "$1")"; then
@@ -220,6 +223,16 @@ run_merge_tool () {
 		fi
 		status=$?
 		;;
+	tortoisemerge)
+		if $base_present; then
+			touch "$BACKUP"
+			"$merge_tool_path" -base:"$BASE" -mine:"$LOCAL" -theirs:"$REMOTE" -merged:"$MERGED"
+			check_unchanged
+		else
+			echo "TortoiseMerge cannot be used without a base" 1>&2
+			status=1
+		fi
+		;;
 	*)
 		if test -n "$merge_tool_cmd"; then
 			if merge_mode &&
-- 
1.6.2.1.469.gdffc1

^ permalink raw reply related

* Re: Performance issue: initial git clone causes massive repack
From: Nicolas Sebrecht @ 2009-04-05  4:08 UTC (permalink / raw)
  To: Nicolas Sebrecht; +Cc: Robin H. Johnson, Git Mailing List
In-Reply-To: <20090405035453.GB12927@vidovic>

On Sun, Apr 05, 2009 at 05:54:53AM +0200, Nicolas Sebrecht wrote:

> One repo per category could be a good compromise assuming one seperate
> branch per ebuild, then.

s/ebuild/package/

-- 
Nicolas Sebrecht

^ permalink raw reply

* [PATCH] mergetool--lib: make (g)vimdiff workable under Windows
From: David Aguilar @ 2009-04-05  4:15 UTC (permalink / raw)
  To: gitster; +Cc: markus.heidelberg, charles, git

From: Markus Heidelberg <markus.heidelberg@web.de>

Under Windows vimdiff and gvimdiff are not available as symbolic links,
but as batch files vimdiff.bat and gvimdiff.bat. These files weren't
found by 'type vimdiff' which led to the following error:

The merge tool vimdiff is not available as 'vimdiff'

Even if they were found, it wouldn't work to invoke these batch files
from git-mergetool.

To solve this, use vim and gvim (vim.exe and gvim.exe) and pass the -d
command line switch over to them.

Signed-off-by: Markus Heidelberg <markus.heidelberg@web.de>
---

Markus' last patch rebased.

This patch goes on top of the current difftool/mergetool patches from David.
This is 'da/difftool' in the current 'pu' and in addition:
[PATCH] mergetool-lib: refactor run_mergetool and check_unchanged
[PATCH] mergetool-lib: specialize opendiff options when in diff mode


 git-mergetool--lib.sh |   14 ++++++++++----
 1 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
index 268485a..3d3edda 100644
--- a/git-mergetool--lib.sh
+++ b/git-mergetool--lib.sh
@@ -12,6 +12,12 @@ translate_merge_tool_path () {
 		echo "$2"
 	else
 		case "$1" in
+		vimdiff)
+			path=vim
+			;;
+		gvimdiff)
+			path=gvim
+			;;
 		emerge)
 			path=emacs
 			;;
@@ -146,19 +152,19 @@ run_merge_tool () {
 	vimdiff)
 		if merge_mode; then
 			touch "$BACKUP"
-			"$merge_tool_path" -c "wincmd l" "$LOCAL" "$MERGED" "$REMOTE"
+			"$merge_tool_path" -d -c "wincmd l" "$LOCAL" "$MERGED" "$REMOTE"
 			check_unchanged
 		else
-			"$merge_tool_path" -c "wincmd l" "$LOCAL" "$REMOTE"
+			"$merge_tool_path" -d -c "wincmd l" "$LOCAL" "$REMOTE"
 		fi
 		;;
 	gvimdiff)
 		if merge_mode; then
 			touch "$BACKUP"
-			"$merge_tool_path" -c "wincmd l" -f "$LOCAL" "$MERGED" "$REMOTE"
+			"$merge_tool_path" -d -c "wincmd l" -f "$LOCAL" "$MERGED" "$REMOTE"
 			check_unchanged
 		else
-			"$merge_tool_path" -c "wincmd l" -f "$LOCAL" "$REMOTE"
+			"$merge_tool_path" -d -c "wincmd l" -f "$LOCAL" "$REMOTE"
 		fi
 		;;
 	xxdiff)
-- 
1.6.2.1.469.gdffc1

^ permalink raw reply related

* error in rewriting author history with filter-branch --env-filter
From: Adam Mercer @ 2009-04-05  4:21 UTC (permalink / raw)
  To: GIT

Hi

I'm trying to use git filter-branch to rewrite the author information
on an repo that was imported from CVS a while ago, the commits I'm
trying to rewrite are as follows:

commit 2cdeb25148f02b35391bd80a2c4c1876d7b8b840
Author: <ram>
Date:   Wed Dec 22 13:36:01 2004 +0000

    modify style

I'm trying to modify <ram> to be replaced with my name and email
address. To do this I've been trying to use the following command

$ git filter-branch --env-filter '
an="$GIT_AUTHOR_NAME"
am="$GIT_AUTHOR_EMAIL"
if [ "$GIT_AUTHOR_EMAIL" = "ram"]; then
an="Adam Mercer"
am="ramercer@gmail.com"
fi
export GIT_AUTHOR_NAME=$an
export GIT_AUTHOR_EMAIL=$am
'

however when I run this I get loads of errors of the form:

Rewrite 18242fa722ff393e7fce78e05f37fa7c3b3825e1
(1/216)/opt/local/libexec/git-core/git-filter-branch: line 307: [:
missing `]'

Any ideas what I'm doing wrong? I'm running git-1.6.2.2.

Cheers

Adam

^ permalink raw reply

* Re: [PATCH v2] mergetool--lib: add new merge tool TortoiseMerge
From: Brian Gernhardt @ 2009-04-05  5:10 UTC (permalink / raw)
  To: David Aguilar; +Cc: gitster, markus.heidelberg, charles, git
In-Reply-To: <1238904024-11238-1-git-send-email-davvid@gmail.com>


On Apr 5, 2009, at 12:00 AM, David Aguilar wrote:

> diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
> index dcd4516..268485a 100644
> --- a/git-mergetool--lib.sh
> +++ b/git-mergetool--lib.sh
> @@ -46,10 +46,13 @@ check_unchanged () {
>
> valid_tool () {
> 	case "$1" in
> -	kdiff3 | kompare | tkdiff | xxdiff | meld | opendiff | emerge |  
> vimdiff | gvimdiff | ecmerge | diffuse)
> +	kdiff3 | kompare | tkdiff | xxdiff | meld | opendiff | emerge |  
> vimdiff | gvimdiff | ecmerge | diffuse | tortoisemerge)
> 		if test "$1" = "kompare" && ! diff_mode; then
> 			return 1
> 		fi
> +		if test "$1" = "tortoisemerge" && ! merge_mode; then
> +			return 1
> +		fi
> 		;; # happy
> 	*)
> 		if test -z "$(get_merge_tool_cmd "$1")"; then

Why is `case "$1"` being followed by two `if test "$1" =`s?
Wouldn't it be simpler to have separate case arms for them?
Especially with how long that list is getting...

~~ Brian

^ permalink raw reply

* Re: Performance issue: initial git clone causes massive repack
From: Robin H. Johnson @ 2009-04-05  7:04 UTC (permalink / raw)
  To: Git Mailing List
In-Reply-To: <20090405035453.GB12927@vidovic>

[-- Attachment #1: Type: text/plain, Size: 4992 bytes --]

Before I answer the rest of your post, I'd like to note that the matter
of which choice between single-repo, repo-per-package, repo-per-category
has been flogged to death within Gentoo.

I did not come to the Git mailing list to rehash those choices. I came
here to find a solution to the performance problem. While it shows up
with our repo, I'm certain that we're not the only people with the
problem. The GSoC 2009 ideas contain a potential project for caching the
generated packs, which, while having value in itself, could be partially
avoided by sending suitable pre-built packs (if they exist) without any
repacking.

On Sun, Apr 05, 2009 at 05:54:53AM +0200, Nicolas Sebrecht wrote:
> > That causes incredibly bloat unfortunately.
> > 
> > I'll summarize why here for the git mailing list. Most our developers
> > have the entire tree checked out, and in informal surveys, would like to
> > continue to do so. There are ~13500 packages right now 
> Each developer doesn't work on so many packages, right ? From my point
> of view, checkin'out the entire tree is the wrong way on how to do
> things.
Also, I should note that working on the tree isn't the only reason to
have the tree checked out. While the great majority of Gentoo users have
their trees purely from rsync, there is nothing stopping you from using
a tree from CVS (anonCVS for the users, master CVS server for the
developers).

A quick bit of stats run show that while some developers only touch a
few packages, there are at least 200 developers that have done a major
change to 100 or more packages.

> > Without tail packing, the Gentoo tree is presently around 520MiB (you
> > can fit it into ~190MiB with tail packing). This means that
> > repo-per-package would have an overhead in the range of 400%.
> Don't know about the business for Gentoo, but HDD is cheap.
There's no reason to have bloat just for the layout to change.

> Also, I'd like to know how much space you will gain with the CVS to Git >
> migration.  How bigger is a CVS repo against a Git one ?
For the CVS checkouts right now: 
- ~410MiB of content (w/ 4kb inodes)
- ~240MiB of CVS overhead (w/ 4kb inodes)
(sorry about the earlier 520MiB number, I forgot to exclude a local dir
of stats data on my box when I ran du quickly).

Our experimental Git, with only a single repo for gentoo-x86:
- ~410MiB of content (w/ 4kb inodes)
- 80MiB - 1.6GiB of Git total overhead.

80MiB of overhead is the total overhead with a shallow clone at depth 1.
1.6GiB is with the full history.

And per-package numbers, because we DID do an experimental conversion,
last year, although the packs might not have been optimal:
- ~410MiB of content (w/ 4kb inodes)
- 4.7GiB of Git total overhead, with a breakdown:
  - 1.9GiB in inode waste
  - 2.8GiB in packs

> One repo per category could be a good compromise assuming one seperate
> branch per package, then.
Other downsides to repo-per-category and repo-per-package:
- Raises difficulty in adding a new package/category. 
  You cannot just do 'mkdir && vi ... && git add && git commit' anymore.
- The name of the directory for both of the category AND the package are not
  specified in the ebuild, as such, unless they are checked out to the right
  location, you will get breakage (definitely in the package name, and
  about 10% of the time with categories).
- You cannot use git-cvsserver with them cleanly and have the correct
  behavior (we DO have developers that want to use the CVS emulation
  layer) - adding a category or a package would NOT trigger the
  addition of a new repo on the server when needed.
- Does NOT present a good base for anybody wanting to branch the entire
  tree themselves.
  

> > Additionally, there's a lot of commonality between ebuilds and packages,
> > and having repo-per-package means that the compression algorithms can't
> > make use of it - dictionary algorithms are effective at compression for
> > a reason.
> Please, no. We are in the long term issues. Compression will be
> efficient. It's all about the content of the files and dictionary
> algorithms certainly will do a good job over the ebuilds revisions.
We're already on track to drop the CVS $Header$, and thereafter, some of the
ebuilds are already on track to be smaller. Here's our prototype dev-perl/Sub-Name-0.04.
====
# Copyright 1999-2009 Gentoo Foundation
# Distributed under the terms of the GNU General Public License v2
MODULE_AUTHOR=XMATH
inherit perl-module
DESCRIPTION="(re)name a sub"
LICENSE="|| ( Artistic GPL-2 )"
SLOT="0"
KEYWORDS="~amd64 ~x86"
IUSE=""
SRC_TEST=do
====

We can have all the CPAN packages from CPAN author XMATH, with changing
only the DESCRIPTION string. KEYWORDS then just changes over the package
lifespan.

-- 
Robin Hugh Johnson
Gentoo Linux Developer & Infra Guy
E-Mail     : robbat2@gentoo.org
GnuPG FP   : 11AC BA4F 4778 E3F6 E4ED  F38E B27B 944E 3488 4E85

[-- Attachment #2: Type: application/pgp-signature, Size: 330 bytes --]

^ permalink raw reply

* Re: error in rewriting author history with filter-branch --env-filter
From: Mike Hommey @ 2009-04-05  7:21 UTC (permalink / raw)
  To: Adam Mercer; +Cc: GIT
In-Reply-To: <799406d60904042121o27b4ac39m633354dcf0bcc84d@mail.gmail.com>

On Sat, Apr 04, 2009 at 11:21:36PM -0500, Adam Mercer wrote:
> Hi
> 
> I'm trying to use git filter-branch to rewrite the author information
> on an repo that was imported from CVS a while ago, the commits I'm
> trying to rewrite are as follows:
> 
> commit 2cdeb25148f02b35391bd80a2c4c1876d7b8b840 Author: <ram> Date:
> Wed Dec 22 13:36:01 2004 +0000
> 
>     modify style
> 
> I'm trying to modify <ram> to be replaced with my name and email
> address. To do this I've been trying to use the following command
> 
> $ git filter-branch --env-filter ' an="$GIT_AUTHOR_NAME"
> am="$GIT_AUTHOR_EMAIL" if [ "$GIT_AUTHOR_EMAIL" = "ram"]; then
> an="Adam Mercer" am="ramercer@gmail.com" fi export GIT_AUTHOR_NAME=$an
> export GIT_AUTHOR_EMAIL=$am '
> 
> however when I run this I get loads of errors of the form:
> 
> Rewrite 18242fa722ff393e7fce78e05f37fa7c3b3825e1
> (1/216)/opt/local/libexec/git-core/git-filter-branch: line 307: [:
> missing `]'
> 
> Any ideas what I'm doing wrong? I'm running git-1.6.2.2.

You need a white space between "ram" and ].

Mike

^ permalink raw reply

* Re: [PATCH] revision.h: add includes of "diff.h" and "commit.h"
From: Nanako Shiraishi @ 2009-04-05  7:28 UTC (permalink / raw)
  To: Christian Couder
  Cc: git, Jeff King, Nathaniel P Dawson, Johannes Sixt, Junio C Hamano
In-Reply-To: <20090404214554.75ada117.chriscool@tuxfamily.org>

Quoting Christian Couder <chriscool@tuxfamily.org>:

> Because they are needed by some features included in
> "revision.h".
>
> This makes the following just work:
>
>         $ cat >1.c <<\EOF
>         #include "cache.h"
>         #include "revision.h"
>         EOF
>         $ cc -Wall -DSHA1_HEADER='<openssl/sha.h>' -c 1.c

I'm sorry if this is obvious to experienced people, but I don't understand what benefit there is to make such an empty program compilable.

-- 
Nanako Shiraishi
http://ivory.ap.teacup.com/nanako3/

^ permalink raw reply

* Re: [PATCH] revision.h: add includes of "diff.h" and "commit.h"
From: Junio C Hamano @ 2009-04-05  8:02 UTC (permalink / raw)
  To: Nanako Shiraishi
  Cc: Christian Couder, git, Jeff King, Nathaniel P Dawson,
	Johannes Sixt
In-Reply-To: <20090405162841.6117@nanako3.lavabit.com>

Nanako Shiraishi <nanako3@lavabit.com> writes:

> Quoting Christian Couder <chriscool@tuxfamily.org>:
>
>> Because they are needed by some features included in
>> "revision.h".
>>
>> This makes the following just work:
>>
>>         $ cat >1.c <<\EOF
>>         #include "cache.h"
>>         #include "revision.h"
>>         EOF
>>         $ cc -Wall -DSHA1_HEADER='<openssl/sha.h>' -c 1.c
>
> I'm sorry if this is obvious to experienced people, but I don't understand what benefit there is to make such an empty program compilable.

I believe this was prompted by my earlier comment on the header clean-up
($gmane/115443), but I'd say it does not make much sense.  I have already
explained why it doesn't, and in addition, as the above example shows, you
still have to include "cache.h" in your 1.c file anyway, so it is not
making the header "usable standalone" either.

If you have a follow-up patch that removes the inclusion of diff.h and
commit.h to millions of .c files that already include revision.h, it might
start to make sense, but it goes against one of the rules Christian wanted
to add, namely:

    a header file should be included in a C file only if it is needed to 
    compile the C file (it is not ok to include it only because it includes 
    many other headers that are needed)

in the sense that if somebody wants to run diff in his C code, he should
explicitly include diff.h (or diffcore.h if necessary), instead of relying
on the fact that revision.h happens to include it, and he happens to
include revision.h because he uses setup_revisions() to parse the command
line arguments (and I happen to think that guideline makes sense).

Even though including the same .h file twice is protected with the
standard:

	#ifndef FROTZ_H
        #define FROTZ_H
        ...
        #endif

it does make C preprocessor do extra work to open the header twice (and
skip the whole file in its second inclusion), so there is a slight
performance issue.

You can argue revision.h is somewhat special---it are so central that
almost all core-ish history inspection commands in git revolve around
them, and it is not particularly a bad idea to say "you can rely on
revision.h to include diff.h" in practice.  That would give you an escape
hatch to omit inclusion of diff.h from programs that include revision.h
and avoid the performance issue.

But then that introduces new rules on which ones are special and which
ones are not, and overall it does not help simplifying the life of the
programmers.

So I do not feel strongly supportive about this patch.

^ permalink raw reply

* Re: [PATCH 7/7] user-manual: simplify the user configuration
From: Felipe Contreras @ 2009-04-05  9:14 UTC (permalink / raw)
  To: Jay Soffian; +Cc: git, Junio C Hamano
In-Reply-To: <76718490904042036q26bebc13p91c8cffaf432c02c@mail.gmail.com>

On Sun, Apr 5, 2009 at 6:36 AM, Jay Soffian <jaysoffian@gmail.com> wrote:
> On Sat, Apr 4, 2009 at 5:38 AM, Felipe Contreras
> <felipe.contreras@gmail.com> wrote:
>> This is shorter, avoids the burder to think about the format of the
>> configuration file, and git config is already used in other places in
>> the manual.
>> [...]
>>  ------------------------------------------------
>> -[user]
>> -       name = Your Name Comes Here
>> -       email = you@yourdomain.example.com
>> +$ git config --global user.name "Your Name Comes Here"
>> +$ git config --global user.email you@yourdomain.example.com
>>  ------------------------------------------------
>
> This has come up before, and the idea was to hint to the reader that
> .gitconfig is something they can edit themselves.

Ok, I forgot to explain this again the commit message of this
particular patch, but it's described in the patch series description.

The previous patch adds a 'Getting Started' section that explains
exactly that; you can edit ~/.gitconfig.

So that concern is already addressed.

-- 
Felipe Contreras

^ permalink raw reply

* Re: [PATCH] mergetool-lib: make (g)vimdiff workable under Windows
From: Johannes Schindelin @ 2009-04-05  9:35 UTC (permalink / raw)
  To: Markus Heidelberg
  Cc: Junio C Hamano, git, David Aguilar, Charles Bailey, msysgit
In-Reply-To: <200904041517.55890.markus.heidelberg@web.de>


Hi,

On Sat, 4 Apr 2009, Markus Heidelberg wrote:

> Under Windows vimdiff and gvimdiff are not available as symbolic links, 
> but as batch files vimdiff.bat and gvimdiff.bat. These files weren't 
> found by 'type vimdiff' which led to the following error:
> 
>     The merge tool vimdiff is not available as 'vimdiff'
> 
> Even if they were found, it wouldn't work to invoke these batch files
> from git-mergetool.
> 
> To solve this, use vim and gvim (vim.exe and gvim.exe) and pass the -d
> command line switch over to them.
> 
> Signed-off-by: Markus Heidelberg <markus.heidelberg@web.de>
> ---
> 
> This patch goes on top of the current difftool/mergetool patches from David.

,.. which is a pity, because I could have applied them directly to 
msysGit's 'devel' branch otherwise.

Thanks,
Dscho

^ permalink raw reply

* non-ascii filenames issue
From: Gregory Petrosyan @ 2009-04-05  9:36 UTC (permalink / raw)
  To: git

gregory@home:~$ git --version
git version 1.6.2.2.404.ge96f3
gregory@home:~$ mkdir git-test
gregory@home:~$ cd git-test
gregory@home:~/git-test$ touch файл
gregory@home:~/git-test$ ls -a
.  ..  файл
gregory@home:~/git-test$ git init
Initialized empty Git repository in /home/gregory/git-test/.git/
gregory@home:~/git-test$ git add .
gregory@home:~/git-test$ git status
# On branch master
#
# Initial commit
#
# Changes to be committed:
#   (use "git rm --cached <file>..." to unstage)
#
#  new file:   "\321\204\320\260\320\271\320\273"
#
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
                 "файл" should be here instead

This is on Ubuntu Jaunty beta, with latest git built from source.
Please CC me, I am not subscribed.

	Gregory

^ permalink raw reply

* Re: [PATCH 0/4] start refactoring binary search function
From: Junio C Hamano @ 2009-04-05  9:45 UTC (permalink / raw)
  To: Christian Couder; +Cc: git, Johannes Schindelin
In-Reply-To: <20090404225920.6a10fe78.chriscool@tuxfamily.org>

Christian Couder <chriscool@tuxfamily.org> writes:

> There are many binary search functions in the code base and I have been
> asked to refactor them in these message:
>
> http://thread.gmane.org/gmane.comp.version-control.git/105363/focus=105436
> http://thread.gmane.org/gmane.comp.version-control.git/114735/focus=115391
>
> so here is a start
>
> The following patch applies on top of pu where they can be squashed into other
> patches:
>
>   sha1-lookup: add new "sha1_pos" function to efficiently lookup sha1
>   patch-ids: use the new generic "sha1_pos" function to lookup sha1
>   bisect: use the new generic "sha1_pos" function to lookup sha1
>   replace_object: use the new generic "sha1_pos" function to lookup
>     sha1

I think the refactoring itself does make sense.  Less duplicated code has
better chance to be improved further if there is demonstrated need, and
I like the series for that "clean-up" value alone.

In the last two patches, however, you advertised the use of this new API
for gaining better performance (in exchange for simpler copy-pasted
implementation), but changing a simple (base + index * sizeof(struct that
contains the sha-1 field)) into a call to a function whose address is
passed _may_ have larger negative impact to the performance, than what is
gained by the better initial midpoint selection the new code uses.

If the extra indirect call turns to degrade the performance too much, we
could always reimplement it as a macro, I think, but let's not go there
before somebody runs benchmarks and demonstrates that it is a problem.

I've restructured your existing two branches to take advantage of the
first two patches.

Thanks.

^ 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