Git development
 help / color / mirror / Atom feed
* [PATCH v2] parse-opt: migrate builtin-apply.
From: Miklos Vajna @ 2008-12-27 14:22 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <20081227140533.GX21154@genesis.frugalware.org>

The only incompatible change is that the user how have to use '--'
before a patch named --build-fake-ancestor=something.

Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>
---

Diffstat against v1:

 builtin-apply.c |  125 +++++++++++++------------------------------------------
 1 files changed, 29 insertions(+), 96 deletions(-)

due to the removal of option_parse_stat, option_parse_numstat,
option_parse_summary, option_parse_check, option_parse_index,
option_parse_cached, option_parse_ancestor and option_parse_reject.

 Documentation/git-apply.txt |    4 +-
 builtin-apply.c             |  293 ++++++++++++++++++++++++-------------------
 2 files changed, 167 insertions(+), 130 deletions(-)

diff --git a/Documentation/git-apply.txt b/Documentation/git-apply.txt
index e726510..9400f6a 100644
--- a/Documentation/git-apply.txt
+++ b/Documentation/git-apply.txt
@@ -10,7 +10,7 @@ SYNOPSIS
 --------
 [verse]
 'git apply' [--stat] [--numstat] [--summary] [--check] [--index]
-	  [--apply] [--no-add] [--build-fake-ancestor <file>] [-R | --reverse]
+	  [--apply] [--no-add] [--build-fake-ancestor=<file>] [-R | --reverse]
 	  [--allow-binary-replacement | --binary] [--reject] [-z]
 	  [-pNUM] [-CNUM] [--inaccurate-eof] [--recount] [--cached]
 	  [--whitespace=<nowarn|warn|fix|error|error-all>]
@@ -64,7 +64,7 @@ OPTIONS
 	cached data, apply the patch, and store the result in the index,
 	without using the working tree. This implies '--index'.
 
---build-fake-ancestor <file>::
+--build-fake-ancestor=<file>::
 	Newer 'git-diff' output has embedded 'index information'
 	for each blob to help identify the original version that
 	the patch applies to.  When this flag is given, and if
diff --git a/builtin-apply.c b/builtin-apply.c
index 07244b0..98a988c 100644
--- a/builtin-apply.c
+++ b/builtin-apply.c
@@ -14,6 +14,7 @@
 #include "builtin.h"
 #include "string-list.h"
 #include "dir.h"
+#include "parse-options.h"
 
 /*
  *  --check turns on checking that the working tree matches the
@@ -46,8 +47,10 @@ static int no_add;
 static const char *fake_ancestor;
 static int line_termination = '\n';
 static unsigned long p_context = ULONG_MAX;
-static const char apply_usage[] =
-"git apply [--stat] [--numstat] [--summary] [--check] [--index] [--cached] [--apply] [--no-add] [--index-info] [--allow-binary-replacement] [--reverse] [--reject] [--verbose] [-z] [-pNUM] [-CNUM] [--whitespace=<nowarn|warn|fix|error|error-all>] <patch>...";
+static const char * const apply_usage[] = {
+	"git apply [--stat] [--numstat] [--summary] [--check] [--index] [--cached] [--apply] [--no-add] [--index-info] [--allow-binary-replacement] [--reverse] [--reject] [--verbose] [-z] [-pNUM] [-CNUM] [--whitespace=<nowarn|warn|fix|error|error-all>] <patch>...",
+	NULL
+};
 
 static enum ws_error_action {
 	nowarn_ws_error,
@@ -61,6 +64,8 @@ static int applied_after_fixing_ws;
 static const char *patch_input_file;
 static const char *root;
 static int root_len;
+static int read_stdin = 1;
+static int options;
 
 static void parse_whitespace_option(const char *option)
 {
@@ -3135,150 +3140,182 @@ static int git_apply_config(const char *var, const char *value, void *cb)
 	return git_default_config(var, value, cb);
 }
 
+static int option_parse_stdin(const struct option *opt,
+			      const char *arg, int unset)
+{
+	int *errs = opt->value;
+
+	*errs |= apply_patch(0, "<stdin>", options);
+	read_stdin = 0;
+	return 0;
+}
+
+static int option_parse_exclude(const struct option *opt,
+				const char *arg, int unset)
+{
+	add_name_limit(arg, 1);
+	return 0;
+}
+
+static int option_parse_include(const struct option *opt,
+				const char *arg, int unset)
+{
+	add_name_limit(arg, 0);
+	has_include = 1;
+	return 0;
+}
+
+static int option_parse_p(const struct option *opt,
+			  const char *arg, int unset)
+{
+	p_value = atoi(arg);
+	p_value_known = 1;
+	return 0;
+}
+
+static int option_parse_z(const struct option *opt,
+			  const char *arg, int unset)
+{
+	if (unset)
+		line_termination = '\n';
+	else
+		line_termination = 0;
+	return 0;
+}
+
+static int option_parse_whitespace(const struct option *opt,
+				   const char *arg, int unset)
+{
+	const char **whitespace_option = opt->value;
+
+	*whitespace_option = arg;
+	parse_whitespace_option(arg);
+	return 0;
+}
+
+static int option_parse_inaccurate(const struct option *opt,
+				   const char *arg, int unset)
+{
+	options |= INACCURATE_EOF;
+	return 0;
+}
+
+static int option_parse_recount(const struct option *opt,
+				const char *arg, int unset)
+{
+	options |= RECOUNT;
+	return 0;
+}
+
+static int option_parse_directory(const struct option *opt,
+				  const char *arg, int unset)
+{
+	root_len = strlen(arg);
+	if (root_len && arg[root_len - 1] != '/') {
+		char *new_root;
+		root = new_root = xmalloc(root_len + 2);
+		strcpy(new_root, arg);
+		strcpy(new_root + root_len++, "/");
+	} else
+		root = arg;
+	return 0;
+}
 
 int cmd_apply(int argc, const char **argv, const char *unused_prefix)
 {
 	int i;
-	int read_stdin = 1;
-	int options = 0;
 	int errs = 0;
 	int is_not_gitdir;
+	int binary;
+	int force_apply = 0;
 
 	const char *whitespace_option = NULL;
 
+	struct option builtin_apply_options[] = {
+		{ OPTION_CALLBACK, '-', NULL, &errs, NULL,
+			"read the patch from the standard input",
+			PARSE_OPT_NOARG, option_parse_stdin },
+		{ OPTION_CALLBACK, 0, "exclude", NULL, "path",
+			"don´t apply changes matching the given path",
+			0, option_parse_exclude },
+		{ OPTION_CALLBACK, 0, "include", NULL, "path",
+			"apply changes matching the given path",
+			0, option_parse_include },
+		{ OPTION_CALLBACK, 'p', NULL, NULL, "num",
+			"remove <num> leading slashes from traditional diff paths",
+			0, option_parse_p },
+		OPT_BOOLEAN(0, "no-add", &no_add,
+			"ignore additions made by the patch"),
+		OPT_BOOLEAN(0, "stat", &diffstat,
+			"instead of applying the patch, output diffstat for the input"),
+		OPT_BOOLEAN(0, "allow-binary-replacement", &binary,
+			"now no-op"),
+		OPT_BOOLEAN(0, "binary", &binary,
+			"now no-op"),
+		OPT_BOOLEAN(0, "numstat", &numstat,
+			"shows number of added and deleted lines in decimal notation"),
+		OPT_BOOLEAN(0, "summary", &summary,
+			"instead of applying the patch, output a summary for the input"),
+		OPT_BOOLEAN(0, "check", &check,
+			"instead of applying the patch, see if the patch is applicable"),
+		OPT_BOOLEAN(0, "index", &check_index,
+			"make sure the patch is applicable to the current index"),
+		OPT_BOOLEAN(0, "cached", &cached,
+			"apply a patch without touching the working tree"),
+		OPT_BOOLEAN(0, "apply", &force_apply,
+			"also apply the patch (use with --stat/--summary/--check)"),
+		OPT_STRING(0, "build-fake-ancestor", &fake_ancestor, "file",
+			"build a temporary index based on embedded index information"),
+		{ OPTION_CALLBACK, 'z', NULL, NULL, NULL,
+			"paths are separated with NUL character",
+			PARSE_OPT_NOARG, option_parse_z },
+		OPT_INTEGER('C', NULL, &p_context,
+				"ensure at least <n> lines of context match"),
+		{ OPTION_CALLBACK, 0, "whitespace", &whitespace_option, "action",
+			"detect new or modified lines that have whitespace errors",
+			0, option_parse_whitespace },
+		OPT_BOOLEAN('R', "reverse", &apply_in_reverse,
+			"apply the patch in reverse"),
+		OPT_BOOLEAN(0, "unidiff-zero", &unidiff_zero,
+			"don't expect at least one line of context"),
+		OPT_BOOLEAN(0, "reject", &apply_with_reject,
+			"leave the rejected hunks in corresponding *.rej files"),
+		OPT__VERBOSE(&apply_verbosely),
+		{ OPTION_CALLBACK, 0, "inaccurate-eof", NULL, NULL,
+			"tolerate incorrectly detected missing new-line at the end of file",
+			PARSE_OPT_NOARG, option_parse_inaccurate },
+		{ OPTION_CALLBACK, 0, "recount", NULL, NULL,
+			"do not trust the line counts in the hunk headers",
+			PARSE_OPT_NOARG, option_parse_recount },
+		{ OPTION_CALLBACK, 0, "directory", NULL, "root",
+			"prepend <root> to all filenames",
+			0, option_parse_directory },
+		OPT_END()
+	};
+
 	prefix = setup_git_directory_gently(&is_not_gitdir);
 	prefix_length = prefix ? strlen(prefix) : 0;
 	git_config(git_apply_config, NULL);
 	if (apply_default_whitespace)
 		parse_whitespace_option(apply_default_whitespace);
 
-	for (i = 1; i < argc; i++) {
+	argc = parse_options(argc, argv, builtin_apply_options,
+			apply_usage, 0);
+	if (apply_with_reject)
+		apply = apply_verbosely = 1;
+	if (!force_apply && (diffstat || numstat || summary || check || fake_ancestor))
+		apply = 0;
+	if (check_index && is_not_gitdir)
+		die("--index outside a repository");
+	if (cached) {
+		if (is_not_gitdir)
+			die("--cached outside a repository");
+		check_index = 1;
+	}
+	for (i = 0; i < argc; i++) {
 		const char *arg = argv[i];
-		char *end;
 		int fd;
 
-		if (!strcmp(arg, "-")) {
-			errs |= apply_patch(0, "<stdin>", options);
-			read_stdin = 0;
-			continue;
-		}
-		if (!prefixcmp(arg, "--exclude=")) {
-			add_name_limit(arg + 10, 1);
-			continue;
-		}
-		if (!prefixcmp(arg, "--include=")) {
-			add_name_limit(arg + 10, 0);
-			has_include = 1;
-			continue;
-		}
-		if (!prefixcmp(arg, "-p")) {
-			p_value = atoi(arg + 2);
-			p_value_known = 1;
-			continue;
-		}
-		if (!strcmp(arg, "--no-add")) {
-			no_add = 1;
-			continue;
-		}
-		if (!strcmp(arg, "--stat")) {
-			apply = 0;
-			diffstat = 1;
-			continue;
-		}
-		if (!strcmp(arg, "--allow-binary-replacement") ||
-		    !strcmp(arg, "--binary")) {
-			continue; /* now no-op */
-		}
-		if (!strcmp(arg, "--numstat")) {
-			apply = 0;
-			numstat = 1;
-			continue;
-		}
-		if (!strcmp(arg, "--summary")) {
-			apply = 0;
-			summary = 1;
-			continue;
-		}
-		if (!strcmp(arg, "--check")) {
-			apply = 0;
-			check = 1;
-			continue;
-		}
-		if (!strcmp(arg, "--index")) {
-			if (is_not_gitdir)
-				die("--index outside a repository");
-			check_index = 1;
-			continue;
-		}
-		if (!strcmp(arg, "--cached")) {
-			if (is_not_gitdir)
-				die("--cached outside a repository");
-			check_index = 1;
-			cached = 1;
-			continue;
-		}
-		if (!strcmp(arg, "--apply")) {
-			apply = 1;
-			continue;
-		}
-		if (!strcmp(arg, "--build-fake-ancestor")) {
-			apply = 0;
-			if (++i >= argc)
-				die ("need a filename");
-			fake_ancestor = argv[i];
-			continue;
-		}
-		if (!strcmp(arg, "-z")) {
-			line_termination = 0;
-			continue;
-		}
-		if (!prefixcmp(arg, "-C")) {
-			p_context = strtoul(arg + 2, &end, 0);
-			if (*end != '\0')
-				die("unrecognized context count '%s'", arg + 2);
-			continue;
-		}
-		if (!prefixcmp(arg, "--whitespace=")) {
-			whitespace_option = arg + 13;
-			parse_whitespace_option(arg + 13);
-			continue;
-		}
-		if (!strcmp(arg, "-R") || !strcmp(arg, "--reverse")) {
-			apply_in_reverse = 1;
-			continue;
-		}
-		if (!strcmp(arg, "--unidiff-zero")) {
-			unidiff_zero = 1;
-			continue;
-		}
-		if (!strcmp(arg, "--reject")) {
-			apply = apply_with_reject = apply_verbosely = 1;
-			continue;
-		}
-		if (!strcmp(arg, "-v") || !strcmp(arg, "--verbose")) {
-			apply_verbosely = 1;
-			continue;
-		}
-		if (!strcmp(arg, "--inaccurate-eof")) {
-			options |= INACCURATE_EOF;
-			continue;
-		}
-		if (!strcmp(arg, "--recount")) {
-			options |= RECOUNT;
-			continue;
-		}
-		if (!prefixcmp(arg, "--directory=")) {
-			arg += strlen("--directory=");
-			root_len = strlen(arg);
-			if (root_len && arg[root_len - 1] != '/') {
-				char *new_root;
-				root = new_root = xmalloc(root_len + 2);
-				strcpy(new_root, arg);
-				strcpy(new_root + root_len++, "/");
-			} else
-				root = arg;
-			continue;
-		}
 		if (0 < prefix_length)
 			arg = prefix_filename(prefix, prefix_length, arg);
 
