Git development
 help / color / mirror / Atom feed
* Re: [PATCH] Make git status usage say git status instead of git commit
From: Junio C Hamano @ 2007-12-03  5:34 UTC (permalink / raw)
  To: Shawn Bohrer; +Cc: git, gitster
In-Reply-To: <1196658129-16708-1-git-send-email-shawn.bohrer@gmail.com>

Shawn Bohrer <shawn.bohrer@gmail.com> writes:

> git status shares the same usage information as git commit since it
> shows what would be committed if the same options are given.  However,
> when displaying the usage information for git status it should say it
> is for git status not git commit.
>
> Signed-off-by: Shawn Bohrer <shawn.bohrer@gmail.com>

Thanks.  Will apply.

> As a side question, should the usage information also use the non dash
> notation of the command since it is deprecated?  I noticed all of the
> other commands are presently using the dash form, so I left it as is for
> now.

Wise choice.  We would probably want to clean them up at the same time
early post 1.5.4.

^ permalink raw reply

* [PATCH 2/2] Add flag to make unpack_trees() not print errors.
From: Daniel Barkalow @ 2007-12-03  5:32 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

This will allow builtin-checkout to suppress merge errors if it's
going to try more merging methods.

Signed-off-by: Daniel Barkalow <barkalow@iabervon.org>
---
 unpack-trees.c |   29 +++++++++++++++++------------
 unpack-trees.h |    1 +
 2 files changed, 18 insertions(+), 12 deletions(-)

diff --git a/unpack-trees.c b/unpack-trees.c
index 0958166..a423197 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -362,7 +362,8 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options
 	}
 
 	if (o->trivial_merges_only && o->nontrivial_merge)
-		return error("Merge requires file-level merging");
+		return o->gently ? -1 : 
+			error("Merge requires file-level merging");
 
 	check_updates(active_cache, active_nr, o);
 	return 0;
@@ -416,7 +417,8 @@ static int verify_uptodate(struct cache_entry *ce,
 	}
 	if (errno == ENOENT)
 		return 0;
-	return error("Entry '%s' not uptodate. Cannot merge.", ce->name);
+	return o->gently ? -1 : 
+		error("Entry '%s' not uptodate. Cannot merge.", ce->name);
 }
 
 static void invalidate_ce_path(struct cache_entry *ce)
@@ -502,8 +504,9 @@ static int verify_clean_subdirectory(struct cache_entry *ce, const char *action,
 		d.exclude_per_dir = o->dir->exclude_per_dir;
 	i = read_directory(&d, ce->name, pathbuf, namelen+1, NULL);
 	if (i)
-		return error("Updating '%s' would lose untracked files in it",
-			     ce->name);
+		return o->gently ? -1 :
+			error("Updating '%s' would lose untracked files in it",
+			      ce->name);
 	free(pathbuf);
 	return cnt;
 }
@@ -575,8 +578,9 @@ static int verify_absent(struct cache_entry *ce, const char *action,
 				return 0;
 		}
 
-		return error("Untracked working tree file '%s' "
-			     "would be %s by merge.", ce->name, action);
+		return o->gently ? -1 : 
+			error("Untracked working tree file '%s' "
+			      "would be %s by merge.", ce->name, action);
 	}
 	return 0;
 }
