Git development
 help / color / mirror / Atom feed
* [PATCH 1/2] Introduce leaky().
From: Pierre Habouzit @ 2008-06-24 20:14 UTC (permalink / raw)
  To: git; +Cc: gitster, Pierre Habouzit
In-Reply-To: <1214338474-16822-1-git-send-email-madcoder@debian.org>

This can be used to mark allocated memory as a "leak". This can be used to
collect memory at the exit of the command, so that tools like valgrind can
be used to check for actual memory leak without noise.

COLLECT_LEAKS_AT_EXIT must be set to that purpose, else 'leaky' is the
transparent macro.

Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---
 Makefile |    5 +++++
 alloc.c  |   20 ++++++++++++++++++++
 cache.h  |    5 +++++
 3 files changed, 30 insertions(+), 0 deletions(-)

diff --git a/Makefile b/Makefile
index 9c16482..6be15c8 100644
--- a/Makefile
+++ b/Makefile
@@ -151,6 +151,8 @@ all::
 # Define NO_EXTERNAL_GREP if you don't want "git grep" to ever call
 # your external grep (e.g., if your system lacks grep, if its grep is
 # broken, or spawning external process is slower than built-in grep git has).
+#
+# Define COLLECT_LEAKS_AT_EXIT if you want memory marked as leaky() at exit.
 
 GIT-VERSION-FILE: .FORCE-GIT-VERSION-FILE
 	@$(SHELL_PATH) ./GIT-VERSION-GEN
@@ -947,6 +949,9 @@ endif
 ifdef NO_EXTERNAL_GREP
 	BASIC_CFLAGS += -DNO_EXTERNAL_GREP
 endif
+ifdef COLLECT_LEAKS_AT_EXIT
+	BASIC_CFLAGS += -DCOLLECT_LEAKS_AT_EXIT
+endif
 
 ifeq ($(TCLTK_PATH),)
 NO_TCLTK=NoThanks
diff --git a/alloc.c b/alloc.c
index 216c23a..1afe810 100644
--- a/alloc.c
+++ b/alloc.c
@@ -74,3 +74,23 @@ void alloc_report(void)
 	REPORT(commit);
 	REPORT(tag);
 }
+
+#ifdef COLLECT_LEAKS_AT_EXIT
+static void **leaks;
+int leaknb, leaksz;
+
+static void release_leaks(void)
+{
+	while (leaknb-- > 0)
+		free(*leaks++);
+	free(leaks);
+}
+
+void *leaky(void *ptr)
+{
+	if (leaksz == 0)
+		atexit(&release_leaks);
+	ALLOC_GROW(leaks, leaknb + 1, leaksz);
+	return leaks[leaknb++] = ptr;
+}
+#endif
diff --git a/cache.h b/cache.h
index 101ead5..33603bb 100644
--- a/cache.h
+++ b/cache.h
@@ -777,6 +777,11 @@ int decode_85(char *dst, const char *line, int linelen);
 void encode_85(char *buf, const unsigned char *data, int bytes);
 
 /* alloc.c */
+#ifdef COLLECT_LEAKS_AT_EXIT
+extern void *leaky(void *);
+#else
+# define leaky(x) x
+#endif
 extern void *alloc_blob_node(void);
 extern void *alloc_tree_node(void);
 extern void *alloc_commit_node(void);
-- 
1.5.6.120.g3adb8.dirty

^ permalink raw reply related

* [RFC] leaky()
From: Pierre Habouzit @ 2008-06-24 20:14 UTC (permalink / raw)
  To: git; +Cc: gitster

In the parse-options thread, it appears that we may somethings leak some
tiny bits of memory on purpose. So that it makes life of people
searching for real memory leaks easier.

The second patch marks the leaks I'm responsible for in git (and that
I'm aware of) as leaky().

^ permalink raw reply

* Re: [PATCH 1/2] Introduce leaky().
From: Pierre Habouzit @ 2008-06-24 20:16 UTC (permalink / raw)
  To: git; +Cc: gitster
In-Reply-To: <1214338474-16822-2-git-send-email-madcoder@debian.org>

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

On Tue, Jun 24, 2008 at 08:14:33PM +0000, Pierre Habouzit wrote:
> +static void release_leaks(void)
> +{
> +	while (leaknb-- > 0)
> +		free(*leaks++);
crap I sent the wrong patch... this is supposed to be:
+		free(leaks[leaknb]);
> +	free(leaks);
> +}

-- 
·O·  Pierre Habouzit
··O                                                madcoder@debian.org
OOO                                                http://www.madism.org

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

^ permalink raw reply

* Re: [PATCH] pre-commit hook should ignore carriage returns at EOL
From: Ian Hilt @ 2008-06-24 20:36 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: Christian Holtje, git, gitster
In-Reply-To: <200806242209.07135.jnareb@gmail.com>

On Tue, 24 Jun 2008 at 10:09pm +0200, Jakub Narebski wrote:

> Unfortunately \r matches \s (is whitespace), so if line ends with CR LF
> ("\r\n") it wouldn't match first regexp, so it would go to 'else' 
> clause, where it would match /\s$/ and it shouldn't.

Right.  Sorry for the noise.

-- 
Ian Hilt
Ian.Hilt (at) gmx.com
GnuPG key: 0x4AFC1EE3

^ permalink raw reply

