Git development
 help / color / mirror / Atom feed
* Re: warning in git version 1.6.6.rc0.114.gc8648
From: Alejandro Riveira @ 2009-12-02 19:53 UTC (permalink / raw)
  To: git
In-Reply-To: <7veindgt8v.fsf@alter.siamese.dyndns.org>

El Wed, 02 Dec 2009 09:50:40 -0800, Junio C Hamano escribió:

> Alejandro Riveira <ariveira@gmail.com> writes:
> 
> 
> Yes we are aware of the issue and have a patch to do so which requires
> another change which we also already have patch for.  It will be fixed
> before 1.6.6-rc1

 Thanks for the quick answer :)

> 
> Thanks for reporting.
 
 No; thanks *you* for your work in git ^_^

^ permalink raw reply

* [PATCH v2] Detailed diagnosis when parsing an object name fails.
From: y @ 2009-12-02 20:01 UTC (permalink / raw)
  To: git, gitster; +Cc: Matthieu Moy

From: Matthieu Moy <Matthieu.Moy@imag.fr>

The previous error message was the same in many situations (unknown
revision or path not in the working tree). We try to help the user as
much as possible to understand the error, especially with the
sha1:filename notation. In this case, we say whether the sha1 or the
filename is problematic, and diagnose the confusion between
relative-to-root and relative-to-$PWD confusion precisely.

The 6 new error messages are tested.

Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
Changes since v1:

* Fixed a segfault with

+	if (!prefix)
+		prefix = "";

* Added testcases.

 cache.h                        |    6 ++-
 setup.c                        |   15 +++++-
 sha1_name.c                    |   95 ++++++++++++++++++++++++++++++++++++++--
 t/t1506-rev-parse-diagnosis.sh |   67 ++++++++++++++++++++++++++++
 4 files changed, 176 insertions(+), 7 deletions(-)
 create mode 100755 t/t1506-rev-parse-diagnosis.sh

diff --git a/cache.h b/cache.h
index 0e69384..5c8cb5f 100644
--- a/cache.h
+++ b/cache.h
@@ -708,7 +708,11 @@ static inline unsigned int hexval(unsigned char c)
 #define DEFAULT_ABBREV 7
 
 extern int get_sha1(const char *str, unsigned char *sha1);
-extern int get_sha1_with_mode(const char *str, unsigned char *sha1, unsigned *mode);
+static inline get_sha1_with_mode(const char *str, unsigned char *sha1, unsigned *mode)
+{
+	return get_sha1_with_mode_1(str, sha1, mode, 0, NULL);
+}
+extern int get_sha1_with_mode_1(const char *str, unsigned char *sha1, unsigned *mode, int fatal, const char *prefix);
 extern int get_sha1_hex(const char *hex, unsigned char *sha1);
 extern char *sha1_to_hex(const unsigned char *sha1);	/* static buffer result! */
 extern int read_ref(const char *filename, unsigned char *sha1);
diff --git a/setup.c b/setup.c
index f67250b..3094e8b 100644
--- a/setup.c
+++ b/setup.c
@@ -74,6 +74,18 @@ int check_filename(const char *prefix, const char *arg)
 	die_errno("failed to stat '%s'", arg);
 }
 
