Git development
 help / color / mirror / Atom feed
* Re: [PATCH] Support output ISO 8601 format dates
From: Robin Rosenberg @ 2007-07-15 23:19 UTC (permalink / raw)
  To: Jan Hudec; +Cc: junkio, git
In-Reply-To: <20070715212359.GB18293@efreet.light.src>

From 59eafc201aec3be121b33c2ecf16c70cc3521e92 Mon Sep 17 00:00:00 2001
From: Robin Rosenberg <robin.rosenberg@dewire.com>
Date: Mon, 16 Jul 2007 01:05:19 +0200
Subject: [PATCH] Support output ISO 8601 format dates

Support output of full ISO 8601 style dates in e.g. git log
and other places that use interpolation for formatting.

Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
---
 Documentation/pretty-formats.txt |    2 ++
 cache.h                          |    2 +-
 commit.c                         |    6 +++++-
 date.c                           |    9 +++++++++
 4 files changed, 17 insertions(+), 2 deletions(-)

söndag 15 juli 2007 skrev Jan Hudec:
> Hello,
> 
> On Fri, Jul 13, 2007 at 22:22:58 +0200, Robin Rosenberg wrote:
> > +	else if (mode == DATE_ISO8601)
> > +		sprintf(timebuf, "%04d-%02d-%02d %02d:%02d:%02d %+05d",
> > +				tm->tm_year + 1900,
> > +				tm->tm_mon + 1,
> > +				tm->tm_mday,
> > +				tm->tm_hour, tm->tm_min, tm->tm_sec,
> > +				tz);
> 
> I apologise for nitpicking, but ISO 8601 (and RFC 3339) says separator between
> date and time is 'T' and there is no separator between time and timezone. So
> this should be
> 
> +		sprintf(timebuf, "%04d-%02d-%02dT%02d:%02d:%02d%+05d",

Note that my reference is to ISO 8601, not RFC 3339.  Hower I interpret "NOTE By mutual 
agreement of the partners in information interchange, the character [T] may be omitted in
applications where there is no risk of confusing a date and time of day representation with
others defined in this International Standard" (ISO 8601:2004 4.3.2) as space being
allowed here instead of 'T'.

By "mutual agreement" I mean that I tell you what it looks like and you agree :)

I agree with you about the space before timezone though. I'll go.

> for 100% conformance to the standard. RFC 3339 explicitely mentions using
> space instead of 'T' as separator as allowed, but does not seem to mention
> space before time zone. There may be applications that would stop at such
> space and assume no timezone information.
> 
> Furthermore RFC 3339 seems to require colon in the timezone, so it would be:
> 
> +		sprintf(timebuf, "%04d-%02d-%02dT%02d:%02d:%02d%+03d:%02d",
> +				tm->tm_year + 1900,
> +				tm->tm_mon + 1,
> +				tm->tm_mday,
> +				tm->tm_hour, tm->tm_min, tm->tm_sec,
> +				tz / 100, abs(tz % 100));
> 
> ISO 8601 makes separators optional, so simple 4-digit timezone is OK.

For the sake of consequence I should use the extended format for all parts and not mix
basic and extended formats.

Updated patch follows that formats dates as "2006-08-17 20:59:46+05:30"

-- robin

diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt
index d922e8e..1296b31 100644
--- a/Documentation/pretty-formats.txt
+++ b/Documentation/pretty-formats.txt
@@ -106,12 +106,14 @@ The placeholders are:
 - '%aD': author date, RFC2822 style
 - '%ar': author date, relative
 - '%at': author date, UNIX timestamp
+- '%ai': author date, ISO 8601 format
 - '%cn': committer name
 - '%ce': committer email
 - '%cd': committer date
 - '%cD': committer date, RFC2822 style
 - '%cr': committer date, relative
 - '%ct': committer date, UNIX timestamp
+- '%ci': committer date, ISO 8601 format
 - '%e': encoding
 - '%s': subject
 - '%b': body
diff --git a/cache.h b/cache.h
index 5dff2f1..3dc0def 100644
--- a/cache.h
+++ b/cache.h
@@ -389,7 +389,7 @@ extern void *read_object_with_reference(const unsigned char *sha1,
 					unsigned long *size,
 					unsigned char *sha1_ret);
 
-enum date_mode { DATE_NORMAL = 0, DATE_RELATIVE, DATE_SHORT, DATE_LOCAL };
+enum date_mode { DATE_NORMAL = 0, DATE_RELATIVE, DATE_SHORT, DATE_LOCAL, DATE_ISO8601 };
 const char *show_date(unsigned long time, int timezone, enum date_mode mode);
 const char *show_rfc2822_date(unsigned long time, int timezone);
 int parse_date(const char *date, char *buf, int bufsize);
diff --git a/commit.c b/commit.c
index 5632e32..68df3b4 100644
--- a/commit.c
+++ b/commit.c
@@ -773,6 +773,7 @@ static void fill_person(struct interp *table, const char *msg, int len)
 	interp_set_entry(table, 2, show_date(date, tz, 0));
 	interp_set_entry(table, 3, show_rfc2822_date(date, tz));
 	interp_set_entry(table, 4, show_date(date, tz, 1));
