Git development
 help / color / mirror / Atom feed
* [PATCH] fix handling of multiple untracked files for git mv -k
From: Michael J Gruber @ 2009-01-14 15:42 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano
In-Reply-To: <vpqwscy81o8.fsf@bauges.imag.fr>

The "-k" option to "git mv" should allow specifying multiple untracked
files. Currently, multiple untracked files raise an assertion if they
appear consecutively as arguments. Fix this by decrementing the loop
index after removing one entry from the array of arguments.

Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
---
 builtin-mv.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

Reported by the OP. Do we need a test case for this? It's a really
trivial change. The patch is off master but builtin-mv.c hasn't change
since 81dc2307d0ad87a4da2e753a9d1b5586d6456eed tags/v1.6.0-rc1~1, so I
suggest this patch for maint.

diff --git a/builtin-mv.c b/builtin-mv.c
index 4f65b5a..bce9959 100644
--- a/builtin-mv.c
+++ b/builtin-mv.c
@@ -192,6 +192,7 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
 					memmove(destination + i,
 						destination + i + 1,
 						(argc - i) * sizeof(char *));
+					i--;
 				}
 			} else
 				die ("%s, source=%s, destination=%s",
-- 
1.6.0.6

^ permalink raw reply related

* Re: [PATCH 2/3] remove pathspec_match, use match_pathspec instead
From: Johannes Schindelin @ 2009-01-14 15:40 UTC (permalink / raw)
  To: Clemens Buchacher; +Cc: git, Junio C Hamano, johannes
In-Reply-To: <1231944876-29930-3-git-send-email-drizzd@aon.at>

Hi,

On Wed, 14 Jan 2009, Clemens Buchacher wrote:

> Both versions have the same functionality. This removes any
> redundancy.
> 
> This also adds makes two extensions to match_pathspec:

s/makes//

>  5 files changed, 17 insertions(+), 51 deletions(-)

Nice.  Does it still pass the test suite?  (From my reading, it should, 
but I do not quite have the time to run it right now.)

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH next] git-notes: add test case for multi-line notes
From: Johannes Schindelin @ 2009-01-14 15:34 UTC (permalink / raw)
  To: Tor Arne Vestbø; +Cc: Junio C Hamano, git
In-Reply-To: <496DF936.3060308@trolltech.com>

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

Hi,

On Wed, 14 Jan 2009, Tor Arne Vestbø wrote:

> The tests adds a third commit with a multi-line note. The output of
> git log -2 is then checked to see if the note lines are wrapped
> correctly, and that there's a line separator between the two commits.
> 
> Also, changed from using 'git diff' to test expect vs. output to use
> 'test_cmp', as I had problems getting correct results using the former.

You could skip the part that you had problems, as the test_cmp is 
obviously the correct thing to do.

> diff --git a/t/t3301-notes.sh b/t/t3301-notes.sh
> index ba42c45..76bb6dd 100755
> --- a/t/t3301-notes.sh
> +++ b/t/t3301-notes.sh
> @@ -8,8 +8,8 @@ test_description='Test commit notes'
> . ./test-lib.sh
> 
> cat > fake_editor.sh << \EOF
> -echo "$MSG" > "$1"
> -echo "$MSG" >& 2
> +echo -e "$MSG" > "$1"
> +echo -e "$MSG" >& 2

I seem to recall that we had plenty of fun substituting "echo -e" with 
"printf" whenever it entered the repository (... again...), as some 
platforms -- ahem, macosx, ahem -- are a bit peculiar with such options.

So you might want to make sure no % is passed as "$MSG", and use printf 
instead.

> +test_expect_success 'create multi-line notes (setup)' '
> +	: > a3 &&
> +	git add a3 &&
> +	test_tick &&
> +	git commit -m 3rd &&
> +	MSG="b3\nc3c3c3c3\nd3d3d3" git notes edit
> +
> +'

Minor style nit: maybe you want to have an empty line at the beginning, 
too...

> +cat > expect-multiline << EOF
> +commit 1584215f1d29c65e99c6c6848626553fdd07fd75
> +Author: A U Thor <author@example.com>
> +Date:   Thu Apr 7 15:15:13 2005 -0700
> +
> +    3rd
> +
> +Notes:
> +    b3
> +    c3c3c3c3
> +    d3d3d3
> +EOF
> +
> +echo >> expect-multiline
> +cat expect >> expect-multiline