+static void NORETURN die_verify_filename(const char *prefix, const char *arg)
+{
+	unsigned char sha1[20];
+	unsigned mode;
+	/* try a detailed diagnostic ... */
+	get_sha1_with_mode_1(arg, sha1, &mode, 1, prefix);
+	/* ... or fall back the most general message. */
+	die("ambiguous argument '%s': unknown revision or path not in the working tree.\n"
+	    "Use '--' to separate paths from revisions", arg);
+
+}
+
 /*
  * Verify a filename that we got as an argument for a pathspec
  * entry. Note that a filename that begins with "-" never verifies
@@ -87,8 +99,7 @@ void verify_filename(const char *prefix, const char *arg)
 		die("bad flag '%s' used after filename", arg);
 	if (check_filename(prefix, arg))
 		return;
-	die("ambiguous argument '%s': unknown revision or path not in the working tree.\n"
-	    "Use '--' to separate paths from revisions", arg);
+	die_verify_filename(prefix, arg);
 }
 
 /*
diff --git a/sha1_name.c b/sha1_name.c
index 44bb62d..030e2ac 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -804,7 +804,77 @@ int get_sha1(const char *name, unsigned char *sha1)
 	return get_sha1_with_mode(name, sha1, &unused);
 }
 
-int get_sha1_with_mode(const char *name, unsigned char *sha1, unsigned *mode)
+static void diagnose_invalid_sha1_path(const char *prefix,
+				       const char *filename,
+				       const char *tree_sha1,
+				       const char *object_name)
+{
+	struct stat st;
+	unsigned char sha1[20];
+	unsigned mode;
+
+	if (!prefix)
+		prefix = "";
+
+	if (!lstat(filename, &st))
+		die("Path '%s' exists on disk, but not in '%s'.",
+		    filename, object_name);
+	if (errno == ENOENT || errno == ENOTDIR) {
+		char *fullname = malloc(strlen(filename)
+					     + strlen(prefix) + 1);
+		strcpy(fullname, prefix);
+		strcat(fullname, filename);
+
+		if (!get_tree_entry(tree_sha1, fullname,
+				    sha1, &mode)) {
+			die("Path '%s' exists, but not '%s'.\n"
+			    "Did you mean '%s:%s'?",
+			    fullname,
+			    filename,
+			    object_name,
+			    fullname);
+		}
+		die("Path '%s' does not exist in '%s'",
+		    filename, object_name);
+	}
+}
+
+static void diagnose_invalid_index_path(int stage,
+					const char *prefix,
+					const char *filename)
+{
+	struct stat st;
+
+	if (!prefix)
+		prefix = "";
+
+	if (!lstat(filename, &st))
+		die("Path '%s' exists on disk, but not in the index.", filename);
+	if (errno == ENOENT || errno == ENOTDIR) {
+		struct cache_entry *ce;
+		int pos;
+		int namelen = strlen(filename) + strlen(prefix);
+		char *fullname = malloc(namelen + 1);
+		strcpy(fullname, prefix);
+		strcat(fullname, filename);
+		pos = cache_name_pos(fullname, namelen);
+		if (pos < 0)
+			pos = -pos - 1;
+		ce = active_cache[pos];
+		if (ce_namelen(ce) == namelen &&
+		    !memcmp(ce->name, fullname, namelen))
+			die("Path '%s' is in the index, but not '%s'.\n"
+			    "Did you mean ':%d:%s'?",
+			    fullname, filename,
+			    stage, fullname);
+
+		die("Path '%s' does not exist (neither on disk nor in the index).",
+		    filename);
+	}
+}
+
+
+int get_sha1_with_mode_1(const char *name, unsigned char *sha1, unsigned *mode, int fatal, const char *prefix)
 {
 	int ret, bracket_depth;
 	int namelen = strlen(name);
@@ -850,6 +920,8 @@ int get_sha1_with_mode(const char *name, unsigned char *sha1, unsigned *mode)
 			}
 			pos++;
 		}
+		if (fatal)
+			diagnose_invalid_index_path(stage, prefix, cp);
 		return -1;
 	}
 	for (cp = name, bracket_depth = 0; *cp; cp++) {
@@ -862,9 +934,24 @@ int get_sha1_with_mode(const char *name, unsigned char *sha1, unsigned *mode)
 	}
 	if (*cp == ':') {
 		unsigned char tree_sha1[20];
-		if (!get_sha1_1(name, cp-name, tree_sha1))
-			return get_tree_entry(tree_sha1, cp+1, sha1,
-					      mode);
+		char *object_name;
+		if (fatal) {
+			object_name = malloc(cp-name+1);
+			strncpy(object_name, name, cp-name);
+			object_name[cp-name] = '\0';
+		}
+		if (!get_sha1_1(name, cp-name, tree_sha1)) {
+			const char *filename = cp+1;
+			ret = get_tree_entry(tree_sha1, filename, sha1, mode);
+			if (fatal)
+				diagnose_invalid_sha1_path(prefix, filename,
+							   tree_sha1, object_name);
+
+			return ret;
+		} else {
+			if (fatal)
+				die("Invalid object name '%s'.", object_name);
+		}
 	}
 	return ret;
 }
diff --git a/t/t1506-rev-parse-diagnosis.sh b/t/t1506-rev-parse-diagnosis.sh
new file mode 100755
index 0000000..8112d56
--- /dev/null
+++ b/t/t1506-rev-parse-diagnosis.sh
@@ -0,0 +1,67 @@
+#!/bin/sh
+
+test_description='test git rev-parse diagnosis for invalid argument'
+
+exec </dev/null
+
+. ./test-lib.sh
+
+HASH_file=
+
+test_expect_success 'set up basic repo' '
+	echo one > file.txt &&
+	mkdir subdir &&
+	echo two > subdir/file.txt &&
+	echo three > subdir/file2.txt &&
+	git add . &&
+	git commit -m init &&
+	echo four > index-only.txt &&
+	git add index-only.txt &&
+	echo five > disk-only.txt
+'
+
+test_expect_success 'correct file objects' '
+	HASH_file=$(git rev-parse HEAD:file.txt) &&
+	git rev-parse HEAD:subdir/file.txt &&
+	git rev-parse :index-only.txt &&
+	cd subdir &&
+	git rev-parse HEAD:file.txt &&
+	git rev-parse HEAD:subdir/file2.txt &&
+	test $HASH_file = $(git rev-parse HEAD:file.txt) &&
+	test $HASH_file = $(git rev-parse :file.txt) &&
+	test $HASH_file = $(git rev-parse :0:file.txt) &&
+	cd ..
+'
+
+test_expect_success 'incorrect revision id' '
+	test_must_fail git rev-parse foobar:file.txt 2>&1 |
+		grep "Invalid object name '"'"'foobar'"'"'." &&
+	test_must_fail git rev-parse foobar 2>&1 |
+		grep "unknown revision or path not in the working tree."
+'
+
+test_expect_success 'incorrect file in sha1:path' '
+	test_must_fail git rev-parse HEAD:nothing.txt 2>&1 |
+		grep "fatal: Path '"'"'nothing.txt'"'"' does not exist in '"'"'HEAD'"'"'" &&
+	test_must_fail git rev-parse HEAD:index-only.txt 2>&1 |
+		grep "fatal: Path '"'"'index-only.txt'"'"' exists on disk, but not in '"'"'HEAD'"'"'." &&
+	cd subdir &&
+	test_must_fail git rev-parse HEAD:file2.txt 2>&1 |
+		grep "Did you mean '"'"'HEAD:subdir/file2.txt'"'"'?" &&
+	cd ..
+'
+
+test_expect_success 'incorrect file in :path and :0:path' '
+	test_must_fail git rev-parse :nothing.txt 2>&1 |
+		grep "fatal: Path '"'"'nothing.txt'"'"' does not exist (neither on disk nor in the index)." &&
+	test_must_fail git rev-parse :1:nothing.txt 2>&1 |
+		grep "Path '"'"'nothing.txt'"'"' does not exist (neither on disk nor in the index)." &&
+	cd subdir &&
+	test_must_fail git rev-parse :file2.txt 2>&1 |
+		grep "Did you mean '"'"':0:subdir/file2.txt'"'"'?" &&
+	cd .. &&
+	test_must_fail git rev-parse :disk-only.txt 2>&1 |
+		grep "fatal: Path '"'"'disk-only.txt'"'"' exists on disk, but not in the index."
+'
+
+test_done
-- 
1.6.6.rc0.256.g6060

^ permalink raw reply related

* [PATCH v2] Detailed diagnosis when parsing an object name fails.
From: y @ 2009-12-02 20:01 UTC (permalink / raw)
  To: git, gitster; +Cc: Matthieu Moy

From: Matthieu Moy <Matthieu.Moy@imag.fr>

The previous error message was the same in many situations (unknown
revision or path not in the working tree). We try to help the user as
much as possible to understand the error, especially with the
sha1:filename notation. In this case, we say whether the sha1 or the
filename is problematic, and diagnose the confusion between
relative-to-root and relative-to-$PWD confusion precisely.

The 6 new error messages are tested.

Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
Changes since v1:

* Fixed a segfault with

+	if (!prefix)
+		prefix = "";

* Added testcases.

 cache.h                        |    6 ++-
 setup.c                        |   15 +++++-
 sha1_name.c                    |   95 ++++++++++++++++++++++++++++++++++++++--
 t/t1506-rev-parse-diagnosis.sh |   67 ++++++++++++++++++++++++++++
 4 files changed, 176 insertions(+), 7 deletions(-)
 create mode 100755 t/t1506-rev-parse-diagnosis.sh

diff --git a/cache.h b/cache.h
index 0e69384..5c8cb5f 100644
--- a/cache.h
+++ b/cache.h
@@ -708,7 +708,11 @@ static inline unsigned int hexval(unsigned char c)
 #define DEFAULT_ABBREV 7
 
 extern int get_sha1(const char *str, unsigned char *sha1);
-extern int get_sha1_with_mode(const char *str, unsigned char *sha1, unsigned *mode);
+static inline get_sha1_with_mode(const char *str, unsigned char *sha1, unsigned *mode)
+{
+	return get_sha1_with_mode_1(str, sha1, mode, 0, NULL);
+}
+extern int get_sha1_with_mode_1(const char *str, unsigned char *sha1, unsigned *mode, int fatal, const char *prefix);
 extern int get_sha1_hex(const char *hex, unsigned char *sha1);
 extern char *sha1_to_hex(const unsigned char *sha1);	/* static buffer result! */
 extern int read_ref(const char *filename, unsigned char *sha1);
diff --git a/setup.c b/setup.c
index f67250b..3094e8b 100644
--- a/setup.c
+++ b/setup.c
@@ -74,6 +74,18 @@ int check_filename(const char *prefix, const char *arg)
 	die_errno("failed to stat '%s'", arg);
 }
 
