Git development
 help / color / mirror / Atom feed
* 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: [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

* [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: [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 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

* [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 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 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

* 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

* 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 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: [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

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

Hi,

On Wed, 14 Jan 2009, Clemens Buchacher wrote:

> @@ -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;
> +}

I would prefer something like this:

	static int has_special(const char *p)
	{
		while (*p)
			if (isspecial(*(p++)))
				return 1;
		return 0;
	}

but that is probably a matter of taste.

Ciao,
Dscho

^ permalink raw reply

* Re: [BUG] assertion failure in builtin-mv.c with "git mv -k"
From: Johannes Schindelin @ 2009-01-14 15:54 UTC (permalink / raw)
  To: Michael J Gruber; +Cc: Matthieu Moy, git
In-Reply-To: <496DFC75.2000904@drmicha.warpmail.net>

Hi,

On Wed, 14 Jan 2009, Michael J Gruber wrote:

> I'll send a patch but I'm not sure if this needs a test case.

Umm, Michael, you have been here long enough to know that the answer is a 
"YES!".  If you fix something, you want to provide a test case just to 
make sure you do not need to fix it again later.

Ciao,
Dscho

^ permalink raw reply

* Re: git merge and cherry-pick and duplicated commits?
From: Sitaram Chamarty @ 2009-01-14 15:53 UTC (permalink / raw)
  To: git
In-Reply-To: <2729632a0901132333h6caf9facu871869abce5597c1@mail.gmail.com>

On 2009-01-14, skillzero@gmail.com <skillzero@gmail.com> wrote:
> I guess maybe a better question is how do people normally handle
> situations like mine where I did some work on branch X and I later
> realize I need only a portion of that work on branch Y? I'm not sure
> how I can change my workflow to completely eliminate these situations.
> For example, I often start a branch to add a new feature and I end up
> fixing bug A on that branch. Then other people on my team decide they
> need the fix for bug A immediately and can't wait for me to finish my
> feature branch and do a full merge.
>
> Is there some way I can change my workflow such that I can fix bug A
> (maybe on a separate branch?) and somehow apply it to both both
> branches in a way that won't result in duplicate commits?
>
> Does this kind of thing ever happen with the Linux kernel or git
> itself: somebody does a fix as part of their topic branch and the
> Linux kernel or git master wants that particular fix now, but is not
> ready for the full topic branch? Would they just suggest the fix be
> separated into its own topic branch and that merged? If so, how would
> that new topic branch merge into the original topic branch without
> resulting in a duplicate commit when it's later merged into master?

If I understand you right, you're talking about a situation
where a particular change is in two different branches, but
the SHA is different.

And yet, in your scenario, both of these branches somehow
get merged into the final branch.

In other words, a DAG of branches merging itself has a
merge, if that makes sense :-)

You may need to think about how commits flow from branch to
branch, and what branches are private, and therefore can be
rebased or change history, and what are public, and how
often and when the private ones rebase off of the public
ones.

^ permalink raw reply

* [StGit] import --mail/--mbox question
From: Shinya Kuribayashi @ 2009-01-14 15:35 UTC (permalink / raw)
  To: git

Hi,

I've been wondering this for quite some time, now try to sort out.

---

Question: when importing Mozilla Thunderbird mails (eml or mbox), the
imported patch always have committer's date in git log, not submitter's
date.  However, if importing the same mails with git am, we could see
submitter's date in git log.

Is this intentionally-designed log management of StGit?  I would expect
the submitter's date & locale is kept when importing patches from
e-mails.

Reproducible: Always

Steps to Reproduce:

1. Prepare eml or mbox file.

  [Thunderbird eml file]
   - Select a mail,
   - File -> Save as -> File, 
   - Format -> Mail files

  [mbox file]
   - use add-ons, such as ImportExportTools(MboxImport)
   - extract directly from profile/ dir

2. Import eml/mbox file with --mail/--mbox option

  $ stg import --mail [eml file]
  $ stg import --mbox [mbox file]

3. Check git log

Example:

  Mbox header (this mbox contains one patch)
  ------------------------------------------
  From - Wed Jan  7 00:41:57 2009
  X-Account-Key: account3
  X-UIDL: 6d997fc0c53b4541c56eeeeb45732171
  X-Mozilla-Status: 0001
  X-Mozilla-Status2: 00000000
  X-Mozilla-Keys:                                                                                 X-DTI-Virus-Check: checked
  X-DTI-Recipient: <skuribay@ruby.dti.ne.jp>
  Return-Path: <sr@denx.de>
  Received: from moutng.kundenserver.de (moutng.kundenserver.de [212.227.126.188]) by pop.ruby.dti.ne.jp (3.10p) with ESMTP id n06AZr2v009835 for <skuribay@ruby.dti.ne.jp>; Tue, 6 Jan 2009 19:35:53 +0900 (JST)
  Received: from localhost.localdomain (p5B2A461A.dip.t-dialin.net [91.42.70.26])
          by mrelayeu.kundenserver.de (node=mrelayeu1) with ESMTP (Nemesis)
          id 0MKwpI-1LK9Hn0ym1-0002Ja; Tue, 06 Jan 2009 11:35:51 +0100
  From: Stefan Roese <sr@denx.de>
  To: u-boot@lists.denx.de
  Cc: skuribay@ruby.dti.ne.jp
  Subject: [PATCH 1/3 v2] MIPS: Add VCT board series support (Part 1/3)
  Date: Tue,  6 Jan 2009 11:35:46 +0100
  Message-Id: <1231238146-2813-1-git-send-email-sr@denx.de>
  X-Mailer: git-send-email 1.6.1
  X-Provags-ID: V01U2FsdGVkX19dTpfkGdmBDPbfKKBfEQYFLGbts7teQcJyLTZ
   xUdZWxGujKNfCpOIRv/s7i7nrf13E4zGP6P0nibSP6yFfDLBv3
   0qZDbTBuPI1ViEQrnIGZCASl9Z6H8em

  This patch adds support for the Micronas VCT board series.
  Currently the following platforms are supported:
  :
  :

  Resulting git log
  -----------------
  commit b659910dd0d75220c3cdbd3408a5025340e6a562
  Author: Stefan Roese <sr@denx.de>
  Date:   Thu Jan 15 00:06:27 2009 +0900 <-- this is my date & locale(JP)

      MIPS: Add VCT board series support (Part 1/3)

      This patch adds support for the Micronas VCT board series.
      Currently the following platforms are supported:

        vct_premium
        vct_premium_small
        vct_premium_onenan
  :
  :

Version:

  Stacked GIT 0.14.3.327.ge4f6.dirty
  git version 1.5.4.3
  Python version 2.5.2 (r252:60911, Jul 31 2008, 17:28:52) 
  [GCC 4.2.3 (Ubuntu 4.2.3-2ubuntu7)]


Thanks in advance,

  Shinya

^ permalink raw reply

* Re: [PATCH 3/3] implement pattern matching in ce_path_match
From: Sverre Rabbelier @ 2009-01-14 15:55 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Clemens Buchacher, git, Junio C Hamano, johannes
In-Reply-To: <alpine.DEB.1.00.0901141641500.3586@pacific.mpi-cbg.de>

On Wed, Jan 14, 2009 at 16:44, Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
>> +static int has_special(const char *p)
>> +{
>> +     int x;
>> +
>> +     while ((x = *p++) != '\0')
>> +             if (isspecial(x))
>> +                     return 1;
>> +
>> +     return 0;
>> +}
>
> I would prefer something like this:
>
>        static int has_special(const char *p)
>        {
>                while (*p)
>                        if (isspecial(*(p++)))
>                                return 1;
>                return 0;
>        }
>
> but that is probably a matter of taste.

FWIW, I think the above is a lot less readable due to the assignment
in the while loop's conditional. Whereas in Dscho's version it is
intuitively obvious what the termination condition of the while loop
is.

-- 
Cheers,

Sverre Rabbelier

^ permalink raw reply

* Re: git Thunderbird Synching
From: Sitaram Chamarty @ 2009-01-14 16:01 UTC (permalink / raw)
  To: git
In-Reply-To: <496D99BA.6000208@vt.edu>

On 2009-01-14, Nicholas LaRoche <nlaroche@vt.edu> wrote:
> I want to do something like this with my main profile, but I'm concerned 
> that if I send/receive email on either machine independently that there 
> will be corruption in some of the files when I push back to my main box.

The mbox files making up the actual mail are probably safe
enough, assuming you do the git operations with TB shut
down, not running.  In effect, each message is one chunk of
code, and you're basically deleting or adding them.  Ugly,
but it would probably work.

But I expect serious trouble with the MSF files that TB
maintains, symptoms being quick searches not working or
showing something in the message list pane and some other
unrelated message in the preview pane.

And that's if you can get git to merge them in the first
place -- which I very much doubt.  For all practical
purposes they're binary blobs.

So if you don't care about the MSF at all and can rebuild
them each time, this would work.

Otherwise IMAP is a better option :-)

^ permalink raw reply

* Re: [BUG] assertion failure in builtin-mv.c with "git mv -k"
From: Michael J Gruber @ 2009-01-14 16:04 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Matthieu Moy, git
In-Reply-To: <alpine.DEB.1.00.0901141653540.3586@pacific.mpi-cbg.de>

Johannes Schindelin venit, vidit, dixit 14.01.2009 16:54:
> Hi,
> 
> On Wed, 14 Jan 2009, Michael J Gruber wrote:
> 
>> I'll send a patch but I'm not sure if this needs a test case.
> 
> Umm, Michael, you have been here long enough to know that the answer is a 
> "YES!".  If you fix something, you want to provide a test case just to 
> make sure you do not need to fix it again later.
> 
> Ciao,
> Dscho
> 

It was a lame attempt at getting around it, it's just one line... I
didn't know I've been being noticed long enough ;)
So, should I prepare a series like:

1: test case and mark known fail
2: the 1 line fix
3: mark test pass

Or should 2+3 be squashed into one?

Cheers,
Michael

^ permalink raw reply

* Re: [PATCH 2/3] remove pathspec_match, use match_pathspec instead
From: Clemens Buchacher @ 2009-01-14 16:09 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git, Junio C Hamano, johannes
In-Reply-To: <alpine.DEB.1.00.0901141637170.3586@pacific.mpi-cbg.de>

On Wed, Jan 14, 2009 at 04:40:42PM +0100, Johannes Schindelin wrote:
> > Both versions have the same functionality. This removes any
> > redundancy.
> >
> > This also adds makes two extensions to match_pathspec:
> 
> s/makes//

Thanks.

> 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.)