* Re: [NON-TOY PATCH] git bisect: introduce 'fixed' and 'unfixed'
From: Johannes Schindelin @ 2008-06-24 20:38 UTC (permalink / raw)
  To: Michael Haggerty; +Cc: Jeff King, git
In-Reply-To: <486153DB.3070502@alum.mit.edu>

Hi,

On Tue, 24 Jun 2008, Michael Haggerty wrote:

> Johannes Schindelin wrote:
> > When you look for a fix instead of a regression, it can be quite hard
> > to twist your brain into choosing the correct bisect command between
> > 'git bisect bad' and 'git bisect good'.
> > 
> > So introduce the commands 'git bisect fixed' and 'git bisect unfixed'.
> 
> It seems to me that your problem is that git-bisect requires the "good" 
> revision to be older than the "bad" one.  If this requirement were 
> removed, would there still be a need for "fixed" vs. "unfixed"?

Nope.

The thing that makes "fixed" and "bad" special is that _one_ commit 
introduced that.

Ciao,
Dscho

^ permalink raw reply

* Re: What's cooking in git.git (topics)
From: Junio C Hamano @ 2008-06-24 20:44 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Miklos Vajna, Pieter de Bie, git
In-Reply-To: <alpine.DEB.1.00.0806242007150.9925@racer>

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

>> Using fc48199 ("Merge branch 'master' into next", which includes
>> ed99a225) on the server, v1.5.6 on the client, I get: 
>> 
>> $ git clone server:/home/vmiklos/git/test next
>> Initialize next/.git
>> Initialized empty Git repository in /home/vmiklos/scm/git/next/.git/
>> vmiklos@server's password:
>> bash: git-upload-pack: command not found
>> fatal: The remote end hung up unexpectedly
>
> Hmm.  Probably the client needs to be newer, too.  This is going to be 
> painful.  Junio?

Even with maint client accessing an account with next git-shell as its
login shell, I do not get the above failure.

Is git-shell installed and configured correctly at all in Miklos's setup?
Why does the other side say "bash: git-upload-pack" when login shell is
git-shell and not bash?

^ permalink raw reply

* Re: [PATCH v2/RFC] git.el: Commands for committing patches
From: Nikolaj Schumacher @ 2008-06-24 20:44 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Alexandre Julliard
In-Reply-To: <7vlk0vhmeb.fsf@gitster.siamese.dyndns.org>

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

> I do not use things in contrib/emacs/git.el myself, but this looks like a
> lot of code to do what "M-| git am <Enter>" already does...

I admit committing email buffers might be unnecessary.  I consider it
more of a cherry-on-top.  The important part is committing patches,
because it gives way to partial commits (below file level).


regards,
Nikolaj Schumacher

^ permalink raw reply

* Re: [PATCH 6/7] parse-opt: add PARSE_OPT_KEEP_ARGV0 parser option.
From: Pierre Habouzit @ 2008-06-24 20:55 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: git, gitster, peff, Johannes.Schindelin
In-Reply-To: <alpine.LFD.1.10.0806241015390.2926@woody.linux-foundation.org>

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