+static void NORETURN die_verify_filename(const char *prefix, const char *arg)
+{
+	unsigned char sha1[20];
+	unsigned mode;
+	/* try a detailed diagnostic ... */
+	get_sha1_with_mode_1(arg, sha1, &mode, 1, prefix);
+	/* ... or fall back the most general message. */
+	die("ambiguous argument '%s': unknown revision or path not in the working tree.\n"
+	    "Use '--' to separate paths from revisions", arg);
+
+}
+
 /*
  * Verify a filename that we got as an argument for a pathspec
  * entry. Note that a filename that begins with "-" never verifies
@@ -87,8 +99,7 @@ void verify_filename(const char *prefix, const char *arg)
 		die("bad flag '%s' used after filename", arg);
 	if (check_filename(prefix, arg))
 		return;
-	die("ambiguous argument '%s': unknown revision or path not in the working tree.\n"
-	    "Use '--' to separate paths from revisions", arg);
+	die_verify_filename(prefix, arg);
 }
 
 /*
diff --git a/sha1_name.c b/sha1_name.c
index 44bb62d..030e2ac 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -804,7 +804,77 @@ int get_sha1(const char *name, unsigned char *sha1)
 	return get_sha1_with_mode(name, sha1, &unused);
 }
 
-int get_sha1_with_mode(const char *name, unsigned char *sha1, unsigned *mode)
+static void diagnose_invalid_sha1_path(const char *prefix,
+				       const char *filename,
+				       const char *tree_sha1,
+				       const char *object_name)
+{
+	struct stat st;
+	unsigned char sha1[20];
+	unsigned mode;
+
+	if (!prefix)
+		prefix = "";
+
+	if (!lstat(filename, &st))
+		die("Path '%s' exists on disk, but not in '%s'.",
+		    filename, object_name);
+	if (errno == ENOENT || errno == ENOTDIR) {
+		char *fullname = malloc(strlen(filename)
+					     + strlen(prefix) + 1);
+		strcpy(fullname, prefix);
+		strcat(fullname, filename);
+
+		if (!get_tree_entry(tree_sha1, fullname,
+				    sha1, &mode)) {
+			die("Path '%s' exists, but not '%s'.\n"
+			    "Did you mean '%s:%s'?",
+			    fullname,
+			    filename,
+			    object_name,
+			    fullname);
+		}
+		die("Path '%s' does not exist in '%s'",
+		    filename, object_name);
+	}
+}
+
+static void diagnose_invalid_index_path(int stage,
+					const char *prefix,
+					const char *filename)
+{
+	struct stat st;
+
+	if (!prefix)
+		prefix = "";
+
+	if (!lstat(filename, &st))
+		die("Path '%s' exists on disk, but not in the index.", filename);
+	if (errno == ENOENT || errno == ENOTDIR) {
+		struct cache_entry *ce;
+		int pos;
+		int namelen = strlen(filename) + strlen(prefix);
+		char *fullname = malloc(namelen + 1);
+		strcpy(fullname, prefix);
+		strcat(fullname, filename);
+		pos = cache_name_pos(fullname, namelen);
+		if (pos < 0)
+			pos = -pos - 1;
+		ce = active_cache[pos];
+		if (ce_namelen(ce) == namelen &&
+		    !memcmp(ce->name, fullname, namelen))
+			die("Path '%s' is in the index, but not '%s'.\n"
+			    "Did you mean ':%d:%s'?",
+			    fullname, filename,
+			    stage, fullname);
+
+		die("Path '%s' does not exist (neither on disk nor in the index).",
+		    filename);
+	}
+}
+
+
+int get_sha1_with_mode_1(const char *name, unsigned char *sha1, unsigned *mode, int fatal, const char *prefix)
 {
 	int ret, bracket_depth;
 	int namelen = strlen(name);
@@ -850,6 +920,8 @@ int get_sha1_with_mode(const char *name, unsigned char *sha1, unsigned *mode)
 			}
 			pos++;
 		}
+		if (fatal)
+			diagnose_invalid_index_path(stage, prefix, cp);
 		return -1;
 	}
 	for (cp = name, bracket_depth = 0; *cp; cp++) {
@@ -862,9 +934,24 @@ int get_sha1_with_mode(const char *name, unsigned char *sha1, unsigned *mode)
 	}
 	if (*cp == ':') {
 		unsigned char tree_sha1[20];
-		if (!get_sha1_1(name, cp-name, tree_sha1))
-			return get_tree_entry(tree_sha1, cp+1, sha1,
-					      mode);
+		char *object_name;
+		if (fatal) {
+			object_name = malloc(cp-name+1);
+			strncpy(object_name, name, cp-name);
+			object_name[cp-name] = '\0';
+		}
+		if (!get_sha1_1(name, cp-name, tree_sha1)) {
+			const char *filename = cp+1;
+			ret = get_tree_entry(tree_sha1, filename, sha1, mode);
+			if (fatal)
+				diagnose_invalid_sha1_path(prefix, filename,
+							   tree_sha1, object_name);
+
+			return ret;
+		} else {
+			if (fatal)
+				die("Invalid object name '%s'.", object_name);
+		}
 	}
 	return ret;
 }
diff --git a/t/t1506-rev-parse-diagnosis.sh b/t/t1506-rev-parse-diagnosis.sh
new file mode 100755
index 0000000..8112d56
--- /dev/null
+++ b/t/t1506-rev-parse-diagnosis.sh
@@ -0,0 +1,67 @@
+#!/bin/sh
+
+test_description='test git rev-parse diagnosis for invalid argument'
+
+exec </dev/null
+
+. ./test-lib.sh
+
+HASH_file=
+
+test_expect_success 'set up basic repo' '
+	echo one > file.txt &&
+	mkdir subdir &&
+	echo two > subdir/file.txt &&
+	echo three > subdir/file2.txt &&
+	git add . &&
+	git commit -m init &&
+	echo four > index-only.txt &&
+	git add index-only.txt &&
+	echo five > disk-only.txt
+'
+
+test_expect_success 'correct file objects' '
+	HASH_file=$(git rev-parse HEAD:file.txt) &&
+	git rev-parse HEAD:subdir/file.txt &&
+	git rev-parse :index-only.txt &&
+	cd subdir &&
+	git rev-parse HEAD:file.txt &&
+	git rev-parse HEAD:subdir/file2.txt &&
+	test $HASH_file = $(git rev-parse HEAD:file.txt) &&
+	test $HASH_file = $(git rev-parse :file.txt) &&
+	test $HASH_file = $(git rev-parse :0:file.txt) &&
+	cd ..
+'
+
+test_expect_success 'incorrect revision id' '
+	test_must_fail git rev-parse foobar:file.txt 2>&1 |
+		grep "Invalid object name '"'"'foobar'"'"'." &&
+	test_must_fail git rev-parse foobar 2>&1 |
+		grep "unknown revision or path not in the working tree."
+'
+
+test_expect_success 'incorrect file in sha1:path' '
+	test_must_fail git rev-parse HEAD:nothing.txt 2>&1 |
+		grep "fatal: Path '"'"'nothing.txt'"'"' does not exist in '"'"'HEAD'"'"'" &&
+	test_must_fail git rev-parse HEAD:index-only.txt 2>&1 |
+		grep "fatal: Path '"'"'index-only.txt'"'"' exists on disk, but not in '"'"'HEAD'"'"'." &&
+	cd subdir &&
+	test_must_fail git rev-parse HEAD:file2.txt 2>&1 |
+		grep "Did you mean '"'"'HEAD:subdir/file2.txt'"'"'?" &&
+	cd ..
+'
+
+test_expect_success 'incorrect file in :path and :0:path' '
+	test_must_fail git rev-parse :nothing.txt 2>&1 |
+		grep "fatal: Path '"'"'nothing.txt'"'"' does not exist (neither on disk nor in the index)." &&
+	test_must_fail git rev-parse :1:nothing.txt 2>&1 |
+		grep "Path '"'"'nothing.txt'"'"' does not exist (neither on disk nor in the index)." &&
+	cd subdir &&
+	test_must_fail git rev-parse :file2.txt 2>&1 |
+		grep "Did you mean '"'"':0:subdir/file2.txt'"'"'?" &&
+	cd .. &&
+	test_must_fail git rev-parse :disk-only.txt 2>&1 |
+		grep "fatal: Path '"'"'disk-only.txt'"'"' exists on disk, but not in the index."
+'
+
+test_done
-- 
1.6.6.rc0.256.g6060


^ permalink raw reply related

* Re: "git merge" merges too much!
From: Jeff King @ 2009-12-02 20:09 UTC (permalink / raw)
  To: The Git Mailing List
In-Reply-To: <m1NFAji-000kn2C@most.weird.com>

On Mon, Nov 30, 2009 at 01:12:31PM -0500, Greg A. Woods wrote:

> (From a first pass through the documentation I would never have guessed
> that "tags" were also a form of "refs".  All these different names for

I find git is much simpler to use and understand if you start "at the
bottom" with the basic concepts (because for the most part, git is
really a set of tools for manipulating the few basic data structures).
For a short intro, try:

  http://eagain.net/articles/git-for-computer-scientists/

I think Scott Chacon's "Pro Git" book also takes a similar approach, but
I confess that I have not actually read it carefully. At this point, I
know enough about git to make reading it not very interesting. :) You
can find it online at:

  http://progit.org/book/

> features.  Even the gitglossary(7) is somewhat inconsistent on how it
> uses "ref" and "refs".  Perhaps all that's needed is some firm editing
> and clean-up of the manuals and documentation by a good strong technical
> editor.)

I skimmed it and didn't see any inconsistency. If you have something
specific in mind, please point it out so we can fix it.

> "git rebase" will not work for me unless it grows a "copy" option ,
> i.e. one which does not delete the original branch (i.e. avoids the
> "reset" phase of its operation).  This option would likely only make
> sense when used with the "--onto" option, I would guess.

I think Dmitry already mentioned this, but you probably want to create a
new branch to hold your rebased history if you don't want to modify the
existing branch.

> (git-log(1) is worse than ls(1) for having too many options, but worst
> of all in the release I'm still using it doesn't respond sensibly nor
> consistently with other commands when given the "-?" option.)

$ ls -?
ls: invalid option -- '?'
Try `ls --help' for more information.

$ ls --help ;# or ls -h
[copious usage information]

$ git log -?
fatal: unrecognized argument: -?

$ git log --help
[the man page]

$ git log -h
usage: git log [<options>] [<since>..<until>] [[--] <path>...]
   or: git show [options] <object>...

$ cd /outside/of/git/repo
$ git log -?
fatal: Not a git repository (or any of the parent directories): .git

So "-?" is bogus for both ls and git. But there are two failings I see:

  1. Outside of a repository, "git log" does not even get to the
     argument-parsing phase to see that "-?" is bogus. We short-circuit
     "-h" and "--help" to avoid actually looking for a git repository,
     but obviously cannot do so for every "--bogus" argument we see.
     We could potentially also short-circuit "-?" (and probably map it
     to "-h" if we were going to do that). However, I didn't think "-?"
     was in common use.

  2. "git log -h" doesn't mention any of the options specifically,
     though other git commands do (e.g., try "git archive -h"). This is
     because the option list is generated by our parseopt library, but
     the revision and diff options (which are the only ones that "git
     log" takes) do not use parseopt. Maybe we should point to "--help"
     for the full list in that case.

-Peff

^ permalink raw reply

* Re: [RFC PATCH 4/8] Support remote helpers implementing smart transports
From: Ilari Liusvaara @ 2009-12-02 20:10 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: git
In-Reply-To: <20091202170457.GC31648@spearce.org>

On Wed, Dec 02, 2009 at 09:04:57AM -0800, Shawn O. Pearce wrote:
> Ilari Liusvaara <ilari.liusvaara@elisanet.fi> wrote:
> 
> Drop invoke-r.

Dropped.

> Modify transport-helper.c to allow pushing TRANS_OPT_UPLOADPACK and
> TRANS_OPT_RECEIVEPACK down into the helper via the option capability.

NAK. Modified _process_connect_or_invoke (now _process_connect) to pass
new option that appiles to connecting all subprotocols (if needed).

It looks like following:

 > capabilities
 < option
 < connect
 <
 > option servpath <blahblah>
 < ok
 > connect git-upload-pack
 < 

And from helper POV, all subprotocols should appear identical from
layer 6 POV so it doesn't make sense to diffrentiate between path
for upload-pack and receive-pack (or upload-archive!).

> I'd rename connect-r to just connect.

Yeah, putting repository in RPC explicit signature is bit ugly (there
isn't probaby ever going to be signature that doesn't contain repo as
argument). That would make it 'connect'.

Renamed.

-Ilari

^ permalink raw reply

* Re: [PATCH] builtin-commit: add --date option
From: Miklos Vajna @ 2009-12-02 20:33 UTC (permalink / raw)
  To: Jeff King; +Cc: Junio C Hamano, git
In-Reply-To: <20091202192425.GC30778@coredump.intra.peff.net>

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

On Wed, Dec 02, 2009 at 02:24:25PM -0500, Jeff King <peff@peff.net> wrote:
> On Wed, Dec 02, 2009 at 09:35:48AM -0800, Junio C Hamano wrote:
> 
> > > Is there any documentation describing what does parse_date() accept?
> > [...]
> > The above are all supported (you can label 2 as ISO even though the
> > official ISO8601 wants "T" instead of " " between date and time).
> > 
> > For more amusing ones, see
> > 
> >     http://article.gmane.org/gmane.comp.version-control.git/12241
> > 
> > and follow the discussion there ;-)
> 
> Aren't the amusing ones the result of approxidate, and not parse_date?
> At least that is my recollection from working on the date code when I
> ate 30 hot dogs last August.

I think you are right, at least --date="2008-12-02 18:04:00" works here,
but not --date="teatime".

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

^ permalink raw reply

* Re: [RFC/PATCHv9 01/11] fast-import: Proper notes tree manipulation
From: Shawn O. Pearce @ 2009-12-02 20:39 UTC (permalink / raw)
  To: Johan Herland; +Cc: git, gitster
In-Reply-To: <1259719783-4674-2-git-send-email-johan@herland.net>

Johan Herland <johan@herland.net> wrote:
> diff --git a/fast-import.c b/fast-import.c
> index b41d29f..b51ffbc 100644
> --- a/fast-import.c
> +++ b/fast-import.c

This new version is much cleaner, thank you.

> @@ -161,6 +161,7 @@ Format of STDIN stream:
>  #define MAX_PACK_ID ((1<<PACK_ID_BITS)-1)
>  #define DEPTH_BITS 13
>  #define MAX_DEPTH ((1<<DEPTH_BITS)-1)
> +#define NOTE_MODE 0170000

Be consistent with S_IFGITLINK:

#define S_IFNOTE 0170000
#define S_ISNOTE(m) (((m) & S_IFMT) == S_IFNOTE)
 
> @@ -245,6 +246,7 @@ struct branch
>  	const char *name;
>  	struct tree_entry branch_tree;
>  	uintmax_t last_commit;
> +	unsigned int num_notes;

Use uintmax_t here?

> +static unsigned int do_change_note_fanout(
> +		struct tree_entry *orig_root, struct tree_entry *root,
> +		char *hex_sha1, unsigned int hex_sha1_len,
> +		char *fullpath, unsigned int fullpath_len,
> +		unsigned char fanout)
> +{
> +	struct tree_content *t = root->tree;
> +	struct tree_entry *e, leaf;
> +	unsigned int i, tmp_hex_sha1_len, tmp_fullpath_len, num_notes = 0;
> +	unsigned char sha1[20];
> +	char realpath[60];
> +	int is_note;
> +
> +	for (i = 0; i < t->entry_count; i++) {
> +		e = t->entries[i];
> +		is_note = (e->versions[1].mode & NOTE_MODE) == NOTE_MODE;
> +		tmp_hex_sha1_len = hex_sha1_len + e->name->str_len;
> +		tmp_fullpath_len = fullpath_len;
> +
> +		if (tmp_hex_sha1_len <= 40 && e->name->str_len >= 2) {
> +			memcpy(hex_sha1 + hex_sha1_len, e->name->str_dat,
> +				e->name->str_len);
> +			if (tmp_fullpath_len)
> +				fullpath[tmp_fullpath_len++] = '/';
> +			memcpy(fullpath + tmp_fullpath_len, e->name->str_dat,
> +				e->name->str_len);
> +			tmp_fullpath_len += e->name->str_len;
> +			assert(tmp_fullpath_len < 60);
> +			fullpath[tmp_fullpath_len] = '\0';
> +		} else {
> +			assert(!is_note);
> +			continue;
> +		}

Are we rejecting a mixed content-tree here?  I thought a notes
tree was allowed to hold anything, e.g. isn't it ok to put a
".gitattributes" file into a notes tree.

I think we'd do better to have at the top of our loop:

	if (!is_note && !S_ISDIR(e->versions[1].mode))
		continue;

so that we ignore non-notes and non-subdirectories which might
contain notes.

Also, fast-import never uses assert.  I'd prefer to die() because
then the recent command trace can go into the crash report.
It gives the user more context about what the hell just went wrong.

> +		/* The above may have reallocated the current tree_content */
> +		if (t != root->tree)
> +			t = root->tree;