@@ -708,7 +712,7 @@ int threeway_merge(struct cache_entry **stages,
 	/* #14, #14ALT, #2ALT */
 	if (remote && !df_conflict_head && head_match && !remote_match) {
 		if (index && !same(index, remote) && !same(index, head))
-			return reject_merge(index);
+			return o->gently ? -1 : reject_merge(index);
 		return merged_entry(remote, index, o);
 	}
 	/*
@@ -716,7 +720,7 @@ int threeway_merge(struct cache_entry **stages,
 	 * make sure that it matches head.
 	 */
 	if (index && !same(index, head)) {
-		return reject_merge(index);
+		return o->gently ? -1 : reject_merge(index);
 	}
 
 	if (head) {
@@ -867,11 +871,11 @@ int twoway_merge(struct cache_entry **src,
 			/* all other failures */
 			remove_entry(remove);
 			if (oldtree)
-				return reject_merge(oldtree);
+				return o->gently ? -1 : reject_merge(oldtree);
 			if (current)
-				return reject_merge(current);
+				return o->gently ? -1 : reject_merge(current);
 			if (newtree)
-				return reject_merge(newtree);
+				return o->gently ? -1 : reject_merge(newtree);
 			return -1;
 		}
 	}
@@ -898,7 +902,8 @@ int bind_merge(struct cache_entry **src,
 		return error("Cannot do a bind merge of %d trees\n",
 			     o->merge_size);
 	if (a && old)
-		return error("Entry '%s' overlaps.  Cannot bind.", a->name);
+		return o->gently ? -1 : 
+			error("Entry '%s' overlaps.  Cannot bind.", a->name);
 	if (!a)
 		return keep_entry(old, o);
 	else
diff --git a/unpack-trees.h b/unpack-trees.h
index 5517faa..619950d 100644
--- a/unpack-trees.h
+++ b/unpack-trees.h
@@ -16,6 +16,7 @@ struct unpack_trees_options {
 	int trivial_merges_only;
 	int verbose_update;
 	int aggressive;
+	int gently;
 	const char *prefix;
 	int pos;
 	struct dir_struct *dir;
-- 
1.5.3.6.886.gb204

^ permalink raw reply related

* [PATCH 1/2] Allow callers of unpack_trees() to handle failure
From: Daniel Barkalow @ 2007-12-03  5:32 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

Return an error from unpack_trees() instead of calling die(), and exit
with an error in read-tree. The merge function can return negative to
abort.

This will be used in builtin-checkout -m.

Signed-off-by: Daniel Barkalow <barkalow@iabervon.org>
---
 builtin-read-tree.c |    3 +-
 unpack-trees.c      |   85 ++++++++++++++++++++++++++++----------------------
 2 files changed, 50 insertions(+), 38 deletions(-)

diff --git a/builtin-read-tree.c b/builtin-read-tree.c
index 43cd56a..4f680c3 100644
--- a/builtin-read-tree.c
+++ b/builtin-read-tree.c
@@ -269,7 +269,8 @@ int cmd_read_tree(int argc, const char **argv, const char *unused_prefix)
 		parse_tree(tree);
 		init_tree_desc(t+i, tree->buffer, tree->size);
 	}
-	unpack_trees(nr_trees, t, &opts);
+	if (unpack_trees(nr_trees, t, &opts))
+		return 128;
 
 	/*
 	 * When reading only one tree (either the most basic form,
diff --git a/unpack-trees.c b/unpack-trees.c
index e9eb795..0958166 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -219,6 +219,8 @@ static int unpack_trees_rec(struct tree_entry_list **posns, int len,
 				}
 #endif
 				ret = o->fn(src, o, remove);
+				if (ret < 0)
+					return ret;
 
 #if DBRT_DEBUG > 1
 				printf("Added %d entries\n", ret);
@@ -360,7 +362,7 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options
 	}
 
 	if (o->trivial_merges_only && o->nontrivial_merge)
-		die("Merge requires file-level merging");
+		return error("Merge requires file-level merging");
 
 	check_updates(active_cache, active_nr, o);
 	return 0;
@@ -368,10 +370,10 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options
 
 /* Here come the merge functions */
 
-static void reject_merge(struct cache_entry *ce)
+static int reject_merge(struct cache_entry *ce)
 {
-	die("Entry '%s' would be overwritten by merge. Cannot merge.",
-	    ce->name);
+	return error("Entry '%s' would be overwritten by merge. Cannot merge.",
+		     ce->name);
 }
 
 static int same(struct cache_entry *a, struct cache_entry *b)
@@ -389,18 +391,18 @@ static int same(struct cache_entry *a, struct cache_entry *b)
  * When a CE gets turned into an unmerged entry, we
  * want it to be up-to-date
  */
-static void verify_uptodate(struct cache_entry *ce,
+static int verify_uptodate(struct cache_entry *ce,
 		struct unpack_trees_options *o)
 {
 	struct stat st;
 
 	if (o->index_only || o->reset)
-		return;
+		return 0;
 
 	if (!lstat(ce->name, &st)) {
 		unsigned changed = ce_match_stat(ce, &st, CE_MATCH_IGNORE_VALID);
 		if (!changed)
-			return;
+			return 0;
 		/*
 		 * NEEDSWORK: the current default policy is to allow
 		 * submodule to be out of sync wrt the supermodule
@@ -409,12 +411,12 @@ static void verify_uptodate(struct cache_entry *ce,
 		 * checked out.
 		 */
 		if (S_ISGITLINK(ntohl(ce->ce_mode)))
-			return;
+			return 0;
 		errno = 0;
 	}
 	if (errno == ENOENT)
-		return;
-	die("Entry '%s' not uptodate. Cannot merge.", ce->name);
+		return 0;
+	return error("Entry '%s' not uptodate. Cannot merge.", ce->name);
 }
 
 static void invalidate_ce_path(struct cache_entry *ce)
@@ -480,7 +482,8 @@ static int verify_clean_subdirectory(struct cache_entry *ce, const char *action,
 		 * ce->name is an entry in the subdirectory.
 		 */
 		if (!ce_stage(ce)) {
-			verify_uptodate(ce, o);
+			if (verify_uptodate(ce, o))
+				return -1;
 			ce->ce_mode = 0;
 		}
 		cnt++;
@@ -499,8 +502,8 @@ static int verify_clean_subdirectory(struct cache_entry *ce, const char *action,
 		d.exclude_per_dir = o->dir->exclude_per_dir;
 	i = read_directory(&d, ce->name, pathbuf, namelen+1, NULL);
 	if (i)
-		die("Updating '%s' would lose untracked files in it",
-		    ce->name);
+		return error("Updating '%s' would lose untracked files in it",
+			     ce->name);
 	free(pathbuf);
 	return cnt;
 }
@@ -509,16 +512,16 @@ static int verify_clean_subdirectory(struct cache_entry *ce, const char *action,
  * We do not want to remove or overwrite a working tree file that
  * is not tracked, unless it is ignored.
  */
-static void verify_absent(struct cache_entry *ce, const char *action,
-		struct unpack_trees_options *o)
+static int verify_absent(struct cache_entry *ce, const char *action,
+			 struct unpack_trees_options *o)
 {
 	struct stat st;
 
 	if (o->index_only || o->reset || !o->update)
-		return;
+		return 0;
 
 	if (has_symlink_leading_path(ce->name, NULL))
-		return;
+		return 0;
 
 	if (!lstat(ce->name, &st)) {
 		int cnt;
@@ -528,7 +531,7 @@ static void verify_absent(struct cache_entry *ce, const char *action,
 			 * ce->name is explicitly excluded, so it is Ok to
 			 * overwrite it.
 			 */
-			return;
+			return 0;
 		if (S_ISDIR(st.st_mode)) {
 			/*
 			 * We are checking out path "foo" and
@@ -557,7 +560,7 @@ static void verify_absent(struct cache_entry *ce, const char *action,
 			 * deleted entries here.
 			 */
 			o->pos += cnt;
-			return;
+			return 0;
 		}
 
 		/*
@@ -569,12 +572,13 @@ static void verify_absent(struct cache_entry *ce, const char *action,
 		if (0 <= cnt) {
 			struct cache_entry *ce = active_cache[cnt];
 			if (!ce_stage(ce) && !ce->ce_mode)
-				return;
+				return 0;
 		}
 
-		die("Untracked working tree file '%s' "
-		    "would be %s by merge.", ce->name, action);
+		return error("Untracked working tree file '%s' "
+			     "would be %s by merge.", ce->name, action);
 	}
+	return 0;
 }
 
 static int merged_entry(struct cache_entry *merge, struct cache_entry *old,
@@ -592,12 +596,14 @@ static int merged_entry(struct cache_entry *merge, struct cache_entry *old,
 		if (same(old, merge)) {
 			*merge = *old;
 		} else {
-			verify_uptodate(old, o);
+			if (verify_uptodate(old, o))
+				return -1;
 			invalidate_ce_path(old);
 		}
 	}
 	else {
-		verify_absent(merge, "overwritten", o);
+		if (verify_absent(merge, "overwritten", o))
+			return -1;
 		invalidate_ce_path(merge);
 	}
 
@@ -609,10 +615,12 @@ static int merged_entry(struct cache_entry *merge, struct cache_entry *old,
 static int deleted_entry(struct cache_entry *ce, struct cache_entry *old,
 		struct unpack_trees_options *o)
 {
-	if (old)
-		verify_uptodate(old, o);
-	else
-		verify_absent(ce, "removed", o);
+	if (old) {
+		if (verify_uptodate(old, o))
+			return -1;
+	} else
+		if (verify_absent(ce, "removed", o))
+			return -1;
 	ce->ce_mode = 0;
 	add_cache_entry(ce, ADD_CACHE_OK_TO_ADD|ADD_CACHE_OK_TO_REPLACE);
 	invalidate_ce_path(ce);
@@ -700,7 +708,7 @@ int threeway_merge(struct cache_entry **stages,
 	/* #14, #14ALT, #2ALT */
 	if (remote && !df_conflict_head && head_match && !remote_match) {
 		if (index && !same(index, remote) && !same(index, head))
-			reject_merge(index);
+			return reject_merge(index);
 		return merged_entry(remote, index, o);
 	}
 	/*
@@ -708,7 +716,7 @@ int threeway_merge(struct cache_entry **stages,
 	 * make sure that it matches head.
 	 */
 	if (index && !same(index, head)) {
-		reject_merge(index);
+		return reject_merge(index);
 	}
 
 	if (head) {
@@ -759,8 +767,10 @@ int threeway_merge(struct cache_entry **stages,
 			remove_entry(remove);
 			if (index)
 				return deleted_entry(index, index, o);
-			else if (ce && !head_deleted)
-				verify_absent(ce, "removed", o);
+			else if (ce && !head_deleted) {
+				if (verify_absent(ce, "removed", o))
+					return -1;
+			}
 			return 0;
 		}
 		/*
@@ -776,7 +786,8 @@ int threeway_merge(struct cache_entry **stages,
 	 * conflict resolution files.
 	 */
 	if (index) {
-		verify_uptodate(index, o);
+		if (verify_uptodate(index, o))
+			return -1;
 	}
 
 	remove_entry(remove);
@@ -856,11 +867,11 @@ int twoway_merge(struct cache_entry **src,
 			/* all other failures */
 			remove_entry(remove);
 			if (oldtree)
-				reject_merge(oldtree);
+				return reject_merge(oldtree);
 			if (current)
-				reject_merge(current);
+				return reject_merge(current);
 			if (newtree)
-				reject_merge(newtree);
+				return reject_merge(newtree);
 			return -1;
 		}
 	}
@@ -887,7 +898,7 @@ int bind_merge(struct cache_entry **src,
 		return error("Cannot do a bind merge of %d trees\n",
 			     o->merge_size);
 	if (a && old)
-		die("Entry '%s' overlaps.  Cannot bind.", a->name);
+		return error("Entry '%s' overlaps.  Cannot bind.", a->name);
 	if (!a)
 		return keep_entry(old, o);
 	else
-- 
1.5.3.6.886.gb204

^ permalink raw reply related

* [PATCH 0/2] Fail merge gently
From: Daniel Barkalow @ 2007-12-03  5:32 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

This series makes it possible to call unpack_trees() and get an error 
result instead of having it call die(). Additionally, it allows the caller 
to suppress error messages, in case it's just going to try something else 
instead of actually failing. This passes all of the tests, and should 
have no effect on existing code except for making the exit messages not 
say "fatal:", and merge-recursive will free its tree cache before exiting, 
and print an additional message already in the code but previously 
inaccessible.

This is preliminary for builtin-checkout, which I'm partway through. I 
think I'm down to the case where a real merge is needed for -m, but I'm 
not sure, and I think I need to write a bunch of tests for checkout.

Daniel Barkalow (2):
      Allow callers of unpack_trees() to handle failure
      Add flag to make unpack_trees() not print errors.

 builtin-read-tree.c |    3 +-
 unpack-trees.c      |   90 ++++++++++++++++++++++++++++++---------------------
 unpack-trees.h      |    1 +
 3 files changed, 56 insertions(+), 38 deletions(-)

	-Daniel
*This .sig left intentionally blank*

^ permalink raw reply

* [PATCH] quote_path: fix collapsing of relative paths
From: Jeff King @ 2007-12-03  5:30 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Johannes Schindelin, git

The code tries to collapse identical leading components
between the prefix and the path. So if we're in "dir1", the
path "dir1/file" should become just "file". However, we were
ending up with "../dir1/file". The included test expected
the wrong output.

Because the "len" parameter to quote_path can be passed in
as -1 to indicate a NUL-terminated string, we have to
consider that possibility in our loop conditional (but no
additional checks are necessary, since we already check that
prefix[off] and in[off] are identical, and that prefix[off]
is not NUL.

Signed-off-by: Jeff King <peff@peff.net>
---
This behavior in git-status had been bugging me, and when I went to fix
it, I was surprised to find code already there to do it. :) Dscho,
please confirm that the test is in fact in error, and that I've read the
intent of your code correctly.

 t/t7502-status.sh |    2 +-
 wt-status.c       |    3 ++-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/t/t7502-status.sh b/t/t7502-status.sh
index 269b334..d6ae69d 100755
--- a/t/t7502-status.sh
+++ b/t/t7502-status.sh
@@ -68,7 +68,7 @@ cat > expect << \EOF
 # Changed but not updated:
 #   (use "git add <file>..." to update what will be committed)
 #
-#	modified:   ../dir1/modified
+#	modified:   modified
 #
 # Untracked files:
 #   (use "git add <file>..." to include in what will be committed)
diff --git a/wt-status.c b/wt-status.c
index e77120d..09666ec 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -90,7 +90,8 @@ static char *quote_path(const char *in, int len,
 
 	if (prefix) {
 		int off = 0;
-		while (prefix[off] && off < len && prefix[off] == in[off])
+		while (prefix[off] && (len < 0 || off < len)
+				&& prefix[off] == in[off])
 			if (prefix[off] == '/') {
 				prefix += off + 1;
 				in += off + 1;
-- 
1.5.3.7.2070.g88cf2-dirty

^ permalink raw reply related

* Re: [PATCH v4] Allow update hooks to update refs on their own.
From: Junio C Hamano @ 2007-12-03  5:25 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: Steven Grimm, git
In-Reply-To: <20071203040108.GS14735@spearce.org>

"Shawn O. Pearce" <spearce@spearce.org> writes:

> This probably requires exporting the name of the ref we currently
> have locked in an environment variable (and teach lockfile.c it)
> so we can effectively do recursive locking.  That way the update
> hook can still use git-update-ref to change the ref safely.

Heh, I like that, although I suspect getting this right would mean the
topic should be post 1.5.4 (which I do not mind).  

^ permalink raw reply

* [PATCH] Make git status usage say git status instead of git commit
From: Shawn Bohrer @ 2007-12-03  5:02 UTC (permalink / raw)
  To: git; +Cc: gitster, Shawn Bohrer

git status shares the same usage information as git commit since it
shows what would be committed if the same options are given.  However,
when displaying the usage information for git status it should say it
is for git status not git commit.

Signed-off-by: Shawn Bohrer <shawn.bohrer@gmail.com>
---

As a side question, should the usage information also use the non dash
notation of the command since it is deprecated?  I noticed all of the
other commands are presently using the dash form, so I left it as is for
now.

 builtin-commit.c |   15 ++++++++++-----
 1 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/builtin-commit.c b/builtin-commit.c
index f6e8e44..5e85a22 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -27,6 +27,11 @@ static const char * const builtin_commit_usage[] = {
 	NULL
 };
 
+static const char * const builtin_status_usage[] = {
+	"git-status [options] [--] <filepattern>...",
+	NULL
+};
+
 static unsigned char head_sha1[20], merge_head_sha1[20];
 static char *use_message_buffer;
 static const char commit_editmsg[] = "COMMIT_EDITMSG";
@@ -495,12 +500,12 @@ static void determine_author_info(struct strbuf *sb)
 	strbuf_addf(sb, "author %s\n", fmt_ident(name, email, date, 1));
 }
 
-static int parse_and_validate_options(int argc, const char *argv[])
+static int parse_and_validate_options(int argc, const char *argv[],
+				      const char * const usage[])
 {
 	int f = 0;
 
-	argc = parse_options(argc, argv, builtin_commit_options,
-			     builtin_commit_usage, 0);
+	argc = parse_options(argc, argv, builtin_commit_options, usage, 0);
 
 	if (logfile || message.len || use_message)
 		no_edit = 1;
@@ -597,7 +602,7 @@ int cmd_status(int argc, const char **argv, const char *prefix)
 
 	git_config(git_status_config);
 
-	argc = parse_and_validate_options(argc, argv);
+	argc = parse_and_validate_options(argc, argv, builtin_status_usage);
 
 	index_file = prepare_index(argc, argv, prefix);
 
@@ -689,7 +694,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 
 	git_config(git_commit_config);
 
-	argc = parse_and_validate_options(argc, argv);
+	argc = parse_and_validate_options(argc, argv, builtin_commit_usage);
 
 	index_file = prepare_index(argc, argv, prefix);
 
-- 
1.5.3.6

^ permalink raw reply related

* [PATCH] Trace and quote with argv: get rid of unneeded count argument.
From: Christian Couder @ 2007-12-03  4:51 UTC (permalink / raw)
  To: Junio Hamano; +Cc: git

Now that str_buf takes care of all the allocations, there is
no more gain to pass an argument count.

So this patch removes the "count" argument from:
	- "sq_quote_argv"
	- "trace_argv_printf"
and all the callers.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 builtin-rev-parse.c |    2 +-
 cache.h             |    2 +-
 exec_cmd.c          |    2 +-
 git.c               |    6 +++---
 quote.c             |   13 +++----------
 quote.h             |    3 +--
 trace.c             |    4 ++--
 7 files changed, 12 insertions(+), 20 deletions(-)

	I wrote to Junio:
	> Minor nit: now that the number of arguments is known, we could perhaps use 
	> the argument count instead of -1 in trace_argv_printf, so that it is not 
	> computed again in quote.c:sq_quote_argv, like this:
	>
	>trace_argv_printf(nargv, argc + 1, "trace: exec:");

	After looking at it a little more, I think it's better to get rid of the
	"count" argument altogether with a patch like this one.

diff --git a/builtin-rev-parse.c b/builtin-rev-parse.c
index d1038a0..20d1789 100644
--- a/builtin-rev-parse.c
+++ b/builtin-rev-parse.c
@@ -327,7 +327,7 @@ static int cmd_parseopt(int argc, const char **argv, const char *prefix)
 	                     keep_dashdash ? PARSE_OPT_KEEP_DASHDASH : 0);
 
 	strbuf_addf(&parsed, " --");
-	sq_quote_argv(&parsed, argv, argc, 0);
+	sq_quote_argv(&parsed, argv, 0);
 	puts(parsed.buf);
 	return 0;
 }
diff --git a/cache.h b/cache.h
index 4e59646..9f63199 100644
--- a/cache.h
+++ b/cache.h
@@ -617,7 +617,7 @@ extern void alloc_report(void);
 
 /* trace.c */
 extern void trace_printf(const char *format, ...);
-extern void trace_argv_printf(const char **argv, int count, const char *format, ...);
+extern void trace_argv_printf(const char **argv, const char *format, ...);
 
 /* convert.c */
 /* returns 1 if *dst was used */
diff --git a/exec_cmd.c b/exec_cmd.c
index 2d0a758..e189cac 100644
--- a/exec_cmd.c
+++ b/exec_cmd.c
@@ -80,7 +80,7 @@ int execv_git_cmd(const char **argv)
 	tmp = argv[0];
 	argv[0] = cmd.buf;
 
-	trace_argv_printf(argv, -1, "trace: exec:");
+	trace_argv_printf(argv, "trace: exec:");
 
 	/* execvp() can only ever return if it fails */
 	execvp(cmd.buf, (char **)argv);
diff --git a/git.c b/git.c
index f406c4b..c8b7e74 100644
--- a/git.c
+++ b/git.c
@@ -178,7 +178,7 @@ static int handle_alias(int *argcp, const char ***argv)
 
 				strbuf_init(&buf, PATH_MAX);
 				strbuf_addstr(&buf, alias_string);
-				sq_quote_argv(&buf, (*argv) + 1, *argcp - 1, PATH_MAX);
+				sq_quote_argv(&buf, (*argv) + 1, PATH_MAX);
 				free(alias_string);
 				alias_string = buf.buf;
 			}
@@ -207,7 +207,7 @@ static int handle_alias(int *argcp, const char ***argv)
 		if (!strcmp(alias_command, new_argv[0]))
 			die("recursive alias: %s", alias_command);
 
-		trace_argv_printf(new_argv, count,
+		trace_argv_printf(new_argv,
 				  "trace: alias expansion: %s =>",
 				  alias_command);
 
@@ -261,7 +261,7 @@ static int run_command(struct cmd_struct *p, int argc, const char **argv)
 	if (p->option & NEED_WORK_TREE)
 		setup_work_tree();
 
-	trace_argv_printf(argv, argc, "trace: built-in: git");
+	trace_argv_printf(argv, "trace: built-in: git");
 
 	status = p->fn(argc, argv, prefix);
 	if (status)
diff --git a/quote.c b/quote.c
index 0455783..6986b44 100644
--- a/quote.c
+++ b/quote.c
@@ -56,20 +56,13 @@ void sq_quote_print(FILE *stream, const char *src)
 	fputc('\'', stream);
 }
 
-void sq_quote_argv(struct strbuf *dst, const char** argv, int count,
-                   size_t maxlen)
+void sq_quote_argv(struct strbuf *dst, const char** argv, size_t maxlen)
 {
 	int i;
 
-	/* Count argv if needed. */
-	if (count < 0) {
-		for (count = 0; argv[count]; count++)
-			; /* just counting */
-	}
-
 	/* Copy into destination buffer. */
-	strbuf_grow(dst, 32 * count);
-	for (i = 0; i < count; ++i) {
+	strbuf_grow(dst, 255);
+	for (i = 0; argv[i]; ++i) {
 		strbuf_addch(dst, ' ');
 		sq_quote_buf(dst, argv[i]);
 		if (maxlen && dst->len > maxlen)
diff --git a/quote.h b/quote.h
index 4287990..ab7596f 100644
--- a/quote.h
+++ b/quote.h
@@ -31,8 +31,7 @@
 extern void sq_quote_print(FILE *stream, const char *src);
 
 extern void sq_quote_buf(struct strbuf *, const char *src);
-extern void sq_quote_argv(struct strbuf *, const char **argv, int count,
-                          size_t maxlen);
+extern void sq_quote_argv(struct strbuf *, const char **argv, size_t maxlen);
 
 /* This unwraps what sq_quote() produces in place, but returns
  * NULL if the input does not look like what sq_quote would have
diff --git a/trace.c b/trace.c
index d3d1b6d..4713f91 100644
--- a/trace.c
+++ b/trace.c
@@ -93,7 +93,7 @@ void trace_printf(const char *fmt, ...)
 		close(fd);
 }
 
-void trace_argv_printf(const char **argv, int count, const char *fmt, ...)
+void trace_argv_printf(const char **argv, const char *fmt, ...)
 {
 	struct strbuf buf;
 	va_list ap;
@@ -117,7 +117,7 @@ void trace_argv_printf(const char **argv, int count, const char *fmt, ...)
 	}
 	strbuf_setlen(&buf, len);
 
-	sq_quote_argv(&buf, argv, count, 0);
+	sq_quote_argv(&buf, argv, 0);
 	strbuf_addch(&buf, '\n');
 	write_or_whine_pipe(fd, buf.buf, buf.len, err_msg);
 	strbuf_release(&buf);
-- 
1.5.3.6.2115.gb9452-dirty

^ permalink raw reply related

* Re: importing bk into git
From: David Kettler @ 2007-12-03  3:02 UTC (permalink / raw)
  To: christoph.duelli; +Cc: git
In-Reply-To: <200711292232.03352.christoph.duelli@gmx.de>

G'day,

I modified that script to convert a number of our repositories in
February.  The version below worked for me at the time, but I'm not
able to test it now as our BK license has expired.  In particular I'm
not sure if the bk_info.split line is correct; I had a reduced form of
this line in the file which now looks obviously wrong.

The script is slow; most of the time is in the bk export for every
revision.  There are probably dumb things in there; I don't know
python and I was just starting with git.

Changes from the version I downloaded from the web include:
  - sundry changes to make it work for me
  - separate committers file to translate user names to full names
  - specify a git dir template
  - copy tags from BK
  - minimal conversion of ignore files
  - increased recursion limit to handle large number of commits

I hope this is useful to someone.

regards, David.

--snip,snip--
# Convert a BK repository to GIT
# usage: bk2git BK_REPO [GIT_REPO]
# Single branch only.

import os
import time
import sys

sys.setrecursionlimit(10000);

templates_dir = "/tmp/bk2git/templates"
committers_file = "/tmp/bk2git/committers"
tmp_dir = "/tmp/bk-export%d" % os.getpid()

# Get repository locations.
if len(sys.argv) < 2 or len(sys.argv) > 3:
    print "usage: bk2git BK_REPO [GIT_REPO]"
    sys.exit(1);

bk_dir = sys.argv[1]
if len(sys.argv) == 3:
    git_dir = sys.argv[2]
else:
    git_dir = bk_dir + ".git"

print "BK  " + bk_dir
print "GIT " + git_dir


# Get committer names.
f = file(committers_file, "r")
committers = {}
for line in f:
    [m,n] = line.split(" ",1)
    committers[m] = n.strip();
f.close()

# Get tree of commits.
f = os.popen("cd %s; bk prs -d':REV:\\t:PARENT:\\t:MPARENT:\\t\\n' ChangeSet" % bk_dir)
f.readline()
parents={}
for rev in f:
    [n,p] = rev.rstrip().split("\t",1)
    parents[n] = p.split("\t")
f.close()

# Get tags.
f = os.popen("cd %s;  bk changes -t -n -d':I:\\t:TAG:'" % bk_dir)
tags={}
for rev in f:
    [n,t] = rev.rstrip().split("\t",1)
    tags[n] = t
f.close()

# Initialize git repository.
os.system("mkdir %s" % git_dir)
os.chdir(git_dir)
os.system("git --bare init --template=%s" % templates_dir)
os.system("git-config core.bare false")

unknown = {}
def get_name(email):
    if committers.has_key(email):
        return committers[email]
    unknown[email] = True
    return "*Unknown*"

def git_commit(rev, p):
    os.chdir(tmp_dir)
    os.symlink(git_dir, "%s/.git" % tmp_dir)
#    os.system("pwd; ls -AlR")
    os.system("git-ls-files -z --deleted | xargs -0 git-update-index --remove")
    os.system("git-ls-files -z --others | xargs -0 git-update-index --add")
    os.system("git-ls-files -z | xargs -0 git-update-index")
    treeid = os.popen("git-write-tree").read().rstrip()
    print "wrote tree as %s" % treeid
    os.chdir(git_dir)
    os.system("rm -Rf %s" % tmp_dir)

    bk_info = os.popen("cd %s; bk prs -r%s -d':KEY:\\n:UTC:\\n:USER:@:HOST:\\n$each(:C:){:C\\n}\\n' ChangeSet | sed 1d" % (bk_dir, rev)).read()

    [key, date, user, comments] = bk_info.split("\n", 3)
#    [key, date, user] = bk_info.split("\n", 2)
    [name, machine] = user.split("@", 1);
    f = file("/tmp/git-comments","w")
    f.write(comments)
    f.write("BK KEY: %s\n" % key)
    f.close()
    sdate = str(int(time.mktime(time.strptime(date+" UTC", "%Y%m%d%H%M%S %Z"))))
    os.putenv("GIT_AUTHOR_DATE", sdate)
    os.putenv("GIT_AUTHOR_EMAIL", user)
    os.putenv("GIT_AUTHOR_NAME", get_name(name))
    os.putenv("GIT_COMMITTER_DATE", sdate)
    os.putenv("GIT_COMMITTER_EMAIL", user)
    os.putenv("GIT_COMMITTER_NAME", get_name(name))
    
    commitid = os.popen("git-commit-tree %s %s < /tmp/git-comments" % (treeid, " ".join(["-p "+a for a in p]))).read().rstrip()
    print "committed %s as %s" % (rev, commitid)

    if tags.has_key(rev):
        os.system("git-tag %s %s" % (tags[rev], commitid))
        print "tagged %s" % tags[rev]

    return commitid

os.system("mkdir %s; touch %s/initial" % (tmp_dir, tmp_dir))
resolved = {'1.1': git_commit("1.1",[])}

def res(ver):
    if resolved.has_key(ver):
        return
    
    for v in parents[ver]:
        res(v)

    os.system("cd %s; bk export -r%s %s" % (bk_dir, ver, tmp_dir))
    ignore = "%s/.gitignore" % tmp_dir
    os.system("cd %s; bk co -kpq -r@%s BitKeeper/etc/ignore | sed '/^BitKeeper/d;/^PENDING/d' > %s" % (bk_dir, ver, ignore))
    os.system("test -s %s || rm %s" % (ignore, ignore))

    resolved[ver] = git_commit(ver, [resolved[v] for v in parents[ver]])
    return resolved[ver]

tot = os.popen("cd %s; bk prs -r+ -d':REV:' ChangeSet | tail -n 1" % bk_dir).read()
print "Exporting bitkeeper up to version %s" % tot

HEAD = res(tot)
print "HEAD: %s" % HEAD
file("%s/refs/heads/master" % git_dir,"w").write(HEAD + "\n")
os.system("git-config core.bare true")
os.system("git gc")
print unknown.keys()
--snip,snip--
-- 
IMPORTANT: This email remains the property of the Australian Defence
Organisation and is subject to the jurisdiction of section 70 of the
CRIMES ACT 1914. If you have received this email in error, you are
requested to contact the sender and delete the email.

^ permalink raw reply

* [PATCH/RFC] Use regex for :/ matching
From: Jeff King @ 2007-12-03  4:32 UTC (permalink / raw)
  To: git; +Cc: Johannes Schindelin

The sha1 syntax :/ used to be a strict prefix match.
Instead, let's use a regular expression, which can save on
typing. E.g.,

  git show :/"object name: introduce ':/<oneline prefix>'"

vs

  git show :/introduce..:/.oneline

Signed-off-by: Jeff King <peff@peff.net>
---
Obviously this changes the semantics of existing queries.
Specifically:

  - regex metacharacters are now interpreted
  - the pattern is no longer anchored at the start

I find it much more useful than the original, but perhaps it deserves
its own syntax instead?

I also considered that it might be much slower than the original, but it
is not:

  # original
  $ time git log :/notfound >/dev/null
  real    0m1.055s
  user    0m1.020s
  sys     0m0.024s

  # regex
  $ time git log :/notfound >/dev/null
  real    0m1.065s
  user    0m1.028s
  sys     0m0.036s

Curiously, both are much slower than --grep:

  $ time git log --grep=notfound >/dev/null
  real    0m0.677s
  user    0m0.640s
  sys     0m0.036s

And finally, if accepted, a followup patch should change the "prefix"
variable to "search" or similar; putting it in here made the diff a bit
noisy.

 Documentation/git-rev-parse.txt |    4 ++--
 sha1_name.c                     |   25 ++++++++++++++++++++++---
 2 files changed, 24 insertions(+), 5 deletions(-)

diff --git a/Documentation/git-rev-parse.txt b/Documentation/git-rev-parse.txt
index 329fce0..41dd684 100644
--- a/Documentation/git-rev-parse.txt
+++ b/Documentation/git-rev-parse.txt
@@ -208,8 +208,8 @@ blobs contained in a commit.
   and dereference the tag recursively until a non-tag object is
   found.
 
-* A colon, followed by a slash, followed by a text: this names
-  a commit whose commit message starts with the specified text.
+* A colon, followed by a slash, followed by a regular expression: this names
+  a commit whose commit message starts with a line matching the expression.
   This name returns the youngest matching commit which is
   reachable from any ref.  If the commit message starts with a
   '!', you have to repeat that;  the special sequence ':/!',
diff --git a/sha1_name.c b/sha1_name.c
index d364244..2ad91a3 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -588,8 +588,8 @@ static int handle_one_ref(const char *path,
 
 /*
  * This interprets names like ':/Initial revision of "git"' by searching
- * through history and returning the first commit whose message starts
- * with the given string.
+ * through history and returning the first commit whose first line
+ * matches the given regex.
  *
  * For future extension, ':/!' is reserved. If you want to match a message
  * beginning with a '!', you have to repeat the exclamation mark.
@@ -600,12 +600,22 @@ static int get_sha1_oneline(const char *prefix, unsigned char *sha1)
 {
 	struct commit_list *list = NULL, *backup = NULL, *l;
 	int retval = -1;
+	regex_t re;
+	int r;
 
 	if (prefix[0] == '!') {
 		if (prefix[1] != '!')
 			die ("Invalid search pattern: %s", prefix);
 		prefix++;
 	}
+
+	r = regcomp(&re, prefix, REG_NOSUB);
+	if (r != 0) {
+		char errbuf[256];
+		regerror(r, &re, errbuf, sizeof(errbuf));
+		return error("unable to compile regex: %s", errbuf);
+	}
+
 	if (!save_commit_buffer)
 		return error("Could not expand oneline-name.");
 	for_each_ref(handle_one_ref, &list);
@@ -613,13 +623,21 @@ static int get_sha1_oneline(const char *prefix, unsigned char *sha1)
 		commit_list_insert(l->item, &backup);
 	while (list) {
 		char *p;
+		char *end;
 		struct commit *commit;
 
 		commit = pop_most_recent_commit(&list, ONELINE_SEEN);
 		parse_object(commit->object.sha1);
 		if (!commit->buffer || !(p = strstr(commit->buffer, "\n\n")))
 			continue;
-		if (!prefixcmp(p + 2, prefix)) {
+		p += 2;
+		end = strchr(p, '\n');
+		if (end)
+			*end = '\0';
+		r = regexec(&re, p, 0, NULL, 0);
+		if (end)
+			*end = '\n';
+		if (r == 0) {
 			hashcpy(sha1, commit->object.sha1);
 			retval = 0;
 			break;
@@ -628,6 +646,7 @@ static int get_sha1_oneline(const char *prefix, unsigned char *sha1)
 	free_commit_list(list);
 	for (l = backup; l; l = l->next)
 		clear_commit_marks(l->item, ONELINE_SEEN);
+	regfree(&re);
 	return retval;
 }
 
-- 
1.5.3.7.2068.g5949-dirty

^ permalink raw reply related

* What's in git-gui.git
From: Shawn O. Pearce @ 2007-12-03  4:10 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano

The following changes since gitgui-0.9.0 are now in my git-gui
master branch.  I'm considering tagging a 0.9.1 release next week.

 gitweb:  http://repo.or.cz/w/git-gui.git
 clone:   git://repo.or.cz/git-gui.git
          http://repo.or.cz/r/git-gui.git

----

Christian Stimming (2):
      Update git-gui.pot with latest (few) string additions and changes.
      Update German translation. 100% completed.

Johannes Sixt (1):
      git-gui: Improve the application icon on Windows.

Michele Ballabio (2):
      git-gui: fix a typo in lib/commit.tcl
      git-gui: update it.po and glossary/it.po

Robert Schiele (1):
      git-gui: install-sh from automake does not like -m755

brian m. carlson (1):
      git-gui: Reorder msgfmt command-line arguments

-- 
Shawn.

^ permalink raw reply

* Re: [PATCH] Do check_repository_format() early
From: Nguyen Thai Ngoc Duy @ 2007-12-03  4:03 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Johannes Schindelin, git
In-Reply-To: <7v4pf25jiq.fsf@gitster.siamese.dyndns.org>

On Dec 2, 2007 1:58 AM, Junio C Hamano <gitster@pobox.com> wrote:
> "Nguyen Thai Ngoc Duy" <pclouds@gmail.com> writes:
>
> > On Dec 1, 2007 9:36 AM, Junio C Hamano <gitster@pobox.com> wrote:
> >>
> >> Looks sensible, but can this be accompanied with a trivial test to
> >> demonstrate the existing breakage?
> >
> > How can I reliably check setup_git_directory_gently()? I can pick one
> > command that uses setup_git_directory_gently(). But commands change.
> > Once they turn to setup_git_directory(), the test will no longer be
> > valid.
>
> The commands' implementation may change but what I meant was to test the
> intent.
>
> What's the difference between commands that call "gently" kind and
> non-gently kind?  The former is "I do not _have_ to be in a git
> repository but if I am then I want to know about it and use some
> information from that repository", as opposed to "I need to be in a git
> repository and will find one otherwise I barf" which is the latter kind.
>
> The intent of the change, from reading your patch, is that currently the
> former kind of commands that take "an optional git repository" are happy
> if they find a directory that looks like a git repository and go ahead
> with their operation without checking the repository format version, and
> your patch addresses this issue by making sure that the git repository
> found via "gently" are also checked for the format version.
>
> Examples of commands that do not necessarily require a valid git
> repository are:
>
>  * apply: when being used as a "patch that is better than GNU", that is,
>    without --index, --cached, nor --check option.
>
>  * bundle: when verifying and listing the contained head of an existing
>    bundle file.
>
>  * config: without being in a git repository, you can still interact with
>    $HOME/.gitconfig and /etc/gitconfig [*1*].
>
>  * ls-remote: without being in a git repository, you can still list refs
>    from a remote repository.  If you are in a git repository, you can
>    use nicknames you have in your repositories' remote.$nickname.url
>    configuration.
>
> So what I would suggest would be:
>
>  * The directory your tests run in, t/trash, is a valid git repository.
>    Leave it as is.
>
>  * mkdir test inside t/trash, cd there, and run "git init" there to
>    initialize t/trash/test/.git (the shell function test_create_repo can
>    be used for this).
>
>  * corrupt this by updating the core.repositoryformatversion to a large
>    value, by doing something like:
>
>         V=$(git config core.repositoryformatversion)
>         (
>                 cd test
>                 N=$(( ${V:-0} + 99 ))
>                 git config core.repositoryformatversion $N
>         )
>
>  * make sure t/trash/test/.git/config file, and not t/trash/.git/config
>    file, got that change by doing something like:
>
>         GIT_CONFIG=.git/config git config core.repositoryformatversion
>         GIT_CONFIG=test/.git/config git config core.repositoryformatversion
>
>    The former would report the current version ($V above) while the
>    latter should error out.
>
> Up to this step is the "test setup".  The actual tests would be done in
> t/trash/test directory.
>
>  * Use a few commands that have the "we can run in git repository but we
>    do not have to" behaviour, in modes that _require_ git repository.
>    For example, "git apply --check" wants a valid repository to check
>    the patch against the index.  They should fail because the repository
>    format is too new for them to understand.
>
>  * Similarly, run a few commands in modes that do not require git
>    repository.  For example, "git apply --stat" of an existing patch
>    should be viewable no matter where you are (that is just a "better
>    diffstat" mode), so ideally it should not barf only because you
>    happen to be in a repository that is too new for you to understand.
>    I do not know offhand how your patch would handle this situation.
>
> Note that making sure the latter works is tricky to do right, for a few
> reasons.
>
>  (1) It is not absolutely clear what the right behaviour is.  It could
>      be argued that we should just barf saying we found a repository we
>      do not understand, refraining from doing any damange on it [*2*].
>
>  (2) If we choose not to barf on such a repository, it remains to be
>      decided what "gently" should do --- if it should still treat
>      t/trash/test (which has too new a version) as the found repository,
>      or ignore it and use t/trash (which we can understand) as the found
>      repository.  I think it should do the former.

The patch's behaviour is barf if the repository version is too new.
The list of files that use setup_git_directory_gently is not long. I
am going to have a look over the files before amending the patch again
to make it only barf if nongit_ok is NULL.
-- 
Duy

^ permalink raw reply

* Re: [PATCH v4] Allow update hooks to update refs on their own.
From: Shawn O. Pearce @ 2007-12-03  4:01 UTC (permalink / raw)
  To: Steven Grimm; +Cc: git, Junio C Hamano
In-Reply-To: <20071202212224.GA22117@midwinter.com>

Steven Grimm <koreth@midwinter.com> wrote:
> This is useful in cases where a hook needs to modify an incoming commit
> in some way, e.g., fixing whitespace errors, adding an annotation to
> the commit message, noting the location of output from a profiling tool,
> or committing to an svn repository using git-svn.
...
> +/* Update hook exit code: hook has updated ref on its own */
> +#define EXIT_CODE_REF_UPDATED 100

Hmm.  I would actually rather move the ref locking to before we run
the update hook, so the ref is locked *while* the hook executes.
If we ran a hook and it exited 0 then re-read the ref to see if the
value differs from old_sha1; if it does then we can assume the update
hook took care of the update.  If the value is still old_sha1 then we
know the hook didn't do the update and we need to do it for the hook.

This probably requires exporting the name of the ref we currently
have locked in an environment variable (and teach lockfile.c it)
so we can effectively do recursive locking.  That way the update
hook can still use git-update-ref to change the ref safely.

The advantage of this approach is the hook programmer doesn't need
to implement their own locking scheme and we also don't make a
special case exit code where there wasn't one before.

-- 
Shawn.

^ permalink raw reply

* Re: [PATCH v4] Allow update hooks to update refs on their own.
From: Junio C Hamano @ 2007-12-03  3:45 UTC (permalink / raw)
  To: Jeff King; +Cc: Steven Grimm, git
In-Reply-To: <7vlk8csetl.fsf@gitster.siamese.dyndns.org>

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

> Jeff King <peff@peff.net> writes:
>
>> ..., but an
>> "ok, but btw I changed your commit" status from receive-pack seems like
>> it would be useful, for two reasons:
>>
>>   - it can be displayed differently, so the user is reminded to do a
>>     fetch afterwards
>>   - we can avoid updating the tracking ref, which makes it less likely
>>     to result in a non-fast forward fetch next time.  For example,
>>     consider:
>>
>>       1. The remote master and my origin/master are at A.
>>       2. I make a commit B on top of A.
>>       3. I push B to remote, who rewrites it to B' on top of A. At the
>>          same time, I move my origin/master to B.
>>       4. I fetch, and get non-ff going from B to B'.
>>
>>     If I had never written anything to my origin/master, it would be a
>>     fast forward. And obviously git handles it just fine, but it is more
>>     useful to the user during the next fetch to see A..B rather than
>>     B'...B.
>
> Sensible argument.  I stand corrected.

Having said that, I think the workflow that this "letting update hook
munge" patch supports has a bit more implications for the people who
interact with it.  The pusher will need to force fetch the result, but
after that he needs to discard his own commit and replace it with
whatever the hook did.  The question is how much to discard, and how to
reconstruct the changes since the last push that was made on top of the
commit that was pushed.

Maybe the push pushed out a string of five pearls and the hook may have
rewritten only the tip, or all of them.  You may have built a few
commits on top since then.  If what you pushed out contained merges and
the hook rewrote it, you would need to potentially replay such a merge
that was rewritten by the hook.

This is all the same with a workflow that deals with a branch that is
advertised to constantly rewound and rebased, and the common approach is
to use "fetch + rebase" (see recent discussion between Nico and Bruce).
So this patch is not creating a new problem (iow, I am not mentioning
this as the reason to reject this patch), but I thought I should bring
it up so that people know what they are doing.

^ permalink raw reply

* Re: [PATCH] git-stash: Display help message if git-stash is run with wrong sub-commands
From: Kevin Leung @ 2007-12-03  3:36 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Git ML, Nanako Shiraishi
In-Reply-To: <7vwsrwqysf.fsf@gitster.siamese.dyndns.org>

The current git-stash behaviour is very error prone to typos. For example,
if you typed "git-stash llist", git-stash would think that you wanted to
save to a stash named "llist", but in fact, you meant "git-stash list".

Signed-off-by: Kevin Leung <kevinlsk@gmail.com>
---

 Thanks, Junio. It should be alright now.

 git-stash.sh |   16 +++++++++-------
 1 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/git-stash.sh b/git-stash.sh
index 77c9421..844a3e5 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -1,7 +1,7 @@
 #!/bin/sh
 # Copyright (c) 2007, Nanako Shiraishi

-USAGE='[ | list | show | apply | clear]'
+USAGE='[  | save | list | show | apply | clear ]'

 SUBDIRECTORY_OK=Yes
 . git-sh-setup
@@ -195,6 +195,10 @@ show)
        shift
        show_stash "$@"
        ;;
+save)
+       shift
+       save_stash "$*" && git-reset --hard
+       ;;
 apply)
        shift
        apply_stash "$@"
@@ -202,14 +206,12 @@ apply)
 clear)
        clear_stash
        ;;
-help | usage)
-       usage
-       ;;
 *)
-       if test $# -gt 0 && test "$1" = save
+       if test "$#" -eq "0"
        then
-               shift
+               save_stash && git-reset --hard
+       else
+               usage
        fi
-       save_stash "$*" && git-reset --hard
        ;;
 esac
-- 
1.5.3.7-dirty

^ permalink raw reply related

* Re: Incorrect git-blame result if I use full path to file
From: Jeff King @ 2007-12-03  2:49 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Anatol Pomozov, git, Robin Rosenberg
In-Reply-To: <7v4pf0sdp7.fsf@gitster.siamese.dyndns.org>

On Sun, Dec 02, 2007 at 06:40:36PM -0800, Junio C Hamano wrote:

> > Even more useful would be to convert
> > /path/to/repo/file to 'file' internally.
> 
> ... that might help "cut & paste from file manager" people, and I think
> we had comment session for such a patch recently on the list.
> 
> Sorry, but I lost track of that the current status of that patch.  Did
> it die?

I didn't pay attention to it originally, but I assume you mean the
recent patch from Robin Rosenberg (cc'd). Looking it over, I see one
obvious omission: there is no canonicalization of the paths. IOW, I
think it will break in the presence of symlinks (if I specify
/path/to/repo/file, /path/to is a symlink to /other/path, I think the
worktree will end up as /other/path/repo, and fail a string comparison
with /path/to/repo).

-Peff

^ permalink raw reply

* Re: [PATCH] git-stash: Display help message if git-stash is run with wrong sub-commands
From: Junio C Hamano @ 2007-12-03  2:48 UTC (permalink / raw)
  To: Kevin Leung; +Cc: Git ML, Junio C Hamano, Nanako Shiraishi
In-Reply-To: <e66701d40712021834h64bf8d0y14f0e222d0f9a617@mail.gmail.com>

"Kevin Leung" <kevinlsk@gmail.com> writes:

> The current git-stash behaviour is very error prone to typos. For example,
> if you typed "git-stash llist", git-stash would think that you wanted to
> save to a stash named "llist", but in fact, you meant "git-stash list".
>
> Signed-off-by: Kevin Leung <kevinlsk@gmail.com>

Thanks.  Looks good.  except...

> @@ -195,6 +195,10 @@ show)
>         shift
>         show_stash "$@"
>         ;;
> +save)
> +       shift
> +       save_stash "$@" && git-reset --hard
> +       ;;

... this should be "$*" as it was originally spelled.

Save this script in foo.sh and run "foo.sh a b c" to see what I mean.

#!/bin/sh

foo () {
	msg="$1"
	echo "Foo here <$1>"
}

foo "$@"
foo "$*"

^ permalink raw reply

* Re: Incorrect git-blame result if I use full path to file
From: Junio C Hamano @ 2007-12-03  2:40 UTC (permalink / raw)
  To: Jeff King; +Cc: Anatol Pomozov, git
In-Reply-To: <20071203022729.GD8322@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

> IOW, it's not intended for users to use absolute paths in this way.
> However, the results for git-blame are obviously quite confusing. It
> might be worth fixing, but I suspect there are many more such traps
> waiting in other commands. I wonder if it would make sense to reject
> pathspecs starting with '/' entirely, which would at least give us a
> saner error message (and I can't think of a time when such a pathspec
> would be useful)?

All correct, except...

> Even more useful would be to convert
> /path/to/repo/file to 'file' internally.

... that might help "cut & paste from file manager" people, and I think
we had comment session for such a patch recently on the list.

Sorry, but I lost track of that the current status of that patch.  Did
it die?

^ permalink raw reply

* [PATCH] git-stash: Display help message if git-stash is run with wrong sub-commands
From: Kevin Leung @ 2007-12-03  2:34 UTC (permalink / raw)
  To: Git ML, Junio C Hamano, Nanako Shiraishi

The current git-stash behaviour is very error prone to typos. For example,
if you typed "git-stash llist", git-stash would think that you wanted to
save to a stash named "llist", but in fact, you meant "git-stash list".

Signed-off-by: Kevin Leung <kevinlsk@gmail.com>
---
 git-stash.sh |   16 +++++++++-------
 1 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/git-stash.sh b/git-stash.sh
index 77c9421..a17cc25 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -1,7 +1,7 @@
 #!/bin/sh
 # Copyright (c) 2007, Nanako Shiraishi

-USAGE='[ | list | show | apply | clear]'
+USAGE='[  | save | list | show | apply | clear ]'

 SUBDIRECTORY_OK=Yes
 . git-sh-setup
@@ -195,6 +195,10 @@ show)
        shift
        show_stash "$@"
        ;;
+save)
+       shift
+       save_stash "$@" && git-reset --hard
+       ;;
 apply)
        shift
        apply_stash "$@"
@@ -202,14 +206,12 @@ apply)
 clear)
        clear_stash
        ;;