-- 
1.6.1.rc1.35.gae26e.dirty

^ permalink raw reply related

* Re: [git-new-workdir RFC] Backlinking $workdir/logs/HEAD to $GIT_DIR/logs/workdir?
From: Shawn O. Pearce @ 2008-12-27 17:26 UTC (permalink / raw)
  To: Adeodato Simó; +Cc: git
In-Reply-To: <20081227121526.GA12120@chistera.yi.org>

Adeodato Simó <dato@net.com.org.es> wrote:
> I use git new-workdir for some of my projects. Apart from the usual
> caveat "don't checkout the same branch twice", I found another small
> issue, surely known: the reflog for HEAD in the workidirs does not exist
> in the source git repo, hence git-gc will happily prune what it believes
> to be dangling commits.
> 
> Would it be, perhaps, be okay to create a logs/workdir/<name>/HEAD
> symlink pointing to $workdir/logs/HEAD, so that this does not happen?

You also need to link the ref.  A log is only scanned if the
ref by the same name is scanned.

So you'd need to create a refs/workdir/<name>/HEAD in the parent
repository.  Using a symlink to point to the HEAD file from the
workdir might just work.

-- 
Shawn.

^ permalink raw reply

* [JGIT PATCH 11/23 v2] Rewrite WindowCache to use a hash table
From: Shawn O. Pearce @ 2008-12-27 17:32 UTC (permalink / raw)
  To: Robin Rosenberg; +Cc: git
In-Reply-To: <200812271430.06052.robin.rosenberg.lists@dewire.com>

This shaved 10s off my linux-2.6 "jgit rev-list --objects" test.
We're also getting slightly better cache hit ratios, approaching
95% during commit access and 80% during tree access on the same
repository.  Its a significant win.