Why bother with the condition?  Just do the assignment every time
in the loop.

> @@ -2080,8 +2195,10 @@ static void note_change_n(struct branch *b)
>  			    typename(type), command_buf.buf);
>  	}
> 
> -	tree_content_set(&b->branch_tree, sha1_to_hex(commit_sha1), sha1,
> -		S_IFREG | 0644, NULL);
> +	construct_path_with_fanout(sha1_to_hex(commit_sha1), fanout, path);
> +	b->num_notes += adjust_num_notes(&b->branch_tree, path, sha1);
> +	mode = (is_null_sha1(sha1) ? S_IFREG : NOTE_MODE) | 0644;
> +	tree_content_set(&b->branch_tree, path, sha1, mode, NULL);

I wonder if it wouldn't be better to compute the fan out here on
each put.  That way if an importer drives 2,000,000 notes at once
to us in a single commit, we don't wind up with a flat 0 fan-out
tree and trying to perform a linear insert on each one, but instead
will start to increase the fan out as the number of entries goes up.

Basically, tree_content_set/remove are O(N) operations on N paths
in the tree, because their structures aren't necessarily sorted.
IIRC at one point in time I tried to do this with a binary search but
gave up and just did it unsorted.  At least using the fan out here
would help partition the search space dramatically on large inserts.