-help | usage)
-       usage
-       ;;
 *)
-       if test $# -gt 0 && test "$1" = save
+       if test "$#" -eq "0"
        then
-               shift
+               save_stash && git-reset --hard
+       else
+               usage
        fi
-       save_stash "$*" && git-reset --hard
        ;;
 esac
-- 
1.5.3.7-dirty

^ permalink raw reply related

* Re: v1.5.4 plans
From: Junio C Hamano @ 2007-12-03  2:32 UTC (permalink / raw)
  To: David Symonds; +Cc: git
In-Reply-To: <ee77f5c20712021539r3075fc57ld6a4cec737e6043d@mail.gmail.com>

"David Symonds" <dsymonds@gmail.com> writes:

> On Dec 3, 2007 9:04 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> Please do not take this as the final decision made by the Emperor, whose
>> subjects now must follow.  This is a sanity-check to see if everybody is
>> on the same page.
>>
>> I am not the Emperor anyway ;-)
>>
>
>> Topics not in 'master' yet but should be in v1.5.4
>> --------------------------------------------------
>>
>> I think the following should go in, along with what we already have in
>> 'master':
>
> Can we add the git-status/git-checkout relative path stuff that's
> currently been sitting in 'next'? It would be a good step forward for
> usability.

I think checkout from subdirectory with relative was merged on November
18th to master, with d577bc58.  Relative path output for git-status is
part of the "git-commit in C" series, which is planned to go in.