Yeah.  My initial reaction was: "you could have that echo inside the cat 
<<EOF", but this is clearer.  Except that you should make sure that 
nothing is printed (M$' echo outputs something if you pass no parameters); 
printf "\n" would be my choice.

Other than that, very good: ACK.

Ciao,
Dscho

^ permalink raw reply

* Re: jgit merge question
From: Shawn O. Pearce @ 2009-01-14 15:30 UTC (permalink / raw)
  To: Johannes Schindelin, David Birchfield; +Cc: git
In-Reply-To: <alpine.DEB.1.00.0901141124460.3586@pacific.mpi-cbg.de>

Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> On Wed, 14 Jan 2009, David Birchfield wrote:
> 
> > I have been working with the JGit library and with the pgm files and 
> > other documentation I have been able to implement most of the basic 
> > functions that I need for my testing application - including fetch.  
> > This is great!

I'm glad JGit has been useful for you.

> > However, I have not been able to successfully implement 
> > the merge function with JGit. There is some reference to this in the 
> > mail archive, but I cannot find any of the merge code that is referenced 
> > in the distribution.

David is probably talking about the 8 patch series I proposed to add a
crude merge API to JGit.  The patches are available here, based on the
current JGit master:

  http://android.git.kernel.org/?p=tools/egit.git;a=shortlog;h=refs/heads/for-gerrit2
  git://android.git.kernel.org/tools/egit.git for-gerrit2

as I'm still highly dependent upon this in Gerrit2, but I haven't
been able to fix enough bugs and add enough unit tests to make
Robin happy with accepting them into the library.  I plan to get
back to that as soon as Gerrit2 is live for Android.

That API is a very, very crude merge API.  I don't like coding
with it from the application level.  It doesn't give Gerrit2
enough error information when it fails, and that means any other
application will also be unhappy with it.

It also has at least one bug, but at least its a conservative bug
(it won't merge subtrees sometimes and aborts even though they
should have merged clean).  Its also missing a huge block of code
about head simplification; that block is in Gerrit2 application
code rather than down in the library (that was a mistake in design).

So basically, yea, I've proposed something, but there are some very
good reasons why there isn't merge support yet in JGit.

> http://thread.gmane.org/gmane.comp.version-control.git/100524/focus=100589

Yea, we also need the diff implementation so we can do file-level
merges.  That merge API above only does path-level merges.  If a
file is modified on both sides, the merge API I use in Gerrit2
just aborts.  That typically isn't what a human who is trying to
use Git for development wants.
 
> Yes, it has not been implemented, basically because it needs a diff 
> implementation, and I constantly run out of time working on it.  I have 
> something that works, but needs lots of improvements to be usable 
> (basically, it has to avoid deep recursion depths).

What's a little stack space between friends?  ;-)

-- 
Shawn.

^ permalink raw reply

* Re: [PATCH 3/3] implement pattern matching in ce_path_match
From: Clemens Buchacher @ 2009-01-14 15:25 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, johannes
In-Reply-To: <1231944876-29930-4-git-send-email-drizzd@aon.at>

On Wed, Jan 14, 2009 at 03:54:36PM +0100, Clemens Buchacher wrote:
> This also implies pattern matching for many other git commands, such
> as add -u, blame, log, etc.

Oops, I thought I had removed that. AFAICT blame is not affected by this.

^ permalink raw reply

* [PATCH 3/3] implement pattern matching in ce_path_match
From: Clemens Buchacher @ 2009-01-14 14:54 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, johannes, Clemens Buchacher
In-Reply-To: <1231944876-29930-3-git-send-email-drizzd@aon.at>

For tracked files, matching globbing pattern in "git add" was broken
by 1e5f764c (builtin-add.c: optimize -A option and "git add ."), which
uses ce_path_match instead of match_pathspec for tracked files.
But ce_path_match does not implement pattern matching.

With this patch ce_path_match uses match_pathspec in order to perform
pattern matching.

This also implies pattern matching for many other git commands, such
as add -u, blame, log, etc.  For "git log --follow", we have to
disallow globbing pattern, because they potentially match more than
one file.

Reported-by: Johannes Schneider <johannes@familieschneider.info>
Signed-off-by: Clemens Buchacher <drizzd@aon.at>
---
 builtin-log.c         |   13 ++++++++++++-
 read-cache.c          |   23 +----------------------
 t/t2200-add-update.sh |   10 ++++++++++
 3 files changed, 23 insertions(+), 23 deletions(-)

diff --git a/builtin-log.c b/builtin-log.c
index c7aa48e..16e9207 100644
--- a/builtin-log.c
+++ b/builtin-log.c
@@ -25,6 +25,17 @@ static int default_show_root = 1;
 static const char *fmt_patch_subject_prefix = "PATCH";
 static const char *fmt_pretty;
 
+static int has_special(const char *p)
+{
+	int x;
+
+	while ((x = *p++) != '\0')
+		if (isspecial(x))
+			return 1;
+
+	return 0;
+}
+
 static void cmd_log_init(int argc, const char **argv, const char *prefix,
 		      struct rev_info *rev)
 {
@@ -49,7 +60,7 @@ static void cmd_log_init(int argc, const char **argv, const char *prefix,
 		rev->always_show_header = 0;
 	if (DIFF_OPT_TST(&rev->diffopt, FOLLOW_RENAMES)) {
 		rev->always_show_header = 0;
-		if (rev->diffopt.nr_paths != 1)
+		if (rev->diffopt.nr_paths != 1 || has_special(rev->diffopt.paths[0]))
 			usage("git logs can only follow renames on one pathname at a time");
 	}
 	for (i = 1; i < argc; i++) {
diff --git a/read-cache.c b/read-cache.c
index b1475ff..8a5f306 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -640,28 +640,7 @@ int ce_same_name(struct cache_entry *a, struct cache_entry *b)
 
 int ce_path_match(const struct cache_entry *ce, const char **pathspec)
 {
-	const char *match, *name;
-	int len;
-
-	if (!pathspec)
-		return 1;
-
-	len = ce_namelen(ce);
-	name = ce->name;
-	while ((match = *pathspec++) != NULL) {
-		int matchlen = strlen(match);
-		if (matchlen > len)
-			continue;
-		if (memcmp(name, match, matchlen))
-			continue;
-		if (matchlen && name[matchlen-1] == '/')
-			return 1;
-		if (name[matchlen] == '/' || !name[matchlen])
-			return 1;
-		if (!matchlen)
-			return 1;
-	}
-	return 0;
+	return match_pathspec(pathspec, ce->name, ce_namelen(ce), 0, NULL);
 }
 
 /*
diff --git a/t/t2200-add-update.sh b/t/t2200-add-update.sh
index cd9231c..6d99461 100755
--- a/t/t2200-add-update.sh
+++ b/t/t2200-add-update.sh
@@ -128,4 +128,14 @@ test_expect_success 'add -n -u should not add but just report' '
 
 '
 
+test_expect_success 'wildcard update' '
+
+	: >a.x &&
+	git add "*.x" &&
+	echo asdf >a.x &&
+	git add -u "*.x" &&
+	test -z "`git ls-files -m a.x`"
+
+'
+
 test_done
-- 
1.6.1

^ permalink raw reply related

* [PATCH 2/3] remove pathspec_match, use match_pathspec instead
From: Clemens Buchacher @ 2009-01-14 14:54 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, johannes, Clemens Buchacher
In-Reply-To: <1231944876-29930-2-git-send-email-drizzd@aon.at>

Both versions have the same functionality. This removes any
redundancy.

This also adds makes two extensions to match_pathspec:

- If pathspec is NULL, return 1. This reflects the behavior of git
  commands, for which no paths usually means "match all paths".

- If seen is NULL, do not use it.

Signed-off-by: Clemens Buchacher <drizzd@aon.at>
---
 builtin-checkout.c |    6 +++---
 builtin-commit.c   |    2 +-
 builtin-ls-files.c |   40 ++--------------------------------------
 cache.h            |    1 -
 dir.c              |   19 +++++++++++--------
 5 files changed, 17 insertions(+), 51 deletions(-)

diff --git a/builtin-checkout.c b/builtin-checkout.c
index b5dd9c0..84a2825 100644
--- a/builtin-checkout.c
+++ b/builtin-checkout.c
@@ -240,7 +240,7 @@ static int checkout_paths(struct tree *source_tree, const char **pathspec,
 
 	for (pos = 0; pos < active_nr; pos++) {
 		struct cache_entry *ce = active_cache[pos];
-		pathspec_match(pathspec, ps_matched, ce->name, 0);
+		match_pathspec(pathspec, ce->name, ce_namelen(ce), 0, ps_matched);
 	}
 
 	if (report_path_error(ps_matched, pathspec, 0))
@@ -249,7 +249,7 @@ static int checkout_paths(struct tree *source_tree, const char **pathspec,
 	/* Any unmerged paths? */
 	for (pos = 0; pos < active_nr; pos++) {
 		struct cache_entry *ce = active_cache[pos];
-		if (pathspec_match(pathspec, NULL, ce->name, 0)) {
+		if (match_pathspec(pathspec, ce->name, ce_namelen(ce), 0, NULL)) {
 			if (!ce_stage(ce))
 				continue;
 			if (opts->force) {
@@ -274,7 +274,7 @@ static int checkout_paths(struct tree *source_tree, const char **pathspec,
 	state.refresh_cache = 1;
 	for (pos = 0; pos < active_nr; pos++) {
 		struct cache_entry *ce = active_cache[pos];
-		if (pathspec_match(pathspec, NULL, ce->name, 0)) {
+		if (match_pathspec(pathspec, ce->name, ce_namelen(ce), 0, NULL)) {
 			if (!ce_stage(ce)) {
 				errs |= checkout_entry(ce, &state, NULL);
 				continue;
diff --git a/builtin-commit.c b/builtin-commit.c
index 7cf227a..913aa89 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -166,7 +166,7 @@ static int list_paths(struct string_list *list, const char *with_tree,
 		struct cache_entry *ce = active_cache[i];
 		if (ce->ce_flags & CE_UPDATE)
 			continue;
-		if (!pathspec_match(pattern, m, ce->name, 0))
+		if (!match_pathspec(pattern, ce->name, ce_namelen(ce), 0, m))
 			continue;
 		string_list_insert(ce->name, list);
 	}
diff --git a/builtin-ls-files.c b/builtin-ls-files.c
index f72eb85..3434031 100644
--- a/builtin-ls-files.c
+++ b/builtin-ls-files.c
@@ -36,42 +36,6 @@ static const char *tag_other = "";
 static const char *tag_killed = "";
 static const char *tag_modified = "";
 
-
-/*
- * Match a pathspec against a filename. The first "skiplen" characters
- * are the common prefix
- */
-int pathspec_match(const char **spec, char *ps_matched,
-		   const char *filename, int skiplen)
-{
-	const char *m;
-
-	while ((m = *spec++) != NULL) {
-		int matchlen = strlen(m + skiplen);
-
-		if (!matchlen)
-			goto matched;
-		if (!strncmp(m + skiplen, filename + skiplen, matchlen)) {
-			if (m[skiplen + matchlen - 1] == '/')
-				goto matched;
-			switch (filename[skiplen + matchlen]) {
-			case '/': case '\0':
-				goto matched;
-			}
-		}
-		if (!fnmatch(m + skiplen, filename + skiplen, 0))
-			goto matched;
-		if (ps_matched)
-			ps_matched++;
-		continue;
-	matched:
-		if (ps_matched)
-			*ps_matched = 1;
-		return 1;
-	}
-	return 0;
-}
-
 static void show_dir_entry(const char *tag, struct dir_entry *ent)
 {
 	int len = prefix_len;
@@ -80,7 +44,7 @@ static void show_dir_entry(const char *tag, struct dir_entry *ent)
 	if (len >= ent->len)
 		die("git ls-files: internal error - directory entry not superset of prefix");
 
-	if (pathspec && !pathspec_match(pathspec, ps_matched, ent->name, len))
+	if (!match_pathspec(pathspec, ent->name, ent->len, len, ps_matched))
 		return;
 
 	fputs(tag, stdout);
@@ -156,7 +120,7 @@ static void show_ce_entry(const char *tag, struct cache_entry *ce)
 	if (len >= ce_namelen(ce))
 		die("git ls-files: internal error - cache entry not superset of prefix");
 
-	if (pathspec && !pathspec_match(pathspec, ps_matched, ce->name, len))
+	if (!match_pathspec(pathspec, ce->name, ce_namelen(ce), len, ps_matched))
 		return;
 
 	if (tag && *tag && show_valid_bit &&
diff --git a/cache.h b/cache.h
index 2dbd546..eba8afc 100644
--- a/cache.h
+++ b/cache.h
@@ -940,7 +940,6 @@ extern int ws_fix_copy(char *, const char *, int, unsigned, int *);
 extern int ws_blank_line(const char *line, int len, unsigned ws_rule);
 
 /* ls-files */
-int pathspec_match(const char **spec, char *matched, const char *filename, int skiplen);
 int report_path_error(const char *ps_matched, const char **pathspec, int prefix_offset);
 void overlay_tree_on_cache(const char *tree_name, const char *prefix);
 
diff --git a/dir.c b/dir.c
index 87a9758..d3b92de 100644
--- a/dir.c
+++ b/dir.c
@@ -108,25 +108,28 @@ static int match_one(const char *match, const char *name, int namelen)
  * and a mark is left in seen[] array for pathspec element that
  * actually matched anything.
  */
-int match_pathspec(const char **pathspec, const char *name, int namelen, int prefix, char *seen)
+int match_pathspec(const char **pathspec, const char *name, int namelen,
+		int prefix, char *seen)
 {
-	int retval;
-	const char *match;
+	int i, retval = 0;
+
+	if (!pathspec)
+		return 1;
 
 	name += prefix;
 	namelen -= prefix;
 
-	for (retval = 0; (match = *pathspec++) != NULL; seen++) {
+	for (i = 0; pathspec[i] != NULL; i++) {
 		int how;
-		if (*seen == MATCHED_EXACTLY)
+		const char *match = pathspec[i] + prefix;
+		if (seen && seen[i] == MATCHED_EXACTLY)
 			continue;
-		match += prefix;
 		how = match_one(match, name, namelen);
 		if (how) {
 			if (retval < how)
 				retval = how;
-			if (*seen < how)
-				*seen = how;
+			if (seen && seen[i] < how)
+				seen[i] = how;
 		}
 	}
 	return retval;
-- 
1.6.1

^ permalink raw reply related

* [PATCH 0/3] fix "git add" pattern matching
From: Clemens Buchacher @ 2009-01-14 14:54 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, johannes

Hi,

Johannes Schneider reported that "git add '*'" did not add matching files.
And indeed pattern matching for tracked files is broken since 1.6.1. I've
never used this feature myself, but I can imagine it's useful in some
situations. For example, you can do "git add '*.c'" to add all .c files. The
equivalent command without pattern matching would be
"find .  -name '*.c' | xargs git add".

[PATCH 1/3] clean up pathspec matching
[PATCH 2/3] remove pathspec_match, use match_pathspec instead
[PATCH 3/3] implement pattern matching in ce_path_match

This patch series fixes pattern matching for "git add". The first two
patches are cleanups only. PATCH 3/3 then implements pattern matching in
ce_path_match. It is very intrusive in the sense that all commands using
ce_path_match will now have pattern matching. I suppose this is good in
general, but can also have undesired side-effects, as in "git log --follow
'*'", for example. I'd appreciate some double-checking in that context.

Cheers,
Clemens

^ permalink raw reply

* [PATCH 1/3] clean up pathspec matching
From: Clemens Buchacher @ 2009-01-14 14:54 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, johannes, Clemens Buchacher
In-Reply-To: <1231944876-29930-1-git-send-email-drizzd@aon.at>

If pathspec already matched exactly, it cannot match any more.
Originally, we had to continue anyways, because we did not
differentiate between exact, recursive and globbing matches.

Signed-off-by: Clemens Buchacher <drizzd@aon.at>
---
 dir.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/dir.c b/dir.c
index 7c59829..87a9758 100644
--- a/dir.c
+++ b/dir.c
@@ -118,7 +118,7 @@ int match_pathspec(const char **pathspec, const char *name, int namelen, int pre
 
 	for (retval = 0; (match = *pathspec++) != NULL; seen++) {
 		int how;
-		if (retval && *seen == MATCHED_EXACTLY)
+		if (*seen == MATCHED_EXACTLY)
 			continue;
 		match += prefix;
 		how = match_one(match, name, namelen);
-- 
1.6.1

^ permalink raw reply related

* Re: [BUG] assertion failure in builtin-mv.c with "git mv -k"
From: Michael J Gruber @ 2009-01-14 14:53 UTC (permalink / raw)
  To: Matthieu Moy; +Cc: git
In-Reply-To: <vpqwscy81o8.fsf@bauges.imag.fr>

Matthieu Moy venit, vidit, dixit 14.01.2009 14:20:
> Hi,
> 
> Just found a bug in builtin-mv.c. Here's a script to reproduce:
> 
> mkdir git
> cd git
> git init
> touch controled
> git add controled && git commit -m "init"
> touch foo1 foo2
> mkdir dir
> git mv -k foo* dir/
> 
> The output is the following:
> 
> Initialized empty Git repository in /tmp/git/.git/
> [master (root-commit)]: created 694563b: "init"
>  0 files changed, 0 insertions(+), 0 deletions(-)
>  create mode 100644 controled
> git: builtin-mv.c:216: cmd_mv: Assertion `pos >= 0' failed.
> ./bug.sh: line 10: 12919 Aborted                 git mv -k foo* dir/
> 
> Apparently, this happens when using "git mv -k" with more than one
> unversionned file. The code ignores the first one, but still goes
> through this
> 
> 	for (i = 0; i < argc; i++) {
> 		const char *src = source[i], *dst = destination[i];
> 		enum update_mode mode = modes[i];
> 		int pos;
> 		if (show_only || verbose)
> 			printf("Renaming %s to %s\n", src, dst);
> 		if (!show_only && mode != INDEX &&
> 				rename(src, dst) < 0 && !ignore_errors)
> 			die ("renaming %s failed: %s", src, strerror(errno));
> 
> 		if (mode == WORKING_DIRECTORY)
> 			continue;
> 
> 		pos = cache_name_pos(src, strlen(src));
> 		assert(pos >= 0); /* <----- this is the one */
> 		if (!show_only)
> 			rename_cache_entry_at(pos, dst);
> 	}
> 
> for the second, and it crashes on the assertion (gdb says "pos" here
> is an unversionned file name).
> 
> If anyone has time to fix this ...
> 
> Thanks,
> 

The problem is actually in the loop before, with just one line missing.
I'll send a patch but I'm not sure if this needs a test case.

Michael

^ permalink raw reply

* [PATCH next] git-notes: add test case for multi-line notes
From: Tor Arne Vestbø @ 2009-01-14 14:39 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Johannes Schindelin, git
In-Reply-To: <7v3afm758u.fsf@gitster.siamese.dyndns.org>

The tests adds a third commit with a multi-line note. The output of
git log -2 is then checked to see if the note lines are wrapped
correctly, and that there's a line separator between the two commits.

Also, changed from using 'git diff' to test expect vs. output to use
'test_cmp', as I had problems getting correct results using the former.

Signed-off-by: Tor Arne Vestbø <tavestbo@trolltech.com>
---
 t/t3301-notes.sh |   35 ++++++++++++++++++++++++++++++++---
 1 files changed, 32 insertions(+), 3 deletions(-)

diff --git a/t/t3301-notes.sh b/t/t3301-notes.sh
index ba42c45..76bb6dd 100755
--- a/t/t3301-notes.sh
+++ b/t/t3301-notes.sh
@@ -8,8 +8,8 @@ test_description='Test commit notes'
 . ./test-lib.sh
 
 cat > fake_editor.sh << \EOF
-echo "$MSG" > "$1"
-echo "$MSG" >& 2
+echo -e "$MSG" > "$1"
+echo -e "$MSG" >& 2
 EOF
 chmod a+x fake_editor.sh
 VISUAL=./fake_editor.sh
@@ -59,7 +59,36 @@ EOF
 test_expect_success 'show notes' '
 	! (git cat-file commit HEAD | grep b1) &&
 	git log -1 > output &&
-	git diff expect output
+	test_cmp expect output
+'
+test_expect_success 'create multi-line notes (setup)' '
+	: > a3 &&
+	git add a3 &&
+	test_tick &&
+	git commit -m 3rd &&
+	MSG="b3\nc3c3c3c3\nd3d3d3" git notes edit
+
+'
+
+cat > expect-multiline << EOF
+commit 1584215f1d29c65e99c6c6848626553fdd07fd75
+Author: A U Thor <author@example.com>
+Date:   Thu Apr 7 15:15:13 2005 -0700
+
+    3rd
+
+Notes:
+    b3
+    c3c3c3c3
+    d3d3d3
+EOF
+
+echo >> expect-multiline
+cat expect >> expect-multiline
+
+test_expect_success 'show multi-line notes' '
+	git log -2 > output &&
+	test_cmp expect-multiline output
 '
 
 test_done
-- 
1.6.0.2.GIT

^ permalink raw reply related

* Re: [PATCH 2/3] git-daemon: use getnameinfo to resolve hostname
From: Jay Soffian @ 2009-01-14 14:22 UTC (permalink / raw)
  To: Jeff King; +Cc: Jan Engelhardt, git
In-Reply-To: <20090114122536.GA5939@coredump.intra.peff.net>

On Wed, Jan 14, 2009 at 7:25 AM, Jeff King <peff@peff.net> wrote:
> So at the very least, you should be adding REMOTE_HOST in _addition_ to
> REMOTE_ADDR, not instead of. But that still leaves one final concern,
> which is that some git-daemon admins might not want to pay the cost for
> a reverse lookup for every request. It's extra network traffic, and adds
> extra latency to the process (but I don't personally run git-daemon, and
> I don't know whether big sites like kernel.org actually care about
> this).

Speaking for large sites everywhere, yes they do care. Enabling DNS
lookups must be configurable.

j.

^ permalink raw reply

* Re: [PATCH 2/3] git-daemon: use getnameinfo to resolve hostname
From: Adeodato Simó @ 2009-01-14 14:17 UTC (permalink / raw)
  To: Jeff King; +Cc: Jan Engelhardt, git
In-Reply-To: <20090114122536.GA5939@coredump.intra.peff.net>

* Jeff King [Wed, 14 Jan 2009 07:25:36 -0500]:

> On Wed, Jan 14, 2009 at 11:48:38AM +0100, Jan Engelhardt wrote:

> > This is much shorter than inet_ntop'ing, and also translated
> > unresolvable addresses into a string.

> Er, doesn't this totally change the meaning of REMOTE_ADDR from an IP
> address to a hostname?

Yes, I believe so.

However, AFAIK you can obtain the intended behavior if you pass
NI_NUMERICHOST as a flag to the getnameinfo() call. With that, this
patch can be still considered for inclusing if the original "don't
hardcode protocol-specific bits" is still deemed worthy.

-- 
Adeodato Simó                                     dato at net.com.org.es
Debian Developer                                  adeodato at debian.org
 
- Why are you whispering?
- Because I just think that no matter where she is, my mom can hear this
  conversation.
                -- Rory and Lane

^ permalink raw reply

* Re: Help! My ISP blocks repo.or.cz. How to push changes? (a solution)
From: Jakub Narebski @ 2009-01-14 13:55 UTC (permalink / raw)
  To: Mike Hommey
  Cc: Alex Riesen, git, Markus Heidelberg, Asheesh Laroia,
	Luciano Rocha, J.H.
In-Reply-To: <200901130043.04772.jnareb@gmail.com>

Jakub Narebski wrote:

> The ISP I use (Telekomunikacja Polska S.A., aka TP) made some
> unannounced changes for ADSL service (Neostrada) which made it block
> repo.or.cz (and of course its aliases, including git.or.cz where git
> wiki resides).

Thank you all for your help with arriving at solution. I'll describe it 
below; perhaps it would help somebody else (now that it is in mailing 
list archive).


First, let me explain what I am working with:

I have access to shell account with set up SSH key access; let's name 
this machine host.example.com. I don't have admin rights there, and 
quota is quite tight; I have installed netcat (nc) in ~/bin - it is 
only 22 kB.

I don't know where to find SOCKS5 proxy, and I don't have 'tsocks'
installed either on my computer, or on shell account... I think.


Now, solutions:

1. For reading gitweb at repo.or.cz, and for reading and editing git 
   wiki at http://git.or.cz/gitwiki/ I use one of free HTTP proxies:
   http://www.4proxy.de (first such proxy I have found that has an
   option to _not_ obfuscate URLs; it still unnecessary escapes some
   things like '/' in the query argument).

2. For pushing changes to repo.or.cz I use SSH tunnel (I could have
   used ProxyCommand solution with netcat instead[1]). I run:

   $ autossh -M 2000 -f -N -L 2222:repo.or.cz:22 jnareb@host.example.com

   at startup, and I have set the following in ~/.ssh/config:

   # TP S.A. blocks repo.or.cz
   Host repo.or.cz
   	#ssh -f -N -L 2222:repo.or.cz:22 host.example.com
   	NoHostAuthenticationForLocalhost yes
   	HostName localhost
   	Port 2222

   [1] Alternate solution:

   # TP S.A. blocks repo.or.cz
   Host repo.or.cz
	ProxyCommand ssh host.example.com exec /home/jnareb/bin/nc %h %p

3. For fetching changes via git:// protocol from repo.or.cz I use the
   following setup in git config:

   [core]
   	gitProxy = ssh-proxy for "repo.or.cz"

   Unfortunately example from Documentation/config.txt with "ssh" as
   git proxy command doesn't work, and neither putting command with
   options (e.g. "ssh host.example.com /home/jnareb/bin/nc") doesn't
   work: the command is _not_ split on whitespace. So I had to use
   helper script ~/bin/ssh-proxy:

   #!/bin/sh

   ssh host.example.com /home/jnareb/bin/nc "$1" "$2"


I hope that would help somebody... and if somebody notices better 
solution, hs/she would provide me with it :-)

-- 
Jakub Narebski
Poland

^ permalink raw reply

* Re: [RFC/PATCH] gitweb: Fix nested links problem with ref markers
From: Giuseppe Bilotta @ 2009-01-14 13:52 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: git
In-Reply-To: <200901141139.42263.jnareb@gmail.com>

On Wed, Jan 14, 2009 at 5:39 AM, Jakub Narebski <jnareb@gmail.com> wrote:
> On Wed, 14 Jan 2009, Giuseppe Bilotta wrote:
>> On Tue, Jan 13, 2009 at 7:17 PM, Jakub Narebski <jnareb@gmail.com> wrote:
>>> On Tue, 13 Jan 2009, Giuseppe Bilotta wrote:
>
>>>> Can you provide a patch I can apply to my tree for testing to see how
>>>> it comes up?
>>>
>>> Here it is. Note that CSS could be I think reduced. The size of
>>> gitweb.perl changes is affected by changing calling convention for
>>> git_print_header_html subroutine.
>>
>> It's funny, I was working on a very similar patch myself a couple of
>> days ago, but couldn't get the horizontal filler after the link to
>> work properly, which is why I asked on www-style.
>>
>> I'll test your patch and let you know.
>
> I am checking 'log' view of git repository; it should have enough
> ref markers to test this issue.
>
> It works also in Konqueror 3.5.3-0.2.fc4...
>
>>> There is also strange artifact at least in Mozilla 1.17.2: if I hover
>>> over ref marker, the subject (title) gets darker background. Curious...
>>
>> Might be some kind of bug with the capturing vs bubbling phase.
>
> ...but the same artifact can be seen too.  Also I am not entirely
> pleased with the way things behave on mouseover.  It is a pity that
> you cannot style element using CSS2.1 based on the pseudo-class :hover
> of descendant element, or/and pseudo-class of sibling element, which
> nevertheless overlays given element.

I know, I've been needing something like this in other occasion. And
that's precisely what I was talking about for the limits of CSS, and
why I really wonder if the illegal XHMTL but valid XML shouldn't
rather be our option ...



-- 
Giuseppe "Oblomov" Bilotta

^ permalink raw reply

* Re: git merge and cherry-pick and duplicated commits?
From: Alex Riesen @ 2009-01-14 13:47 UTC (permalink / raw)
  To: Thomas Rast; +Cc: skillzero, git
In-Reply-To: <200901140941.17110.trast@student.ethz.ch>

2009/1/14 Thomas Rast <trast@student.ethz.ch>:
> skillzero@gmail.com wrote:
>> That's what I was somewhat disappointed by. Even though the result of
>> the commit had a different hash, I assumed git would keep some kind of
>> internal per-commit hash so it could tell later that two commits were
>> the same and not re-apply them.
>
> I think there's an important misunderstanding here: merging A into B
> does *not* have anything to do with commits, or history for that
> matter, beyond the differences from $(git merge-base A B) to A and
> B.[*]
>
> Along the same lines, nothing is ever re-applied during merging.
> git-merge just figures out that you made the same change on both
> sides, so it must have been a good change, so it must go into the end
> result.  *How* you arrived at the same change---say, by
> cherry-picking, or by getting the same result in that region from
> otherwise different commits, or even from several commits---does *not*
> matter in any way.

Yes, merge only considers what bytes (aka contents of
trees-directories and blobs-files) do the branches to be merged
have, compares them (by comparing their hashes) and if there
are differences tries to mix them together according to the merge
rules described somewhere in Documentation.

So this all is really just about what the branches contain, not
how they got it. It is the conflict resolution algorithm which uses
the history to find the best possible source blob or tree which was
changed by conflicting branches so the "mix" can be prepared as
close as possible to what would we do if we went looking for the
pieces manually.

^ permalink raw reply

* [BUG] assertion failure in builtin-mv.c with "git mv -k"
From: Matthieu Moy @ 2009-01-14 13:20 UTC (permalink / raw)
  To: git

Hi,

Just found a bug in builtin-mv.c. Here's a script to reproduce:

mkdir git
cd git
git init
touch controled
git add controled && git commit -m "init"
touch foo1 foo2
mkdir dir
git mv -k foo* dir/

The output is the following:

Initialized empty Git repository in /tmp/git/.git/
[master (root-commit)]: created 694563b: "init"
 0 files changed, 0 insertions(+), 0 deletions(-)
 create mode 100644 controled
git: builtin-mv.c:216: cmd_mv: Assertion `pos >= 0' failed.
./bug.sh: line 10: 12919 Aborted                 git mv -k foo* dir/

Apparently, this happens when using "git mv -k" with more than one
unversionned file. The code ignores the first one, but still goes
through this

	for (i = 0; i < argc; i++) {
		const char *src = source[i], *dst = destination[i];
		enum update_mode mode = modes[i];
		int pos;
		if (show_only || verbose)
			printf("Renaming %s to %s\n", src, dst);
		if (!show_only && mode != INDEX &&
				rename(src, dst) < 0 && !ignore_errors)
			die ("renaming %s failed: %s", src, strerror(errno));

		if (mode == WORKING_DIRECTORY)
			continue;

		pos = cache_name_pos(src, strlen(src));
		assert(pos >= 0); /* <----- this is the one */
		if (!show_only)
			rename_cache_entry_at(pos, dst);
	}

for the second, and it crashes on the assertion (gdb says "pos" here
is an unversionned file name).

If anyone has time to fix this ...

Thanks,

-- 
Matthieu

^ permalink raw reply

* Re: [PATCH 3/3] git-daemon: vhost support
From: Jan Engelhardt @ 2009-01-14 13:15 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vmydu2kbj.fsf@gitster.siamese.dyndns.org>


On Wednesday 2009-01-14 12:33, Junio C Hamano wrote:
>> git-daemon: support vhosts
>>
>> Since git clients usually send the target hostname in the request
>> similar to the "Host:" HTTP header, one can do virtual hosting.
>
>Isn't this what --interpolated-path option (especially H and CH
>interpolations) is about?

Looks like it. In this case this third patch is not needed.

^ permalink raw reply

* Re: [PATCH 2/3] git-daemon: use getnameinfo to resolve hostname
From: Jan Engelhardt @ 2009-01-14 13:06 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vsknm2kbs.fsf@gitster.siamese.dyndns.org>


On Wednesday 2009-01-14 12:33, Junio C Hamano wrote:
>> git-daemon: use getnameinfo to resolve hostname
>>
>> This is much shorter than inet_ntop'ing, and also translated
>> unresolvable addresses into a string.
>
>translated?  (I think you meant "translates" but my English is bad, so I
>am double checking).

yes, keyboard slipped away.

>This indeed is much nicer, provided if it is available at least as widely
>as inet_ntop() is.

Both inet_ntop and getnameinfo are in POSIX.1-2001.

> (1) Do we need similar compat/ function for getnameinfo()?  I am guessing
>     that most likely places are the ones that need NO_INET_NTOP and
>     NO_INET_PTON, and googling seems to indicate old Cygwin and HP-UX
>     11.00 may be among them.
>
> (2) Do we still use inet_ntop() elsewhere, and if not, can we remove the
>     compat/ definitions?
>

Yes, it is still used elsewhere.

^ permalink raw reply

* Re: [PATCH 1/3] git-daemon: single-line logs
From: Jan Engelhardt @ 2009-01-14 13:03 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vy6xe2kbx.fsf@gitster.siamese.dyndns.org>


On Wednesday 2009-01-14 12:33, Junio C Hamano wrote:
>
>> parent v1.6.1
>>
>> git-daemon: single-line logs
>
>Please drop these two needless lines when/if you are submitting patches
>for inclusion..

The patches are produced by my git-export-patch script (or in this
specific case it was quilt); so they usually look like this.
Especially when I do make mental notes that do not go into the
commit log, the patch is tacked on, as in, for example,
http://marc.info/?l=netfilter-devel&m=123191159015731&w=2

>> Having just a single line per connection attempt, much like Apache
>> httpd2 access logs, makes log parsing much easier, especially when
>> just glancing over it non-automated.
>
>While I like the motivation, and I wish the log were as terse as possible
>from the day one,

Well, why did no one do it like that then? My guess is that it was not a 
"real" logging infrastructure but more of a debug aid. Especially when 
it required --syslog --verbose to pass to git-daemon this seems like 
--debug=yesPlease.

>I think changing the output format unconditionally like
>this patch does is a horrible idea.  I'd expect there are many people who
>already have their infrastructure set up to parse the current output; this
>patch actively breaks things for them, doesn't it?

Probably. Which just shows that git-daemon is in need of
some configuration .. thing so that each user can choose
his own if desired.


>> @@ -295,12 +295,13 @@ static int git_daemon_config(const char
>> -static int run_service(char *dir, struct daemon_service *service)
>> +static int run_service(char *dir, struct daemon_service *service,
>> +    const char *origin, const char *vhost)
>>  {
>>  	const char *path;
>>  	int enabled = service->enabled;
>>  
>> -	loginfo("Request %s for '%s'", service->name, dir);
>> +	loginfo("%s->%s %s \"%s\"\n", origin, vhost, service->name, dir);
>
>Mental note.  You are adding origin and vhost probably because you are
>losing them from elsewhere..

Not quite sure what you mean by losing.

But in 1.6.0.x, run_service had a variable interp of type itable or so 
and it was possible to use interp[SLOT_DIR].val without someone raising 
a hand declaring I lost them elsewhere ;-)

>> @@ -529,7 +530,6 @@ static int execute(struct sockaddr *addr
>>  			port = ntohs(sin6_addr->sin6_port);
>>  #endif
>>  		}
>> -		loginfo("Connection from %s:%d", addrbuf, port);
>
>Mental note.  Port is not logged anymore here.

Right, I did not see a need for it, and it in itself just stood
in the way of getting 1-line-output.

>> @@ -541,10 +541,6 @@ static int execute(struct sockaddr *addr
>>  	alarm(0);
>>  
>>  	len = strlen(line);
>> -	if (pktlen != len)
>> -		loginfo("Extended attributes (%d bytes) exist <%.*s>",
>> -			(int) pktlen - len,
>> -			(int) pktlen - len, line + len + 1);
>
>Mental note.  XA are not logged here anymore.

I only ever saw the hostname XA, and this is still logged.

>> @@ -569,7 +565,8 @@ static int execute(struct sockaddr *addr
>>  			 * Note: The directory here is probably context sensitive,
>>  			 * and might depend on the actual service being performed.
>>  			 */
>> -			return run_service(line + namelen + 5, s);
>> +			return run_service(line + namelen + 5, s,
>> +			       addrbuf, hostname);
>>  		}
>>  	}
>
>So not just you are changing the format, but you are losing information as
>well.
>
>By the way, I think hostname has already been freed and NULLed at this
>call site.  Aren't you getting entries like:
>
>	192.168.0.1->(null) upload-pack "/pub/git.git"
>
>in your log?

No. Which means someone succeeded at obfuscating daemon.c.
It seems that parse_extra_args() sets hostname again after it has been 
NULLified just moments ago.

^ permalink raw reply

* Re: [PATCH 0/4] refactor the --color-words to make it more hackable
From: Santi Béjar @ 2009-01-14 13:00 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git, Thomas Rast
In-Reply-To: <alpine.DEB.1.00.0901112057300.3586@pacific.mpi-cbg.de>

2009/1/11 Johannes Schindelin <Johannes.Schindelin@gmx.de>:
>

[...]

> The basic idea is to decouple the original text from the text that is
> passed to libxdiff to find the word differences.
>
> To that end, the words of the pre and post texts are put into two lists that
> are fed to libxdiff.  While the words are extracted, an array is created which
> contains pointers back to the word boundaries in the original text.
>

Thanks. With this I will no longer need to add some spurious spaces in
my latex files :-)

I've tested and it seems to work, but there are some corner cases that
it does not handle well. If you have this two files:

---8<--- pre
h(4)

a = b + c
---8<--- post
h(4),hh[44]

a = b + c

aa = a

aeff = aeff * ( aaa )
---8<---

The "git diff" is okay, but not the "git diff --color-words", the
addition of "aeff = ..." is not shown.

Additionally with "git diff --no-index --color-words='^[A-Za-z0-9]*'
the ']' character is not shown as an addition, and instead of the
"aeff" line you get a ")" in green, as:

h(4),{GREEN}hh[44{ENDGREEN}]

a = b + c

{GREEN}aa = a
 ){ENDGREEN}

Also if the lost text is at the end the next "diff --git" line is
printed in read:

--8<---
#!/bin/bash
git init
cat > file <<EOF
a

aa
EOF
cat > gfile <<EOF
a
EOF
git add .
git commit -m "Initial import"
git rm file
cat > gfile <<EOF
b
EOF
git add gfile
git commit -m "changes"
git show --color-words
---8<---

Thanks,
Santi

P.D.: I've test the version that is in 'pu', it does not have the
patch to fix the segfault but I've also tested with it.

^ permalink raw reply

* Re: [PATCH 2/3] git-daemon: use getnameinfo to resolve hostname
From: Jeff King @ 2009-01-14 12:25 UTC (permalink / raw)
  To: Jan Engelhardt; +Cc: git
In-Reply-To: <alpine.LSU.2.00.0901141148130.16109@fbirervta.pbzchgretzou.qr>

On Wed, Jan 14, 2009 at 11:48:38AM +0100, Jan Engelhardt wrote:

> This is much shorter than inet_ntop'ing, and also translated
> unresolvable addresses into a string.

Er, doesn't this totally change the meaning of REMOTE_ADDR from an IP
address to a hostname? That seems bad because:

  - people already have hooks that compare REMOTE_ADDR against an
    address, so we are breaking their hooks

  - we are losing IP information in favor of hostname information; since
    (I assume) this is primarily intended for IP-based access control,
    we are adding an extra layer of indirection in the middle of our
    security model (i.e., I used to have to spoof an IP to fool your
    hook, but now I can do that _or_ spoof DNS).

So at the very least, you should be adding REMOTE_HOST in _addition_ to
REMOTE_ADDR, not instead of. But that still leaves one final concern,
which is that some git-daemon admins might not want to pay the cost for
a reverse lookup for every request. It's extra network traffic, and adds
extra latency to the process (but I don't personally run git-daemon, and
I don't know whether big sites like kernel.org actually care about
this).

-Peff

^ permalink raw reply

* Re: [PATCH 3/3] git-daemon: vhost support
From: Junio C Hamano @ 2009-01-14 11:33 UTC (permalink / raw)
  To: Jan Engelhardt; +Cc: git
In-Reply-To: <alpine.LSU.2.00.0901141148390.16109@fbirervta.pbzchgretzou.qr>

Jan Engelhardt <jengelh@medozas.de> writes:

> parent v1.6.1
>
> git-daemon: support vhosts
>
> Since git clients usually send the target hostname in the request
> similar to the "Host:" HTTP header, one can do virtual hosting.

Isn't this what --interpolated-path option (especially H and CH
interpolations) is about?

^ permalink raw reply

* Re: [PATCH 2/3] git-daemon: use getnameinfo to resolve hostname
From: Junio C Hamano @ 2009-01-14 11:33 UTC (permalink / raw)
  To: Jan Engelhardt; +Cc: git
In-Reply-To: <alpine.LSU.2.00.0901141148130.16109@fbirervta.pbzchgretzou.qr>

Jan Engelhardt <jengelh@medozas.de> writes:

> parent v1.6.1
>
> git-daemon: use getnameinfo to resolve hostname
>
> This is much shorter than inet_ntop'ing, and also translated
> unresolvable addresses into a string.

translated?  (I think you meant "translates" but my English is bad, so I
am double checking).

This indeed is much nicer, provided if it is available at least as widely
as inet_ntop() is.

We seem to ship inet_ntop() in compat/; a few questions.

 (1) Do we need similar compat/ function for getnameinfo()?  I am guessing
     that most likely places are the ones that need NO_INET_NTOP and
     NO_INET_PTON, and googling seems to indicate old Cygwin and HP-UX
     11.00 may be among them.

 (2) Do we still use inet_ntop() elsewhere, and if not, can we remove the
     compat/ definitions?

^ permalink raw reply

* Re: [PATCH 1/3] git-daemon: single-line logs
From: Junio C Hamano @ 2009-01-14 11:33 UTC (permalink / raw)
  To: Jan Engelhardt; +Cc: git
In-Reply-To: <alpine.LSU.2.00.0901141147120.16109@fbirervta.pbzchgretzou.qr>

Jan Engelhardt <jengelh@medozas.de> writes:

> parent v1.6.1
>
> git-daemon: single-line logs

Please drop these two needless lines when/if you are submitting patches
for inclusion..

> Having just a single line per connection attempt, much like Apache
> httpd2 access logs, makes log parsing much easier, especially when
> just glancing over it non-automated.

While I like the motivation, and I wish the log were as terse as possible
from the day one, I think changing the output format unconditionally like
this patch does is a horrible idea.  I'd expect there are many people who
already have their infrastructure set up to parse the current output; this
patch actively breaks things for them, doesn't it?

> @@ -295,12 +295,13 @@ static int git_daemon_config(const char
>  	return 0;
>  }
>  
> -static int run_service(char *dir, struct daemon_service *service)
> +static int run_service(char *dir, struct daemon_service *service,
> +    const char *origin, const char *vhost)
>  {
>  	const char *path;
>  	int enabled = service->enabled;
>  
> -	loginfo("Request %s for '%s'", service->name, dir);
> +	loginfo("%s->%s %s \"%s\"\n", origin, vhost, service->name, dir);

Mental note.  You are adding origin and vhost probably because you are
losing them from elsewhere..

> @@ -507,10 +508,10 @@ static void parse_extra_args(char *extra
>  static int execute(struct sockaddr *addr)
>  {
>  	static char line[1000];
> +	char addrbuf[256] = "";
>  	int pktlen, len, i;
>  
>  	if (addr) {
> -		char addrbuf[256] = "";
>  		int port = -1;
>  
>  		if (addr->sa_family == AF_INET) {
> @@ -529,7 +530,6 @@ static int execute(struct sockaddr *addr
>  			port = ntohs(sin6_addr->sin6_port);
>  #endif
>  		}
> -		loginfo("Connection from %s:%d", addrbuf, port);

Mental note.  Port is not logged anymore here.

> @@ -541,10 +541,6 @@ static int execute(struct sockaddr *addr
>  	alarm(0);
>  
>  	len = strlen(line);
> -	if (pktlen != len)
> -		loginfo("Extended attributes (%d bytes) exist <%.*s>",
> -			(int) pktlen - len,
> -			(int) pktlen - len, line + len + 1);

Mental note.  XA are not logged here anymore.

> @@ -569,7 +565,8 @@ static int execute(struct sockaddr *addr
>  			 * Note: The directory here is probably context sensitive,
>  			 * and might depend on the actual service being performed.
>  			 */
> -			return run_service(line + namelen + 5, s);
> +			return run_service(line + namelen + 5, s,
> +			       addrbuf, hostname);
>  		}
>  	}

So not just you are changing the format, but you are losing information as
well.

By the way, I think hostname has already been freed and NULLed at this
call site.  Aren't you getting entries like:

	192.168.0.1->(null) upload-pack "/pub/git.git"

in your log?

^ 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