-- 
Shawn.

^ permalink raw reply

* Re: [PATCH 2/5] git-svn: Make merge metadata accessible to make_log_entry
From: Alex Vandiver @ 2009-12-02 20:46 UTC (permalink / raw)
  To: git
In-Reply-To: <1259780874-14706-3-git-send-email-alex@chmrr.net>

At Wed Dec 02 14:07:51 -0500 2009, Alex Vandiver wrote:
> @@ -2944,14 +2944,15 @@ sub find_extra_svk_parents {
>                  # wahey!  we found it, but it might be
>                  # an old one (!)
>                  push @known_parents, [ $rev, $commit ];
> +                push @known_parents, [ $rev, $path, $commit ];
>              }
>          }
>      }

This hunk is wrong due to a mis-merge on my part -- the first 'push'
line should have been removed, obviously.  I'll wait for other
comments before I push a v2, however.

 - Alex
-- 
Networking -- only one letter away from not working

^ permalink raw reply

* How do you best store structured data in git repositories?
From: Sebastian Setzer @ 2009-12-02 21:08 UTC (permalink / raw)
  To: git

Hi,
when you design a file format to store structured data, and you want to
manage these files with git, how do you do this best?

I'd like to hear about best practices, experiences, links to discussions
on this subject, ...

Here are some of my questions:

Do you store everything in a single file and configure git to use
special diff- and merge-tools?
Do you use XML for this purpose?
Do you take care that the contents of your file is as stable as possible
when it's saved or do you let your diff tools cope with issues like
reordering, reassignment of identifiers (for example when identifiers
are offsets in the file), ...?

Do you store one object/record per file (with filename=id, for example
with GUID-s) and hope that git will not mess them up when it merges
them?

Do you store records as directories, with very small files which contain
single attributes (because records can be considered sets of
key-value-pairs and the same applies to directories)? Do you configure
git to do a scalar merge on non-text "attributes" (with special file
extensions)?

When you don't store everything in a single, binary file: Do you use git
hooks to update an index for efficient queries on your structured data?
Do you update the whole index for every change? Or do you use git hashes
to decide which segment of your index needs to be updated?

greetings,
Sebastian

^ permalink raw reply

* Re: [PATCH v2] Add --track option to git clone
From: Nanako Shiraishi @ 2009-12-02 21:07 UTC (permalink / raw)
  To: Jeff King; +Cc: David Soria Parra, git
In-Reply-To: <20091202190807.GB30778@coredump.intra.peff.net>

Quoting Jeff King <peff@peff.net>

> I would find something like this useful for cloning git.git, where I
> explicitly fetch maint, master, next, and pu, but none of html, man, or
> todo. This makes "gitk --all" much nicer to view.

Thank you for explaining. I now can understand why it can be useful.

>   # most general case
>   git clone -f 'refs/heads/subset/*:refs/remotes/origin/*' remote.git

Because this is only about branches and no other kinds of 
references, I think this is an overkill.

>   git clone -f 'subset/*' remote.git

But I think this is a good idea.

>   # multiple -f should add multiple refspec lines
>   git clone -f maint -f master -f next -f pu git.git
>
>   # choose your favorite branch
>   git clone -f maint -f master -f next -f pu -b next git.git
> ...
> What do you think?

I think your rule to make first branch given by -f the default 
for -b is a good idea. But I'm not very happy with the example 
with four -f. Can we probably write it like this?

  git clone -f maint,master,next,pu git.git

If it isn't a good idea to use comma, we can use colon to split 
the list of branch names instead.

-- 
Nanako Shiraishi
http://ivory.ap.teacup.com/nanako3/

^ permalink raw reply

* Re: [PATCH] builtin-commit: add --date option
From: Miklos Vajna @ 2009-12-02 21:07 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jeff King, git
In-Reply-To: <7vpr6xcgki.fsf@alter.siamese.dyndns.org>

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

On Wed, Dec 02, 2009 at 11:38:05AM -0800, Junio C Hamano <gitster@pobox.com> wrote:
>  - We should honor GIT_AUTHOR_DATE if --reset-author is given.

This is already a situation, as far as I see.

>  - I _think_ we should ignore GIT_AUTHOR_DATE if --reset-author is not
>    given, as --amend/-c/-C is stronger for being command line options than
>    an environment variable.

We already ignore GIT_AUTHOR_DATE if --reset-author is not given, also
when I change it like this:

diff --git a/builtin-commit.c b/builtin-commit.c
index e93a647..7234c7d 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -397,7 +397,8 @@ static void determine_author_info(void)

                name = xstrndup(a + 8, lb - (a + 8));
                email = xstrndup(lb + 2, rb - (lb + 2));