But now you mention it, I realize that I ran "git-topic.perl" (found in
my 'todo' branch) without "--all" option when I made that list, and I
missed stuff fully merged to 'next'.  Sorry.

Here is a corrected list.

Topics not in 'master' yet but should be in v1.5.4
--------------------------------------------------

I think the following should go in, along with what we already have in
'master':

 * git-commit in C (Kristian and others)
 * git-add --patch (Wincent)
 * git-prune --expire (Dscho)
 * git-add --interactive coloring (Dan Zwell)
 * whitespace error classes in diff and patch, using gitattributes (Bruce and me)
 * cvsserver runs post-receive (Michael Witten)
 * git-rebase -i gives chance to rerere (Dscho)
 * git-rebase gives more appropriate help text (Wincent)
 * make refspec matching logic in git-push and git-fetch saner (Steffen Prohaska)
 * work-tree related minor fixes (Nguyen and Dscho)
 * allow update hook to munge commit (Steven Grimm)
 * git-fast-export (Dscho)
 * Add commitdiff to gitweb grep page (Denis Cheng)
 * "git pull --rebase" (Dscho)
 * "git config --get-color" (me)
 * "color.diff = true" means "auto" (me)
 * Rewrite "export VAR=VAL" to "VAR=VAL; export VAR" (Dscho)
 * run correct perl in Documentation (me, waiting for Merlyn)