It sure does. I am not confident enough to send untested patches yet. :-)

On Wed, Jan 14, 2009 at 04:44:36PM +0100, Johannes Schindelin wrote:
> I would prefer something like this:
> 
>       static int has_special(const char *p)
>       {
>               while (*p)
>                       if (isspecial(*(p++)))
>                               return 1;
>               return 0;
>       }

Ok, no problem.

^ permalink raw reply

* [Q] git rebase -i -p conflicts with squash
From: Constantine Plotnikov @ 2009-01-14 16:13 UTC (permalink / raw)
  To: git

If I run git rebase --interactive with --preserve-merges option and
select "squash" for one of the commit, the rebase process fails with
the message "Refusing to squash a merge:
5e775c536654640c173ba71a0af7e84bf8bc618a". However the neither commit
participating in the squash is a merge commit. Even more, there are no
merge commits in the repository at all.

>From my limited understanding of squash operation, it should fail only
if one of squashed commits is a merge commit, but it should be
possible to squash non-merge commits without problem as it looks like
quite safe and local operation (and I might want to preserve merges
that happened after squashed commits). Is it the current behaviour a
bug or a feature?

Constantine

^ permalink raw reply

* [PATCH next v2] git-notes: add test case for multi-line notes
From: Tor Arne Vestbø @ 2009-01-14 16:28 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Junio C Hamano, git
In-Reply-To: <alpine.DEB.1.00.0901141627440.3586@pacific.mpi-cbg.de>

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.