-               date = xstrndup(rb + 2, eol - (rb + 2));
+               if (!date)
+                       date = xstrndup(rb + 2, eol - (rb + 2));
        }

        if (force_author) {

tests 27, 33, 38 and 39 fail in t7501-commit.sh.

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

^ permalink raw reply related

* [PATCH] gitweb: Describe (possible) gitweb.js minification in gitweb/README
From: Jakub Narebski @ 2009-12-02 21:14 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Jakub Narebski
In-Reply-To: <7vaay2tkfh.fsf@alter.siamese.dyndns.org>

On Tue, 1 Dec 2009, Junio C Hamano wrote:

> * jn/gitweb-blame (2009-11-24) 8 commits.
>   (merged to 'next' on 2009-11-25 at 0a5b649)
>  + gitweb.js: fix padLeftStr() and its usage
>  + gitweb.js: Harden setting blamed commit info in incremental blame
>  + gitweb.js: fix null object exception in initials calculation
>  + gitweb: Minify gitweb.js if JSMIN is defined

This commit is somehow missing description of JSMIn in gitweb/README
from the original patch.  Here is its completion.

>  + gitweb: Create links leading to 'blame_incremental' using JavaScript
>   (merged to 'next' on 2009-10-11 at 73c4a83)
>  + gitweb: Colorize 'blame_incremental' view during processing
>  + gitweb: Incremental blame (using JavaScript)
>  + gitweb: Add optional "time to generate page" info in footer
> 
> With two more changes to disable this by default to make it
> suitable as "new feature with known breakages" for 1.6.6

-- >8 --
Subject: [PATCH] gitweb: Describe (possible) gitweb.js minification in gitweb/README

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
 gitweb/README |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/gitweb/README b/gitweb/README
index b69b0e5..e34ee79 100644
--- a/gitweb/README
+++ b/gitweb/README
@@ -95,7 +95,8 @@ You can specify the following configuration variables when building GIT:
  * GITWEB_JS
    Points to the localtion where you put gitweb.js on your web server
    (or to be more generic URI of JavaScript code used by gitweb).
-   Relative to base URI of gitweb.  [Default: gitweb.js]
+   Relative to base URI of gitweb.  [Default: gitweb.js (or gitweb.min.js
+   if JSMIN build variable is defined / JavaScript minifier is used)]
  * GITWEB_CONFIG
    This Perl file will be loaded using 'do' and can be used to override any
    of the options above as well as some other options -- see the "Runtime
-- 
1.6.5.3

^ permalink raw reply related

* Re: Git GUI client SmartGit released
From: Marc Strapetz @ 2009-12-02 21:15 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: git, Alexander Kitaev
In-Reply-To: <20091202164810.GB31648@spearce.org>

> I noticed you use JGit and the Trilead SSH client.
> 
> I'm curious, did you guys replace JSch because its a pile of junk?
> Did you patch JGit to use Trilead SSH instead of JSch?  If so,
> would you be interested in contributing that change back to JGit?
> I'm rather fed up with JSch...  :-)

We currently don't use JGit's transport, but we plant a custom SSH
client on the git executable which connects back to SmartGit and just
tunnels SSH data through. Anyway, I can remember that SVNKit was using
JSch initially and they switched to Trilead because of problems with
JSch (maybe Alexander in Cc can shed more light on that).

--
Best regards,
Marc Strapetz
=============
syntevo GmbH
http://www.syntevo.com
http://blog.syntevo.com



Shawn O. Pearce wrote:
> Marc Strapetz <marc.strapetz@syntevo.com> wrote:
>> We are proud to announce the general availability of our Git client
>> SmartGit[1]:
>>
>>  http://www.syntevo.com/smartgit/index.html
> 
> Congrats on your release.
> 
> I noticed you use JGit and the Trilead SSH client.
> 
> I'm curious, did you guys replace JSch because its a pile of junk?
> Did you patch JGit to use Trilead SSH instead of JSch?  If so,
> would you be interested in contributing that change back to JGit?
> I'm rather fed up with JSch...  :-)
> 

^ permalink raw reply

* Re: How do you best store structured data in git repositories?
From: Avery Pennarun @ 2009-12-02 21:17 UTC (permalink / raw)
  To: sebastianspublicaddress; +Cc: git
In-Reply-To: <1259788097.3590.29.camel@nord26-amd64>

On Wed, Dec 2, 2009 at 4:08 PM, Sebastian Setzer
<sebastianspublicaddress@googlemail.com> wrote:
> Do you store everything in a single file and configure git to use
> special diff- and merge-tools?
> Do you use XML for this purpose?

XML is terrible for most data storage purposes.  Data exchange, maybe,
but IMHO the best thing you can do when you get XML data is to put it
in some other format ASAP.

As it happens, I've been doing a project where we store a bunch of
stuff in csv format in git, and it works fairly well.  We made a
special merge driver that can merge csv data (based on knowing which
columns should be treated as the "primary key").

> Do you take care that the contents of your file is as stable as possible
> when it's saved or do you let your diff tools cope with issues like
> reordering, reassignment of identifiers (for example when identifiers
> are offsets in the file), ...?

A custom merge driver is better, by far, than the builtin ones (which
were designed for source code) if you have any kind of structured data
that you don't want to have to merge by hand.

That said, however, you should still try to make your files as stable
as possible, because:

- If your program outputs the data in random order, it's just being
sloppy anyway

- 'git diff' doesn't work usefully otherwise (for examining the data
and debugging)

Of course, all bets are off if your file is actually binary; merging
and diffing is mostly impossible unless you use a totally custom
engine.  And if your file contains byte offsets, then it's a binary
file, no matter that it looks like in your text editor.  Adding a byte
in the middle would make such a file entirely nonsense, which is not
an attribute of a text file.

> Do you store one object/record per file (with filename=id, for example
> with GUID-s) and hope that git will not mess them up when it merges
> them?
>
> Do you store records as directories, with very small files which contain
> single attributes (because records can be considered sets of
> key-value-pairs and the same applies to directories)? Do you configure
> git to do a scalar merge on non-text "attributes" (with special file
> extensions)?

In git, you have to balance between its different limitations.  If you
have a tonne of small files, it'll take you longer to retrieve a large
amount of data.  If you have one big huge file, git will suck a lot of
memory when repacking.  The best is to achieve a reasonable balance.

One trick that I've been using lately is to split large files
according to a rolling checksum:
http://alumnit.ca/~apenwarr/log/?m=200910#04

This generally keeps diffs useful, but keeps individual file sizes
down.  Obviously the implementation pointed to there is just a toy,
but the idea is sound.

> When you don't store everything in a single, binary file: Do you use git
> hooks to update an index for efficient queries on your structured data?
> Do you update the whole index for every change? Or do you use git hashes
> to decide which segment of your index needs to be updated?

We keep a separate index file that's not part of git.  When the git
repo is updated, we note which rows have changed, then update the
index.

Avery

^ permalink raw reply

* [PATCH] General --quiet improvements
From: Felipe Contreras @ 2009-12-02 21:28 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Felipe Contreras

'git reset' is missing --quiet, and 'git gc' is not using OPT__QUIET.
Let's fix that.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 Documentation/git-reset.txt |    1 +
 builtin-gc.c                |    2 +-
 builtin-reset.c             |    3 +--
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-reset.txt b/Documentation/git-reset.txt
index 2d27e40..9df6de2 100644
--- a/Documentation/git-reset.txt
+++ b/Documentation/git-reset.txt
@@ -62,6 +62,7 @@ This means that `git reset -p` is the opposite of `git add -p` (see
 linkgit:git-add[1]).
 
 -q::
+--quiet::
 	Be quiet, only report errors.
 
 <commit>::
diff --git a/builtin-gc.c b/builtin-gc.c
index 093517e..c304638 100644
--- a/builtin-gc.c
+++ b/builtin-gc.c
@@ -180,12 +180,12 @@ int cmd_gc(int argc, const char **argv, const char *prefix)
 	char buf[80];
 
 	struct option builtin_gc_options[] = {
+		OPT__QUIET(&quiet),
 		{ OPTION_STRING, 0, "prune", &prune_expire, "date",
 			"prune unreferenced objects",
 			PARSE_OPT_OPTARG, NULL, (intptr_t)prune_expire },
 		OPT_BOOLEAN(0, "aggressive", &aggressive, "be more thorough (increased runtime)"),
 		OPT_BOOLEAN(0, "auto", &auto_gc, "enable auto-gc mode"),
-		OPT_BOOLEAN('q', "quiet", &quiet, "suppress progress reports"),
 		OPT_END()
 	};
 
diff --git a/builtin-reset.c b/builtin-reset.c
index 73e6022..25b38ce 100644
--- a/builtin-reset.c
+++ b/builtin-reset.c
@@ -202,6 +202,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
 	struct commit *commit;
 	char *reflog_action, msg[1024];
 	const struct option options[] = {
+		OPT__QUIET(&quiet),
 		OPT_SET_INT(0, "mixed", &reset_type,
 						"reset HEAD and index", MIXED),
 		OPT_SET_INT(0, "soft", &reset_type, "reset only HEAD", SOFT),
@@ -209,8 +210,6 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
 				"reset HEAD, index and working tree", HARD),
 		OPT_SET_INT(0, "merge", &reset_type,
 				"reset HEAD, index and working tree", MERGE),
-		OPT_BOOLEAN('q', NULL, &quiet,
-				"disable showing new HEAD in hard reset and progress message"),
 		OPT_BOOLEAN('p', "patch", &patch_mode, "select hunks interactively"),
 		OPT_END()
 	};
-- 
1.6.6.rc0.63.g0471c

^ permalink raw reply related

* Re: [PATCH] builtin-commit: add --date option
From: Miklos Vajna @ 2009-12-02 21:40 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vaay1gt0z.fsf@alter.siamese.dyndns.org>

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

On Wed, Dec 02, 2009 at 09:55:24AM -0800, Junio C Hamano <gitster@pobox.com> wrote:
> I should have mentioned ones with more importance in real-life before
> referring you to the amusing ones: US and European dates.
> 
> date.c::match_multi_number() groks these:
> 
>     mm/dd/yy[yy] (US)
>     dd.mm.yy[yy] (European)

As far as I see these are also about approxidate() as well. But thanks
for the ISO number, now I have enough info to add documentation about
it. Patch is coming soon... ;)

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