^ permalink raw reply

* Re: Incorrect git-blame result if I use full path to file
From: Jeff King @ 2007-12-03  2:28 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Anatol Pomozov, git
In-Reply-To: <7vhcj0seok.fsf@gitster.siamese.dyndns.org>

On Sun, Dec 02, 2007 at 06:19:23PM -0800, Junio C Hamano wrote:

> I think it is rather a sloppy error checking than a bug.  It should be
> throwing a stone back at you when you feed it a full path, or converting
> it back to work tree relative path before using.

I think it's not the only place. Doing "git diff /path/to/repo/file"
silently produces an empty diff, even if there are changes in the file.

-Peff

^ permalink raw reply

* Re: Incorrect git-blame result if I use full path to file
From: Jeff King @ 2007-12-03  2:27 UTC (permalink / raw)
  To: Anatol Pomozov; +Cc: git
In-Reply-To: <3665a1a00712021652tbdfe9d1tdc4575d225bfed36@mail.gmail.com>

On Sun, Dec 02, 2007 at 04:52:36PM -0800, Anatol Pomozov wrote:

> I just start learning git and I found a bug (but sorry if the
> functionality I am trying to blame as a bug not actually bug and it
> was made by intention)

Some of both, I think. :)

> It is exaclty what we expect. But lets try full path for master.txt
> $pwd
> /personal/sources/learn/gitea/repo
> $git blame /personal/sources/learn/gitea/repo/master.txt
> ^69bce74 (Anatol Pomozov 2007-12-02 16:44:07 -0800 1) On master
> ^69bce74 (Anatol Pomozov 2007-12-02 16:44:07 -0800 2) On master
> ^69bce74 (Anatol Pomozov 2007-12-02 16:44:07 -0800 3) On master