+	interp_set_entry(table, 6, show_date(date, tz, DATE_ISO8601));
 }
 
 static long format_commit_message(const struct commit *commit,
@@ -791,12 +792,14 @@ static long format_commit_message(const struct commit *commit,
 		{ "%aD" },	/* author date, RFC2822 style */
 		{ "%ar" },	/* author date, relative */
 		{ "%at" },	/* author date, UNIX timestamp */
+		{ "%ai" },	/* author date, ISO 8601 */
 		{ "%cn" },	/* committer name */
 		{ "%ce" },	/* committer email */
 		{ "%cd" },	/* committer date */
 		{ "%cD" },	/* committer date, RFC2822 style */
 		{ "%cr" },	/* committer date, relative */
 		{ "%ct" },	/* committer date, UNIX timestamp */
+		{ "%ci" },	/* committer date, ISO 8601 */
 		{ "%e" },	/* encoding */
 		{ "%s" },	/* subject */
 		{ "%b" },	/* body */
@@ -813,10 +816,11 @@ static long format_commit_message(const struct commit *commit,
 		IPARENTS, IPARENTS_ABBREV,
 		IAUTHOR_NAME, IAUTHOR_EMAIL,
 		IAUTHOR_DATE, IAUTHOR_DATE_RFC2822, IAUTHOR_DATE_RELATIVE,
-		IAUTHOR_TIMESTAMP,
+		IAUTHOR_TIMESTAMP, IAUTHOR_ISO8601,
 		ICOMMITTER_NAME, ICOMMITTER_EMAIL,
 		ICOMMITTER_DATE, ICOMMITTER_DATE_RFC2822,
 		ICOMMITTER_DATE_RELATIVE, ICOMMITTER_TIMESTAMP,
+		ICOMMITTER_ISO8601,
 		IENCODING,
 		ISUBJECT,
 		IBODY,
diff --git a/date.c b/date.c
index 4690371..c96100e 100644
--- a/date.c
+++ b/date.c
@@ -137,7 +137,15 @@ const char *show_date(unsigned long time, int tz, enum date_mode mode)
 	if (mode == DATE_SHORT)
 		sprintf(timebuf, "%04d-%02d-%02d", tm->tm_year + 1900,
 				tm->tm_mon + 1, tm->tm_mday);
+	else if (mode == DATE_ISO8601)
+		sprintf(timebuf, "%04d-%02d-%02d %02d:%02d:%02d%+03d:%02d",
+				tm->tm_year + 1900,
+				tm->tm_mon + 1,
+				tm->tm_mday,
+				tm->tm_hour, tm->tm_min, tm->tm_sec,
+				tz/100,
+				abs(tz%100)
+				);
 	else
 		sprintf(timebuf, "%.3s %.3s %d %02d:%02d:%02d %d%c%+05d",
 				weekday_names[tm->tm_wday],
-- 
1.5.2.3

^ permalink raw reply related

* [PATCH 0/6] Introduce commit notes
From: Johannes Schindelin @ 2007-07-15 23:19 UTC (permalink / raw)
  To: Alberto Bertogli, git, gitster, Johan Herland

Hi,

This patch series replaces the series I sent out a while ago, which added 
"commit annotations".  Since "commit notes" was liked much better, here 
they are.

It picks up the same idea, having a pseudo-branch whose revisions contain 
a .git/objects/??/* like file structure, and whose blobs are the commit 
notes.

By default, that pseudo-branch is "refs/notes/commits", but it is 
overridable by the config variable core.notesRef, which in turn can be 
overridden by the environment variable GIT_NOTES_REF.  If the given ref 
does not exist yet, it is interpreted as empty.

The biggest obstacle was a thinko about the scalability.  Tree objects 
take free form name entries, and therefore a binary search by name is not 
possible.

Patch 6/6 is only a WIP patch, but it shows the road ahead.  It adds code 
to generate .git/notes-index from refs/notes/commits (or any other ref you 
specify as notes ref), which is reused until refs/notes/commits^{tree} 
changes.  Patch 6/6 is only meant to assess which data structure yields 
best performance, and how big the costs are.

However, as long as there are no public, fetchable commit notes, I think 
the first 5 patches are safe for application and testing.

Ciao,
Dscho

Johannes Schindelin (6):
      Rename git_one_line() to git_line_length() and export it
      Introduce commit notes
      Add git-notes
      Add a test script for "git notes"
      Document git-notes
      notes: add notes-index for a substantial speedup.

 .gitignore                       |    1 +
 Documentation/cmd-list.perl      |    1 +
 Documentation/config.txt         |   15 ++
 Documentation/git-notes.txt      |   45 ++++
 Makefile                         |    5 +-
 cache.h                          |    1 +
 commit.c                         |   15 +-
 commit.h                         |    1 +
 config.c                         |    5 +
 environment.c                    |    1 +
 git-notes.sh                     |   61 ++++++
 notes.c                          |  416 ++++++++++++++++++++++++++++++++++++++
 notes.h                          |    9 +
 t/t3301-notes.sh                 |   63 ++++++
 t/t3302-notes-index-expensive.sh |  118 +++++++++++
 15 files changed, 750 insertions(+), 7 deletions(-)
 create mode 100644 Documentation/git-notes.txt
 create mode 100755 git-notes.sh
 create mode 100644 notes.c
 create mode 100644 notes.h
 create mode 100755 t/t3301-notes.sh
 create mode 100755 t/t3302-notes-index-expensive.sh

^ permalink raw reply

* Re: Questions about git-fast-import for cvs2svn
From: Robin H. Johnson @ 2007-07-15 23:21 UTC (permalink / raw)
  To: Git Mailing List
In-Reply-To: <200707152356.58304.robin.rosenberg.lists@dewire.com>

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

On Sun, Jul 15, 2007 at 11:56:56PM +0200, Robin Rosenberg wrote:
> s?ndag 15 juli 2007 skrev Michael Haggerty:
> >    b. CVS tag/branch creation events do not even include a username.
> > Any suggestions for what to use here?
> The CVSROOT/history file contains the user name and timestamp of the tag
> creation. CVS can be told not to update the file. It is appended to by default.
This assumes that said file is intact, and that LogFormat was not
changed to not put entries into the file on tag/branch.

-- 
Robin Hugh Johnson
Gentoo Linux Developer & Council Member
E-Mail     : robbat2@gentoo.org
GnuPG FP   : 11AC BA4F 4778 E3F6 E4ED  F38E B27B 944E 3488 4E85

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

^ permalink raw reply

* [PATCH 1/6] Rename git_one_line() to git_line_length() and export it
From: Johannes Schindelin @ 2007-07-15 23:22 UTC (permalink / raw)
  To: Alberto Bertogli, git, gitster, Johan Herland
In-Reply-To: <Pine.LNX.4.64.0707152326080.14781@racer.site>


The function get_one_line() really returns the line length, not the
whole line, but it is really useful, so do not hide it in commit.c,
under the wrong name.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 commit.c |   10 +++++-----
 commit.h |    1 +
 2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/commit.c b/commit.c
index 4c5dfa9..0c350bc 100644
--- a/commit.c
+++ b/commit.c
@@ -458,7 +458,7 @@ void clear_commit_marks(struct commit *commit, unsigned int mark)
 /*
  * Generic support for pretty-printing the header
  */
-static int get_one_line(const char *msg, unsigned long len)
+int get_line_length(const char *msg, unsigned long len)
 {
 	int ret = 0;
 
@@ -950,7 +950,7 @@ static void pp_header(enum cmit_fmt fmt,
 	for (;;) {
 		const char *line = *msg_p;
 		char *dst;
-		int linelen = get_one_line(*msg_p, *len_p);
+		int linelen = get_line_length(*msg_p, *len_p);
 		unsigned long len;
 
 		if (!linelen)
@@ -1041,7 +1041,7 @@ static void pp_title_line(enum cmit_fmt fmt,
 	title = xmalloc(title_alloc);
 	for (;;) {
 		const char *line = *msg_p;
-		int linelen = get_one_line(line, *len_p);
+		int linelen = get_line_length(line, *len_p);
 		*msg_p += linelen;
 		*len_p -= linelen;
 
@@ -1118,7 +1118,7 @@ static void pp_remainder(enum cmit_fmt fmt,
 	int first = 1;
 	for (;;) {
 		const char *line = *msg_p;
-		int linelen = get_one_line(line, *len_p);
+		int linelen = get_line_length(line, *len_p);
 		*msg_p += linelen;
 		*len_p -= linelen;
 
@@ -1214,7 +1214,7 @@ unsigned long pretty_print_commit(enum cmit_fmt fmt,
 
 	/* Skip excess blank lines at the beginning of body, if any... */
 	for (;;) {
-		int linelen = get_one_line(msg, len);
+		int linelen = get_line_length(msg, len);
 		int ll = linelen;
 		if (!linelen)
 			break;
diff --git a/commit.h b/commit.h
index 467872e..fc6df23 100644
--- a/commit.h
+++ b/commit.h
@@ -60,6 +60,7 @@ enum cmit_fmt {
 	CMIT_FMT_UNSPECIFIED,
 };
 
+extern int get_line_length(const char *msg, unsigned long len);
 extern enum cmit_fmt get_commit_format(const char *arg);
 extern unsigned long pretty_print_commit(enum cmit_fmt fmt, const struct commit *, unsigned long len, char **buf_p, unsigned long *space_p, int abbrev, const char *subject, const char *after_subject, enum date_mode dmode);
 
-- 
1.5.3.rc1.2718.gd2dc9-dirty

^ permalink raw reply related

* [PATCH 2/6] Introduce commit notes
From: Johannes Schindelin @ 2007-07-15 23:23 UTC (permalink / raw)
  To: Alberto Bertogli, git, gitster, Johan Herland
In-Reply-To: <Pine.LNX.4.64.0707152326080.14781@racer.site>


Commit notes are blobs which are shown together with the commit
message.  These blobs are taken from the notes ref, which you can
configure by the config variable core.notesRef, which in turn can
be overridden by the environment variable GIT_NOTES_REF.

The notes ref is a branch which contains trees much like the
loose object trees in .git/objects/.  In other words, to get
at the commit notes for a given SHA-1, take the first two
hex characters as directory name, and the remaining 38 hex
characters as base name, and look that up in the notes ref.

The rationale for putting this information into a ref is this: we
want to be able to fetch and possibly union-merge the notes,
maybe even look at the date when a note was introduced, and we
want to store them efficiently together with the other objects.

There is one severe shortcoming, though.  Since tree objects can
contain file names of a variable length, it is not possible to
do a binary search for the correct base name in the tree object's
contents.  Therefore this approach does not scale well, because
the average lookup time will be proportional to the number of
commit objects, and therefore the slowdown will be quadratic in
that number.

However, a remedy is near: in a later commit, a .git/notes-index
will be introduced, a cached mapping from commits to commit notes,
to be written when the tree name of the notes ref changes.  In
case that notes-index cannot be written, the current (possibly
slow) code will come into effect again.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 Documentation/config.txt |   15 +++++++++++
 Makefile                 |    3 +-
 cache.h                  |    1 +
 commit.c                 |    5 +++
 config.c                 |    5 +++
 environment.c            |    1 +
 notes.c                  |   64 ++++++++++++++++++++++++++++++++++++++++++++++
 notes.h                  |    9 ++++++
 8 files changed, 102 insertions(+), 1 deletions(-)
 create mode 100644 notes.c
 create mode 100644 notes.h

diff --git a/Documentation/config.txt b/Documentation/config.txt
index d0e9a17..5fe833d 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -285,6 +285,21 @@ core.pager::
 	The command that git will use to paginate output.  Can be overridden
 	with the `GIT_PAGER` environment variable.
 
+core.notesRef::
+	When showing commit messages, also show notes which are stored in
+	the given ref.  This ref is expected to contain paths of the form
+	??/*, where the directory name consists of the first two
+	characters of the commit name, and the base name consists of
+	the remaining 38 characters.
++
+If such a path exists in the given ref, the referenced blob is read, and
+appended to the commit message, separated by a "Notes:" line.  If the
+given ref itself does not exist, it is not an error, but means that no
+notes should be print.
++
+This setting defaults to "refs/notes/commits", and can be overridden by
+the `GIT_NOTES_REF` environment variable.
+
 alias.*::
 	Command aliases for the gitlink:git[1] command wrapper - e.g.
 	after defining "alias.last = cat-file commit HEAD", the invocation
diff --git a/Makefile b/Makefile
index d7541b4..119d949 100644
--- a/Makefile
+++ b/Makefile
@@ -322,7 +322,8 @@ LIB_OBJS = \
 	write_or_die.o trace.o list-objects.o grep.o match-trees.o \
 	alloc.o merge-file.o path-list.o help.o unpack-trees.o $(DIFF_OBJS) \
 	color.o wt-status.o archive-zip.o archive-tar.o shallow.o utf8.o \
-	convert.o attr.o decorate.o progress.o mailmap.o symlinks.o remote.o
+	convert.o attr.o decorate.o progress.o mailmap.o symlinks.o remote.o \
+	notes.o
 
 BUILTIN_OBJS = \
 	builtin-add.o \
diff --git a/cache.h b/cache.h
index 328c1ad..c89cac5 100644
--- a/cache.h
+++ b/cache.h
@@ -309,6 +309,7 @@ extern size_t packed_git_window_size;
 extern size_t packed_git_limit;
 extern size_t delta_base_cache_limit;
 extern int auto_crlf;
+extern char *notes_ref_name;
 
 #define GIT_REPO_VERSION 0
 extern int repository_format_version;
diff --git a/commit.c b/commit.c
index 0c350bc..3529b6a 100644
--- a/commit.c
+++ b/commit.c
@@ -6,6 +6,7 @@
 #include "interpolate.h"
 #include "diff.h"
 #include "revision.h"
+#include "notes.h"
 
 int save_commit_buffer = 1;
 
@@ -1254,6 +1255,10 @@ unsigned long pretty_print_commit(enum cmit_fmt fmt,
 	 */
 	if (fmt == CMIT_FMT_EMAIL && offset <= beginning_of_body)
 		buf[offset++] = '\n';
+
+	if (fmt != CMIT_FMT_ONELINE)
+		get_commit_notes(commit, buf_p, &offset, space_p);
+
 	buf[offset] = '\0';
 	free(reencoded);
 	return offset;
diff --git a/config.c b/config.c
index f89a611..05d2ad6 100644
--- a/config.c
+++ b/config.c
@@ -395,6 +395,11 @@ int git_default_config(const char *var, const char *value)
 		return 0;
 	}
 
+	if (!strcmp(var, "core.notesref")) {
+		notes_ref_name = xstrdup(value);
+		return 0;
+	}
+
 	if (!strcmp(var, "user.name")) {
 		strlcpy(git_default_name, value, sizeof(git_default_name));
 		return 0;
diff --git a/environment.c b/environment.c
index f83fb9e..2e677d3 100644
--- a/environment.c
+++ b/environment.c
@@ -34,6 +34,7 @@ char *pager_program;
 int pager_in_use;
 int pager_use_color = 1;
 int auto_crlf = 0;	/* 1: both ways, -1: only when adding git objects */
+char *notes_ref_name;
 
 static const char *git_dir;
 static char *git_object_dir, *git_index_file, *git_refs_dir, *git_graft_file;
diff --git a/notes.c b/notes.c
new file mode 100644
index 0000000..5d1bb1a
--- /dev/null
+++ b/notes.c
@@ -0,0 +1,64 @@
+#include "cache.h"
+#include "commit.h"
+#include "notes.h"
+#include "refs.h"
+
+static int initialized;
+
+void get_commit_notes(const struct commit *commit,
+		char **buf_p, unsigned long *offset_p, unsigned long *space_p)
+{
+	char name[80];
+	const char *hex;
+	unsigned char sha1[20];
+	char *msg;
+	unsigned long msgoffset, msglen;
+	enum object_type type;
+
+	if (!initialized) {
+		const char *env = getenv(GIT_NOTES_REF);
+		if (env) {
+			if (notes_ref_name)
+				free(notes_ref_name);
+			notes_ref_name = xstrdup(getenv(GIT_NOTES_REF));
+		} else if (!notes_ref_name)
+			notes_ref_name = xstrdup("refs/notes/commits");
+		if (notes_ref_name && read_ref(notes_ref_name, sha1)) {
+			free(notes_ref_name);
+			notes_ref_name = NULL;
+		}
+		initialized = 1;
+	}
+	if (!notes_ref_name)
+		return;
+
+	hex = sha1_to_hex(commit->object.sha1);
+	snprintf(name, sizeof(name), "%s:%.*s/%.*s",
+			notes_ref_name, 2, hex, 38, hex + 2);
+	if (get_sha1(name, sha1))
+		return;
+
+	if (!(msg = read_sha1_file(sha1, &type, &msglen)) || !msglen)
+		return;
+	/* we will end the annotation by a newline anyway. */
+	if (msg[msglen - 1] == '\n')
+		msglen--;
+
+	ALLOC_GROW(*buf_p, *offset_p + 14 + msglen, *space_p);
+	*offset_p += sprintf(*buf_p + *offset_p, "\nNotes:\n");
+
+	for (msgoffset = 0; msgoffset < msglen;) {
+		int linelen = get_line_length(msg + msgoffset, msglen);
+
+		ALLOC_GROW(*buf_p, *offset_p + linelen + 6, *space_p);
+		*offset_p += sprintf(*buf_p + *offset_p,
+				"    %.*s", linelen, msg + msgoffset);
+		msgoffset += linelen;
+	}
+	ALLOC_GROW(*buf_p, *offset_p + 1, *space_p);
+	(*buf_p)[*offset_p] = '\n';
+	(*offset_p)++;
+	free(msg);
+}
+
+
diff --git a/notes.h b/notes.h
new file mode 100644
index 0000000..aed80e7
--- /dev/null
+++ b/notes.h
@@ -0,0 +1,9 @@
+#ifndef NOTES_H
+#define NOTES_H
+
+void get_commit_notes(const struct commit *commit,
+	char **buf_p, unsigned long *offset_p, unsigned long *space_p);
+
+#define GIT_NOTES_REF "GIT_NOTES_REF"
+
+#endif
-- 
1.5.3.rc1.2718.gd2dc9-dirty

^ permalink raw reply related

* [PATCH 3/6] Add git-notes
From: Johannes Schindelin @ 2007-07-15 23:23 UTC (permalink / raw)
  To: Alberto Bertogli, git, gitster, Johan Herland
In-Reply-To: <Pine.LNX.4.64.0707152326080.14781@racer.site>


This script allows you to edit and show commit notes easily.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 .gitignore   |    1 +
 Makefile     |    2 +-
 git-notes.sh |   61 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 63 insertions(+), 1 deletions(-)
 create mode 100755 git-notes.sh

diff --git a/.gitignore b/.gitignore
index 20ee642..125613f 100644
--- a/.gitignore
+++ b/.gitignore
@@ -83,6 +83,7 @@ git-mktag
 git-mktree
 git-name-rev
 git-mv
+git-notes
 git-pack-redundant
 git-pack-objects
 git-pack-refs
diff --git a/Makefile b/Makefile
index 119d949..10a9342 100644
--- a/Makefile
+++ b/Makefile
@@ -213,7 +213,7 @@ SCRIPT_SH = \
 	git-merge-resolve.sh git-merge-ours.sh \
 	git-lost-found.sh git-quiltimport.sh git-submodule.sh \
 	git-filter-branch.sh \
-	git-stash.sh
+	git-stash.sh git-notes.sh
 
 SCRIPT_PERL = \
 	git-add--interactive.perl \
diff --git a/git-notes.sh b/git-notes.sh
new file mode 100755
index 0000000..e0ad0b9
--- /dev/null
+++ b/git-notes.sh
@@ -0,0 +1,61 @@
+#!/bin/sh
+
+USAGE="(edit | show) [commit]"
+. git-sh-setup
+
+test -n "$3" && usage
+
+test -z "$GIT_NOTES_REF" && GIT_NOTES_REF="$(git config core.notesref)"
+test -z "$GIT_NOTES_REF" &&
+	die "No notes ref set."
+
+COMMIT=$(git rev-parse --verify --default HEAD "$2")
+NAME=$(echo $COMMIT | sed "s/^../&\//")
+
+case "$1" in
+edit)
+	MESSAGE="$GIT_DIR"/new-notes
+	GIT_NOTES_REF= git log -1 $COMMIT | sed "s/^/#/" > "$MESSAGE"
+
+	GIT_INDEX_FILE="$MESSAGE".idx
+	export GIT_INDEX_FILE
+
+	CURRENT_HEAD=$(git show-ref $GIT_NOTES_REF | cut -f 1 -d ' ')
+	if [ -z "$CURRENT_HEAD" ]; then
+		PARENT=
+	else
+		PARENT="-p $OLDTIP"
+		git read-tree $GIT_NOTES_REF || die "Could not read index"
+		git cat-file blob :$NAME >> "$MESSAGE" 2> /dev/null
+	fi
+
+	${VISUAL:-${EDITOR:-vi}} "$MESSAGE"
+
+	grep -v ^# < "$MESSAGE" | git stripspace > "$MESSAGE".processed
+	mv "$MESSAGE".processed "$MESSAGE"
+	if [ -z "$(cat "$MESSAGE")" ]; then
+		test -z "$CURRENT_HEAD" &&
+			die "Will not initialise with empty tree"
+		git update-index --force-remove $NAME ||
+			die "Could not update index"
+	else
+		BLOB=$(git hash-object -w "$MESSAGE") ||
+			die "Could not write into object database"
+		git update-index --add --cacheinfo 0644 $BLOB $NAME ||
+			die "Could not write index"
+	fi
+
+	TREE=$(git write-tree) || die "Could not write tree"
+	NEW_HEAD=$(: | git commit-tree $TREE $PARENT) ||
+		die "Could not annotate"
+	case "$CURRENT_HEAD" in
+	'') git update-ref $GIT_NOTES_REF $NEW_HEAD ;;
+	*) git update-ref $GIT_NOTES_REF $NEW_HEAD $CURRENT_HEAD;;
+	esac
+;;
+show)
+	git show "$GIT_NOTES_REF":$NAME
+;;
+*)
+	usage
+esac
-- 
1.5.3.rc1.2718.gd2dc9-dirty

^ permalink raw reply related

* [PATCH 4/6] Add a test script for "git notes"
From: Johannes Schindelin @ 2007-07-15 23:24 UTC (permalink / raw)
  To: Alberto Bertogli, git, gitster, Johan Herland
In-Reply-To: <Pine.LNX.4.64.0707152326080.14781@racer.site>


Incidentally, a test for "git notes" implies a test for the
whole commit notes machinery.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 t/t3301-notes.sh |   63 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 63 insertions(+), 0 deletions(-)
 create mode 100755 t/t3301-notes.sh

diff --git a/t/t3301-notes.sh b/t/t3301-notes.sh
new file mode 100755
index 0000000..eb50191
--- /dev/null
+++ b/t/t3301-notes.sh
@@ -0,0 +1,63 @@
+#!/bin/sh
+#
+# Copyright (c) 2007 Johannes E. Schindelin
+#
+
+test_description='Test commit notes'
+
+. ./test-lib.sh
+
+test_expect_success setup '
+	: > a1 &&
+	git add a1 &&
+	test_tick &&
+	git commit -m 1st &&
+	: > a2 &&
+	git add a2 &&
+	test_tick &&
+	git commit -m 2nd
+'
+
+cat > fake_editor.sh << EOF
+echo "\$MSG" > "\$1"
+echo "\$MSG" >& 2
+EOF
+chmod a+x fake_editor.sh
+VISUAL="$(pwd)"/fake_editor.sh
+export VISUAL
+
+
+test_expect_success 'need notes ref' '
+	! MSG=1 git notes edit &&
+	! MSG=2 git notes show
+'
+
+test_expect_success 'create notes' '
+	git config core.notesRef refs/notes/commits &&
+	MSG=b1 git notes edit &&
+cat .git/new-notes &&
+test b1 = "$(cat .git/new-notes)" &&
+	test 1 = $(git ls-tree refs/notes/commits | wc -l) &&
+	test b1 = $(git notes show) &&
+	git show HEAD^ &&
+	! git notes show HEAD^
+'
+
+cat > expect << EOF
+commit 268048bfb8a1fb38e703baceb8ab235421bf80c5
+Author: A U Thor <author@example.com>
+Date:   Thu Apr 7 15:14:13 2005 -0700
+
+    2nd
+
+Notes:
+    b1
+EOF
+
+test_expect_success 'show notes' '
+	! (git cat-file commit HEAD | grep b1) &&
+	git log -1 > output &&
+	git diff expect output
+'
+
+test_done
-- 
1.5.3.rc1.2718.gd2dc9-dirty

^ permalink raw reply related

* [PATCH 5/6] Document git-notes
From: Johannes Schindelin @ 2007-07-15 23:24 UTC (permalink / raw)
  To: Alberto Bertogli, git, gitster, Johan Herland
In-Reply-To: <Pine.LNX.4.64.0707152326080.14781@racer.site>


Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 Documentation/cmd-list.perl |    1 +
 Documentation/git-notes.txt |   45 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 46 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/git-notes.txt

diff --git a/Documentation/cmd-list.perl b/Documentation/cmd-list.perl
index 2143995..f05e291 100755
--- a/Documentation/cmd-list.perl
+++ b/Documentation/cmd-list.perl
@@ -140,6 +140,7 @@ git-mergetool                           ancillarymanipulators
 git-mktag                               plumbingmanipulators
 git-mktree                              plumbingmanipulators
 git-mv                                  mainporcelain
+git-notes                               mainporcelain
 git-name-rev                            plumbinginterrogators
 git-pack-objects                        plumbingmanipulators
 git-pack-redundant                      plumbinginterrogators
diff --git a/Documentation/git-notes.txt b/Documentation/git-notes.txt
new file mode 100644
index 0000000..331ed89
--- /dev/null
+++ b/Documentation/git-notes.txt
@@ -0,0 +1,45 @@
+git-notes(1)
+============
+
+NAME
+----
+git-notes - Add commit notes
+
+SYNOPSIS
+--------
+[verse]
+'git-notes' (edit | show) [commit
+
+DESCRIPTION
+-----------
+This command allows you to add notes to commit messages, after the
+fact.  To discern these notes from the message stored in the commit
+object, the notes are indented like the message, after an unindented
+line saying "Notes:".
+
+To enable commit notes, you have to set the config variable
+core.notesRef to something like "refs/notes/commits".  This setting
+can be overridden by the environment variable "GIT_NOTES_REF".
+
+
+SUBCOMMANDS
+-----------
+
+edit::
+	Edit the notes for a given commit (defaults to HEAD).
+
+show::
+	Show the notes for a given commit (defaults to HEAD).
+
+
+Author
+------
+Written by Johannes Schindelin <johannes.schindelin@gmx.de>
+
+Documentation
+-------------
+Documentation by Johannes Schindelin
+
+GIT
+---
+Part of the gitlink:git[7] suite
-- 
1.5.3.rc1.2718.gd2dc9-dirty

^ permalink raw reply related

* [WIP PATCH 6/6] notes: add notes-index for a substantial speedup.
From: Johannes Schindelin @ 2007-07-15 23:26 UTC (permalink / raw)
  To: Alberto Bertogli, git, gitster, Johan Herland
In-Reply-To: <Pine.LNX.4.64.0707152326080.14781@racer.site>


Actually, this commit adds two methods for a notes index:

- a sorted list with a fan out to help binary search, and
- a modified hash table.

It also adds a test which is used to determine the best algorithm.
---
	Not signed off because this is not suitable to be applied as-is.
	It is only meant to test the different approaches.

 notes.c                          |  392 ++++++++++++++++++++++++++++++++++++--
 t/t3302-notes-index-expensive.sh |  118 ++++++++++++
 2 files changed, 490 insertions(+), 20 deletions(-)
 create mode 100755 t/t3302-notes-index-expensive.sh

diff --git a/notes.c b/notes.c
index 5d1bb1a..5a90abf 100644
--- a/notes.c
+++ b/notes.c
@@ -1,10 +1,370 @@
 #include "cache.h"
 #include "commit.h"
+#include "tree-walk.h"
 #include "notes.h"
 #include "refs.h"
 
 static int initialized;
 
+/*
+ * There are two choices of data structure for the notes index.
+ *
+ * A) Fan out enhanced sorted list.
+ *
+ * This is a regular sorted list with a 256 entry fan out.  In other words,
+ * every time an entry is looked up, a binary search is performed over the
+ * sublist defined by the first byte of the SHA-1.
+ *
+ * The disadvantage is an average runtime logarithmic in the number of
+ * commit notes.  The advantages are a compact representation on disk,
+ * and a _guaranteed_ logarithmic runtime.
+ *
+ * You could even squeeze out one more byte per entry, since the
+ * first byte is known from the fan out list.  This would complicate our
+ * algorithm, though.
+ *
+ * B) Hash
+ *
+ * This is not your classical hash.  It is _mostly_ like a hash, with
+ * a few notable exceptions:
+ *
+ * - it is possibly larger than size suggests: since it is file based,
+ *   it is easier to write at the end than to wrap around.
+ *
+ * - as a consequence we can make the entries _strictly_ sorted. This
+ *   is not only nice to look at, but makes incremental updates much,
+ *   much easier.
+ *
+ * The disadvantages of a hash is its loose packing.  In order to operate
+ * reasonably well, it needs a size roughly double the number of entries.
+ * It also has a worst runtime linear in the number of entries.
+ *
+ * The advantage is an expected constant lookup time.
+ *
+ * The performance of a hash map depends highly on a good hashing
+ * algorithm, to avoid collisions.  Lucky us!  SHA-1 is a pretty good
+ * hashing algorithm.
+ *
+ * There is another advantage to hash maps: with not much effort, the
+ * incremental update can be performed in place, relying on O_TRUNC to
+ * detect interruptions.  This operation has an expected constant runtime.
+ */
+
+struct notes_entry {
+	unsigned char commit_sha1[20];
+	unsigned char notes_sha1[20];
+};
+
+struct notes_index {
+	char signature[4]; /* FANO for fan our, HASH for hash */
+
+	unsigned char tree_sha1[20];
+	unsigned char subtree_sha1[256][20]; /* for incremental caching */
+	off_t offsets[256]; /* for fan out */
+	off_t count, size; /* for hash */
+} notes_index;
+
+static int notes_index_fd;
+static int (*get_notes)(const unsigned char *commit_sha1,
+		unsigned char *notes_sha1);
+
+#define GIT_NOTES_MODE "GIT_NOTES_MODE"
+static int use_hash;
+
+static int index_uptodate_check(struct tree *tree) {
+	const char *signature = use_hash ? "HASH" : "FANO";
+	int fd = open(git_path("notes-index"), O_RDONLY);
+
+	if (fd < 0)
+		return fd;
+
+	notes_index_fd = fd;
+
+	return read_in_full(fd, &notes_index, sizeof(notes_index)) < 0||
+			memcmp(notes_index.signature, signature, 4) ||
+			memcmp(notes_index.tree_sha1,
+				&tree->object.sha1, 20);
+}
+
+struct lock_file update_lock;
+
+/* this reads the remaining 38 hexchars */
+static int get_remaining_hexchars(unsigned char *sha1, const char *path)
+{
+	int i, j1, j2;
+	for (i = 0; i < 38; i += 2)
+		if ((j1 = hexval(path[i])) < 0 ||
+				(j2 = hexval(path[i + 1])) < 0)
+			return -1;
+		else
+			sha1[1 + i / 2] = (j1 << 4) | j2;
+	return path[38] != '\0';
+}
+
+static int get_notes_hash_count(struct tree *tree) {
+	struct tree_desc desc, desc2;
+	struct name_entry entry;
+	void *buf;
+	unsigned long count = 0;
+
+	buf = fill_tree_descriptor(&desc, notes_index.tree_sha1);
+	if (!buf)
+		return 0;
+	while (tree_entry(&desc, &entry)) {
+		void *buf2 = fill_tree_descriptor(&desc2, entry.sha1);
+		if (!buf2)
+			continue;
+		while (tree_entry(&desc2, &entry))
+			count++;
+		free(buf2);
+	}
+	free(buf);
+
+	return count;
+}
+
+static unsigned long get_hash_index(const unsigned char *sha1)
+{
+	return (ntohl(*(unsigned long *)sha1) % notes_index.size);
+}
+
+static int write_hash_gap(int fd, unsigned char *sha1)
+{
+	off_t min_offset = sizeof(notes_index) +
+		get_hash_index(sha1) * sizeof(struct notes_entry);
+	while (min_offset > lseek(fd, 0, SEEK_CUR))
+		if (write_in_full(fd, null_sha1, 20) < 0 ||
+				write_in_full(fd, null_sha1, 20) < 0)
+			return error("Could not write gaps in notes-index");
+	return 0;
+}
+
+static int update_index(struct tree *tree) {
+	/*
+	 * Fan out sorted list:
+	 *
+	 * Write out the header, and seek back to it, in order to update it.
+	 * Actually only seek at the end, and make sure that you write
+	 * something big-endian.
+	 *
+	 * Plan for incremental: if subtree_sha1 is equal, copy out.
+	 * Otherwise construct, and remember in the copy of the header.
+	 *
+	 * Hash:
+	 *
+	 * Always use a power of two as size.  Not the next higher one, but
+	 * the next next higher one.
+	 *
+	 * Read the tree recursively, and leave as many zeros as needed
+	 * until the next entry comes.  Or if the entry has a hash larger
+	 * than the last free entry, write it at once.
+	 */
+
+	/* Plan for incremental: (not in-place)
+	 * Look at tree differences.  Write null_sha1 until next, or next
+	 * subtree.  Continue writing until original entry is null_sha1 or
+	 * greater than current subtree entry.
+	 */
+
+	int new_fd = hold_lock_file_for_update(&update_lock,
+			git_path("notes-index"), 0);
+	struct tree_desc desc;
+	struct name_entry entry;
+	void *buf;
+	int i;
+
+	if (new_fd < 0)
+		return error("Could not construct notes-index");
+
+	memset(&notes_index, 0, sizeof(notes_index));
+	hashcpy(notes_index.tree_sha1, tree->object.sha1);
+	notes_index.offsets[0] = sizeof(notes_index);
+	if (use_hash) {
+		notes_index.count = get_notes_hash_count(tree);
+		for (notes_index.size = 1; notes_index.size / 2
+				>= notes_index.count; notes_index.size <<= 1)
+			; /* do nothing */
+		memcpy(notes_index.signature, "HASH", 4);
+	} else
+		memcpy(notes_index.signature, "FANO", 4);
+
+	if (write_in_full(new_fd, &notes_index, sizeof(notes_index)) < 0)
+		return error("Could not write notes-index");
+
+	buf = fill_tree_descriptor(&desc, notes_index.tree_sha1);
+	if (!buf)
+		return error("Could not read %s for notes-index",
+				sha1_to_hex(notes_index.tree_sha1));
+
+	i = 0;
+	while (tree_entry(&desc, &entry)) {
+		int j1, j2;
+		unsigned char sha1[20];
+		struct tree_desc desc2;
+		struct name_entry entry2;
+		void *buf2;
+
+		if (!S_ISDIR(entry.mode) ||
+				(j1 = hexval(entry.path[0])) < 0 ||
+				(j2 = hexval(entry.path[1])) < 0)
+			continue;
+		sha1[0] = j1 * 16 + j2;
+		while (++i < sha1[0])
+			notes_index.offsets[i] = notes_index.offsets[i - 1];
+
+		hashcpy(notes_index.subtree_sha1[i], entry.sha1);
+		buf2 = fill_tree_descriptor(&desc2, entry.sha1);
+		if (!buf2)
+			continue;
+		while(tree_entry(&desc2, &entry2)) {
+			if (get_remaining_hexchars(sha1, entry2.path))
+				continue;
+			if (use_hash && write_hash_gap(new_fd, sha1))
+				return -1;
+			if (write_in_full(new_fd, sha1, 20) < 0 ||
+					write_in_full(new_fd,
+						entry2.sha1, 20) < 0)
+				return error("Could not write notes-index");
+		}
+		free(buf2);
+		notes_index.offsets[i] = lseek(new_fd, 0, SEEK_CUR);
+	}
+	free(buf);
+
+	while (++i < 256)
+		notes_index.offsets[i] = notes_index.offsets[i - 1];
+
+	/* update fan_out */
+	lseek(new_fd, 0, SEEK_SET);
+	write(new_fd, &notes_index, sizeof(notes_index));
+	lseek(new_fd, notes_index.offsets[255], SEEK_SET);
+
+	return close(new_fd) || commit_lock_file(&update_lock) ||
+		(notes_index_fd = open(git_path("notes-index"), O_RDONLY));
+}
+
+static void *notes_mmap;
+
+static void unmap_notes_mmap(void)
+{
+	munmap(notes_mmap, notes_index.offsets[255]);
+}
+
+static int get_notes_fan_out(const unsigned char *commit_sha1,
+		unsigned char *notes_sha1)
+{
+	/*
+	 * Header is assumed to be read.
+	 *
+	 * mmap() the area, and bisect.
+	 */
+	off_t off;
+	size_t size;
+	int i, i2, ret = -1;
+	struct notes_entry *list;
+
+	i = commit_sha1[0];
+	off = i ? notes_index.offsets[i - 1] : sizeof(notes_index);
+	size = notes_index.offsets[i] - off;
+	if (!size)
+		return -1;
+
+	if (!notes_mmap) {
+		notes_mmap = xmmap(NULL, notes_index.offsets[255],
+				PROT_READ, MAP_PRIVATE, notes_index_fd, 0);
+		atexit(unmap_notes_mmap);
+	}
+
+	list = (void *)((char *)notes_mmap + off);
+
+	i = 0;
+	i2 = size / sizeof(*list);
+	while (i + 1 < i2) {
+		int middle = (i + i2) / 2;
+		int cmp = hashcmp(commit_sha1, list[middle].commit_sha1);
+		if (cmp < 0)
+			i2 = middle;
+		else if (cmp > 0)
+			i = middle;
+		else {
+			hashcpy(notes_sha1, list[middle].notes_sha1);
+			i = middle;
+			ret = 0;
+			break;
+		}
+	}
+	if (i == 0 && !hashcmp(commit_sha1, list[i].commit_sha1)) {
+		hashcpy(notes_sha1, list[i].notes_sha1);
+		ret = 0;
+	}
+
+	return ret;
+}
+
+static int get_notes_hash(const unsigned char *commit_sha1,
+		unsigned char *notes_sha1)
+{
+	/*
+	 * Header is assumed to be read. fd is still open.
+	 *
+	 * Seek to hash, read until lower or equal (0000... is lower...)
+	 */
+	int i = get_hash_index(commit_sha1);
+	struct notes_entry entry;
+
+	lseek(notes_index_fd,
+			sizeof(notes_index) + i * sizeof(entry), SEEK_SET);
+	while (!read_in_full(notes_index_fd, &entry, sizeof(entry)) &&
+			!is_null_sha1(entry.commit_sha1)) {
+		int cmp = hashcmp(commit_sha1, entry.commit_sha1);
+		if (!cmp) {
+			hashcpy(notes_sha1, entry.notes_sha1);
+			return 0;
+		} else if (cmp < 0)
+			break;
+	}
+	return -1;
+}
+
+static inline void init_notes_index(void)
+{
+	const char *env;
+	struct commit *notes_ref;
+	unsigned char sha1[20];
+
+	if (initialized)
+		return;
+
+	initialized = 1;
+	env = getenv(GIT_NOTES_REF);
+	if (env) {
+		if (notes_ref_name)
+			free(notes_ref_name);
+		notes_ref_name = xstrdup(env);
+	} else if (!notes_ref_name)
+		notes_ref_name = xstrdup("refs/notes/commits");
+
+	if (!notes_ref_name)
+		return;
+	if (read_ref(notes_ref_name, sha1)) {
+		free(notes_ref_name);
+		notes_ref_name = NULL;
+		return;
+	}
+	env = getenv("GIT_NOTES_MODE");
+	if (env && !strcmp("HASH", env)) {
+		use_hash = 1;
+		get_notes = get_notes_hash;
+	} else if (env && !strcmp("FANO", env))
+		get_notes = get_notes_fan_out;
+	if (get_notes && !get_sha1(notes_ref_name, sha1) &&
+			(notes_ref = (struct commit *)parse_object(sha1)) &&
+			notes_ref->object.type == OBJ_COMMIT)
+		if (index_uptodate_check(notes_ref->tree))
+			if (update_index(notes_ref->tree))
+				get_notes = NULL; /* disable notes-index */
+}
+
 void get_commit_notes(const struct commit *commit,
 		char **buf_p, unsigned long *offset_p, unsigned long *space_p)
 {
@@ -15,28 +375,20 @@ void get_commit_notes(const struct commit *commit,
 	unsigned long msgoffset, msglen;
 	enum object_type type;
 
-	if (!initialized) {
-		const char *env = getenv(GIT_NOTES_REF);
-		if (env) {
-			if (notes_ref_name)
-				free(notes_ref_name);
-			notes_ref_name = xstrdup(getenv(GIT_NOTES_REF));
-		} else if (!notes_ref_name)
-			notes_ref_name = xstrdup("refs/notes/commits");
-		if (notes_ref_name && read_ref(notes_ref_name, sha1)) {
-			free(notes_ref_name);
-			notes_ref_name = NULL;
-		}
-		initialized = 1;
-	}
-	if (!notes_ref_name)
-		return;
+	init_notes_index();
 
-	hex = sha1_to_hex(commit->object.sha1);
-	snprintf(name, sizeof(name), "%s:%.*s/%.*s",
-			notes_ref_name, 2, hex, 38, hex + 2);
-	if (get_sha1(name, sha1))
+	if (!notes_ref_name)
 		return;
+	if (get_notes) {
+		if (get_notes(commit->object.sha1, sha1))
+			return;
+	} else {
+		hex = sha1_to_hex(commit->object.sha1);
+		snprintf(name, sizeof(name), "%s:%.*s/%.*s",
+				notes_ref_name, 2, hex, 38, hex + 2);
+		if (get_sha1(name, sha1))
+			return;
+	}
 
 	if (!(msg = read_sha1_file(sha1, &type, &msglen)) || !msglen)
 		return;
diff --git a/t/t3302-notes-index-expensive.sh b/t/t3302-notes-index-expensive.sh
new file mode 100755
index 0000000..075b8e2
--- /dev/null
+++ b/t/t3302-notes-index-expensive.sh
@@ -0,0 +1,118 @@
+#!/bin/sh
+#
+# Copyright (c) 2007 Johannes E. Schindelin
+#
+
+test_description='Test commit notes index (expensive!)'
+
+. ./test-lib.sh
+
+test -z "$GIT_NOTES_TIMING_TESTS" && {
+	say Skipping timing tests
+	test_done
+	exit
+}
+
+create_repo () {
+	number_of_commits=$1
+	nr=0
+	parent=
+	test -d .git || {
+	git init &&
+	tree=$(git write-tree) &&
+	while [ $nr -lt $number_of_commits ]; do
+		test_tick &&
+		commit=$(echo $nr | git commit-tree $tree $parent) ||
+			return
+		parent="-p $commit"
+		nr=$(($nr+1))
+	done &&
+	git update-ref refs/heads/master $commit &&
+	{
+		export GIT_INDEX_FILE=.git/temp;
+		git rev-list HEAD | cat -n | sed "s/^[ 	][ 	]*/ /g" |
+		while read nr sha1; do
+			blob=$(echo note $nr | git hash-object -w --stdin) &&
+			echo $sha1 | sed "s/^../0644 $blob 0	&\//"
+		done | git update-index --index-info &&
+		tree=$(git write-tree) &&
+		test_tick &&
+		commit=$(echo notes | git commit-tree $tree) &&
+		git update-ref refs/notes/commits $commit
+	} &&
+	git config core.notesRef refs/notes/commits
+	}
+}
+
+test_notes () {
+	count=$1 &&
+	git config core.notesRef refs/notes/commits &&
+	git log | grep "^    " > output &&
+	i=1 &&
+	while [ $i -le $count ]; do
+		echo "    $(($count-$i))" &&
+		echo "    note $i" &&
+		i=$(($i+1));
+	done > expect &&
+	git diff expect output
+}
+
+cat > time_notes << EOF
+	mode=\$1
+	i=1
+	while [ \$i -lt \$2 ]; do
+		case \$1 in
+		no-notes)
+			export GIT_NOTES_REF=non-existing
+		;;
+		no-cash)
+			unset GIT_NOTES_REF
+			export GIT_NOTES_MODE=NONE
+		;;
+		hash-cache-create)
+			unset GIT_NOTES_REF
+			export GIT_NOTES_MODE=HASH
+			rm .git/notes-index
+		;;
+		hash-cache)
+			unset GIT_NOTES_REF
+			export GIT_NOTES_MODE=HASH
+		;;
+		sorted-list-cache-create)
+			unset GIT_NOTES_REF
+			export GIT_NOTES_MODE=FANO
+			rm .git/notes-index
+		;;
+		sorted-list-cache)
+			unset GIT_NOTES_REF
+			export GIT_NOTES_MODE=FANO
+		;;
+		esac
+		git log >/dev/null
+		i=\$((\$i+1))
+	done
+EOF
+
+time_notes () {
+	for mode in no-notes no-cash \
+			hash-cache-create hash-cache \
+			sorted-list-cache-create sorted-list-cache; do
+		echo $mode
+		/usr/bin/time sh ../trash/time_notes $mode $1
+	done
+}
+
+for count in 10 100 1000; do
+
+	test -d ../trash-$count || mkdir ../trash-$count
+	(cd ../trash-$count;
+
+	test_expect_success "setup $count" "create_repo $count"
+
+	test_expect_success 'notes work' "test_notes $count"
+
+	test_expect_success 'notes timing' "time_notes 100"
+	)
+done
+
+test_done
-- 
1.5.3.rc1.2718.gd2dc9-dirty

^ permalink raw reply related

* Re: git-config: replaces ~/.gitconfig symlink with real file
From: Johannes Schindelin @ 2007-07-15 23:30 UTC (permalink / raw)
  To: Bradford Smith; +Cc: git
In-Reply-To: <f158199e0707151427h52da3e38rae3be6e44e27e918@mail.gmail.com>

Hi,

On Sun, 15 Jul 2007, Bradford Smith wrote:

> If I were to fix this, I'd be tempted to use realpath(3) to follow the 
> symlink, but I don't think it's very reliably available cross-platform.  
> Certainly, it isn't used anywhere in the current git code.  Can anyone 
> suggest a more portable fix?

I'd use readlink(2) and test for EINVAL to fall back to the current 
behaviour.

Hth,
Dscho

^ permalink raw reply

* Re: [WIP PATCH 6/6] notes: add notes-index for a substantial speedup.
From: Johannes Schindelin @ 2007-07-15 23:33 UTC (permalink / raw)
  To: Alberto Bertogli, git, gitster, Johan Herland
In-Reply-To: <Pine.LNX.4.64.0707160025480.14781@racer.site>

Hi,

[this explains what Patch 6/6 is all about:]

If GIT_NOTES_TIMING_TESTS is set, t3302 will output some timing data.

It will create three repositories, the first with 10 commits and a
commit note for each, the second with 100, the third with 1000.

For each repository, it times "git log" 100 times in several modes:

- with GIT_NOTES_REF set to a non-existing ref (should be equivalent to
  the timings without this patch series),

- with no .git/notes-index,

- recreating .git/notes-index as a hash map _every_ time,

- creating .git/notes-index as a hash map, and using it the rest of the time,

- recreating .git/notes-index as a sorted list _every_ time, and

- creating .git/notes-index as a sorted list only the first time, and then
  using it to find the notes by binary search.

Here is the output:

* expecting success: create_repo 10
*   ok 1: setup 10

* expecting success: test_notes 10
diff --git a/expect b/output
*   ok 2: notes work

* expecting success: time_notes 100
no-notes
0.13user 0.08system 0:00.22elapsed 98%CPU (0avgtext+0avgdata 0maxresident)k
0inputs+0outputs (0major+45271minor)pagefaults 0swaps
no-cash
0.14user 0.13system 0:00.28elapsed 99%CPU (0avgtext+0avgdata 0maxresident)k
0inputs+0outputs (0major+49421minor)pagefaults 0swaps
hash-cache-create
0.16user 0.24system 0:00.41elapsed 99%CPU (0avgtext+0avgdata 0maxresident)k
0inputs+0outputs (0major+73111minor)pagefaults 0swaps
hash-cache
0.10user 0.08system 0:00.18elapsed 101%CPU (0avgtext+0avgdata 0maxresident)k
0inputs+0outputs (0major+45660minor)pagefaults 0swaps
sorted-list-cache-create
0.23user 0.17system 0:00.40elapsed 100%CPU (0avgtext+0avgdata 0maxresident)k
0inputs+0outputs (0major+72056minor)pagefaults 0swaps
sorted-list-cache
0.12user 0.08system 0:00.20elapsed 99%CPU (0avgtext+0avgdata 0maxresident)k
0inputs+0outputs (0major+46854minor)pagefaults 0swaps
*   ok 3: notes timing

* expecting success: create_repo 100
*   ok 1: setup 100

* expecting success: test_notes 100
diff --git a/expect b/output
*   ok 2: notes work

* expecting success: time_notes 100
no-notes
0.38user 0.18system 0:00.56elapsed 100%CPU (0avgtext+0avgdata 0maxresident)k
0inputs+0outputs (0major+54386minor)pagefaults 0swaps
no-cash
1.45user 0.66system 0:02.13elapsed 99%CPU (0avgtext+0avgdata 0maxresident)k
0inputs+0outputs (0major+93980minor)pagefaults 0swaps
hash-cache-create
1.56user 0.98system 0:02.56elapsed 99%CPU (0avgtext+0avgdata 0maxresident)k
0inputs+0outputs (0major+132604minor)pagefaults 0swaps
hash-cache
0.38user 0.17system 0:00.56elapsed 99%CPU (0avgtext+0avgdata 0maxresident)k
0inputs+0outputs (0major+54785minor)pagefaults 0swaps
sorted-list-cache-create
1.56user 0.88system 0:02.47elapsed 99%CPU (0avgtext+0avgdata 0maxresident)k
0inputs+0outputs (0major+124215minor)pagefaults 0swaps
sorted-list-cache
0.44user 0.23system 0:00.68elapsed 99%CPU (0avgtext+0avgdata 0maxresident)k
0inputs+0outputs (0major+64952minor)pagefaults 0swaps
*   ok 3: notes timing

* expecting success: create_repo 1000
*   ok 1: setup 1000

* expecting success: test_notes 1000
diff --git a/expect b/output
*   ok 2: notes work

* expecting success: time_notes 100
no-notes
2.95user 1.19system 0:04.18elapsed 99%CPU (0avgtext+0avgdata 0maxresident)k
0inputs+0outputs (0major+144766minor)pagefaults 0swaps
no-cash
23.05user 5.86system 0:33.06elapsed 87%CPU (0avgtext+0avgdata 0maxresident)k
0inputs+0outputs (0major+639774minor)pagefaults 0swaps
hash-cache-create
23.86user 7.21system 0:32.67elapsed 95%CPU (0avgtext+0avgdata 0maxresident)k
0inputs+0outputs (0major+710958minor)pagefaults 0swaps
hash-cache
3.16user 1.18system 0:04.35elapsed 99%CPU (0avgtext+0avgdata 0maxresident)k
0inputs+0outputs (0major+145160minor)pagefaults 0swaps
sorted-list-cache-create
23.22user 7.32system 0:31.66elapsed 96%CPU (0avgtext+0avgdata 0maxresident)k
0inputs+0outputs (0major+686007minor)pagefaults 0swaps
sorted-list-cache
3.74user 1.81system 0:05.77elapsed 96%CPU (0avgtext+0avgdata 0maxresident)k
0inputs+0outputs (0major+241987minor)pagefaults 0swaps
*   ok 3: notes timing

Results:

These timings were taken from a desktop machine with a few background
processes running, so take them with a grain of salt.

As expected, without a .git/notes-index, it scales pretty badly.  Creating
.git/notes-index is slightly worse than that, but it typically happens
much less often than looking at a commit message.  Therefore the work is
worth it, since the lookup _with_ .git/notes-index is in the same ball park
as no notes at all, with the hash map being better than the sorted
list lookup.

Therefore I will go with the hash map approach, when cleaning up patch 6/6.
But not tonight.

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH 2/6] Introduce commit notes
From: Junio C Hamano @ 2007-07-15 23:36 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Alberto Bertogli, git, Johan Herland
In-Reply-To: <Pine.LNX.4.64.0707160022560.14781@racer.site>

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

> The notes ref is a branch which contains trees much like the
> loose object trees in .git/objects/.  In other words, to get
> at the commit notes for a given SHA-1, take the first two
> hex characters as directory name, and the remaining 38 hex
> characters as base name, and look that up in the notes ref.
> ...
> However, a remedy is near: in a later commit, a .git/notes-index
> will be introduced, a cached mapping from commits to commit notes,
> to be written when the tree name of the notes ref changes.  In
> case that notes-index cannot be written, the current (possibly
> slow) code will come into effect again.

I wonder if it is worth using the fan-out tree structure for the
underlying "note" trees, as the notes-index would be the primary
way to access them.

Not that I've looked at the code too deeply with an intention of
possibly including it early.  I was hoping to see fixes to d/f
code in merge-recursive from either you or Alex instead ;-)

^ permalink raw reply

* Re: finding the right remote branch for a commit
From: Johannes Schindelin @ 2007-07-15 23:48 UTC (permalink / raw)
  To: Matthias Lederhofer; +Cc: git, gitster
In-Reply-To: <20070715223341.GA3797@moooo.ath.cx>

Hi,

you left enough hints to convince me that you will not fix the bugs.  
So I will bite the bullet, and find some time this week to fix the issues.

Junio, I'd really appreciated if you considered waiting with 1.5.3 (maybe 
do an -rc2?) before these bugs are squashed.

On Mon, 16 Jul 2007, Matthias Lederhofer wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> > In practice, and I consider these all bugs, it does not work:
> > 
> > - you have to say
> > 
> >   $ git --work-tree=$HOME --bare init
> > 
> >   which is a bit counterintuitive.  After all, it is _not_ a bare 
> >   repository.  The whole purpose of worktree, as far as I understand, is 
> >   to have a _detached_ repository, which would otherwise be called bare.
> 
> Use
> 
>     $ git --work-tree "$HOME" --git-dir . init
> 
> instead.

Why _should_ that be necessary at all?  I _already_ told git that the 
working tree is somewhere else.  It makes _no sense at all_ to treat the 
cwd as anything else than the GIT_DIR, when --work-tree but no --git-dir 
were specified.

> IMHO the --bare flag did not make much sense before the introduction
> of GIT_WORK_TREE and doesn't after, at least not with the meaning it
> has: why should 'git --bare' mean to use the repository from cwd?

To the contrary, it makes tons of sense.  If you want to initialise a bare 
repository, what _more_ natural way than to say "git init --bare"?  And 
what _more_ natural place to pick for GIT_DIR than the cwd, when you did 
not specify --git-dir?

> > [descriptions of bugs, that have been largely ignored]
>
> Up to now you are supposed to be in the working tree all the time when 
> using it.  Therefore I'd call these feature requests rather than bugs :)

Feature requests? WTF? What reason is there for the _requirement_ to 
specify a working tree, when git does not make use of it?  Hmm?

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH 2/6] Introduce commit notes
From: Johannes Schindelin @ 2007-07-15 23:52 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Alberto Bertogli, git, Johan Herland
In-Reply-To: <7vlkdhck8d.fsf@assigned-by-dhcp.cox.net>

Hi,

On Sun, 15 Jul 2007, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > The notes ref is a branch which contains trees much like the
> > loose object trees in .git/objects/.  In other words, to get
> > at the commit notes for a given SHA-1, take the first two
> > hex characters as directory name, and the remaining 38 hex
> > characters as base name, and look that up in the notes ref.
> > ...
> > However, a remedy is near: in a later commit, a .git/notes-index
> > will be introduced, a cached mapping from commits to commit notes,
> > to be written when the tree name of the notes ref changes.  In
> > case that notes-index cannot be written, the current (possibly
> > slow) code will come into effect again.
> 
> I wonder if it is worth using the fan-out tree structure for the
> underlying "note" trees, as the notes-index would be the primary
> way to access them.

The fan-out tree is a nice fallback solution when you cannot write the 
notes-index.

> Not that I've looked at the code too deeply with an intention of 
> possibly including it early.  I was hoping to see fixes to d/f code in 
> merge-recursive from either you or Alex instead ;-)