^ permalink raw reply

* Re: [PATCH] builtin-commit: add --date option
From: Miklos Vajna @ 2009-12-02 22:12 UTC (permalink / raw)
  To: Jeff King; +Cc: git
In-Reply-To: <20091202192614.GD30778@coredump.intra.peff.net>

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

On Wed, Dec 02, 2009 at 02:26:14PM -0500, Jeff King <peff@peff.net> wrote:
> Do you really want to set the date to something arbitrary, or do you
> just want to set it to "now"? If the latter case, do you really just
> want the recently discussed --reset-author?

No, I want to set it to two days ago, for example when I forgot to
commit at the end of the day and I notice it two days later before I
start to work on something again.

> Also, is there a good reason why GIT_AUTHOR_DATE is not respected in
> this case?  If not, should we simply be fixing that bug instead?

GIT_AUTHOR_DATE is respected by --reset-author, just given that we have
--author already, using environment variables for a user can be
uncomfortable.

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

^ permalink raw reply

* [PATCH 2/2] Document date formats accepted by parse_date()
From: Miklos Vajna @ 2009-12-02 22:16 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jeff King, git
In-Reply-To: <3d547f4e32c026efc76a7dfe1572da617714f8c9.1259791789.git.vmiklos@frugalware.org>

---
 Documentation/date-formats.txt    |   23 +++++++++++++++++++++++
 Documentation/git-commit-tree.txt |    1 +
 Documentation/git-commit.txt      |    2 ++
 3 files changed, 26 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/date-formats.txt

diff --git a/Documentation/date-formats.txt b/Documentation/date-formats.txt
new file mode 100644
index 0000000..626c887
--- /dev/null
+++ b/Documentation/date-formats.txt
@@ -0,0 +1,23 @@
+DATE FORMATS
+------------
+
+The GIT_AUTHOR_DATE, GIT_COMMITTER_DATE environment variables
+ifdef::git-commit[]
+and the `--date` option
+endif::git-commit[]
+support the following date formats:
+
+Git native format::
+	It is `<unix timestamp> <timezone offset>`, where `<unix
+	timestamp>` is the number of seconds since the UNIX epoch.
+	`<timezone offset>` is a positive or negative offset from UTC.
+	For example CET (which is 2 hours ahead UTC) is `+0200`.
+
+RFC 2822::
+	The standard email format as described by RFC 2822, for example
+	`Thu, 07 Apr 2005 22:13:13 +0200`.
+
+ISO 8601::
+	Time and date specified by the ISO 8601 standard, for example
+	`2005-04-07T22:13:13`. The parser accepts a space instead of the
+	`T` character as well.
diff --git a/Documentation/git-commit-tree.txt b/Documentation/git-commit-tree.txt
index b8834ba..4fec5d5 100644
--- a/Documentation/git-commit-tree.txt
+++ b/Documentation/git-commit-tree.txt
@@ -73,6 +73,7 @@ A commit comment is read from stdin. If a changelog
 entry is not provided via "<" redirection, 'git-commit-tree' will just wait
 for one to be entered and terminated with ^D.
 
+include::date-formats.txt[]
 
 Diagnostics
 -----------
diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt
index cbbbeeb..17783b4 100644
--- a/Documentation/git-commit.txt
+++ b/Documentation/git-commit.txt
@@ -220,6 +220,8 @@ specified.
 	these files are also staged for the next commit on top
 	of what have been staged before.
 
+:git-commit: 1
+include::date-formats.txt[]
 
 EXAMPLES
 --------
-- 
1.6.5.2

^ permalink raw reply related

* [PATCH 1/2] builtin-commit: add --date option
From: Miklos Vajna @ 2009-12-02 22:16 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jeff King, git
In-Reply-To: <7vpr6xcgki.fsf@alter.siamese.dyndns.org>

This is like --author: allow a user to specify a given date without
using the GIT_AUTHOR_DATE environment variable.

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

On Wed, Dec 02, 2009 at 11:38:05AM -0800, Junio C Hamano <gitster@pobox.com> wrote:
> So I do not think --date is something we urgently need, even though it
> might be nice to have it to be consistent with --author.

Changes from the previous version:

* updated the commit message

* rebased on top of current master that has --reset-author

 Documentation/git-commit.txt |    5 ++++-
 builtin-commit.c             |    6 +++++-
 t/t7501-commit.sh            |   15 +++++++++++++++
 3 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt
index d227cec..cbbbeeb 100644
--- a/Documentation/git-commit.txt
+++ b/Documentation/git-commit.txt
@@ -11,7 +11,7 @@ SYNOPSIS
 'git commit' [-a | --interactive] [-s] [-v] [-u<mode>] [--amend] [--dry-run]
 	   [(-c | -C) <commit>] [-F <file> | -m <msg>] [--reset-author]
 	   [--allow-empty] [--no-verify] [-e] [--author=<author>]
-	   [--cleanup=<mode>] [--] [[-i | -o ]<file>...]
+	   [--date=<date>] [--cleanup=<mode>] [--] [[-i | -o ]<file>...]
 
 DESCRIPTION
 -----------
@@ -85,6 +85,9 @@ OPTIONS
 	an existing commit that matches the given string and its author
 	name is used.
 
+--date=<date>::
+	Override the date used in the commit.
+
 -m <msg>::
 --message=<msg>::
 	Use the given <msg> as the commit message.
diff --git a/builtin-commit.c b/builtin-commit.c
index e93a647..9a1264a 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -52,7 +52,7 @@ static char *edit_message, *use_message;
 static char *author_name, *author_email, *author_date;
 static int all, edit_flag, also, interactive, only, amend, signoff;
 static int quiet, verbose, no_verify, allow_empty, dry_run, renew_authorship;
-static char *untracked_files_arg;
+static char *untracked_files_arg, *force_date;
 /*
  * The default commit message cleanup mode will remove the lines
  * beginning with # (shell comments) and leading and trailing
@@ -90,6 +90,7 @@ static struct option builtin_commit_options[] = {
 
 	OPT_FILENAME('F', "file", &logfile, "read log from file"),
 	OPT_STRING(0, "author", &force_author, "AUTHOR", "override author for commit"),
+	OPT_STRING(0, "date", &force_date, "DATE", "override date for commit"),
 	OPT_CALLBACK('m', "message", &message, "MESSAGE", "specify commit message", opt_parse_m),
 	OPT_STRING('c', "reedit-message", &edit_message, "COMMIT", "reuse and edit message from specified commit"),
 	OPT_STRING('C', "reuse-message", &use_message, "COMMIT", "reuse message from specified commit"),
@@ -410,6 +411,9 @@ static void determine_author_info(void)
 		email = xstrndup(lb + 2, rb - (lb + 2));
 	}
 
+	if (force_date)
+		date = force_date;
+
 	author_name = name;
 	author_email = email;
 	author_date = date;
diff --git a/t/t7501-commit.sh b/t/t7501-commit.sh
index a603f6d..a529701 100755
--- a/t/t7501-commit.sh
+++ b/t/t7501-commit.sh
@@ -211,6 +211,21 @@ test_expect_success 'amend commit to fix author' '
 
 '
 
+test_expect_success 'amend commit to fix date' '
+
+	test_tick &&
+	newtick=$GIT_AUTHOR_DATE &&
+	git reset --hard &&
+	git cat-file -p HEAD |
+	sed -e "s/author.*/author $author $newtick/" \
+		-e "s/^\(committer.*> \).*$/\1$GIT_COMMITTER_DATE/" > \
+		expected &&
+	git commit --amend --date="$newtick" &&
+	git cat-file -p HEAD > current &&
+	test_cmp expected current
+
+'
+
 test_expect_success 'sign off (1)' '
 
 	echo 1 >positive &&
-- 
1.6.5.2

^ permalink raw reply related

* Re: [PATCH] builtin-commit: add --date option
From: Jeff King @ 2009-12-02 22:22 UTC (permalink / raw)
  To: Miklos Vajna; +Cc: git