We talk about many git commands taking "files" or "paths" but really
they are git "pathspecs", meaning a path specifier that is relative to
the repository root, and which is generally used for limiting the parts
of the history we are looking at.

So I think what is happening is that git-blame is looking for content
from /personal/sources/..., which of course as a git pathspec doesn't
match any of the files. So everything ends up being blamed on
'^69bce74' (which really means "beyond where we started looking"). But
of course it still finds the content to try blaming in the first place,
because in that instance it treats /personal/sources/... as a file to be
opened.

IOW, it's not intended for users to use absolute paths in this way.
However, the results for git-blame are obviously quite confusing. It
might be worth fixing, but I suspect there are many more such traps
waiting in other commands. I wonder if it would make sense to reject
pathspecs starting with '/' entirely, which would at least give us a
saner error message (and I can't think of a time when such a pathspec
would be useful)? Even more useful would be to convert
/path/to/repo/file to 'file' internally.

-Peff

^ permalink raw reply

* Re: Incorrect git-blame result if I use full path to file
From: Junio C Hamano @ 2007-12-03  2:19 UTC (permalink / raw)
  To: Anatol Pomozov; +Cc: git
In-Reply-To: <3665a1a00712021652tbdfe9d1tdc4575d225bfed36@mail.gmail.com>