The cache is also a bit better about memory usage.  We now make
sure the byte[] or ByteBuffer handle is cleared out of the soft
reference when we push the reference into the queue manually. A
small change, but it makes a marked difference with the overall
JVM memory usage.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
  Robin Rosenberg <robin.rosenberg.lists@dewire.com> wrote:
  > > -/**
  > > - * The WindowCache manages reusable <code>Windows</code> and inflaters used by
  > > - * the other windowed file access classes.
  > > - */
  > > -public class WindowCache {
  > > +class WindowCache {
  > 
  > This breaks the Eclipse plugin which want to call WindowCache.reconfigure. Package level
  > class can also have javadocs. Even though we do not require it, I still thinks it's a good idea. Not
  > sure how useful that one was.

  Dammit, good catch.  I was working on this in the context of only
  having the JGit code in my workspace, so I missed the impact on
  the Eclipse plugin, and possibly on the NetBeans one as well.

  Corrected patch follows.

  
 .../src/org/spearce/jgit/lib/ByteWindow.java       |   10 +-
 .../src/org/spearce/jgit/lib/WindowCache.java      |  317 ++++++++++++--------
 .../src/org/spearce/jgit/lib/WindowedFile.java     |    2 +-
 3 files changed, 198 insertions(+), 131 deletions(-)

diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/ByteWindow.java b/org.spearce.jgit/src/org/spearce/jgit/lib/ByteWindow.java
index 1f6beb8..8d37de7 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/ByteWindow.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/ByteWindow.java
@@ -56,14 +56,20 @@
  *            type of object reference used to manage the window data.
  */
 abstract class ByteWindow<T> extends SoftReference<T> {
+	boolean sizeActive = true;
+
+	ByteWindow<?> chainNext;
+
+	ByteWindow<?> lruPrev;
+
+	ByteWindow<?> lruNext;
+
 	final WindowedFile provider;
 
 	final int id;
 
 	final int size;
 
-	int lastAccessed;
-
 	final long start;
 
 	final long end;
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/WindowCache.java b/org.spearce.jgit/src/org/spearce/jgit/lib/WindowCache.java
index f617845..e71ca73 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/WindowCache.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/WindowCache.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2007, Robin Rosenberg <robin.rosenberg@dewire.com>
+ * Copyright (C) 2008, Google Inc.
  * Copyright (C) 2008, Shawn O. Pearce <spearce@spearce.org>
  *
  * All rights reserved.
@@ -68,23 +68,51 @@ private static final int bits(int newSize) {
 
 	static final ReferenceQueue<?> clearedWindowQueue;
 
-	private static ByteWindow[] windows;
+	private static ByteWindow[] cache;
 
-	private static int openWindowCount;
+	private static ByteWindow lruHead;
+
+	private static ByteWindow lruTail;
 
 	private static int openByteCount;
 
-	private static int accessClock;
+	private static int hits;
+
+	private static int reqs;
+
+	private static int opens;
+
+	private static int closes;
 
 	static {
 		maxByteCount = 10 * MB;
 		windowSizeShift = bits(8 * KB);
 		windowSize = 1 << windowSizeShift;
 		mmap = false;
-		windows = new ByteWindow[maxByteCount / windowSize];
+		cache = new ByteWindow[cacheTableSize()];
 		clearedWindowQueue = new ReferenceQueue<Object>();
 	}
 
+	static synchronized String statString() {
+		int maxChain = 0, tot = 0;
+		for (ByteWindow<?> e : cache) {
+			int n = 0;
+			for (; e != null; e = e.chainNext) {
+				n++;
+				tot++;
+			}
+			maxChain = Math.max(maxChain, n);
+		}
+		return "WindowCache[ hits: " + (hits * 100 / reqs) + "%"
+				+ "; windows: " + tot + " maxChain: " + maxChain + "; kb:"
+				+ (openByteCount / KB) + "; cache: " + cache.length + " fds: "
+				+ (opens - closes) + "," + opens + "," + closes + " ]";
+	}
+
+	private static int cacheTableSize() {
+		return 5 * (maxByteCount / windowSize) / 2;
+	}
+
 	/**
 	 * Modify the configuration of the window cache.
 	 * <p>
@@ -135,27 +163,36 @@ private static synchronized void reconfigureImpl(final int packedGitLimit,
 			// We have to throw away every window we have. None
 			// of them are suitable for the new configuration.
 			//
-			for (int i = 0; i < openWindowCount; i++) {
-				final ByteWindow win = windows[i];
-				if (--win.provider.openCount == 0)
-					win.provider.cacheClose();
-				windows[i] = null;
+			for (ByteWindow<?> e : cache) {
+				for (; e != null; e = e.chainNext)
+					clear(e);
+			}
+			runClearedWindowQueue();
+			cache = new ByteWindow[cacheTableSize()];
+
+		} else {
+			if (prune) {
+				// We should decrease our memory usage.
+				//
+				releaseMemory();
+				runClearedWindowQueue();
 			}
-			windows = new ByteWindow[maxByteCount / windowSize];
-			openWindowCount = 0;
-			openByteCount = 0;
-		} else if (prune) {
-			// Our memory limit was decreased so we should try
-			// to drop windows to ensure we meet the new lower
-			// limit we were just given.
-			//
-			final int wincnt = maxByteCount / windowSize;
-			releaseMemory(wincnt, null, 0, 0);
 
-			if (wincnt != windows.length) {
-				final ByteWindow[] n = new ByteWindow[wincnt];
-				System.arraycopy(windows, 0, n, 0, openWindowCount);
-				windows = n;
+			if (cache.length != cacheTableSize()) {
+				// The cache table should be resized.
+				// Rehash every entry.
+				//
+				final ByteWindow[] priorTable = cache;
+
+				cache = new ByteWindow[cacheTableSize()];
+				for (ByteWindow<?> e : priorTable) {
+					for (ByteWindow<?> n; e != null; e = n) {
+						n = e.chainNext;
+						final int idx = hash(e.provider, e.id);
+						e.chainNext = cache[idx];
+						cache[idx] = e;
+					}
+				}
 			}
 		}
 	}
@@ -178,19 +215,27 @@ private static synchronized void reconfigureImpl(final int packedGitLimit,
 	 */
 	public static synchronized final void get(final WindowCursor curs,
 			final WindowedFile wp, final long position) throws IOException {
+		reqs++;
 		final int id = (int) (position >> windowSizeShift);
-		int idx = binarySearch(wp, id);
-		if (0 <= idx) {
-			final ByteWindow<?> w = windows[idx];
-			if ((curs.handle = w.get()) != null) {
-				w.lastAccessed = ++accessClock;
-				curs.window = w;
-				return;
+		final int idx = hash(wp, id);
+		for (ByteWindow<?> e = cache[idx]; e != null; e = e.chainNext) {
+			if (e.provider == wp && e.id == id) {
+				if ((curs.handle = e.get()) != null) {
+					curs.window = e;
+					makeMostRecent(e);
+					hits++;
+					return;
+				}
+
+				clear(e);
+				break;
 			}
 		}
 
-		if (++wp.openCount == 1) {
+		if (wp.openCount == 0) {
 			try {
+				opens++;
+				wp.openCount = 1;
 				wp.cacheOpen();
 			} catch (IOException ioe) {
 				wp.openCount = 0;
@@ -201,106 +246,55 @@ public static synchronized final void get(final WindowCursor curs,
 			} catch (Error ioe) {
 				wp.openCount = 0;
 				throw ioe;
+			} finally {
+				wp.openCount--;
 			}
 
 			// The cacheOpen may have mapped the window we are trying to
 			// map ourselves. Retrying the search ensures that does not
 			// happen to us.
 			//
-			idx = binarySearch(wp, id);
-			if (0 <= idx) {
-				final ByteWindow<?> w = windows[idx];
-				if ((curs.handle = w.get()) != null) {
-					w.lastAccessed = ++accessClock;
-					curs.window = w;
-					return;
+			for (ByteWindow<?> e = cache[idx]; e != null; e = e.chainNext) {
+				if (e.provider == wp && e.id == id) {
+					if ((curs.handle = e.get()) != null) {
+						curs.window = e;
+						makeMostRecent(e);
+						return;
+					}
+
+					clear(e);
+					break;
 				}
 			}
 		}
 
-		idx = -(idx + 1);
-		final int wSz = windowSize(wp, id);
-		idx = releaseMemory(windows.length, wp, idx, wSz);
-
-		if (idx < 0)
-			idx = 0;
-		final int toMove = openWindowCount - idx;
-		if (toMove > 0)
-			System.arraycopy(windows, idx, windows, idx + 1, toMove);
-		wp.loadWindow(curs, id, id << windowSizeShift, wSz);
-		windows[idx] = curs.window;
-		openWindowCount++;
-		openByteCount += curs.window.size;
+		final int wsz = windowSize(wp, id);
+		wp.openCount++;
+		openByteCount += wsz;
+		releaseMemory();
+		runClearedWindowQueue();
+
+		wp.loadWindow(curs, id, id << windowSizeShift, wsz);
+		final ByteWindow<?> e = curs.window;
+		e.chainNext = cache[idx];
+		cache[idx] = e;
+		insertLRU(e);
 	}
 
-	private static int releaseMemory(final int maxWindowCount,
-			final WindowedFile willRead, int insertionIndex, final int willAdd) {
-		for (;;) {
-			final ByteWindow<?> w = (ByteWindow<?>) clearedWindowQueue.poll();
-			if (w == null)
-				break;
-			final int oldest = binarySearch(w.provider, w.id);
-			if (oldest < 0 || windows[oldest] != w)
-				continue; // Must have been evicted by our other controls.
-
-			final WindowedFile p = w.provider;
-			if (--p.openCount == 0 && p != willRead)
-				p.cacheClose();
-
-			openByteCount -= w.size;
-			final int toMove = openWindowCount - oldest - 1;
-			if (toMove > 0)
-				System.arraycopy(windows, oldest + 1, windows, oldest, toMove);
-			windows[--openWindowCount] = null;
-			if (oldest < insertionIndex)
-				insertionIndex--;
-		}
-
-		while (openWindowCount >= maxWindowCount
-				|| (openWindowCount > 0 && openByteCount + willAdd > maxByteCount)) {
-			int oldest = 0;
-			for (int k = openWindowCount - 1; k > 0; k--) {
-				if (windows[k].lastAccessed < windows[oldest].lastAccessed)
-					oldest = k;
-			}
-
-			final ByteWindow w = windows[oldest];
-			final WindowedFile p = w.provider;
-			if (--p.openCount == 0 && p != willRead)
-				p.cacheClose();
-
-			openByteCount -= w.size;
-			final int toMove = openWindowCount - oldest - 1;
-			if (toMove > 0)
-				System.arraycopy(windows, oldest + 1, windows, oldest, toMove);
-			windows[--openWindowCount] = null;
-			w.enqueue();
-			if (oldest < insertionIndex)
-				insertionIndex--;
+	private static void makeMostRecent(ByteWindow<?> e) {
+		if (lruHead != e) {
+			unlinkLRU(e);
+			insertLRU(e);
 		}
-
-		return insertionIndex;
 	}
 
-	private static final int binarySearch(final WindowedFile sprov,
-			final int sid) {
-		if (openWindowCount == 0)
-			return -1;
-		final int shc = sprov.hash;
-		int high = openWindowCount;
-		int low = 0;
-		do {
-			final int mid = (low + high) / 2;
-			final ByteWindow mw = windows[mid];
-			if (mw.provider == sprov && mw.id == sid)
-				return mid;
-			final int mhc = mw.provider.hash;
-			if (mhc < shc || (shc == mhc && mw.id < sid))
-				low = mid + 1;
-			else
-				high = mid;
-		} while (low < high);
-		return -(low + 1);
+	private static void releaseMemory() {
+		ByteWindow<?> e = lruTail;
+		while (openByteCount > maxByteCount && e != null) {
+			final ByteWindow<?> p = e.lruPrev;
+			clear(e);
+			e = p;
+		}
 	}
 
 	/**
@@ -316,22 +310,89 @@ private static final int binarySearch(final WindowedFile sprov,
 	 *            cache.
 	 */
 	public static synchronized final void purge(final WindowedFile wp) {
-		int d = 0;
-		for (int s = 0; s < openWindowCount; s++) {
-			final ByteWindow win = windows[s];
-			if (win.provider != wp)
-				windows[d++] = win;
-			else
-				openByteCount -= win.size;
+		for (ByteWindow e : cache) {
+			for (; e != null; e = e.chainNext) {
+				if (e.provider == wp)
+					clear(e);
+			}
+		}
+		runClearedWindowQueue();
+	}
+
+	private static void runClearedWindowQueue() {
+		ByteWindow<?> e;
+		while ((e = (ByteWindow) clearedWindowQueue.poll()) != null) {
+			unlinkSize(e);
+			unlinkLRU(e);
+			unlinkCache(e);
+			e.chainNext = null;
+			e.lruNext = null;
+			e.lruPrev = null;
 		}
-		openWindowCount = d;
+	}
+
+	private static void clear(final ByteWindow<?> e) {
+		unlinkSize(e);
+		e.clear();
+		e.enqueue();
+	}
 
-		if (wp.openCount > 0) {
-			wp.openCount = 0;
-			wp.cacheClose();
+	private static void unlinkSize(final ByteWindow<?> e) {
+		if (e.sizeActive) {
+			if (--e.provider.openCount == 0) {
+				closes++;
+				e.provider.cacheClose();
+			}
+			openByteCount -= e.size;
+			e.sizeActive = false;
 		}
 	}
 
+	private static void unlinkCache(final ByteWindow dead) {
+		final int idx = hash(dead.provider, dead.id);
+		ByteWindow<?> e = cache[idx], p = null, n;
+		for (; e != null; p = e, e = n) {
+			n = e.chainNext;
+			if (e == dead) {
+				if (p == null)
+					cache[idx] = n;
+				else
+					p.chainNext = n;
+				break;
+			}
+		}
+	}
+
+	private static void unlinkLRU(final ByteWindow e) {
+		final ByteWindow<?> prev = e.lruPrev;
+		final ByteWindow<?> next = e.lruNext;
+
+		if (prev != null)
+			prev.lruNext = next;
+		else
+			lruHead = next;
+
+		if (next != null)
+			next.lruPrev = prev;
+		else
+			lruTail = prev;
+	}
+
+	private static void insertLRU(final ByteWindow<?> e) {
+		final ByteWindow h = lruHead;
+		e.lruPrev = null;
+		e.lruNext = h;
+		if (h != null)
+			h.lruPrev = e;
+		else
+			lruTail = e;
+		lruHead = e;
+	}
+
+	private static int hash(final WindowedFile wp, final int id) {
+		return ((wp.hash + id) >>> 1) % cache.length;
+	}
+
 	private static int windowSize(final WindowedFile file, final int id) {
 		final long len = file.length();
 		final long pos = id << windowSizeShift;
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/WindowedFile.java b/org.spearce.jgit/src/org/spearce/jgit/lib/WindowedFile.java
index dd94f24..9c5cf1e 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/WindowedFile.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/WindowedFile.java
@@ -88,7 +88,7 @@
 	 */
 	public WindowedFile(final File file) {
 		fPath = file;
-		hash = System.identityHashCode(this);
+		hash = System.identityHashCode(this) * 31;
 		length = Long.MAX_VALUE;
 	}
 
-- 
1.6.1.302.gccd4d

^ permalink raw reply related

* Re: Questions about repo and git submodule
From: Shawn O. Pearce @ 2008-12-27 17:45 UTC (permalink / raw)
  To: Emily Ren; +Cc: Git Mailinglist
In-Reply-To: <856bfe0e0812242228o6a428702i296dc87c76cf9dba@mail.gmail.com>

Emily Ren <lingyan.ren@gmail.com> wrote:
> Thank you for your guide ! I've created git repository with submodules
> with a simple script successfully.
> 
> Now I have another question,  since my android repo will always sync
> up from android.git.kernel.org, my git repository needs to be updated
> accordingly.  Is there a tool I can use to sync up from repo to git
> repositoy ?

I think this discussion has moved to the repo-discuss list:

  https://groups.google.com/group/repo-discuss/browse_thread/thread/2fa368ed7cac5d79

-- 
Shawn.

^ permalink raw reply

* Re: [PATCH] Add a commit.signoff configuration variable to always use --signoff.
From: Jan Krüger @ 2008-12-27 18:08 UTC (permalink / raw)
  To: Adeodato Simó; +Cc: git, gitster
In-Reply-To: <1230296219-16408-1-git-send-email-dato@net.com.org.es>

Hi,

On Fri, 26 Dec 2008 13:56:59 +0100
"Adeodato Simó" <dato@net.com.org.es> wrote:

> > Has there even been talk of a commit.signoff configuration variable
> > to always add a S-o-b line? This could allow to enable it on a
> > per-project basis, which would be cool.
> 
> Well, it seemed easy enough to do, so I went ahead. Comments would be
> welcome.

I think it might be a good idea to allow overriding the config variable
in the other direction, i.e. a --no-signoff option to commit. Otherwise,
for example, rebase would have no way of suppressing the S-o-b lines in
rebased commits (and you might want to not automatically sign off
commits you rebase).

-Jan

^ permalink raw reply

* Re: [PATCH] git-shortlog.txt: improve documentation about .mailmap files
From: Adeodato Simó @ 2008-12-27 18:21 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7viqp5et48.fsf@gitster.siamese.dyndns.org>

* Junio C Hamano [Sat, 27 Dec 2008 03:48:39 -0800]:

> Adeodato Simó <dato@net.com.org.es> writes:

> > The previous .mailmap example made it seem like .mailmap files are only
> > useful for commits with a wrong address for an author, when they are about
> > fixing the real name. Explained this better in the text, and replaced the
> > existing example with a new one that hopefully makes things clearer.

> Thanks.

Thanks for your review!

> > -If the file `.mailmap` exists, it will be used for mapping author
> > -email addresses to a real author name. One mapping per line, first
> > -the author name followed by the email address enclosed by
> > -'<' and '>'. Use hash '#' for comments. Example:
> > +If a file `.mailmap` exists in the toplevel directory of the repository,
> > +it will be used for mapping author email addresses to a canonical real
> > +name. This can be used to coalesce together commits by the same person
> > +where their name was spelled differently (whether with the same email
> > +address or not).

> We didn't stress "the toplevel" earlier, partly because it is obvious that
> the file cannot be anything but project-tree wide (as opposed to being per
> subdirectory, similar to .gitignore and .gitattributes).  I guess it would
> not hurt to be explicit, even though it feels slightly silly.

Hm. I'd prefer to keep it in if you don't mind much.

> "..., it is used to map author email addresses to..." would flow easier.

Changed.

> > +The format of the file is one mapping per line, first the desired author
> > +name followed by the email address enclosed by '<' and '>'. Use hash '#'
> > +for comments.

> You already introduced the term "a canonical real name" in the earlier
> description.  It would be easier to read if you stick to it and say "Each
> line consists of the canonical real name of an author, whitespaces, and an
> email address, enclosed by '<' and '>', to map to the name".

Changed.

> Can a hash '#' character be anywhere on a line?  E.g. how is an entry like
> this processed?

> 	Jane Doe <jane@desktop.(none)> # early mistake...

It is processed correctly. Good suggestion to have it documented. I did:

  Use hash '#' for comments, either on their own line, or after the
  email address.

> > +... For example, if your history contains commits by these
> > +committers:

> I think you meant "authors", not "committers".

Ok.

> > +------------
> > +Author: Joe Developer <joe@random.com>
> > +Author: Joe R. Developer <joe@random.com>
> > +Author: Jane Doe <jane@the-does.name>
> > +Author: Jane Doe <jane@laptop.(none)>
> > +Author: Jane D. <jane@desktop.(none)>
> > +------------

> I'd suggest dropping "Author: ".  You said you are listing people.

And ok.

> Isn't random.com a real domain (the same goes for the-does.name)?  It
> would be preferrable to use addresses from .example (or .xz) top-level
> domain.

I wanted to use a real TLD to clearly convey that these were for real
addresses and not misconfigured ones. How about "example.com"? (This is
used in other Documentation files.)

> Clarify that there are actually two people in the list above, and explain
> that they are one Joe with two spellings who prefers to be referred to
> with his middle initial, and one Jane with three spellings who prefers to
> show the family name fully spelled out.  Do not force your readers guess
> which spelling is preferred for each person in the example.  It would make
> it easier for them to understand the example you will give them next and
> to agree that the mailmap is "proper".

Good point. As always, what it's obvious to the writer may not be to the
reader. Thanks.

I also added a sentence mentioning that names can appear more than once,
but addresses can't.

Can you take another look? (Amended patch coming.)

-- 
Adeodato Simó                                     dato at net.com.org.es
Debian Developer                                  adeodato at debian.org
 
- You look beaten.
- I just caught Tara laughing with another man.
- Are you sure they weren't just... kissing or something?
- No, they were laughing.
                -- Denny Crane and Alan Shore

^ permalink raw reply

* [PATCH v2] git-shortlog.txt: improve documentation about .mailmap files
From: Adeodato Simó @ 2008-12-27 18:23 UTC (permalink / raw)
  To: git, gitster; +Cc: Adeodato Simó
In-Reply-To: <20081227182140.GA28946@chistera.yi.org>

The previous .mailmap example made it seem like .mailmap files are only
useful for commits with a wrong address for an author, when they are about
fixing the real name. Explained this better in the text, and replaced the
existing example with a new one that hopefully makes things clearer.

Signed-off-by: Adeodato Simó <dato@net.com.org.es>
---
 Documentation/git-shortlog.txt |   40 +++++++++++++++++++++++++++++++++-------
 1 files changed, 33 insertions(+), 7 deletions(-)

diff --git a/Documentation/git-shortlog.txt b/Documentation/git-shortlog.txt
index 7ccf31c..4a76b7f 100644
--- a/Documentation/git-shortlog.txt
+++ b/Documentation/git-shortlog.txt
@@ -48,15 +48,41 @@ OPTIONS
 FILES
 -----
 
-If the file `.mailmap` exists, it will be used for mapping author
-email addresses to a real author name. One mapping per line, first
-the author name followed by the email address enclosed by
-'<' and '>'. Use hash '#' for comments. Example:
+If a file `.mailmap` exists in the toplevel directory of the repository,
+it is used to map author email addresses to a canonical real name. This
+can be used to coalesce together commits by the same person where their
+name was spelled differently (whether with the same email address or
+not).
+
+Each line in the file consists, in this order, of the canonical real name
+of an author, whitespace, and an email address (enclosed by '<' and '>')
+to map to the name. Use hash '#' for comments, either on their own line,
+or after the email address.
+
+A canonical name may appear in more than one line, associated with
+different email addresses, but it doesn't make sense for a given address
+to appear more than once (if that happens, the latest line in which it
+appears will take effect).
+
+So, for example, if your history contains commits by two authors, Jane
+and Joe, whose names appear in the repository under several forms:
+
+------------
+Joe Developer <joe@example.com>
+Joe R. Developer <joe@example.com>
+Jane Doe <jane@example.com>
+Jane Doe <jane@laptop.(none)>
+Jane D. <jane@desktop.(none)>
+------------
+
+Then, supposing Joe wants his middle name initial used, and Jane prefers
+her family name fully spelled out, a proper `.mailmap` file would be:
 
 ------------
-# Keep alphabetized
-Adam Morrow <adam@localhost.localdomain>
-Eve Jones <eve@laptop.(none)>
+# Note how we don't need an entry for <jane@laptop.(none)>, because the
+# real name of that author is correct already, and coalesced directly.
+Jane Doe <jane@desktop.(none)>
+Joe R. Developer <joe@random.com>
 ------------
 
 Author
-- 
1.6.1.307.g07803

^ permalink raw reply related

* Re: [PATCH] Add a commit.signoff configuration variable to always use --signoff.
From: Adeodato Simó @ 2008-12-27 18:40 UTC (permalink / raw)
  To: Jan Krüger; +Cc: git, gitster
In-Reply-To: <20081227190819.7257932a@neuron>

* Jan Krüger [Sat, 27 Dec 2008 19:08:19 +0100]:

> I think it might be a good idea to allow overriding the config variable
> in the other direction, i.e. a --no-signoff option to commit. Otherwise,
> for example, rebase would have no way of suppressing the S-o-b lines in
> rebased commits (and you might want to not automatically sign off
> commits you rebase).

Good catch.

--no-signoff exists already, so maybe git-rebase should just use it?

-- 
Adeodato Simó                                     dato at net.com.org.es
Debian Developer                                  adeodato at debian.org
 
Man is certainly stark mad; he cannot make a flea, yet he makes gods by the
dozens.
                -- Michel de Montaigne

^ permalink raw reply

* Re: [PATCH] Add a commit.signoff configuration variable to always use --signoff.
From: Jan Krüger @ 2008-12-27 19:15 UTC (permalink / raw)
  To: Adeodato Simó; +Cc: git, gitster
In-Reply-To: <20081227184001.GA31893@chistera.yi.org>

Hi,

On Sat, 27 Dec 2008 19:40:01 +0100
"Adeodato Simó" <dato@net.com.org.es> wrote:

> > I think it might be a good idea to allow overriding the config
> > variable in the other direction, i.e. a --no-signoff option to
> > commit. [...]
> 
> Good catch.
> 
> --no-signoff exists already, so maybe git-rebase should just use it?

Oh, yeah, I didn't notice the option mechanism worked this way and the
options got parsed after processing the config variables. I guess
rebase and rebase--interactive should use it, then, and perhaps there
are other commands that should that I just don't know about (a grep
across all commands implemented in shell script didn't find anything
else, though).

-Jan

^ permalink raw reply

* Fix commit message after push?
From: skillzero @ 2008-12-27 21:13 UTC (permalink / raw)
  To: git

Is there a way to fix a commit message after the commit has already
pushed to someone else? I typed the wrong commit message then I pushed
to the remote repository before realizing it. I tried git commit
--amend to fix it in my local repository, but it rejects the
subsequent push. I somewhat understand why it's rejecting it (my
--amend rewrote history and now they are out-of-sync), but I was
wondering if there is a general way to handle situations like this
with git.

^ permalink raw reply

* [PATCH] Documentation/git-format-patch.txt: fix weird backslash at --root
From: jidanni @ 2008-12-27 19:57 UTC (permalink / raw)
  To: git

However note --root is not mentioned in SYNOPSIS nor on its own line.
Also fixed grammar.

Signed-off-by: jidanni <jidanni@jidanni.org>

diff --git a/git-format-patch.txt b/git-format-patch.txt
index ee27eff..51c25cd 100644
--- a/git-format-patch.txt
+++ b/git-format-patch.txt
@@ -48 +48 @@ everything since project inception to one commit, say "git
-format-patch \--root <commit>" to make it clear that it is the
+format-patch --root <commit>" to make it clear that it is the
@@ -166 +166 @@ not add any suffix.
-	that they differ.  Note that this disable the patch to be properly
+	that they differ.  Note that this disables the patch from being properly
@@ -209 +209 @@ project:
-$ git format-patch \--root origin
+$ git format-patch --root origin
-- 
1.5.6.5

^ permalink raw reply related

* [PATCH] Documentation/diff-options.txt: unify options
From: jidanni @ 2008-12-27 20:12 UTC (permalink / raw)
  To: git


Signed-off-by: jidanni <jidanni@jidanni.org>

diff --git a/diff-options.txt b/diff-options.txt
index 5721548..b05503a 100644
--- a/diff-options.txt
+++ b/diff-options.txt
@@ -21,0 +22 @@ ifndef::git-format-patch[]
+-u::
@@ -26,3 +26,0 @@ endif::git-format-patch[]
--u::
-	Synonym for "-p".
-
@@ -30,2 +27,0 @@ endif::git-format-patch[]
-	Shorthand for "--unified=<n>".
-
@@ -189,0 +186 @@ endif::git-format-patch[]
+-a::
@@ -193,3 +189,0 @@ endif::git-format-patch[]
--a::
-	Shorthand for "--text".
-
@@ -198,0 +193 @@ endif::git-format-patch[]
+-b::
@@ -204,3 +199 @@ endif::git-format-patch[]
--b::
-	Shorthand for "--ignore-space-change".
-
+-w::
@@ -212,3 +204,0 @@ endif::git-format-patch[]
--w::
-	Shorthand for "--ignore-all-space".
-
-- 
1.5.6.5

^ permalink raw reply related

* [PATCH] Documentation/git-tag.txt grammar
From: jidanni @ 2008-12-27 19:49 UTC (permalink / raw)
  To: git


Signed-off-by: jidanni <jidanni@jidanni.org>

diff --git a/git-tag.txt b/git-tag.txt
index 6cf11ce..e5503be 100644
--- a/git-tag.txt
+++ b/git-tag.txt
@@ -72 +72 @@ OPTIONS
-	If multiple `-m` options are given, there values are
+	If multiple `-m` options are given, their values are
@@ -209 +209 @@ the boundary between one circle of people (e.g. "people who are
-primarily interested in networking part of the kernel") who may
+primarily interested in the networking part of the kernel") who may
-- 
1.5.6.5

^ permalink raw reply related

* Re: Fix commit message after push?
From: Miklos Vajna @ 2008-12-27 21:19 UTC (permalink / raw)
  To: skillzero; +Cc: git
In-Reply-To: <2729632a0812271313l74e9bb85l857a3ad3a3f0d8d8@mail.gmail.com>

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

On Sat, Dec 27, 2008 at 01:13:46PM -0800, skillzero@gmail.com wrote:
> Is there a way to fix a commit message after the commit has already
> pushed to someone else? I typed the wrong commit message then I pushed
> to the remote repository before realizing it. I tried git commit
> --amend to fix it in my local repository, but it rejects the
> subsequent push. I somewhat understand why it's rejecting it (my
> --amend rewrote history and now they are out-of-sync), but I was
> wondering if there is a general way to handle situations like this
> with git.

git push -f

http://git.or.cz/gitwiki/GitFaq#head-c1dc263aca199d347f28872249e6c1f5d519a2df

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

^ permalink raw reply

* Re: [PATCH v2] parse-opt: migrate builtin-apply.
From: Junio C Hamano @ 2008-12-27 21:47 UTC (permalink / raw)
  To: Miklos Vajna; +Cc: git
In-Reply-To: <1230387764-11230-1-git-send-email-vmiklos@frugalware.org>

Miklos Vajna <vmiklos@frugalware.org> writes:

> +static int option_parse_inaccurate(const struct option *opt,
> +				   const char *arg, int unset)
> +{
> +	options |= INACCURATE_EOF;
> +	return 0;
> +}
> +
> +static int option_parse_recount(const struct option *opt,
> +				const char *arg, int unset)
> +{
> +	options |= RECOUNT;
> +	return 0;
> +}

I still haven't applied and ran the testsuite myself, but these makes me
wonder if there isn't a built-in "bit" type support in parse_options().

^ permalink raw reply

* Re: [PATCH v2] parse-opt: migrate builtin-apply.
From: René Scharfe @ 2008-12-27 21:53 UTC (permalink / raw)
  To: Miklos Vajna; +Cc: Junio C Hamano, git
In-Reply-To: <1230387764-11230-1-git-send-email-vmiklos@frugalware.org>

Miklos Vajna schrieb:
> -static const char apply_usage[] =
> -"git apply [--stat] [--numstat] [--summary] [--check] [--index] [--cached] [--apply] [--no-add] [--index-info] [--allow-binary-replacement] [--reverse] [--reject] [--verbose] [-z] [-pNUM] [-CNUM] [--whitespace=<nowarn|warn|fix|error|error-all>] <patch>...";
> +static const char * const apply_usage[] = {
> +	"git apply [--stat] [--numstat] [--summary] [--check] [--index] [--cached] [--apply] [--no-add] [--index-info] [--allow-binary-replacement] [--reverse] [--reject] [--verbose] [-z] [-pNUM] [-CNUM] [--whitespace=<nowarn|warn|fix|error|error-all>] <patch>...",
> +	NULL
> +};

A useful convention with parse_options is to display "[options]" as a
place holder instead of listing all options explicitly in the usage
string.  They are listed and explained in the full help message anyway
shown by "git apply -?").

> +static int option_parse_inaccurate(const struct option *opt,
> +				   const char *arg, int unset)
> +{
> +	options |= INACCURATE_EOF;
> +	return 0;
> +}
> +
> +static int option_parse_recount(const struct option *opt,
> +				const char *arg, int unset)
> +{
> +	options |= RECOUNT;
> +	return 0;
> +}

OPT_BIT?

> +		OPT_INTEGER('C', NULL, &p_context,
> +				"ensure at least <n> lines of context match"),

p_context is an unsigned long variable; OPT_INTEGER expects a pointer
to an int.  You'd either need an OPT_ULONG macro or change p_context
to in int.  Doing the latter fixed the two test cases t4105 and t4252
for me.

René

^ permalink raw reply

* Re: [PATCH] git-send-email.txt: move --format-patch paragraph to a proper location
From: Junio C Hamano @ 2008-12-27 21:54 UTC (permalink / raw)
  To: Adeodato Simó; +Cc: git
In-Reply-To: <1230367830-6114-1-git-send-email-dato@net.com.org.es>

Adeodato Simó <dato@net.com.org.es> writes:

> When introducing --format-patch, its documentation was accidentally inserted
> in the middle of documentation for --validate.

Good eyes.  Thanks.

^ permalink raw reply

* for newbs = little exercise / tutorial / warmup for windows and other non-sophisticated new Git users :-)
From: Zorba @ 2008-12-27 21:56 UTC (permalink / raw)
  To: git

Here is a little exercise / tutorial / warm-up for someone starting out with 
Git. If you're anyting like me you may find the tutorials etc. on git.or.cz 
a bit daunting. I recommend you try this after reading the user manual but 
before tearing your hair out trying to follow all the examples in the user 
manual. After you've followed this simple workflow, then go back to the more 
advanced stuff  in the tutorials and user manuals (like cloning repositories 
and creating and merging branches).

I created this exercise to try and model our workflow and what we wanted to 
use git for = tracking a project with multiple files where the filebase 
might change frequently from one version to the next.

http://siliconmouth.wordpress.com/category/nerdy/

look for December 27, 2008 or "git warmup" 

^ permalink raw reply

* Re: [PATCH] Documentation/diff-options.txt: unify options
From: Junio C Hamano @ 2008-12-27 22:01 UTC (permalink / raw)
  To: jidanni; +Cc: git
In-Reply-To: <E1LGfY3-0001pM-S9@jidanni.org>

jidanni@jidanni.org writes:

> Signed-off-by: jidanni <jidanni@jidanni.org>
>
> diff --git a/diff-options.txt b/diff-options.txt
> index 5721548..b05503a 100644
> --- a/diff-options.txt
> +++ b/diff-options.txt
> @@ -21,0 +22 @@ ifndef::git-format-patch[]
> +-u::
> @@ -26,3 +26,0 @@ endif::git-format-patch[]
> --u::
> -	Synonym for "-p".
> -

Sorry, but this patch is very unusual in that it lacks any context lines,
which makes it impossible to review.

^ permalink raw reply

* Re: [PATCH] Documentation/diff-options.txt: unify options
From: jidanni @ 2008-12-27 22:08 UTC (permalink / raw)
  To: gitster; +Cc: git
In-Reply-To: <7vfxk9cm6w.fsf@gitster.siamese.dyndns.org>

JCH> Sorry, but this patch is very unusual in that it lacks any context lines,
JCH> which makes it impossible to review.

Trust me, I tried it with the default context lines and it was just
the same hard reading. OK, next time I send patches for other files, I
will always use the default and forget about saving bytes.

^ permalink raw reply

* Re: [PATCH] Documentation/git-format-patch.txt: fix weird backslash at --root
From: Junio C Hamano @ 2008-12-27 22:15 UTC (permalink / raw)
  To: jidanni; +Cc: git
In-Reply-To: <E1LGfLz-0001nM-AO@jidanni.org>

jidanni@jidanni.org writes:

> @@ -209 +209 @@ project:
> -$ git format-patch \--root origin
> +$ git format-patch --root origin

I thought these backslashes before double-dash are protecting the latter
from being turned into em-dashes.  I do not see any weird backslash in the
"git help" (aka "manpage") output nor in the HTML documentation, e.g.

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

without your patch, so I have to say that this chunk is a useless churn.

All of your three patches lack context, which is essential enabler for
easier review.  They have "diff --git" header, but are not made relative
to the root of the project tree (i.e. lack "Documentation/" prefix), which
is the norm for both the tool and for this project and this mailing list.

I guess that you may be experimenting with various options to see how they
work, and the curiosity by itself is a good thing, but please do not make
other people suffer with results from such experiments by sending patches
in nonstandard forms.

By submitting a patch to try improving the system, you are already making
a difference in substance.  Please do not try to be creative in form.  It
only makes lives of other people unnecessarily harder with no real reason
and wastes other people's time.  And it wastes yours, too.  A patch
conforming to the norm is much easier to review, comment on and apply.
Ok?

"Don't try to be different in form, make a difference in substance" also
applies to your S-o-b line, by the way.

^ permalink raw reply

* Re: [PATCH] Documentation/git-format-patch.txt: fix weird backslash at --root
From: jidanni @ 2008-12-27 22:36 UTC (permalink / raw)
  To: gitster; +Cc: git
In-Reply-To: <7v4p0pcliy.fsf@gitster.siamese.dyndns.org>

JCH> http://www.kernel.org/pub/software/scm/git/docs/git-format-patch.html
That looks OK. All I know is on Debian (1:1.5.6.5-2) it gets rendered:
$ w3m -dump /usr/share/doc/git-doc/git-format-patch.html|fgrep \\-
    $ git format-patch \--root origin

I was patching offline from Debian. OK, in the future I will clone
upstream directly properly first.

^ permalink raw reply

* Re: [PATCH 2/2] grep: grep cache entries if they are "assume unchanged"
From: Junio C Hamano @ 2008-12-27 22:39 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy; +Cc: git, Daniel Barkalow
In-Reply-To: <1230366064-1306-1-git-send-email-pclouds@gmail.com>

Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:

> ... External grep case has not been fixed yet. But given that
> on the platform that CE_VALID bit may be used like Windows, external
> grep is not available anyway, I would wait for people to raise their
> hands before touching it.

My gut feeling is that it would suffice to always use the internal grep if
there is any index entry with CE_VALID set.  Having the bit is an unusual
case.

Also I am guessing that your above statement that users on Windows would
be the ones who want CE_VALID implies that CE_VALID is for platforms where
file operations are slow.  If that is really the case, the internal grep
might even have performance advantage over external grep.

No, I haven't benched the internal grep recently.

^ permalink raw reply

* [PATCH] parse-opt: migrate builtin-apply.
From: Miklos Vajna @ 2008-12-27 23:03 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Rene Scharfe, git
In-Reply-To: <4956A3E7.7070208@lsrfire.ath.cx>

The only incompatible change is that the user how have to use '--'
before a patch named --build-fake-ancestor=something.

Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>
---

On Sat, Dec 27, 2008 at 10:53:43PM +0100, René Scharfe <rene.scharfe@lsrfire.ath.cx> wrote:
> Miklos Vajna schrieb:
> > -static const char apply_usage[] =
> > -"git apply [--stat] [--numstat] [--summary] [--check] [--index] [--cached] [--apply] [--no-add] [--index-info] [--allow-binary-replacement] [--reverse] [--reject] [--verbose] [-z] [-pNUM] [-CNUM] [--whitespace=<nowarn|warn|fix|error|error-all>] <patch>...";
> > +static const char * const apply_usage[] = {
> > +   "git apply [--stat] [--numstat] [--summary] [--check] [--index] [--cached] [--apply] [--no-add] [--index-info] [--allow-binary-replacement] [--reverse] [--reject] [--verbose] [-z] [-pNUM] [-CNUM] [--whitespace=<nowarn|warn|fix|error|error-all>] <patch>...",
> > +   NULL
> > +};
>
> A useful convention with parse_options is to display "[options]" as a
> place holder instead of listing all options explicitly in the usage
> string.  They are listed and explained in the full help message anyway
> shown by "git apply -?").

True, changed.

> > +static int option_parse_recount(const struct option *opt,
> > +                           const char *arg, int unset)
> > +{
> > +   options |= RECOUNT;
> > +   return 0;
> > +}
>
> OPT_BIT?

I haven't discovered it, you are right. :-)

> > +           OPT_INTEGER('C', NULL, &p_context,
> > +                           "ensure at least <n> lines of context match"),
>
> p_context is an unsigned long variable; OPT_INTEGER expects a pointer
> to an int.  You'd either need an OPT_ULONG macro or change p_context
> to in int.  Doing the latter fixed the two test cases t4105 and t4252
> for me.

Thanks, the trick was that the test did not fail on i686, just on
x86_64. I have changed it as you suggested and it now passes for me on
x86_64 as well.

 Documentation/git-apply.txt |    4 +-
 builtin-apply.c             |  281 +++++++++++++++++++++++--------------------
 2 files changed, 154 insertions(+), 131 deletions(-)

diff --git a/Documentation/git-apply.txt b/Documentation/git-apply.txt
index e726510..9400f6a 100644
--- a/Documentation/git-apply.txt
+++ b/Documentation/git-apply.txt
@@ -10,7 +10,7 @@ SYNOPSIS
 --------
 [verse]
 'git apply' [--stat] [--numstat] [--summary] [--check] [--index]
-	  [--apply] [--no-add] [--build-fake-ancestor <file>] [-R | --reverse]
+	  [--apply] [--no-add] [--build-fake-ancestor=<file>] [-R | --reverse]
 	  [--allow-binary-replacement | --binary] [--reject] [-z]
 	  [-pNUM] [-CNUM] [--inaccurate-eof] [--recount] [--cached]
 	  [--whitespace=<nowarn|warn|fix|error|error-all>]
@@ -64,7 +64,7 @@ OPTIONS
 	cached data, apply the patch, and store the result in the index,
 	without using the working tree. This implies '--index'.
 
---build-fake-ancestor <file>::
+--build-fake-ancestor=<file>::
 	Newer 'git-diff' output has embedded 'index information'
 	for each blob to help identify the original version that
 	the patch applies to.  When this flag is given, and if
diff --git a/builtin-apply.c b/builtin-apply.c
index 07244b0..7603658 100644
--- a/builtin-apply.c
+++ b/builtin-apply.c
@@ -14,6 +14,7 @@
 #include "builtin.h"
 #include "string-list.h"
 #include "dir.h"
+#include "parse-options.h"
 
 /*
  *  --check turns on checking that the working tree matches the
@@ -45,9 +46,11 @@ static int apply_verbosely;
 static int no_add;
 static const char *fake_ancestor;
 static int line_termination = '\n';
-static unsigned long p_context = ULONG_MAX;
-static const char apply_usage[] =
-"git apply [--stat] [--numstat] [--summary] [--check] [--index] [--cached] [--apply] [--no-add] [--index-info] [--allow-binary-replacement] [--reverse] [--reject] [--verbose] [-z] [-pNUM] [-CNUM] [--whitespace=<nowarn|warn|fix|error|error-all>] <patch>...";
+static unsigned int p_context = UINT_MAX;
+static const char * const apply_usage[] = {
+	"git apply [options] [<patch>...]",
+	NULL
+};
 
 static enum ws_error_action {
 	nowarn_ws_error,
@@ -61,6 +64,8 @@ static int applied_after_fixing_ws;
 static const char *patch_input_file;
 static const char *root;
 static int root_len;
+static int read_stdin = 1;
+static int options;
 
 static void parse_whitespace_option(const char *option)
 {
@@ -3135,150 +3140,168 @@ static int git_apply_config(const char *var, const char *value, void *cb)
 	return git_default_config(var, value, cb);
 }
 
+static int option_parse_stdin(const struct option *opt,
+			      const char *arg, int unset)
+{
+	int *errs = opt->value;
+
+	*errs |= apply_patch(0, "<stdin>", options);
+	read_stdin = 0;
+	return 0;
+}
+
+static int option_parse_exclude(const struct option *opt,
+				const char *arg, int unset)
+{
+	add_name_limit(arg, 1);
+	return 0;
+}
+
+static int option_parse_include(const struct option *opt,
+				const char *arg, int unset)
+{
+	add_name_limit(arg, 0);
+	has_include = 1;
+	return 0;
+}
+
+static int option_parse_p(const struct option *opt,
+			  const char *arg, int unset)
+{
+	p_value = atoi(arg);
+	p_value_known = 1;
+	return 0;
+}
+
+static int option_parse_z(const struct option *opt,
+			  const char *arg, int unset)
+{
+	if (unset)
+		line_termination = '\n';
+	else
+		line_termination = 0;
+	return 0;
+}
+
+static int option_parse_whitespace(const struct option *opt,
+				   const char *arg, int unset)
+{
+	const char **whitespace_option = opt->value;
+
+	*whitespace_option = arg;
+	parse_whitespace_option(arg);
+	return 0;
+}
+
+static int option_parse_directory(const struct option *opt,
+				  const char *arg, int unset)
+{
+	root_len = strlen(arg);
+	if (root_len && arg[root_len - 1] != '/') {
+		char *new_root;
+		root = new_root = xmalloc(root_len + 2);
+		strcpy(new_root, arg);
+		strcpy(new_root + root_len++, "/");
+	} else
+		root = arg;
+	return 0;
+}
 
 int cmd_apply(int argc, const char **argv, const char *unused_prefix)
 {
 	int i;
-	int read_stdin = 1;
-	int options = 0;
 	int errs = 0;
 	int is_not_gitdir;
+	int binary;
+	int force_apply = 0;
 
 	const char *whitespace_option = NULL;
 
+	struct option builtin_apply_options[] = {
+		{ OPTION_CALLBACK, '-', NULL, &errs, NULL,
+			"read the patch from the standard input",
+			PARSE_OPT_NOARG, option_parse_stdin },
+		{ OPTION_CALLBACK, 0, "exclude", NULL, "path",
+			"don´t apply changes matching the given path",
+			0, option_parse_exclude },
+		{ OPTION_CALLBACK, 0, "include", NULL, "path",
+			"apply changes matching the given path",
+			0, option_parse_include },
+		{ OPTION_CALLBACK, 'p', NULL, NULL, "num",
+			"remove <num> leading slashes from traditional diff paths",
+			0, option_parse_p },
+		OPT_BOOLEAN(0, "no-add", &no_add,
+			"ignore additions made by the patch"),
+		OPT_BOOLEAN(0, "stat", &diffstat,
+			"instead of applying the patch, output diffstat for the input"),
+		OPT_BOOLEAN(0, "allow-binary-replacement", &binary,
+			"now no-op"),
+		OPT_BOOLEAN(0, "binary", &binary,
+			"now no-op"),
+		OPT_BOOLEAN(0, "numstat", &numstat,
+			"shows number of added and deleted lines in decimal notation"),
+		OPT_BOOLEAN(0, "summary", &summary,
+			"instead of applying the patch, output a summary for the input"),
+		OPT_BOOLEAN(0, "check", &check,
+			"instead of applying the patch, see if the patch is applicable"),
+		OPT_BOOLEAN(0, "index", &check_index,
+			"make sure the patch is applicable to the current index"),
+		OPT_BOOLEAN(0, "cached", &cached,
+			"apply a patch without touching the working tree"),
+		OPT_BOOLEAN(0, "apply", &force_apply,
+			"also apply the patch (use with --stat/--summary/--check)"),
+		OPT_STRING(0, "build-fake-ancestor", &fake_ancestor, "file",
+			"build a temporary index based on embedded index information"),
+		{ OPTION_CALLBACK, 'z', NULL, NULL, NULL,
+			"paths are separated with NUL character",
+			PARSE_OPT_NOARG, option_parse_z },
+		OPT_INTEGER('C', NULL, &p_context,
+				"ensure at least <n> lines of context match"),
+		{ OPTION_CALLBACK, 0, "whitespace", &whitespace_option, "action",
+			"detect new or modified lines that have whitespace errors",
+			0, option_parse_whitespace },
+		OPT_BOOLEAN('R', "reverse", &apply_in_reverse,
+			"apply the patch in reverse"),
+		OPT_BOOLEAN(0, "unidiff-zero", &unidiff_zero,
+			"don't expect at least one line of context"),
+		OPT_BOOLEAN(0, "reject", &apply_with_reject,
+			"leave the rejected hunks in corresponding *.rej files"),
+		OPT__VERBOSE(&apply_verbosely),
+		OPT_BIT(0, "inaccurate-eof", &options,
+			"tolerate incorrectly detected missing new-line at the end of file",
+			INACCURATE_EOF),
+		OPT_BIT(0, "recount", &options,
+			"do not trust the line counts in the hunk headers",
+			RECOUNT),
+		{ OPTION_CALLBACK, 0, "directory", NULL, "root",
+			"prepend <root> to all filenames",
+			0, option_parse_directory },
+		OPT_END()
+	};
+
 	prefix = setup_git_directory_gently(&is_not_gitdir);
 	prefix_length = prefix ? strlen(prefix) : 0;
 	git_config(git_apply_config, NULL);
 	if (apply_default_whitespace)
 		parse_whitespace_option(apply_default_whitespace);
 
-	for (i = 1; i < argc; i++) {
+	argc = parse_options(argc, argv, builtin_apply_options,
+			apply_usage, 0);
+	if (apply_with_reject)
+		apply = apply_verbosely = 1;
+	if (!force_apply && (diffstat || numstat || summary || check || fake_ancestor))
+		apply = 0;
+	if (check_index && is_not_gitdir)
+		die("--index outside a repository");
+	if (cached) {
+		if (is_not_gitdir)
+			die("--cached outside a repository");
+		check_index = 1;
+	}
+	for (i = 0; i < argc; i++) {
 		const char *arg = argv[i];
-		char *end;
 		int fd;
 
-		if (!strcmp(arg, "-")) {
-			errs |= apply_patch(0, "<stdin>", options);
-			read_stdin = 0;
-			continue;
-		}
-		if (!prefixcmp(arg, "--exclude=")) {
-			add_name_limit(arg + 10, 1);
-			continue;
-		}
-		if (!prefixcmp(arg, "--include=")) {
-			add_name_limit(arg + 10, 0);
-			has_include = 1;
-			continue;
-		}
-		if (!prefixcmp(arg, "-p")) {
-			p_value = atoi(arg + 2);
-			p_value_known = 1;
-			continue;
-		}
-		if (!strcmp(arg, "--no-add")) {
-			no_add = 1;
-			continue;
-		}
-		if (!strcmp(arg, "--stat")) {
-			apply = 0;
-			diffstat = 1;
-			continue;
-		}
-		if (!strcmp(arg, "--allow-binary-replacement") ||
-		    !strcmp(arg, "--binary")) {
-			continue; /* now no-op */
-		}
-		if (!strcmp(arg, "--numstat")) {
-			apply = 0;
-			numstat = 1;
-			continue;
-		}
-		if (!strcmp(arg, "--summary")) {
-			apply = 0;
-			summary = 1;
-			continue;
-		}
-		if (!strcmp(arg, "--check")) {
-			apply = 0;
-			check = 1;
-			continue;
-		}
-		if (!strcmp(arg, "--index")) {
-			if (is_not_gitdir)
-				die("--index outside a repository");
-			check_index = 1;
-			continue;
-		}
-		if (!strcmp(arg, "--cached")) {
-			if (is_not_gitdir)
-				die("--cached outside a repository");
-			check_index = 1;
-			cached = 1;
-			continue;
-		}
-		if (!strcmp(arg, "--apply")) {
-			apply = 1;
-			continue;
-		}
-		if (!strcmp(arg, "--build-fake-ancestor")) {
-			apply = 0;
-			if (++i >= argc)
-				die ("need a filename");
-			fake_ancestor = argv[i];
-			continue;
-		}
-		if (!strcmp(arg, "-z")) {
-			line_termination = 0;
-			continue;
-		}
-		if (!prefixcmp(arg, "-C")) {
-			p_context = strtoul(arg + 2, &end, 0);
-			if (*end != '\0')
-				die("unrecognized context count '%s'", arg + 2);
-			continue;
-		}
-		if (!prefixcmp(arg, "--whitespace=")) {
-			whitespace_option = arg + 13;
-			parse_whitespace_option(arg + 13);
-			continue;
-		}
-		if (!strcmp(arg, "-R") || !strcmp(arg, "--reverse")) {
-			apply_in_reverse = 1;
-			continue;
-		}
-		if (!strcmp(arg, "--unidiff-zero")) {
-			unidiff_zero = 1;
-			continue;
-		}
-		if (!strcmp(arg, "--reject")) {
-			apply = apply_with_reject = apply_verbosely = 1;
-			continue;
-		}
-		if (!strcmp(arg, "-v") || !strcmp(arg, "--verbose")) {
-			apply_verbosely = 1;
-			continue;
-		}
-		if (!strcmp(arg, "--inaccurate-eof")) {
-			options |= INACCURATE_EOF;
-			continue;
-		}
-		if (!strcmp(arg, "--recount")) {
-			options |= RECOUNT;
-			continue;
-		}
-		if (!prefixcmp(arg, "--directory=")) {
-			arg += strlen("--directory=");
-			root_len = strlen(arg);
-			if (root_len && arg[root_len - 1] != '/') {
-				char *new_root;
-				root = new_root = xmalloc(root_len + 2);
-				strcpy(new_root, arg);
-				strcpy(new_root + root_len++, "/");
-			} else
-				root = arg;
-			continue;
-		}
 		if (0 < prefix_length)
 			arg = prefix_filename(prefix, prefix_length, arg);
 
-- 
1.6.1.rc1.35.gae26e.dirty

^ permalink raw reply related

* git clone - failing on cygwin with git:// but not with ssh://
From: Joe Casadonte @ 2008-12-27 22:37 UTC (permalink / raw)
  To: git

Hi,

I'm new to git, so my apologies if I'm missing something pretty
basic.  Here's my setup:

"Public" server
---------------
OS: RedHat clone, 2.6.9 kernel
git version: self-compiled, 1.6.0.6

Test client #1 (works)
----------------------
OS: RedHat clone, 2.6.9 kernel
git version: self-compiled, 1.6.0.6

Test client #2 (fails)
----------------------
OS: Win XP Pro
git version: cygwin compiled, 1.6.0.4

I have a new repository on the "public" server, and have cloned it on
test client #1 with:

$ git clone git://foobar/myproj.git
Initialized empty Git repository in /opt/myproj/.git/
remote: Counting objects: 104, done.
remote: Compressing objects: 100% (72/72), done.
remote: Total 104 (delta 22), reused 104 (delta 22)
Receiving objects: 100% (104/104), 76.97 KiB, done.
Resolving deltas: 100% (22/22), done.


I try the same thing on test box #2 and receive:


D:\temp>git clone git://foobar/otminfmyproj.git
Initialized empty Git repository in /cygdrive/d/temp/foobar/.git/
fatal: read error (Socket operation on non-socket)
fatal: early EOF
fatal: index-pack failed


I've turned on verbose logging in the daemon and I see the following
messages:

Dec 27 17:31:53 foobar git-daemon: [30327] Connection from 192.168.1.102:2598
Dec 27 17:31:53 foobar git-daemon: [30327] Extended attributes (16 bytes) exist <host=foobar>
Dec 27 17:31:53 foobar git-daemon: [30327] Request upload-pack for '/myproj.git'
Dec 27 17:31:55 foobar git-daemon: [30327] Disconnected (with error)

A successful log (from test client #1) shows practically the same
thing:

Dec 27 17:33:22 foobar git-daemon: [30338] Connection from 192.168.2.101:44832
Dec 27 17:33:22 foobar git-daemon: [30338] Extended attributes (16 bytes) exist <host=foobar>
Dec 27 17:33:22 foobar git-daemon: [30338] Request upload-pack for '/myproj.git'
Dec 27 17:33:22 foobar git-daemon: [30338] Disconnected


Running the clone via ssh protocol from test client #2 works, though:


D:\temp>git clone ssh://root@foobar/nfs02/git/myproj
Initialized empty Git repository in /cygdrive/d/temp/myproj/.git/
remote: Counting objects: 104, done.
remote: Compressing objects: 100% (72/72), done.
remote: Total 104 (delta 22), reused 104 (delta 22)
Receiving objects: 100% (104/104), 76.97 KiB | 9 KiB/s, done.
Resolving deltas: 100% (22/22), done.


The same test machine has cloned from a different linux server via the
git protocol just fine.

Any ideas?  Thanks for the help!

--
Regards,


joe
Joe Casadonte
jcasadonte@northbound-train.com

------------------------------------------------------------------------------
         Llama Fresh Farms => http://www.northbound-train.com
    Ramblings of a Gay Man => http://www.northbound-train.com/ramblings
               Emacs Stuff => http://www.northbound-train.com/emacs.html
          Music CD Trading => http://www.northbound-train.com/cdr.html
------------------------------------------------------------------------------
                       Live Free, that's the message!
------------------------------------------------------------------------------

^ 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