Well, yeah.  I was kind of trying to cool off from my unpleasant 
unpack_trees() experience.

But I'll look into the issue again this week.  Promise.

Ciao,
Dscho

^ permalink raw reply

* Re: Draft release notes for v1.5.3, as of -rc1
From: Johannes Schindelin @ 2007-07-15 23:53 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: skimo, git
In-Reply-To: <7vvecmj1ju.fsf@assigned-by-dhcp.cox.net>

Hi,

On Sat, 14 Jul 2007, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > It is really unfortunate that rewrite-commits came in so late in the 
> > release cycle, and I am quite sure it should not be in 1.5.3.  There is 
> > just too much new in it, and too many things to flesh out.
> >
> > Junio, if you want to include it in 1.5.3, it should be marked as alpha 
> > code, as it has not seen any time in "next", let alone "master".
> 
> I am not considering rewrite-commits for inclusion right now.

Fair enough.

> I was hoping that filter-branch will stay.  Its interface is something 
> people are already familiar with since the days of its its older 
> incarnation cg-admin-rewritehist, and it would be really really nice 
> that anything that attempts to replace it builds on and extends its 
> external interface.
> 
> Maybe rewrite-commits can be used to delegate the implementation of 
> heavy lifting from filter-branch?  IOW can the latter be just a thin 
> wrapper around the former?