"Anatol Pomozov" <anatol.pomozov@gmail.com> writes:

> I just start learning git and I found a bug (but sorry if the
> functionality I am trying to blame as a bug not actually bug and it
> was made by intention)

I think it is rather a sloppy error checking than a bug.  It should be
throwing a stone back at you when you feed it a full path, or converting
it back to work tree relative path before using.

^ permalink raw reply

* Re: [PATCH v4] Allow update hooks to update refs on their own.
From: Junio C Hamano @ 2007-12-03  2:16 UTC (permalink / raw)
  To: Jeff King; +Cc: Steven Grimm, git
In-Reply-To: <20071203021333.GC8322@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

> ..., but an
> "ok, but btw I changed your commit" status from receive-pack seems like
> it would be useful, for two reasons:
>
>   - it can be displayed differently, so the user is reminded to do a
>     fetch afterwards
>   - we can avoid updating the tracking ref, which makes it less likely
>     to result in a non-fast forward fetch next time.  For example,
>     consider:
>
>       1. The remote master and my origin/master are at A.
>       2. I make a commit B on top of A.
>       3. I push B to remote, who rewrites it to B' on top of A. At the
>          same time, I move my origin/master to B.
>       4. I fetch, and get non-ff going from B to B'.
>
>     If I had never written anything to my origin/master, it would be a
>     fast forward. And obviously git handles it just fine, but it is more
>     useful to the user during the next fetch to see A..B rather than
>     B'...B.