In-Reply-To: <20091202221232.GF31763@genesis.frugalware.org>

On Wed, Dec 02, 2009 at 11:12:32PM +0100, Miklos Vajna wrote:

> No, I want to set it to two days ago, for example when I forgot to
> commit at the end of the day and I notice it two days later before I
> start to work on something again.

OK. I think you are being overly meticulous about your commit date, but
that is your right. ;)

> > Also, is there a good reason why GIT_AUTHOR_DATE is not respected in
> > this case?  If not, should we simply be fixing that bug instead?
> 
> GIT_AUTHOR_DATE is respected by --reset-author, just given that we have
> --author already, using environment variables for a user can be
> uncomfortable.

OK, I can agree with that reasoning.

-Peff

^ permalink raw reply

* Re: [StGit PATCH v2 0/6] add support for git send-email
From: Catalin Marinas @ 2009-12-02 22:35 UTC (permalink / raw)
  To: Alex Chiang; +Cc: git, Karl Wiberg
In-Reply-To: <20091202003503.7737.51579.stgit@bob.kio>

Hi Alex,

2009/12/2 Alex Chiang <achiang@hp.com>:
> This is v2 of the series that starts teaching stg mail how to
> call git send-email.

Thanks for posting these patches (and thanks to Karl for reviewing
them). I don't have much to comment (Karl did the hard work here) but
I'll give them a try tomorrow and let you know.

Thanks.

-- 
Catalin

^ permalink raw reply

* Re: [PATCH 2/2] Document date formats accepted by parse_date()
From: Nanako Shiraishi @ 2009-12-02 22:33 UTC (permalink / raw)
  To: Miklos Vajna; +Cc: Junio C Hamano, Jeff King, git
In-Reply-To: <831fc8f48429d5a21e29d04760b46b2ddfcb7d80.1259791789.git.vmiklos@frugalware.org>

Quoting Miklos Vajna <vmiklos@frugalware.org>

> ---

I don't think any message needs to be there, but a Signed-Off-By: 
line should be.

> +The GIT_AUTHOR_DATE, GIT_COMMITTER_DATE environment variables
> +ifdef::git-commit[]
> +and the `--date` option
> +endif::git-commit[]
> +support the following date formats:
> +
> +Git native format::
> +	It is `<unix timestamp> <timezone offset>`, where `<unix
> +	timestamp>` is the number of seconds since the UNIX epoch.
> +	`<timezone offset>` is a positive or negative offset from UTC.
> +	For example CET (which is 2 hours ahead UTC) is `+0200`.

It is better to call it 'internal' format, instead of 'native'. 
I think 'native' means the most natural way to Git, and users 
view what 'git show' outputs as 'native'.

  nana.git% git show -s v1.6.5^0 | grep Date
  Date:   Sat Oct 10 00:05:19 2009 -0700
  nana.git% ./test-date parse 'Sat Oct 10 00:05:19 2009 -0700'
  Sat Oct 10 00:05:19 2009 -0700 -> 2009-10-10 07:05:19 +0000

And 'Git native' format is also supported, I think.

> +RFC 2822::
> +	The standard email format as described by RFC 2822, for example
> +	`Thu, 07 Apr 2005 22:13:13 +0200`.
> +
> +ISO 8601::
> +	Time and date specified by the ISO 8601 standard, for example
> +	`2005-04-07T22:13:13`. The parser accepts a space instead of the
> +	`T` character as well.

I didn't know about "T", but it works (^_^).

  nana.git% ./test-date parse '2005-04-07T22:13:13'
  2005-04-07T22:13:13 -> 2005-04-07 13:13:13 +0000

Is there something wrong with 'show'?

  nana.git% ./test-date show '2005-04-07T22:13:13'
  2005-04-07T22:13:13 -> 40 years ago

-- 
Nanako Shiraishi
http://ivory.ap.teacup.com/nanako3/

^ permalink raw reply

* Re: [PATCH v2] Add --track option to git clone
From: Jeff King @ 2009-12-02 22:37 UTC (permalink / raw)
  To: Nanako Shiraishi; +Cc: David Soria Parra, git
In-Reply-To: <20091203060708.6117@nanako3.lavabit.com>

On Thu, Dec 03, 2009 at 06:07:08AM +0900, Nanako Shiraishi wrote:

> >   # most general case
> >   git clone -f 'refs/heads/subset/*:refs/remotes/origin/*' remote.git
> 
> Because this is only about branches and no other kinds of 
> references, I think this is an overkill.

Perhaps. I'm not sure this has to be only about branches. You could do:

  git clone -f tags/v1.6.1 git.git

though I admit I don't really have a burning desire to do so. I just
think it will be simple to make it flexible (since you have to build
such a refspec _anyway_), and there is no reason to restrict people who
might use it creatively.

The biggest argument against it would be that we are confusing the user
by giving too much rope, but I don't think that is the case here. If
you just use branches, you need never know that the full refspec exists
(just as some people use "git fetch origin master" without ever
understanding how "master" can be replaced by a full refspec).

> >   git clone -f 'subset/*' remote.git
> 
> But I think this is a good idea.

One question on this: does it fetch to "refs/remotes/origin/subset/*"
or to "refs/remotes/origin/*"?

I think the latter makes more sense (presumably you don't care that your
branches are in "subset/", since you by definition have asked for
nothing outside of that namespace).

> >   # choose your favorite branch
> >   git clone -f maint -f master -f next -f pu -b next git.git
> > ...
> > What do you think?
> 
> I think your rule to make first branch given by -f the default 
> for -b is a good idea. But I'm not very happy with the example 
> with four -f. Can we probably write it like this?
> 
>   git clone -f maint,master,next,pu git.git

Yeah, that is much nicer. I think "," is allowed in ref names, but I
am tempted not to care here. It is not as if this is a low-level
feature, and most people will not be crazy enough to use commas in their
branch-names. IOW, you will get into trouble only if you have crazy
names _and_ you want to use this particular feature. If we wanted to be
complete, we could provide a quoting mechanism, but that is perhaps
excessive.

> If it isn't a good idea to use comma, we can use colon to split 
> the list of branch names instead.

Colon would work (though of course it would imply not allowing full
refspecs with "-f"). However, I actually find

  git clone -f maint:master:next:pu git.git

to be a bit ugly and confusing.

-Peff

^ permalink raw reply

* Re: [RFC/PATCHv9 01/11] fast-import: Proper notes tree manipulation
From: Johan Herland @ 2009-12-02 22:41 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: git, gitster
In-Reply-To: <20091202203953.GE31648@spearce.org>

On Wednesday 02 December 2009, Shawn O. Pearce wrote:
> Johan Herland <johan@herland.net> wrote:
> > diff --git a/fast-import.c b/fast-import.c
> > index b41d29f..b51ffbc 100644
> > --- a/fast-import.c
> > +++ b/fast-import.c
> 
> This new version is much cleaner, thank you.

Thank you for the suggestions that made it so :)

The rest of your comments all make perfect sense. I'll whip up a replacement 
patch shortly.


Thanks for the review!

...Johan

-- 
Johan Herland, <johan@herland.net>
www.herland.net

^ permalink raw reply

* Re: [PATCH 2/2] Document date formats accepted by parse_date()
From: Jeff King @ 2009-12-02 22:42 UTC (permalink / raw)
  To: Nanako Shiraishi; +Cc: Miklos Vajna, Junio C Hamano, git
In-Reply-To: <20091203073313.6117@nanako3.lavabit.com>

On Thu, Dec 03, 2009 at 07:33:13AM +0900, Nanako Shiraishi wrote:

> Is there something wrong with 'show'?
> 
>   nana.git% ./test-date show '2005-04-07T22:13:13'
>   2005-04-07T22:13:13 -> 40 years ago

No. It doesn't call parse_date, but instead accepts a time_t-like
"seconds since epoch". I intentionally didn't use parse_date because I
didn't want bugs in parse_date to affect 'show' output, to keep testing
simple.

-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