On Tue, Jun 24, 2008 at 05:18:56PM +0000, Linus Torvalds wrote:
> 
> 
> On Tue, 24 Jun 2008, Pierre Habouzit wrote:
> >
> > This way, argv[0] isn't clobbered, to the cost of maybe not having a
> > resulting NULL terminated argv array.
> 
> Umm. I think it's much easier to do by always having
> 
> 	ctx->out  = argv;
> 
> and then just initializing cpix to 0 or 1:
> 
> 	ctx->cpidx = ((flags & PARSE_OPT_KEEP_ARGV0) != 0);
> 
> because now parse_options_end() doesn't need to play games any more. It 
> doesn't need to care about PARSE_OPT_KEEP_ARGV0, it can just do exactly 
> what it always used to do, because "ctx->cpidx + ctx->argc" automatically 
> does the right thing (it is both the return value _and_ the index that you 
> should fill with NULL.

It indeed is now a trivial patch:

-----8<-----

Subject: [PATCH] parse-opt: add PARSE_OPT_KEEP_ARGV0 parser option.

This way, argv[0] isn't clobbered.

Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---
 parse-options.c |    1 +
 parse-options.h |    1 +
 2 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/parse-options.c b/parse-options.c
index 60a11e8..51a44e3 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -248,6 +248,7 @@ void parse_options_start(struct parse_opt_ctx_t *ctx,
 	ctx->argc = argc - 1;
 	ctx->argv = argv + 1;
 	ctx->out  = argv;
+	ctx->cpidx = ((flags & PARSE_OPT_KEEP_ARGV0) != 0);
 	ctx->flags = flags;
 }
 
diff --git a/parse-options.h b/parse-options.h
index b391bb6..6299632 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -20,6 +20,7 @@ enum parse_opt_type {
 enum parse_opt_flags {
 	PARSE_OPT_KEEP_DASHDASH = 1,
 	PARSE_OPT_STOP_AT_NON_OPTION = 2,
+	PARSE_OPT_KEEP_ARGV0 = 4,
 };
 
 enum parse_opt_option_flags {
-- 
1.5.6.120.g3adb8.dirty


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

^ permalink raw reply related

* Re: Segmentation fault on http clone, post-1.5.6
From: Teemu Likonen @ 2008-06-24 20:55 UTC (permalink / raw)
  To: Nicolas Pitre; +Cc: Jeff King, git
In-Reply-To: <alpine.LFD.1.10.0806241524480.2979@xanadu.home>

Nicolas Pitre wrote (2008-06-24 15:34 -0400):

> I'm trying to reproduce your segfault with current master
> (v1.5.6-56-g29b0d01) but I just can't.
> 
> Can you provide a gdb backtrace of the segfault?

Let's hope I'm doing this right. Just installed gdb for the first time.
I started with "gdb git" and then typed "run clone http://...".
Eventually it gave this:


Program received signal SIGSEGV, Segmentation fault.
0x080cb69a in find_pack_entry (sha1=0x81675bc "\224�d�BX\006�\020��\016�2\214\002\n�R", e=0xbfcccd18, ignore_packed=0x0) at cache.h:489
489             return memcmp(sha1, sha2, 20);


And "bt" command prints this:


#0  0x080cb69a in find_pack_entry (sha1=0x81675bc "\224�d�BX\006�\020��\016�2\214\002\n�R", e=0xbfcccd18, ignore_packed=0x0)
    at cache.h:489
#1  0x080cd05f in read_packed_sha1 (sha1=0x6900736b <Address 0x6900736b out of bounds>, type=0xbfccede4, size=0xbfccede8)
    at sha1_file.c:1924
#2  0x080cd211 in read_sha1_file (sha1=0x81675bc "\224�d�BX\006�\020��\016�2\214\002\n�R", type=0xbfccede4, size=0xbfccede8)
    at sha1_file.c:2016
#3  0x080b468a in parse_object (sha1=0x81675bc "\224�d�BX\006�\020��\016�2\214\002\n�R") at object.c:190
#4  0x080d69bd in walker_fetch (walker=0x8147610, targets=89, target=0x8163270, write_ref=0x0, write_ref_log_details=0x0) at walker.c:182
#5  0x080d16d1 in fetch_objs_via_curl (transport=0x81475e0, nr_objs=89, to_fetch=0x81630a0) at transport.c:369
#6  0x080d0c57 in transport_fetch_refs (transport=0x81475e0, refs=0x8163020) at transport.c:814
#7  0x0805be15 in cmd_clone (argc=2, argv=dwarf2_read_address: Corrupted DWARF expression.
) at builtin-clone.c:465
#8  0x0804b081 in handle_internal_command (argc=2, argv=0xbfcd1568) at git.c:252
#9  0x0804b272 in main (argc=0, argv=0x8165240) at git.c:448

^ permalink raw reply

* [REPLACEMENT PATCH] parse-opt: fake short strings for callers to believe in.
From: Pierre Habouzit @ 2008-06-24 20:58 UTC (permalink / raw)
  To: git; +Cc: torvalds, gitster, peff, Johannes.Schindelin
In-Reply-To: <1214298732-6247-6-git-send-email-madcoder@debian.org>

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

If we begin to parse -abc and that the parser knew about -a and -b, it
will fake a -c switch for the caller to deal with.

Of course in the case of -acb (supposing -c is not taking an argument) the
caller will have to be especially clever to do the same thing. We could
think about exposing an API to do so if it's really needed, but oh well...

Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---

  On top of my leaky() mini series, one can on purpose leak some memory
  here, and avoid the strbuf and limitations at once. It means that in my
  current git-blame proof of concept one can drop the strdup(), which
  makes it slightly less disgusting.

 parse-options.c |    9 +++++++++
 parse-options.h |    5 +++++
 2 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/parse-options.c b/parse-options.c
index 90935f3..60a11e8 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -1,5 +1,6 @@
 #include "git-compat-util.h"
 #include "parse-options.h"
+#include "cache.h"
 
 #define OPT_SHORT 1
 #define OPT_UNSET 2
@@ -257,6 +258,9 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,
                        const struct option *options,
                        const char * const usagestr[])
 {
+	/* we must reset ->opt, unknown short option leave it dangling */
+	ctx->opt = NULL;
+
 	for (; ctx->argc; ctx->argc--, ctx->argv++) {
 		const char *arg = ctx->argv[0];
 
@@ -286,6 +290,11 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,
 				case -1:
 					return parse_options_usage(usagestr, options);
 				case -2:
+					/* fake a short option thing to hide the fact that we may have
+					 * started to parse aggregated stuff
+					 */
+					ctx->argv[0] = leaky(xstrdup(ctx->opt - 1));
+					*(char *)ctx->argv[0] = '-';
 					return PARSE_OPT_UNKNOWN;
 				}
 			}
diff --git a/parse-options.h b/parse-options.h
index 9da5e8c..b391bb6 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -119,6 +119,11 @@ enum {
 	PARSE_OPT_UNKNOWN,
 };
 
+/*
+ * It's okay for the caller to consume argv/argc in the usual way.
+ * Other fields of that structure are private to parse-options and should not
+ * be modified in any way.
+ */
 struct parse_opt_ctx_t {
 	const char **argv;
 	const char **out;
-- 
1.5.6.120.g3adb8.dirty


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

^ permalink raw reply related

* [JGIT PATCH 1/4] LsTree: Do not use the default first empty tree in the walker
From: Robin Rosenberg @ 2008-06-24 21:20 UTC (permalink / raw)
  To: git; +Cc: Shawn O. Pearce, Marek Zawirski, Florian Koeberle,
	Robin Rosenberg
In-Reply-To: <20080622233525.GJ11793@spearce.org>

In f0ef5e1ef09d346432fead17bc82d78b7cfbd621 an empty tree
was added to all TreeWalkers.

Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
---
 .../src/org/spearce/jgit/pgm/LsTree.java           |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/org.spearce.jgit/src/org/spearce/jgit/pgm/LsTree.java b/org.spearce.jgit/src/org/spearce/jgit/pgm/LsTree.java
index 1bc7bbd..05ec8c3 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/pgm/LsTree.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/pgm/LsTree.java
@@ -65,6 +65,7 @@ class LsTree extends TextBuiltin {
 		else if (argi + 1 < args.length)
 			throw die("too many arguments");
 
+		walk.reset(); // drop the first empty tree, which we do not need here
 		final String n = args[argi];
 		if (is_WorkDir(n))
 			walk.addTree(new FileTreeIterator(new File(n)));
-- 
1.5.5.1.178.g1f811

^ permalink raw reply related

* [JGIT PATCH 2/4] Create a fnmatch-style pattern TreeFilter
From: Robin Rosenberg @ 2008-06-24 21:20 UTC (permalink / raw)
  To: git; +Cc: Shawn O. Pearce, Marek Zawirski, Florian Koeberle,
	Robin Rosenberg
In-Reply-To: <1214342427-2077-1-git-send-email-robin.rosenberg@dewire.com>

This uses Florian's pattern matcher to perform the matching.

Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
---
 .../jgit/treewalk/filter/WildCardTreeFilter.java   |  101 ++++++++++++++++++++
 1 files changed, 101 insertions(+), 0 deletions(-)
 create mode 100644 org.spearce.jgit/src/org/spearce/jgit/treewalk/filter/WildCardTreeFilter.java

diff --git a/org.spearce.jgit/src/org/spearce/jgit/treewalk/filter/WildCardTreeFilter.java b/org.spearce.jgit/src/org/spearce/jgit/treewalk/filter/WildCardTreeFilter.java
new file mode 100644
index 0000000..dc3faf9
--- /dev/null
+++ b/org.spearce.jgit/src/org/spearce/jgit/treewalk/filter/WildCardTreeFilter.java
@@ -0,0 +1,101 @@
+/*
+ * Copyright (C) 2008, Robin Rosenberg <robin.rosenberg@dewire.com>
+ *
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or
+ * without modification, are permitted provided that the following
+ * conditions are met:
+ *
+ * - Redistributions of source code must retain the above copyright
+ *   notice, this list of conditions and the following disclaimer.
+ *
+ * - Redistributions in binary form must reproduce the above
+ *   copyright notice, this list of conditions and the following
+ *   disclaimer in the documentation and/or other materials provided
+ *   with the distribution.
+ *
+ * - Neither the name of the Git Development Community nor the
+ *   names of its contributors may be used to endorse or promote
+ *   products derived from this software without specific prior
+ *   written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
+ * CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES,
+ * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
+ * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
+ * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
+ * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
+ * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
+ * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+package org.spearce.jgit.treewalk.filter;
+
+import java.io.IOException;
+
+import org.spearce.jgit.errors.IncorrectObjectTypeException;
+import org.spearce.jgit.errors.InvalidPatternException;
+import org.spearce.jgit.errors.MissingObjectException;
+import org.spearce.jgit.fnmatch.FileNameMatcher;
+import org.spearce.jgit.treewalk.TreeWalk;
+import org.spearce.jgit.treewalk.filter.TreeFilter;
+
+/**
+ * This class implements a TreeeFilter that uses the wildcard style pattern
+ * matching like of Posix fnmatch function.
+ */
+public class WildCardTreeFilter extends TreeFilter {
+
+	private final FileNameMatcher matcher;
+
+	private final String pattern;
+
+	protected WildCardTreeFilter(final String pattern) {
+		try {
+			this.pattern = pattern;
+			matcher = new FileNameMatcher(pattern, null);
+		} catch (InvalidPatternException e) {
+			throw new IllegalArgumentException(e);
+		}
+	}
+
+	@Override
+	public TreeFilter clone() {
+		return new WildCardTreeFilter(pattern);
+	}
+
+	@Override
+	public boolean include(TreeWalk walker) throws MissingObjectException,
+			IncorrectObjectTypeException, IOException {
+		matcher.reset();
+		matcher.append(walker.getPathString());
+		if (matcher.isMatch())
+			return true;
+		return false;
+	}
+
+	@Override
+	public boolean shouldBeRecursive() {
+		return true;
+	}
+
+	/**
+	 * Construct a WildCardmatcher like POSIX fnmatch.
+	 *
+	 * @param pattern
+	 *            A POSIX wildcard pattern
+	 * @return a {@link TreeFilter} that matches pattern
+	 * @throws IllegalArgumentException
+	 *             if the pattern is malformed
+	 */
+	public static TreeFilter create(final String pattern) {
+		return new WildCardTreeFilter(pattern);
+	}
+
+}
-- 
1.5.5.1.178.g1f811

^ permalink raw reply related

* [JGIT PATCH 3/4] Added a method to get the non-relative name from the tree walker
From: Robin Rosenberg @ 2008-06-24 21:20 UTC (permalink / raw)
  To: git; +Cc: Shawn O. Pearce, Marek Zawirski, Florian Koeberle,
	Robin Rosenberg
In-Reply-To: <1214342427-2077-2-git-send-email-robin.rosenberg@dewire.com>

Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
---
 .../src/org/spearce/jgit/treewalk/TreeWalk.java    |   21 ++++++++++++++++++++
 1 files changed, 21 insertions(+), 0 deletions(-)

diff --git a/org.spearce.jgit/src/org/spearce/jgit/treewalk/TreeWalk.java b/org.spearce.jgit/src/org/spearce/jgit/treewalk/TreeWalk.java
index 42f8b25..a5eb4d9 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/treewalk/TreeWalk.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/treewalk/TreeWalk.java
@@ -499,6 +499,17 @@ public class TreeWalk {
 	}
 
 	/**
+	 * Get the current entry's name.
+	 * <p>
+	 *
+	 * @return name of the current entry only.
+	 * @see #getPathString()
+	 */
+	public String getName() {
+		return nameOf(currentHead);
+	}
+
+	/**
 	 * Test if the supplied path matches the current entry's path.
 	 * <p>
 	 * This method tests that the supplied path is exactly equal to the current
@@ -659,4 +670,14 @@ public class TreeWalk {
 					+ Constants.CHARACTER_ENCODING, uee);
 		}
 	}
+
+	private static String nameOf(final AbstractTreeIterator t) {
+		try {
+			return new String(t.path, t.pathOffset, t.pathLen - t.pathOffset,
+					Constants.CHARACTER_ENCODING);
+		} catch (UnsupportedEncodingException uee) {
+			throw new RuntimeException("JVM doesn't support "
+					+ Constants.CHARACTER_ENCODING, uee);
+		}
+	}
 }
-- 
1.5.5.1.178.g1f811

^ permalink raw reply related

* [JGIT PATCH 4/4] LsTree: Enable pattern matching in LsTree
From: Robin Rosenberg @ 2008-06-24 21:20 UTC (permalink / raw)
  To: git; +Cc: Shawn O. Pearce, Marek Zawirski, Florian Koeberle,
	Robin Rosenberg
In-Reply-To: <1214342427-2077-3-git-send-email-robin.rosenberg@dewire.com>

Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
---
 .../src/org/spearce/jgit/pgm/LsTree.java           |   14 +++++++++++++-
 .../jgit/treewalk/filter/WildCardTreeFilter.java   |    2 +-
 2 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/org.spearce.jgit/src/org/spearce/jgit/pgm/LsTree.java b/org.spearce.jgit/src/org/spearce/jgit/pgm/LsTree.java
index 05ec8c3..87a003d 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/pgm/LsTree.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/pgm/LsTree.java
@@ -43,8 +43,11 @@ import org.spearce.jgit.lib.Constants;
 import org.spearce.jgit.lib.FileMode;
 import org.spearce.jgit.treewalk.FileTreeIterator;
 import org.spearce.jgit.treewalk.TreeWalk;
+import org.spearce.jgit.treewalk.filter.TreeFilter;
+import org.spearce.jgit.treewalk.filter.WildCardTreeFilter;
 
 class LsTree extends TextBuiltin {
+
 	@Override
 	void execute(final String[] args) throws Exception {
 		final TreeWalk walk = new TreeWalk(db);
@@ -66,12 +69,21 @@ class LsTree extends TextBuiltin {
 			throw die("too many arguments");
 
 		walk.reset(); // drop the first empty tree, which we do not need here
-		final String n = args[argi];
+		final String n = args[argi++];
 		if (is_WorkDir(n))
 			walk.addTree(new FileTreeIterator(new File(n)));
 		else
 			walk.addTree(resolve(n));
 
+		if (argi == args.length - 1) {
+			TreeFilter filter = WildCardTreeFilter.create(args[argi++]);
+			walk.setFilter(filter);
+		}
+		if (argi + 1 == args.length)
+			throw die("usage: [-r] treename [pattern]");
+		else if (argi + 1 < args.length)
+			throw die("too many arguments");
+
 		while (walk.next()) {
 			final FileMode mode = walk.getFileMode(0);
 			if (mode == FileMode.TREE)
diff --git a/org.spearce.jgit/src/org/spearce/jgit/treewalk/filter/WildCardTreeFilter.java b/org.spearce.jgit/src/org/spearce/jgit/treewalk/filter/WildCardTreeFilter.java
index dc3faf9..645d52d 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/treewalk/filter/WildCardTreeFilter.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/treewalk/filter/WildCardTreeFilter.java
@@ -74,7 +74,7 @@ public class WildCardTreeFilter extends TreeFilter {
 	public boolean include(TreeWalk walker) throws MissingObjectException,
 			IncorrectObjectTypeException, IOException {
 		matcher.reset();
-		matcher.append(walker.getPathString());
+		matcher.append(walker.getName());
 		if (matcher.isMatch())
 			return true;
 		return false;
-- 
1.5.5.1.178.g1f811

^ permalink raw reply related

* Re: Segmentation fault on http clone, post-1.5.6
From: Nicolas Pitre @ 2008-06-24 21:24 UTC (permalink / raw)
  To: Teemu Likonen; +Cc: Jeff King, git
In-Reply-To: <20080624205556.GA3565@mithlond.arda.local>

On Tue, 24 Jun 2008, Teemu Likonen wrote:

> Nicolas Pitre wrote (2008-06-24 15:34 -0400):
> 
> > I'm trying to reproduce your segfault with current master
> > (v1.5.6-56-g29b0d01) but I just can't.
> > 
> > Can you provide a gdb backtrace of the segfault?
> 
> Let's hope I'm doing this right. Just installed gdb for the first time.
> I started with "gdb git" and then typed "run clone http://...".
> Eventually it gave this:

Excellent!

The problem is probably fixed with this:

diff --git a/sha1_file.c b/sha1_file.c
index a92f023..b7d1a82 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -844,6 +844,8 @@ struct packed_git *parse_pack_index(unsigned char *sha1)
 
 	strcpy(p->pack_name, path);
 	p->pack_size = 0;
+	p->num_bad_objects = 0;
+	p->bad_object_sha1 = NULL;
 	p->next = NULL;
 	p->windows = NULL;
 	p->pack_fd = -1;

Could you confirm it?

However I just don't like the fact that pack structures are allocated 
and initialized in two places, which makes it error prone as 
demonstrated here.  So I'll cook up a better patch that fixes the 
duplication issue.


Nicolas

^ permalink raw reply related

* Re: [PATCH 1/2] Introduce leaky().
From: Jakub Narebski @ 2008-06-24 21:28 UTC (permalink / raw)
  To: Pierre Habouzit; +Cc: git, gitster
In-Reply-To: <1214338474-16822-2-git-send-email-madcoder@debian.org>

Pierre Habouzit <madcoder@debian.org> writes:

> diff --git a/Makefile b/Makefile

> +#
> +# Define COLLECT_LEAKS_AT_EXIT if you want memory marked as leaky() at exit.

I think s/at exit/to be freed &/;


> diff --git a/cache.h b/cache.h

Hmmm... cache?

>  /* alloc.c */
> +#ifdef COLLECT_LEAKS_AT_EXIT
> +extern void *leaky(void *);
> +#else
> +# define leaky(x) x
> +#endif

Not 

+# define leaky(x) (x)

to be careful?
-- 
Jakub Narebski
Poland
ShadeHawk on #git

^ permalink raw reply

* Re: why is git destructive by default? (i suggest it not be!)
From: Brandon Casey @ 2008-06-24 21:42 UTC (permalink / raw)
  To: David Jeske; +Cc: Jakub Narebski, Boaz Harrosh, git
In-Reply-To: <willow-jeske-01l61=64jMFEDjCiBE>

David Jeske wrote:
>> -- David Jeske wrote:
>>> - improve the man page description of "reset --hard"
>>> - standardize all the potentially destructive operations
>>> (after gc) on "-f/--force" to override
>> The thing is 'force' is not always the most descriptive word
>> for the behavior that you propose enabling with --force.
> I'm not talking about switching "git reset --hard" to "git reset -f". I'm
> talking about requiring a "-f" option to "git reset --hard" when it would
> destroy or dangle information.

I only have the same advice I gave to Boaz. I think you should try to adjust
your workflow so that 'git reset' is not necessary. It seems that for the
functions you're trying to perform, 'checkout' and 'branch' should be used rather
than 'reset'.

Again, as I mentioned to Boaz, there is really no benefit to reusing a single
branch name if that is what you are trying to do. The cost of branching in git
is 41 bytes i.e. nil. The cost of updating the working directory which happens
during the 'reset --hard' is exactly the same whether I do
'reset --hard <some_branch>' or 'checkout -b new_branch <some_branch>'.

In nearly every case where I, personally, have used 'reset --hard', I was using
it because I didn't care what the current state of the working directory or the
index were. They were wrong and I was resetting to the right state. I believe
this was the intended use for the command.

I'm not sure why you want to use reset so often. If there is something in the
documentation that led you to want to use reset maybe it can be changed so that
other users are not led in the same way.

About the reflog..
The reflog is not a storage area. It's just a log, like /var/log/messages. It is
there to provide a way to recover from mistakes. Mistakes are usually recognized
fairly quickly. If you have not realized that you have made a mistake after 30
days, it may be pretty hard to recover from since people have imperfect memories.
If we did not garbage collect the reflog it would just continue to grow appending
useless piece of information after useless piece of information.

-brandon

^ permalink raw reply

* Re: Segmentation fault on http clone, post-1.5.6
From: SZEDER Gábor @ 2008-06-24 21:56 UTC (permalink / raw)
  To: Nicolas Pitre; +Cc: Teemu Likonen, Jeff King, git
In-Reply-To: <alpine.LFD.1.10.0806241720440.2979@xanadu.home>

Hi,

On Tue, Jun 24, 2008 at 05:24:49PM -0400, Nicolas Pitre wrote:
> The problem is probably fixed with this:
> 
> Could you confirm it?
It seems to fix the problem at me.

Gábor

^ permalink raw reply

* Re: [PATCH 1/2] Introduce leaky().
From: Pierre Habouzit @ 2008-06-24 22:10 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: git, gitster
In-Reply-To: <m3skv2jzey.fsf@localhost.localdomain>

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

On Tue, Jun 24, 2008 at 09:28:30PM +0000, Jakub Narebski wrote:
> Pierre Habouzit <madcoder@debian.org> writes:
> 
> > diff --git a/Makefile b/Makefile
> 
> > +#
> > +# Define COLLECT_LEAKS_AT_EXIT if you want memory marked as leaky() at exit.
> 
> I think s/at exit/to be freed &/;

  err obviously.

> > diff --git a/cache.h b/cache.h
> 
> Hmmm... cache?

  well cache.h has the prototypes for alloc.c that feels like the proper
place, but I don't care much :)

> >  /* alloc.c */
> > +#ifdef COLLECT_LEAKS_AT_EXIT
> > +extern void *leaky(void *);
> > +#else
> > +# define leaky(x) x
> > +#endif
> 
> Not 
> 
> +# define leaky(x) (x)
> 
> to be careful?

ack.

-- 
·O·  Pierre Habouzit
··O                                                madcoder@debian.org
OOO                                                http://www.madism.org

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

^ permalink raw reply

* Re: What's cooking in git.git (topics)
From: Miklos Vajna @ 2008-06-24 22:10 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Johannes Schindelin, Pieter de Bie, git
In-Reply-To: <7vskv2d0lp.fsf@gitster.siamese.dyndns.org>

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

On Tue, Jun 24, 2008 at 01:44:34PM -0700, Junio C Hamano <gitster@pobox.com> wrote:
> >> bash: git-upload-pack: command not found
> >> fatal: The remote end hung up unexpectedly
> >
> > Hmm.  Probably the client needs to be newer, too.  This is going to be 
> > painful.  Junio?
> 
> Even with maint client accessing an account with next git-shell as its
> login shell, I do not get the above failure.
> 
> Is git-shell installed and configured correctly at all in Miklos's setup?
> Why does the other side say "bash: git-upload-pack" when login shell is
> git-shell and not bash?

Sorry for the confusion, this is not about git-shell at all. I have
bash as the shell on the server, obviously.

So, in case the server runs next, the client runs master, and I try to
clone via ssh, I get the above error.

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

^ permalink raw reply

* Re: why is git destructive by default? (i suggest it not be!)
From: Steven Walter @ 2008-06-24 22:21 UTC (permalink / raw)
  To: David Jeske; +Cc: Jakub Narebski, Boaz Harrosh, git
In-Reply-To: <willow-jeske-01l61=64jMFEDjCiBE>

On Tue, Jun 24, 2008 at 08:04:30PM -0000, David Jeske wrote:
> I'm not talking about switching "git reset --hard" to "git reset -f". I'm
> talking about requiring a "-f" option to "git reset --hard" when it would
> destroy or dangle information.

I think you're asking for something like the following...
-- 
-Steven Walter <stevenrwalter@gmail.com>
Freedom is the freedom to say that 2 + 2 = 4
B2F1 0ECC E605 7321 E818  7A65 FC81 9777 DC28 9E8F 

^ permalink raw reply

* [PATCH] cmd_reset: don't trash uncommitted changes unless told to
From: Steven Walter @ 2008-06-24 22:21 UTC (permalink / raw)
  To: git, jeske; +Cc: Steven Walter
In-Reply-To: <20080624222105.GA24549@dervierte>

Give "reset --hard" a -f (force) flag, without which it will refuse to
proceed if there are changes in the index or working tree.

Signed-off-by: Steven Walter <stevenrwalter@gmail.com>
---
 builtin-reset.c |   24 +++++++++++++++++++++++-
 1 files changed, 23 insertions(+), 1 deletions(-)

diff --git a/builtin-reset.c b/builtin-reset.c
index f34acb1..6ee8448 100644
--- a/builtin-reset.c
+++ b/builtin-reset.c
@@ -11,8 +11,10 @@
 #include "tag.h"
 #include "object.h"
 #include "commit.h"
+#include "diff.h"
 #include "run-command.h"
 #include "refs.h"
+#include "revision.h"
 #include "diff.h"
 #include "diffcore.h"
 #include "tree.h"
@@ -165,12 +167,26 @@ static void prepend_reflog_action(const char *action, char *buf, size_t size)
 		warning("Reflog action message too long: %.*s...", 50, buf);
 }
 
+/* Stolen from builtin-revert.c... */
+static int index_is_dirty(void)
+{
+	struct rev_info rev;
+        read_cache();
+	init_revisions(&rev, NULL);
+	setup_revisions(0, NULL, &rev, "HEAD");
+	DIFF_OPT_SET(&rev.diffopt, QUIET);
+	DIFF_OPT_SET(&rev.diffopt, EXIT_WITH_STATUS);
+	run_diff_files(&rev, 1);
+	return !!DIFF_OPT_TST(&rev.diffopt, HAS_CHANGES);
+}
+
 enum reset_type { MIXED, SOFT, HARD, NONE };
 static const char *reset_type_names[] = { "mixed", "soft", "hard", NULL };
 
 int cmd_reset(int argc, const char **argv, const char *prefix)
 {
-	int i = 0, reset_type = NONE, update_ref_status = 0, quiet = 0;
+	int i = 0, reset_type = NONE, update_ref_status = 0, quiet = 0,
+            force = 0;
 	const char *rev = "HEAD";
 	unsigned char sha1[20], *orig = NULL, sha1_orig[20],
 				*old_orig = NULL, sha1_old_orig[20];
@@ -184,6 +200,8 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
 				"reset HEAD, index and working tree", HARD),
 		OPT_BOOLEAN('q', NULL, &quiet,
 				"disable showing new HEAD in hard reset and progress message"),
+		OPT_BOOLEAN('f', NULL, &force,
+				"proceed even if there are uncommitted changes"),
 		OPT_END()
 	};
 
@@ -225,6 +243,10 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
 	if (reset_type == HARD && is_bare_repository())
 		die("hard reset makes no sense in a bare repository");
 
+        if (reset_type == HARD && !force && index_is_dirty()) {
+                die("Uncommitted changes; re-run with -f to trash them");
+        }
+
 	/* Soft reset does not touch the index file nor the working tree
 	 * at all, but requires them in a good order.  Other resets reset
 	 * the index file to the tree object we are switching to. */
-- 
1.5.6.dirty

^ permalink raw reply related

* Re: [NON-TOY PATCH] git bisect: introduce 'fixed' and 'unfixed'
From: Junio C Hamano @ 2008-06-24 22:30 UTC (permalink / raw)
  To: Jeff King; +Cc: Johannes Schindelin, git
In-Reply-To: <20080624174157.GB9500@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

> On Tue, Jun 24, 2008 at 06:09:28PM +0100, Johannes Schindelin wrote:
>
>> 	And this is my first attempt at a proper patch for it.
>> 
>> 	Now with documentation, and hopefully all places where the
>> 	user is being told about a "bad" commit.
>
> This looks reasonably sane to me. The only thing I can think of that
> we're missing is that "git bisect visualize" will still show the refs as
> "bisect/bad" and "bisect/good".
>
> To fix that, you'd have to ask people to start the bisect by saying "I
> am bisecting to find a fix, not a breakage." And then you could change
> the refnames and all of the messages as appropriate.

It probably is not just a good idea, but is a necessary fix, to remove
confusion like this example that appears everywhere:

>  		echo >&2 'You '$THEN'need to give me at least one good' \
> -			'and one bad revisions.'
> +			'and one bad (or fixed) revision.'
>  		echo >&2 '(You can use "git bisect bad" and' \
>  			'"git bisect good" for that.)'

People who are reading the change Dscho did in the "patch" form may not
notice it, but imagine how the above looks to the end user who was told
that "new bisect can now look for fixes", who does not need to nor even
want to know that the new feature is implemented by making bad and fixed
synonyms.

They need to mentally reword "good" into "unfixed" and "bisect bad" into
"bisect fixed" while reading the output from the above pieces, but the
point of this new "look for fixes" feature is they do not have to do the
rewording anymore!

^ permalink raw reply

* Re: [NON-TOY PATCH] git bisect: introduce 'fixed' and 'unfixed'
From: Junio C Hamano @ 2008-06-24 22:31 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Michael Haggerty, Jeff King, git
In-Reply-To: <alpine.DEB.1.00.0806242137120.9925@racer>

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

> On Tue, 24 Jun 2008, Michael Haggerty wrote:
> ...
>> It seems to me that your problem is that git-bisect requires the "good" 
>> revision to be older than the "bad" one.  If this requirement were 
>> removed, would there still be a need for "fixed" vs. "unfixed"?
>
> Nope.
>
> The thing that makes "fixed" and "bad" special is that _one_ commit 
> introduced that.

That was my initial reaction, and I actually was about to phrase it more
bluntly: you do not understand what "bisect" is.

But that was a reaction without thinking things through.  It may not be
what "git bisect" currently is, but the suggestion does not go against
what the underlying "git rev-list --bisect" is at all.  I think what
Michael is speculating is different, and it makes sense in its own way.

Instead of having a set of bisect/good-* refs and a single bisect-bad ref,
your "fixed and unfixed" mode could work quite differently.  By noticing
that the topology the user specified with initial good and bad have
ancient bad and recent good --- that is, "it used to be bad but now it is
good" --- you could instead use a set of bisect/bad-* refs and a single
bisect-good ref, and feed good and bad swapped to "rev-list --bisect" in
bisect_next().  That way, the labels given by visualize will match what
the user is doing automatically.

I said "it makes sense in its own way", because it is _quite_ different
from how git-bisect currently assumes, and restructuring git-bisect to
operate naturally in a way Michael describes would be a much larger
surgery with costs (including risks of bugs) associated with it, which
needs to be weighed in when judging that approach would actually make
sense.

^ permalink raw reply

* Re: [PATCH] pre-commit hook should ignore carriage returns at EOL
From: Junio C Hamano @ 2008-06-24 22:31 UTC (permalink / raw)
  To: Christian Holtje; +Cc: Ian Hilt, git
In-Reply-To: <39C2861E-F800-40AE-8C15-4FC3BB51EF16@gmail.com>

Christian Holtje <docwhat@gmail.com> writes:

> The code is checking for \r$ and then doing a different space check
> depending on that, not one after another.
>
> Thanks for the feedback. I'll put up v2 in a second.

Please don't.

It's an ancient sample hook that is not be enabled by default.  I do not
want people to be wasting too much time on the relic.

However, if this sample is to be changed at all, please do it right.

If somebody suddenly adds CR at the end of an existing file that ought to
have LF line endings, we _DO_ want to catch that as a breakage.  So the
title of the commit "should ignore carriage returns at EOL" is WRONG.  It
shouldn't, in general.

One thing the hook could and probably should do these days is if the file
type says you _ought to_ have CRLF line endings, actively make sure your
lines do end with CRLF (this is a much stronger and better check than
blindly ignoring CR before LF for such files).  And on the other hand, if
the file should end with LF, do make sure it does not have CR before it.

The person who did the sample hook you are looking at couldn't do so
because there weren't autocrlf nor gitattributes(5) facility back then.
But you can use them now to rewrite this properly.

I wonder if "git diff --check" can be used for most if not all of the
checking, without the big Perl script you are touching in your patch.
That facility did not exist when the current sample hook was written,
either.

^ 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