Sensible argument.  I stand corrected.

^ permalink raw reply

* Re: [PATCH v4] Allow update hooks to update refs on their own.
From: Jeff King @ 2007-12-03  2:13 UTC (permalink / raw)
  To: Steven Grimm; +Cc: git
In-Reply-To: <20071202212224.GA22117@midwinter.com>

On Sun, Dec 02, 2007 at 01:22:24PM -0800, Steven Grimm wrote:

> 	Since Junio's main objection to this seemed to be the protocol
> 	change to bypass the automatic update of the tracking ref in
> 	git-send-pack, that code is gone (thus reverting this to the
> 	same code change as the initial version!) and I added a section
> 	to the git-send-pack manual page describing the automatic
> 	tracking ref update behavior, which wasn't documented at all
> 	before. Someone please review my terminology there.

I am dubious of the usefulness of passing back the new commit id, but an
"ok, but btw I changed your commit" status from receive-pack seems like
it would be useful, for two reasons:

  - it can be displayed differently, so the user is reminded to do a
    fetch afterwards
  - we can avoid updating the tracking ref, which makes it less likely
    to result in a non-fast forward fetch next time.  For example,
    consider:

      1. The remote master and my origin/master are at A.
      2. I make a commit B on top of A.
      3. I push B to remote, who rewrites it to B' on top of A. At the
         same time, I move my origin/master to B.
      4. I fetch, and get non-ff going from B to B'.

    If I had never written anything to my origin/master, it would be a
    fast forward. And obviously git handles it just fine, but it is more
    useful to the user during the next fetch to see A..B rather than
    B'...B.

-Peff

^ 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