That is a very good idea.

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH] Support output ISO 8601 format dates
From: Linus Torvalds @ 2007-07-15 23:57 UTC (permalink / raw)
  To: Robin Rosenberg; +Cc: Jan Hudec, junkio, git
In-Reply-To: <200707160119.34509.robin.rosenberg@dewire.com>



On Mon, 16 Jul 2007, Robin Rosenberg wrote:
> 
> By "mutual agreement" I mean that I tell you what it looks like and you agree :)

ROTFL.

		Linus

^ permalink raw reply

* Re: [PATCH 2/6] Introduce commit notes
From: Junio C Hamano @ 2007-07-16  0:05 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Alberto Bertogli, git, Johan Herland
In-Reply-To: <7vlkdhck8d.fsf@assigned-by-dhcp.cox.net>

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

> I wonder if it is worth using the fan-out tree structure for the
> underlying "note" trees, as the notes-index would be the primary
> way to access them.

Actually now I think about it I think this was a stupid
suggestion.  Creation of a new note in a reasonably well
populated note tree would be made 256-fold more efficient by
having the fan-out, as write-tree does not have to recompute the
other 255 tree objects thanks to the cache-tree data being
fresh.

^ permalink raw reply

* Re: [PATCH 6/6] Add git-rewrite-commits
From: Johannes Schindelin @ 2007-07-16  0:38 UTC (permalink / raw)
  To: skimo; +Cc: git, Junio C Hamano