Signed-off-by: Tor Arne Vestbø <tavestbo@trolltech.com>
---

Thanks for the feedback Johannes! Here's an updated patch. I removed
the blank line instead of adding another, as that's the current style
of that file.

 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..e260d79 100755
--- a/t/t3301-notes.sh
+++ b/t/t3301-notes.sh
@@ -8,8 +8,9 @@ test_description='Test commit notes'
 . ./test-lib.sh
 
 cat > fake_editor.sh << \EOF
-echo "$MSG" > "$1"
-echo "$MSG" >& 2
+MSG=${MSG//%/}
+printf "$MSG" > "$1"
+printf "$MSG" >& 2
 EOF
 chmod a+x fake_editor.sh
 VISUAL=./fake_editor.sh
@@ -59,7 +60,35 @@ 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
+
+printf "\n" >> 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 3/3] implement pattern matching in ce_path_match
From: Samuel Tardieu @ 2009-01-14 16:18 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Clemens Buchacher, git, Junio C Hamano, johannes
In-Reply-To: <alpine.DEB.1.00.0901141641500.3586@pacific.mpi-cbg.de>

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

Dscho> I would prefer something like this:

My taste would favor:

  static int has_special(const char *p)
  {
    for (; *p; p++)
      if (isspecial(*p))
        return 1;
    return 0;
  }

as it underlines the intent (loop over "p" characters and stop no
later than the end of the string) while avoiding using side effects in
the body to increment the pointer. This habit comes from Ada, where
loop indices are considered read-only in the loop body.

It also eases further extensions such as

  static int has_special(const char *p)
  {
    for (; *p; p++)
      if (isspecial(*p) || isveryspecial(*p))
        return 1;
    return 0;
  }

without having to move the "++" somewhere else.

Dscho> but that is probably a matter of taste.

Agreed.

  Sam
-- 
Samuel Tardieu -- sam@rfc1149.net -- http://www.rfc1149.net/

^ permalink raw reply

* Re: [ANNOUNCE] tig-0.13
From: Ted Pavlic @ 2009-01-14 16:33 UTC (permalink / raw)
  To: Jonas Fonseca; +Cc: git
In-Reply-To: <20090113233643.GA28898@diku.dk>

Thanks for this. It's a nice tool.

> What is tig?
> ------------
> Tig is an ncurses-based text-mode interface for git. It functions mainly
> as a git repository browser, but can also assist in staging changes for
> commit at chunk level and act as a pager for output from various git
> commands.
>
>   - Homepage:	http://jonas.nitro.dk/tig/
>   - Manual:	http://jonas.nitro.dk/tig/manual.html
>   - Tarballs:	http://jonas.nitro.dk/tig/releases/
>   - Git URL:	git://repo.or.cz/tig.git
>   - Gitweb:	http://repo.or.cz/w/tig.git

-- 
Ted Pavlic <ted@tedpavlic.com>

   Please visit my ALS association page:
         http://web.alsa.org/goto/tedpavlic
   My family appreciates your support in the fight to defeat ALS.

^ 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