In-Reply-To: <20070715144435.GH999MdfPADPa@greensroom.kotnet.org>

Hi,

On Sun, 15 Jul 2007, Sven Verdoolaege wrote:

> On Sat, Jul 14, 2007 at 01:49:59PM +0100, Johannes Schindelin wrote:
> > On Thu, 12 Jul 2007, skimo@liacs.nl wrote:
> > > +struct decoration rewrite_decoration = { "rewritten as" };
> > > [...]
> > > +
> > > +struct rewrite_decoration {
> > > +	struct rewrite_decoration *next;
> > > +	unsigned char sha1[20];
> > > +};
> > 
> > I wonder why you give the instance of a "struct decoration" the same name 
> > as that of a _different_ struct. IOW it is confusing that there is a 
> > "struct rewrite_decoration", but the variable "rewrite_decoration" is no 
> > instance of that struct.
> 
> I was just following Linus's example.

*Sigh*  I hate to repeat arguments.

> > > +static char *add_parents(char *dest, struct commit_list *parents)
> > 
> > Since one commit can be rewritten to multiple commits now, you do not 
> > know how much space you need to add the parents.
> 
> I count them beforehand.

Yes.  And it is extremely hard to follow through your code, just to verify 
that it really works.  Using ALLOC_GROW instead of trying to play cute 
games would spare these tours, and I am quite convinced that it makes for 
less bugs.  If not now, then when somebody _else_ than you touches the 
code.

> > > +		    !get_short_sha1(buf, ll, sha1, 1) &&
> > > +		    !get_rewritten_sha1(sha1))
> > > +			memcpy(dest, sha1_to_hex(sha1), ll);
> > > +		else
> > > +			memcpy(dest, buf, ll);
> > 
> > What do you do if the rewritten sha1, truncated to ll characters, is 
> > ambiguous?  Wouldn't you need more than
> 
> More than what?

Right.  I did not complete that sentence.  But isn't it obvious?  Usually, 
you put in short, but (at least for some time) _unambiguous_ short names.  
IMHO people would expect the same to be true of rewritten commit messages.

> Are you saying I should add some more of it then? (As you can see, I 
> simply don't consider that case here, so right now I just leave it 
> possibly ambiguous.)

Yes, I saw that.  Just wanted to remind you that some people might 
consider this a bug.

> > > +	if (!prefixcmp(ref+5, original_prefix))
> > 
> > Should original_prefix not be "refs/original", then?
> 
> The idea was that we could add a command line option later to override 
> it.

Okay, I'll try again.  You do not use the canonical form.  Why not use 
it, if only for consistency?  Besides, if you want to be able to override 
it with a command line option, you _will_ have to canonicalise _that_ 
original_prefix.

> > > +static int is_pruned(struct commit *commit, int path_pruning)
> > > +{
> > > +	if (commit->object.flags & PRUNED)
> > > +		return 1;
> > 
> > Hmm.  I thought about changing get_revision_1() to mark that commit as 
> > uninteresting, since the parents were already added.  But I guess this 
> > has too much side effect potential.
> > 
> > In any case, I think that "NO_MATCH" would be a more descriptive name.
> 
> Right now, it's only set for matching stuff, but it could be set for 
> other kinds of pruning too.

Yes, it could.

> > > +	if (path_pruning &&
> > > +	    !(commit->object.flags & (TREECHANGE | UNINTERESTING)))
> > > +		return 1;
> > 
> > Why only with "path_pruning"?  Ah yes.  Because otherwise, you would 
> > assume "A" in "A..B" to be pruned.
> 
> TREECHANGE is only set when path pruning is in effect.
> If I didn't check for path_pruning, then all commits would be
> considered to have been pruned.  (Or am I missing something?
> Honestyl, I found all that TREECHANGE stuff difficult to follow.)

AFAICT TREECHANGE means that parents were rewritten.

> revision.c itself is also riddled with "prune_fn && ".
> Wouldn't it make sense to invert the meaning of this bit and call
> it, say, PRUNED, so that the default is off and you would only
> have to check if the bit was set ?

You meant the TREECHANGE bit?  No.

BTW what do you plan to do about my objection to UNINTERESTING, given the 
example "git rewrite-commits A..B x/y"?

> > I wonder why you bother at all: my impression was that the revisions 
> > you want to filter out here were already filtered out by 
> > revision.c:rewrite_parents()...
> 
> I also want to make reference that pointed to something that has
> been pruned (in either way) to now point to something in the new
> history.

I did not understand that intention.  Maybe I am too dumb, that's all.

> > > +static void rewrite_sha1(struct object *obj, unsigned char *new_sha1)
> > > +{
> > > +	if (!hashcmp(obj->sha1, new_sha1))
> > > +		return;
> > > +
> > > +	add_rewrite_decoration(obj, new_sha1);
> > 
> > This is not so much "rewrite_sha1", as "append_rewritten_sha1", right?
> 
> Well, it's only called once on each commit.

Didn't I mention that it was a severe limitation to think of the sha1 
mapping of a 1-to-1 mapping?  Think of it more as a relation.

> > > +		get_rewritten_sha1(sha1);
> > > +		if (!is_null_sha1(sha1)) {
> > > +			hashclr(sha1);
> > > +			rewrite_sha1(&list->item->object, sha1);
> > 
> > I guess you'll admit that it is unintuitive to read 
> > "get_rewritten_sha1(sha1); rewrite_sha1(o, sha1);".  I thought: "What?  
> > Again?"  IMHO that is a strong hint that "rewrite_sha1" is not an apt 
> > name for that function.
> 
> I guess our brains work in a slightly different way.

I doubt that.  What would you expect a code

	get_tagged_commit(commit);
	tag_commit(tag, commit);

to do, if not tag a tagged commit _again_?  So I bet if it wasn't your 
code to begin with, you would have experienced the same confusion as me.

> > Hmm.  When looking at that code, I wonder if
> > 
> > 	git rewrite-commits A..B
> > 
> > will rewrite C, too, if it happens to lie in A..B.  That would be not 
> > brilliant.
> 
> That was the idea.  Why is it not brilliant?

Because it is called rewrite-commits, not 
rewrite-all-refs-that-touch-this-commit-range.

> What would you like this to do instead?

Only touch the positive refs given in the command line.  If you say 
"--all", then it's all.  If you say "A..B", then it's "B".

> Assume that in your new history all the commit that used to be pointed 
> to have been removed.  If these pointers are left to point to the old 
> history, then how are you going to get at the new history?

I still have the superproject splitting as the main application in mind.  
Only for big applications like these does the performance improvement of 
rewrite-commits over filter-branch buy us anything.

> > > +	commit = lookup_commit_reference(sha1);
> > > +	pruned = is_pruned(commit, !!cb_data);
> > > +
> > > +	hashcpy(new_sha1, sha1);
> > > +	if (!pruned && get_rewritten_sha1(new_sha1))
> > > +		return 0;
> > 
> > So if pruned == 1, you _do_ rewrite it?  I'm not quite sure.  Care to 
> > explain?
> 
> Yes.  See above.

Sorry, this explanation does not help me.  My impression was that a pruned 
commit should be _mapped_ to the rewritten sha1s of the original parents, 
but not be rewritten.

> > > +	cmd.in = open(commit_path, O_RDONLY);
> > > +	if (cmd.in < 0)
> > > +		die("Unable to read commit from file '%s'", commit_path);
> > > +	unlink(commit_path);
> > 
> > This will fail on Windows.  You do not catch that error, so it is almost 
> > fine: just put the file into rewrite_dir, so the leftovers will be removed 
> > later anyway.
> 
> You mean unlinking an opened file?

Yes, I mean unlinking an opened file.

> > > +		hashclr(sha1);
> > > +		commit->object.flags |= PRUNED;
> > > +		/*
> > > +		 * If the filter returns two or more commits,
> > > +		 * we consider the original commit to have been
> > > +		 * removed and put the list in the old commit's
> > > +		 * parent list so that all the old commit's children
> > > +		 * will copy them.
> > > +		 */
> > > +		if (list) {
> > > +			free_commit_list(commit->parents);
> > > +			commit->parents = list;
> > > +		} else
> > > +		    add_sha1_map(commit->object.sha1, NULL);
> > 
> > That is almost certainly wrong.
> 
> What specifically ?

It takes a few minutes to verify that the correct calls to add_sha1_map() 
are done in every case.

Also, why should commit->parents be reset only in the multiple-sha1 case?

But most seriously, why do you replace the _parents_ of a given commit by 
the sha1s of the _rewritten_ commits?  They are not even the rewritten 
_parents_.

> > If you step away from the notion that 
> > get_rewritten_sha1() returns _one_ SHA-1, all will become clearer.  And 
> > the filters should know about them SHA-1s, too.
> 
> The filters do know about them.  The corresponding map file contains
> all the new SHA1s.

But the code does not reflect that in some parts.  For example when you 
call "get_rewritten_sha1(sha1);" which one is it?  And what is it if the 
commit was rewritten to no commit at all, i.e. the history was cut 
forcefully?

> > > +		add_sha1_map(commit->object.sha1, sha1);
> > > +	} else
> > > +		filter_and_write_commit(buf, p-buf, commit, sha1);
> > > +	free(buf);
> > 
> > You can really discard the commit->buffer, too, no?
> 
> I'm not familiar enough with the internals to say for sure.

Hmm.  You play with them pretty heavily, then, for example when you just 
reset commit->parents.

> > > +static void setup_temp_dir()
> > > +{
> > > +	int aux;
> > > +
> > > +	absolute_git_dir = create_absolute_path(get_git_dir());
> > > +	setenv(GIT_DIR_ENVIRONMENT, absolute_git_dir, 1);
> > > +
> > > +	if (!rewrite_dir) {
> > > +		rewrite_dir = xstrdup(git_path("rewrite"));
> > 
> > I might read the code wrong, but this does not guarantee that rewrite_dir 
> > is an absolute path, right?
> 
> It doesn't right now.

Umm.  Sorry to ask so bluntly: are you planning to change that?

> > > +	if (mkdir(mkpath("%s/map", rewrite_dir), 0777))
> > > +		die("unable to create map directory '%s/map'", rewrite_dir);
> > 
> > Maybe make this dependent on --write-sha1-mappings, and have a check in 
> > "map ()"?
> 
> I'll leave that to you :-)

IMHO the first part should be in the initial commit of 
builtin-rewrite-commits.c, so I will not do that.

> > > +	while ((commit = get_revision(&rev)) != NULL) {
> > > +		rewrite_commit(commit, !!rev.prune_fn);
> > > +	}
> > 
> > Please lose the curly brackets for single lines; otherwise it would look 
> > too perl like.
> 
> That would be
> 	
> 	rewrite_commit($commit) while $commit = get_revision;

No, it would not, because it is not valid C.

> > > +	rm_rf(git_path("refs/%s", original_prefix));
> > 
> > Would it not be better to move refs/original to 
> > refs/original/original?
> 
> Is that what you want? You'd end up with 
> refs/original/original/original/original/original/original/original/ 
> pretty quickly.

We are pretty anal about not losing data too quickly, and possibly 
unwantedly.

It would be surprising to me if this would be the first program to change 
that behaviour.

IOW, refs/original/ is not a temporary directory that you easily blow 
away.  It is meant for the user to be inspected.  And it is the user's 
obligation to say when that happened, and that it should go now.

Thinking about it again, I'd even go so far as to disallow operation with 
an existing refs/original/.

Ciao,
Dscho

^ permalink raw reply

* Re: CVS -> SVN -> Git
From: Martin Langhoff @ 2007-07-16  1:05 UTC (permalink / raw)
  To: esr; +Cc: Michael Haggerty, Julian Phillips, git, dev
In-Reply-To: <20070715013949.GA20850@thyrsus.com>

On 7/15/07, Eric S. Raymond <esr@thyrsus.com> wrote:
> Not quite.  I'm suggesting it's an appropriate lingua franca for centralized
> VCSes with branching, e.g. everything pre-Arch.

That's a huge goal that gets in the way of waht we want to do here: we
are trying to save time, not embark on some huge mission.

cvs2svn has all the "wtf-did-cvs-mean-by-that" algorithms that are
very hard to write and maintain, and it seems to be the best one at
that. Of course, it also writes SVN repos -- but I'm sure that's the
easiest part.

     We don't need no meta VCS for any of this.

All we need is to hook into the "write out a repo based on all the
stuff we parsed from cvs". Perhaps it's doable, and if Michael helps
out abstracting that part a bit, maintainable long term too.

cheers,



m

^ permalink raw reply

* Re: CVS -> SVN -> Git
From: Martin Langhoff @ 2007-07-16  1:08 UTC (permalink / raw)
  To: Michael Haggerty; +Cc: David Frech, esr, Julian Phillips, git, dev
In-Reply-To: <469A099E.6060906@alum.mit.edu>

On 7/15/07, Michael Haggerty <mhagger@alum.mit.edu> wrote:
> > It took a day and half to get the svn dump parsing right (it's an
> > egregiously bad format) but only a couple of hours to write the
> > fast-import backend.
>
> I'm surprised you think that; I find the svn dump format quite easy and
> straightforward.  (Of course it assumes some Subversionisms, like easy
> deep directory copies, which I can imagine would be annoying in other
> contexts.)  What don't you like about the format?

Is there good doco and samples for it? I wouldn't mind doing things by
way of an SVN dump parser.

> Yes, fast-import is a very easy-to-write format and looks to be very
> well documented.  I don't think that having to write output in
> fast-import format would be any kind of a hindrance for such a tool.

Damn! You've now figured out that all my volunteering was for the easy
part of the job ;-)




m

^ permalink raw reply

* Re: CVS -> SVN -> Git
From: Julian Phillips @ 2007-07-16  1:13 UTC (permalink / raw)
  To: Martin Langhoff; +Cc: Michael Haggerty, David Frech, esr, git, dev
In-Reply-To: <46a038f90707151808u67c4e834lb06ed86c855f58ec@mail.gmail.com>

On Mon, 16 Jul 2007, Martin Langhoff wrote:

> On 7/15/07, Michael Haggerty <mhagger@alum.mit.edu> wrote:
>> >  It took a day and half to get the svn dump parsing right (it's an
>> >  egregiously bad format) but only a couple of hours to write the
>> >  fast-import backend.
>>
>>  I'm surprised you think that; I find the svn dump format quite easy and
>>  straightforward.  (Of course it assumes some Subversionisms, like easy
>>  deep directory copies, which I can imagine would be annoying in other
>>  contexts.)  What don't you like about the format?
>
> Is there good doco and samples for it? I wouldn't mind doing things by
> way of an SVN dump parser.

I don't know if it classes as what you call good, but it is documented:

http://svn.collab.net/repos/svn/trunk/notes/fs_dumprestore.txt

>
>>  Yes, fast-import is a very easy-to-write format and looks to be very
>>  well documented.  I don't think that having to write output in
>>  fast-import format would be any kind of a hindrance for such a tool.
>
> Damn! You've now figured out that all my volunteering was for the easy
> part of the job ;-)
>
>
>
>
> m
>
>

-- 
Julian

  ---
This process can check if this value is zero, and if it is, it does
something child-like.
 		-- Forbes Burkowski, CS 454, University of Washington

^ permalink raw reply

* [PATCH] contrib/emacs/Makefile: Also install .el files.
From: David Kastrup @ 2007-07-15  9:46 UTC (permalink / raw)
  To: git
In-Reply-To: <403842ba71506c7b194812cd9a4f669c847eb7bc.1184548803.git.dak@gnu.org>


Signed-off-by: David Kastrup <dak@gnu.org>
---
 contrib/emacs/Makefile |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/contrib/emacs/Makefile b/contrib/emacs/Makefile
index 98aa0aa..92033f0 100644
--- a/contrib/emacs/Makefile
+++ b/contrib/emacs/Makefile
@@ -12,7 +12,7 @@ all: $(ELC)
 
 install: all
 	$(INSTALL) -d $(DESTDIR)$(emacsdir)
-	$(INSTALL_ELC) $(ELC) $(DESTDIR)$(emacsdir)
+	$(INSTALL_ELC) $(ELC:.elc=.el) $(ELC) $(DESTDIR)$(emacsdir)
 
 %.elc: %.el
 	$(EMACS) -batch -f batch-byte-compile $<
-- 
1.4.4.2

^ permalink raw reply related

* [PATCH] Make several improvements and get annotations to work (Emacs support pending).
From: David Kastrup @ 2007-07-15 23:42 UTC (permalink / raw)
  To: git
In-Reply-To: <403842ba71506c7b194812cd9a4f669c847eb7bc.1184548803.git.dak@gnu.org>

(vc-git-symbolic-commit): Allow nil to pass through.
(vc-git-previous-version): Use explicit parent argument.
(vc-git-next-version): Simplify.
(vc-git-annotate-command): Use `vc-do-command'.
(vc-git-annotate-extract-revision-at-line): Rename from
`vc-annotate-extract-revision-at-line'.

Signed-off-by: David Kastrup <dak@gnu.org>
---
 contrib/emacs/vc-git.el |   76 +++++++++++++++++++++++++++--------------------
 1 files changed, 44 insertions(+), 32 deletions(-)

diff --git a/contrib/emacs/vc-git.el b/contrib/emacs/vc-git.el
index 3637c8a..2a0a0fe 100644
--- a/contrib/emacs/vc-git.el
+++ b/contrib/emacs/vc-git.el
@@ -84,19 +84,20 @@
 (defun vc-git-symbolic-commit (commit)
   "Translate COMMIT string into symbolic form.
 Returns nil if not possible."
-  (with-temp-buffer
-    (and
-     (zerop
-      (call-process "git" nil '(t nil) nil "name-rev"
-		    commit))
-     (goto-char (point-max))
-     (bolp)
-     (zerop (forward-line -1))
-     (bobp)
-     (progn
-       (search-forward " " nil t)
-       (not (eolp)))
-     (buffer-substring-no-properties (point) (1- (point-max))))))
+  (and commit
+       (with-temp-buffer
+	 (and
+	  (zerop
+	   (call-process "git" nil '(t nil) nil "name-rev"
+			 commit))
+	  (goto-char (point-max))
+	  (bolp)
+	  (zerop (forward-line -1))
+	  (bobp)
+	  (progn
+	    (search-forward " " nil t)
+	    (not (eolp)))
+	  (buffer-substring-no-properties (point) (1- (point-max)))))))
 
 (defun vc-git-previous-version (file rev)
   "git-specific version of `vc-previous-version'."
@@ -104,15 +105,13 @@ Returns nil if not possible."
    (with-temp-buffer
      (and
       (zerop
-       (call-process "git" nil '(t nil) nil "rev-list" "--abbrev"
-		     "--abbrev-commit" "-2" rev "--" (file-relative-name file)))
-      (goto-char (point-max))
-      (bolp)
-      (zerop (forward-line -1))
-      (not (bobp))
-      (buffer-substring-no-properties
-       (point)
-       (1- (point-max)))))))
+       (call-process "git" nil '(t nil) nil "rev-list"
+		     "--abbrev"
+		     "--abbrev-commit" "-1" "--parents" rev "--"
+		     (file-relative-name file)))
+      (goto-char (point-min))
+      (search-forward-regexp " \\([^ \n]*\\)" nil t)
+      (match-string 1)))))
 
 (defun vc-git-next-version (file rev)
   "git-specific version of `vc-next-version'."
@@ -121,8 +120,9 @@ Returns nil if not possible."
      (and
       (zerop
        (call-process "git" nil '(t nil) nil "rev-list" "--abbrev"
-		     "--abbrev-commit"
+		     "--abbrev-commit"  "--remove-empty"
 		     "HEAD" "--not" rev "--" (file-relative-name file)))
+      (goto-char (point-min))
       (goto-char (point-max))
       (bolp)
       (zerop (forward-line -1))
@@ -174,24 +174,36 @@ Returns nil if not possible."
                        (vc-git--run-command-string file "ls-files" "-z" "--full-name" "--")
                        0 -1))
             (coding-system-for-read 'no-conversion)
-            (coding-system-for-write 'no-conversion))
+            coding-system-for-write)
         (with-temp-file destfile
-          (eq 0 (call-process "git" nil t nil "cat-file" "blob"
-                              (concat (or rev "HEAD") ":" fullname)))))
+	  (prog1
+	      (eq 0 (call-process "git" nil t nil "cat-file" "blob"
+				  (concat (or rev "HEAD") ":" fullname)))
+	    (setq coding-system-for-write 'no-conversion))))
     (vc-git--run-command file "checkout" (or rev "HEAD"))))
 
-(defun vc-git-annotate-command (file buf &optional rev)
-  (let ((name (file-relative-name file)))
-    (if rev
-	(call-process "git" nil buf nil "blame" rev "--" name)
-      (call-process "git" nil buf nil "blame" "--" name))))
+(defun vc-git-annotate-command (file buffer &optional version)
+  "Execute \"git blame\" on FILE, inserting the contents in BUFFER.
+Optional arg VERSION is a version to annotate from."
+  (vc-do-command buffer
+		 'async
+		 "git" file "blame"
+		 (or version "HEAD")
+		 "--"))
+
+;;(defun vc-git-annotate-command (file buf &optional rev)
+;;  (let ((name (file-relative-name file)))
+;;    (vc-setup-buffer buf)
+;;    (if rev
+;;	(call-process "git" nil buf nil "blame" rev "--" name)
+;;      (call-process "git" nil buf nil "blame" "--" name))))
 
 (defun vc-git-annotate-time ()
   (and (re-search-forward "[0-9a-f]+ (.* \\([0-9]+\\)-\\([0-9]+\\)-\\([0-9]+\\) \\([0-9]+\\):\\([0-9]+\\):\\([0-9]+\\) \\([-+0-9]+\\) +[0-9]+)" nil t)
        (vc-annotate-convert-time
         (apply #'encode-time (mapcar (lambda (match) (string-to-number (match-string match))) '(6 5 4 3 2 1 7))))))
 
-(defun vc-annotate-extract-revision-at-line ()
+(defun vc-git-annotate-extract-revision-at-line ()
   (save-excursion
     (move-beginning-of-line 1)
     (and (looking-at "[0-9a-f]+")
-- 
1.4.4.2

^ permalink raw reply related

* Re: CVS -> SVN -> Git
From: Karl Fogel @ 2007-07-16  1:30 UTC (permalink / raw)
  To: Martin Langhoff
  Cc: Michael Haggerty, David Frech, esr, Julian Phillips, git, dev
In-Reply-To: <46a038f90707151808u67c4e834lb06ed86c855f58ec@mail.gmail.com>

"Martin Langhoff" <martin.langhoff@gmail.com> writes:
> On 7/15/07, Michael Haggerty <mhagger@alum.mit.edu> wrote:
>> > It took a day and half to get the svn dump parsing right (it's an
>> > egregiously bad format) but only a couple of hours to write the
>> > fast-import backend.
>>
>> I'm surprised you think that; I find the svn dump format quite easy and
>> straightforward.  (Of course it assumes some Subversionisms, like easy
>> deep directory copies, which I can imagine would be annoying in other
>> contexts.)  What don't you like about the format?
>
> Is there good doco and samples for it? I wouldn't mind doing things by
> way of an SVN dump parser.

   http://svn.collab.net/repos/svn/trunk/notes/dump-load-format.txt

Best,
-Karl

^ permalink raw reply

* Re: [PATCH] contrib/emacs...
From: David Kastrup @ 2007-07-16  1:27 UTC (permalink / raw)
  To: git
In-Reply-To: <09ccbeffdf2ca787844d20fdc3e93d323da206f7.1184548803.git.dak@gnu.org>


I apologize for the somewhat messy state of the patches and commit
messages: I am trying to get a hang of the tools right now.

If people have suggestions how to read up on cleaning up and
reordering commits before submission, I'd be greatful.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

^ 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