* Re: Mercurial vs Updated git HOWTO for kernel hackers
From: Kyle Moffett @ 2005-06-29 3:53 UTC (permalink / raw)
To: Sean; +Cc: mercurial, Petr Baudis, Linux Kernel, Jeff Garzik,
Git Mailing List
In-Reply-To: <2661.10.10.10.24.1120004702.squirrel@linux1>
On Jun 28, 2005, at 20:25:02, Sean wrote:
> there will be a price to pay if the linux community fragments over
> choice
> of scm.
I don't agree. With the current set of SCMs, I don't think it will
be long
before somebody invents a gitweb/Mercurial/whatever gateway, such
that I can
"hg serve" from my Mercurial repository and have Linus "git pull" from a
multiprotocol bridge.
> the good news is that we're no longer locked into the whims of
> some proprietary system. so it should be straight forward for
> those who
> choose any tool to work with those who've chosen another. this is
> already
> evidenced by the fact that the git repository is pulled and re-
> exeported
> with mecurial.
I agree completely! Cheers to the end of proprietary revision storage!
> anyway, all the best, just wish you guys would spend less time
> trying to
> convert git users and more time advancing your own tool.
A project with no users isn't much of a project, now is it? In any
case,
this thread has long since passed its usefulness, so let's let it
die, ok?
Cheers,
Kyle Moffett
--
I lost interest in "blade servers" when I found they didn't throw
knives at people who weren't supposed to be in your machine room.
-- Anthony de Boer
^ permalink raw reply
* Re: kernel.org and GIT tree rebuilding
From: Nicolas Pitre @ 2005-06-29 3:55 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Git Mailing List
In-Reply-To: <Pine.LNX.4.58.0506281424420.19755@ppc970.osdl.org>
On Tue, 28 Jun 2005, Linus Torvalds wrote:
>
>
> On Tue, 28 Jun 2005, Nicolas Pitre wrote:
> >
> > OK. New patch below.
>
> Dammit, I wasted all that time doing it myself.
>
> I just committed and pushed out my version. But mine also does sha1_file.c
> right, so that you can use a packed archive in .git/objects/pack. Yours
> has some other cleanups, so..
>
> Can you double-check my version (it hasn't mirrored out yet, it seems, but
> it should be there soon).
OK... See below the cleanups I merged from my version on top of yours:
pack-objects.c | 70 ++++++++++++++-----------------------------------------
pack.h | 17 ++++++++-----
unpack-objects.c | 66 +++++++++++++++++++++++++--------------------------
3 files changed, 63 insertions(+), 90 deletions(-)
I also restored my original object header size ordering (little endian)
for two reasons:
- it is much simpler to generate and therefore allows for removing
quite some code
- it allows for stable bit position which makes it much easier to look
at an hex dump of the binary data for manual debugging
Also a few code optimizations and one error return fix.
Signed-off-by: Nicolas Pitre <nico@cam.org>
diff --git a/pack-objects.c b/pack-objects.c
--- a/pack-objects.c
+++ b/pack-objects.c
@@ -34,7 +34,7 @@ static void *delta_against(void *buf, un
if (!otherbuf)
die("unable to read %s", sha1_to_hex(entry->delta->sha1));
delta_buf = diff_delta(otherbuf, othersize,
- buf, size, &delta_size, ~0UL);
+ buf, size, &delta_size, 0UL);
if (!delta_buf || delta_size != entry->delta_size)
die("delta size changed");
free(buf);
@@ -42,54 +42,13 @@ static void *delta_against(void *buf, un
return delta_buf;
}
-/*
- * The per-object header is a pretty dense thing, which is
- * - first byte: low four bits are "size", then three bits of "type",
- * and the high bit is "size continues".
- * - each byte afterwards: low seven bits are size continuation,
- * with the high bit being "size continues"
- */
-static int encode_header(enum object_type type, unsigned long size, unsigned char *hdr)
-{
- int n = 1, i;
- unsigned char c;
-
- if (type < OBJ_COMMIT || type > OBJ_DELTA)
- die("bad type %d", type);
-
- /*
- * Shift the size up by 7 bits at a time,
- * until you get bits in the "high four".
- * That will be our beginning. We'll have
- * four size bits in 28..31, then groups
- * of seven in 21..27, 14..20, 7..13 and
- * finally 0..6.
- */
- if (size) {
- n = 5;
- while (!(size & 0xfe000000)) {
- size <<= 7;
- n--;
- }
- }
- c = (type << 4) | (size >> 28);
- for (i = 1; i < n; i++) {
- *hdr++ = c | 0x80;
- c = (size >> 21) & 0x7f;
- size <<= 7;
- }
- *hdr = c;
- return n;
-}
-
static unsigned long write_object(struct sha1file *f, struct object_entry *entry)
{
unsigned long size;
char type[10];
void *buf = read_sha1_file(entry->sha1, type, &size);
- unsigned char header[10];
+ char header[25];
unsigned hdrlen, datalen;
- enum object_type obj_type;
if (!buf)
die("unable to read %s", sha1_to_hex(entry->sha1));
@@ -97,22 +56,31 @@ static unsigned long write_object(struct
die("object %s size inconsistency (%lu vs %lu)", sha1_to_hex(entry->sha1), size, entry->size);
/*
- * The object header is a byte of 'type' followed by zero or
- * more bytes of length. For deltas, the 20 bytes of delta sha1
- * follows that.
+ * The object header first byte has its low 3 bits representing the
+ * object type, the 4 upper bits indicating which of the following
+ * bytes are used to build the object size. For delta objects the
+ * sha1 of the reference object is also appended.
*/
- obj_type = entry->type;
if (entry->delta) {
+ header[0] = OBJ_DELTA;
buf = delta_against(buf, size, entry);
size = entry->delta_size;
- obj_type = OBJ_DELTA;
+ } else
+ header[0] = entry->type;
+ header[0] |= size << 3;
+ hdrlen = 1;
+ datalen = size >> 4;
+ while (datalen) {
+ header[hdrlen - 1] |= 0x80;
+ header[hdrlen++] = datalen;
+ datalen >>= 7;
}
- hdrlen = encode_header(obj_type, size, header);
- sha1write(f, header, hdrlen);
if (entry->delta) {
- sha1write(f, entry->delta, 20);
+ memcpy(header+hdrlen, entry->delta, 20);
hdrlen += 20;
}
+
+ sha1write(f, header, hdrlen);
datalen = sha1write_compressed(f, buf, size);
free(buf);
return hdrlen + datalen;
diff --git a/pack.h b/pack.h
--- a/pack.h
+++ b/pack.h
@@ -1,13 +1,18 @@
#ifndef PACK_H
#define PACK_H
+/*
+ * The packed object type is stored in the low 3 bits of a byte.
+ * The type value 0 is a reserved prefix if ever there is more than 7
+ * object types, or any future format extensions.
+ */
enum object_type {
- OBJ_NONE,
- OBJ_COMMIT,
- OBJ_TREE,
- OBJ_BLOB,
- OBJ_TAG,
- OBJ_DELTA,
+ OBJ_EXT = 0,
+ OBJ_COMMIT = 1,
+ OBJ_TREE = 2,
+ OBJ_BLOB = 3,
+ OBJ_TAG = 4,
+ OBJ_DELTA = 7
};
/*
diff --git a/unpack-objects.c b/unpack-objects.c
--- a/unpack-objects.c
+++ b/unpack-objects.c
@@ -13,6 +13,14 @@ struct pack_entry {
unsigned char sha1[20];
};
+static char *type_string[] = {
+ [OBJ_COMMIT] = "commit",
+ [OBJ_TREE] = "tree",
+ [OBJ_BLOB] = "blob",
+ [OBJ_TAG] = "tag",
+ [OBJ_DELTA] = "delta"
+};
+
static void *pack_base;
static unsigned long pack_size;
static void *index_base;
@@ -93,7 +101,7 @@ static int check_index(void)
}
static int unpack_non_delta_entry(struct pack_entry *entry,
- enum object_type kind,
+ char *type,
unsigned char *data,
unsigned long size,
unsigned long left)
@@ -102,9 +110,8 @@ static int unpack_non_delta_entry(struct
z_stream stream;
char *buffer;
unsigned char sha1[20];
- char *type;
- printf("%s %c %lu\n", sha1_to_hex(entry->sha1), ".CTBGD"[kind], size);
+ printf("%s %s %lu\n", sha1_to_hex(entry->sha1), type, size);
if (dry_run)
return 0;
@@ -121,13 +128,6 @@ static int unpack_non_delta_entry(struct
inflateEnd(&stream);
if ((st != Z_STREAM_END) || stream.total_out != size)
goto err_finish;
- switch (kind) {
- case OBJ_COMMIT: type = "commit"; break;
- case OBJ_TREE: type = "tree"; break;
- case OBJ_BLOB: type = "blob"; break;
- case OBJ_TAG: type = "tag"; break;
- default: goto err_finish;
- }
if (write_sha1_file(buffer, size, type, sha1) < 0)
die("failed to write %s (%s)",
sha1_to_hex(entry->sha1), type);
@@ -135,8 +135,8 @@ static int unpack_non_delta_entry(struct
if (memcmp(sha1, entry->sha1, 20))
die("resulting %s have wrong SHA1", type);
- finish:
st = 0;
+ finish:
free(buffer);
return st;
err_finish:
@@ -185,15 +185,13 @@ static int unpack_delta_entry(struct pac
die("truncated pack file");
data = base_sha1 + 20;
data_size = left - 20;
- printf("%s D %lu", sha1_to_hex(entry->sha1), delta_size);
+ printf("%s delta %lu", sha1_to_hex(entry->sha1), delta_size);
printf(" %s\n", sha1_to_hex(base_sha1));
if (dry_run)
return 0;
- /* pack+5 is the base sha1, unless we have it, we need to
- * unpack it first.
- */
+ /* unless we have the base sha1, we need to unpack it first. */
if (!has_sha1_file(base_sha1)) {
struct pack_entry *base;
if (!find_pack_entry(base_sha1, &base))
@@ -238,8 +236,9 @@ static int unpack_delta_entry(struct pac
static void unpack_entry(struct pack_entry *entry)
{
unsigned long offset, size, left;
- unsigned char *pack, c;
- int type;
+ unsigned char c, *pack = pack_base;
+ int i;
+ enum object_type type;
/* Have we done this one already due to deltas based on it? */
if (lookup_object(entry->sha1))
@@ -247,20 +246,17 @@ static void unpack_entry(struct pack_ent
offset = ntohl(entry->offset);
if (offset >= pack_size)
- goto bad;
-
- pack = pack_base + offset;
- c = *pack++;
- offset++;
- type = (c >> 4) & 7;
- size = (c & 15);
+ goto out_of_bound;
+ c = pack[offset++];
+ type = c & 0x07;
+ size = (c & ~0x80) >> 3;
+ i = 4;
while (c & 0x80) {
if (offset >= pack_size)
- goto bad;
- offset++;
- c = *pack++;
- size = (size << 7) + (c & 0x7f);
-
+ goto out_of_bound;
+ c = pack[offset++];
+ size |= (c & ~0x80) << i;
+ i += 7;
}
left = pack_size - offset;
switch (type) {
@@ -268,14 +264,18 @@ static void unpack_entry(struct pack_ent
case OBJ_TREE:
case OBJ_BLOB:
case OBJ_TAG:
- unpack_non_delta_entry(entry, type, pack, size, left);
+ unpack_non_delta_entry(entry, type_string[type],
+ pack+offset, size, left);
return;
case OBJ_DELTA:
- unpack_delta_entry(entry, pack, size, left);
+ unpack_delta_entry(entry, pack+offset, size, left);
return;
+ default:
+ die("corrupted pack file(unknown object type %d)", type);
}
-bad:
- die("corrupted pack file");
+
+ out_of_bound:
+ die("corrupted pack file (object offset out of bound)");
}
/*
^ permalink raw reply
* [PATCH] denser delta header encoding
From: Nicolas Pitre @ 2005-06-29 4:27 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Git Mailing List
Since the delta data format is not tied to any actual git object
anymore, now is the time to add a small improvement to the delta data
header as it is been done for packed object header. This patch allows
for reducing the delta header of about 2 bytes and makes for simpler
code.
Signed-off-by: Nicolas Pitre <nico@cam.org>
diff --git a/count-delta.c b/count-delta.c
--- a/count-delta.c
+++ b/count-delta.c
@@ -11,16 +11,13 @@
static unsigned long get_hdr_size(const unsigned char **datap)
{
const unsigned char *data = *datap;
- unsigned long size;
- unsigned char cmd;
- int i;
- size = i = 0;
- cmd = *data++;
- while (cmd) {
- if (cmd & 1)
- size |= *data++ << i;
- i += 8;
- cmd >>= 1;
+ unsigned char cmd = *data++;
+ unsigned long size = cmd & ~0x80;
+ int i = 7;
+ while (cmd & 0x80) {
+ cmd = *data++;
+ size |= (cmd & ~0x80) << i;
+ i += 7;
}
*datap = data;
return size;
@@ -47,8 +44,8 @@ int count_delta(void *delta_buf, unsigne
unsigned char cmd;
unsigned long src_size, dst_size, out;
- /* the smallest delta size possible is 6 bytes */
- if (delta_size < 6)
+ /* the smallest delta size possible is 4 bytes */
+ if (delta_size < 4)
return -1;
data = delta_buf;
diff --git a/diff-delta.c b/diff-delta.c
--- a/diff-delta.c
+++ b/diff-delta.c
@@ -228,28 +228,22 @@ void *diff_delta(void *from_buf, unsigne
top = to_buf + to_size;
/* store reference buffer size */
- orig = out + outpos++;
- *orig = i = 0;
- do {
- if (from_size & 0xff) {
- *orig |= (1 << i);
- out[outpos++] = from_size;
- }
- i++;
- from_size >>= 8;
- } while (from_size);
+ out[outpos++] = from_size;
+ from_size >>= 7;
+ while (from_size) {
+ out[outpos - 1] |= 0x80;
+ out[outpos++] = from_size;
+ from_size >>= 7;
+ }
/* store target buffer size */
- orig = out + outpos++;
- *orig = i = 0;
- do {
- if (to_size & 0xff) {
- *orig |= (1 << i);
- out[outpos++] = to_size;
- }
- i++;
- to_size >>= 8;
- } while (to_size);
+ out[outpos++] = to_size;
+ to_size >>= 7;
+ while (to_size) {
+ out[outpos - 1] |= 0x80;
+ out[outpos++] = to_size;
+ to_size >>= 7;
+ }
inscnt = 0;
moff = 0;
diff --git a/patch-delta.c b/patch-delta.c
--- a/patch-delta.c
+++ b/patch-delta.c
@@ -22,33 +22,33 @@ void *patch_delta(void *src_buf, unsigne
unsigned long size;
int i;
- /* the smallest delta size possible is 6 bytes */
- if (delta_size < 6)
+ /* the smallest delta size possible is 4 bytes */
+ if (delta_size < 4)
return NULL;
data = delta_buf;
top = delta_buf + delta_size;
/* make sure the orig file size matches what we expect */
- size = i = 0;
cmd = *data++;
- while (cmd) {
- if (cmd & 1)
- size |= *data++ << i;
- i += 8;
- cmd >>= 1;
+ size = cmd & ~0x80;
+ i = 7;
+ while (cmd & 0x80) {
+ cmd = *data++;
+ size |= (cmd & ~0x80) << i;
+ i += 7;
}
if (size != src_size)
return NULL;
/* now the result size */
- size = i = 0;
cmd = *data++;
- while (cmd) {
- if (cmd & 1)
- size |= *data++ << i;
- i += 8;
- cmd >>= 1;
+ size = cmd & ~0x80;
+ i = 7;
+ while (cmd & 0x80) {
+ cmd = *data++;
+ size |= (cmd & ~0x80) << i;
+ i += 7;
}
dst_buf = malloc(size);
if (!dst_buf)
^ permalink raw reply
* Re: [PATCH] denser delta header encoding
From: Junio C Hamano @ 2005-06-29 4:58 UTC (permalink / raw)
To: Nicolas Pitre; +Cc: Linus Torvalds, git
In-Reply-To: <Pine.LNX.4.63.0506290021050.1667@localhost.localdomain>
Linus, please do not apply this as is.
There are code other than what Nico updated with this patch in
sha1_file.c that also need updating, that count the number of
bytes in the delta-patch result by reading from the delta
header.
I wonder if we can have a helper function in delta suite
somewhere (maybe in diff-delta.c):
int look_at_delta_header(void **delta_data, ulong delta_size,
ulong *src_size, ulong *dst_size)
that:
- checks delta size and barf if it is small;
- reads the header and fills src_size and dst_size;
- advances *delta_data pointer;
and have count-delta, patch-delta and sha1_file.c users use it
consistently. Nico, what do you think?
^ permalink raw reply
* Re: kernel.org and GIT tree rebuilding
From: Nicolas Pitre @ 2005-06-29 5:16 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Git Mailing List
In-Reply-To: <Pine.LNX.4.63.0506282314320.1667@localhost.localdomain>
On Tue, 28 Jun 2005, Nicolas Pitre wrote:
> OK... See below the cleanups I merged from my version on top of yours:
Of course by the time I sent the above you already rewrote the ting to
be streamable.
So again :-) see below the cleanups I merged from my version on top of
yours:
pack-objects.c | 70 ++++++++++++++-----------------------------------------
pack.h | 17 ++++++++-----
unpack-objects.c | 29 ++++++++++++----------
3 files changed, 46 insertions(+), 70 deletions(-)
I also restored my original object header size ordering (little endian)
for two reasons:
- it is much simpler to generate and therefore allows for removing
quite some code
- it allows for stable bit position which makes it much easier to look
at an hex dump of the binary data for manual debugging
Signed-off-by: Nicolas Pitre <nico@cam.org>
diff --git a/pack-objects.c b/pack-objects.c
--- a/pack-objects.c
+++ b/pack-objects.c
@@ -34,7 +34,7 @@ static void *delta_against(void *buf, un
if (!otherbuf)
die("unable to read %s", sha1_to_hex(entry->delta->sha1));
delta_buf = diff_delta(otherbuf, othersize,
- buf, size, &delta_size, ~0UL);
+ buf, size, &delta_size, 0UL);
if (!delta_buf || delta_size != entry->delta_size)
die("delta size changed");
free(buf);
@@ -42,54 +42,13 @@ static void *delta_against(void *buf, un
return delta_buf;
}
-/*
- * The per-object header is a pretty dense thing, which is
- * - first byte: low four bits are "size", then three bits of "type",
- * and the high bit is "size continues".
- * - each byte afterwards: low seven bits are size continuation,
- * with the high bit being "size continues"
- */
-static int encode_header(enum object_type type, unsigned long size, unsigned char *hdr)
-{
- int n = 1, i;
- unsigned char c;
-
- if (type < OBJ_COMMIT || type > OBJ_DELTA)
- die("bad type %d", type);
-
- /*
- * Shift the size up by 7 bits at a time,
- * until you get bits in the "high four".
- * That will be our beginning. We'll have
- * four size bits in 28..31, then groups
- * of seven in 21..27, 14..20, 7..13 and
- * finally 0..6.
- */
- if (size) {
- n = 5;
- while (!(size & 0xfe000000)) {
- size <<= 7;
- n--;
- }
- }
- c = (type << 4) | (size >> 28);
- for (i = 1; i < n; i++) {
- *hdr++ = c | 0x80;
- c = (size >> 21) & 0x7f;
- size <<= 7;
- }
- *hdr = c;
- return n;
-}
-
static unsigned long write_object(struct sha1file *f, struct object_entry *entry)
{
unsigned long size;
char type[10];
void *buf = read_sha1_file(entry->sha1, type, &size);
- unsigned char header[10];
+ char header[25];
unsigned hdrlen, datalen;
- enum object_type obj_type;
if (!buf)
die("unable to read %s", sha1_to_hex(entry->sha1));
@@ -97,22 +56,31 @@ static unsigned long write_object(struct
die("object %s size inconsistency (%lu vs %lu)", sha1_to_hex(entry->sha1), size, entry->size);
/*
- * The object header is a byte of 'type' followed by zero or
- * more bytes of length. For deltas, the 20 bytes of delta sha1
- * follows that.
+ * The object header first byte has its low 3 bits representing the
+ * object type, the 4 upper bits indicating which of the following
+ * bytes are used to build the object size. For delta objects the
+ * sha1 of the reference object is also appended.
*/
- obj_type = entry->type;
if (entry->delta) {
+ header[0] = OBJ_DELTA;
buf = delta_against(buf, size, entry);
size = entry->delta_size;
- obj_type = OBJ_DELTA;
+ } else
+ header[0] = entry->type;
+ header[0] |= size << 3;
+ hdrlen = 1;
+ datalen = size >> 4;
+ while (datalen) {
+ header[hdrlen - 1] |= 0x80;
+ header[hdrlen++] = datalen;
+ datalen >>= 7;
}
- hdrlen = encode_header(obj_type, size, header);
- sha1write(f, header, hdrlen);
if (entry->delta) {
- sha1write(f, entry->delta, 20);
+ memcpy(header+hdrlen, entry->delta, 20);
hdrlen += 20;
}
+
+ sha1write(f, header, hdrlen);
datalen = sha1write_compressed(f, buf, size);
free(buf);
return hdrlen + datalen;
diff --git a/pack.h b/pack.h
--- a/pack.h
+++ b/pack.h
@@ -1,13 +1,18 @@
#ifndef PACK_H
#define PACK_H
+/*
+ * The packed object type is stored in the low 3 bits of a byte.
+ * The type value 0 is a reserved prefix if ever there is more than 7
+ * object types, or any future format extensions.
+ */
enum object_type {
- OBJ_NONE,
- OBJ_COMMIT,
- OBJ_TREE,
- OBJ_BLOB,
- OBJ_TAG,
- OBJ_DELTA,
+ OBJ_EXT = 0,
+ OBJ_COMMIT = 1,
+ OBJ_TREE = 2,
+ OBJ_BLOB = 3,
+ OBJ_TAG = 4,
+ OBJ_DELTA = 7
};
/*
diff --git a/unpack-objects.c b/unpack-objects.c
--- a/unpack-objects.c
+++ b/unpack-objects.c
@@ -6,6 +6,14 @@
static int dry_run;
static const char unpack_usage[] = "git-unpack-objects < pack-file";
+static char *type_string[] = {
+ [OBJ_COMMIT] = "commit",
+ [OBJ_TREE] = "tree",
+ [OBJ_BLOB] = "blob",
+ [OBJ_TAG] = "tag",
+ [OBJ_DELTA] = "delta"
+};
+
/* We always read in 4kB chunks. */
static unsigned char buffer[4096];
static unsigned long offset, len, eof;
@@ -139,19 +147,11 @@ static void added_object(unsigned char *
}
}
-static int unpack_non_delta_entry(enum object_type kind, unsigned long size)
+static int unpack_non_delta_entry(char *type, unsigned long size)
{
void *buf = get_data(size);
unsigned char sha1[20];
- char *type;
- switch (kind) {
- case OBJ_COMMIT: type = "commit"; break;
- case OBJ_TREE: type = "tree"; break;
- case OBJ_BLOB: type = "blob"; break;
- case OBJ_TAG: type = "tag"; break;
- default: die("bad type %d", kind);
- }
if (write_sha1_file(buf, size, type, sha1) < 0)
die("failed to write object");
added_object(sha1, type, buf, size);
@@ -184,26 +184,29 @@ static int unpack_delta_entry(unsigned l
static void unpack_one(void)
{
unsigned char *pack, c;
+ unsigned int i;
unsigned long size;
enum object_type type;
pack = fill(1);
c = *pack;
use(1);
- type = (c >> 4) & 7;
- size = (c & 15);
+ type = c & 0x07;
+ size = (c & ~0x80) >> 3;
+ i = 4;
while (c & 0x80) {
pack = fill(1);
c = *pack++;
use(1);
- size = (size << 7) + (c & 0x7f);
+ size |= (c & ~0x80) << i;
+ i += 7;
}
switch (type) {
case OBJ_COMMIT:
case OBJ_TREE:
case OBJ_BLOB:
case OBJ_TAG:
- unpack_non_delta_entry(type, size);
+ unpack_non_delta_entry(type_string[type], size);
return;
case OBJ_DELTA:
unpack_delta_entry(size);
^ permalink raw reply
* Re: [PATCH] denser delta header encoding
From: Linus Torvalds @ 2005-06-29 5:21 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Nicolas Pitre, git
In-Reply-To: <7vmzp9kbcf.fsf@assigned-by-dhcp.cox.net>
On Tue, 28 Jun 2005, Junio C Hamano wrote:
>
> Linus, please do not apply this as is.
Argh. Too late.
> There are code other than what Nico updated with this patch in
> sha1_file.c that also need updating, that count the number of
> bytes in the delta-patch result by reading from the delta
> header.
Hmm.. I tested this (along with my change to make the pack object also
use the little-endian size encoding) with "gitk" on a packed git archive,
and with git-unpack-objects. So it can't break _too_ seriously. Is it the
"git-cat-file -s" thing that gets the wrong answer?
Ahh. I see it. "packed_delta_info()". And it looks like "diff" uses it
too. Oh, for the copy and rename detection. I don't think I tested that
part, nope.
> I wonder if we can have a helper function in delta suite
> somewhere (maybe in diff-delta.c):
>
> int look_at_delta_header(void **delta_data, ulong delta_size,
> ulong *src_size, ulong *dst_size)
>
> that:
>
> - checks delta size and barf if it is small;
> - reads the header and fills src_size and dst_size;
> - advances *delta_data pointer;
>
> and have count-delta, patch-delta and sha1_file.c users use it
> consistently. Nico, what do you think?
Sounds like a good idea.
Linus
^ permalink raw reply
* Re: [PATCH] denser delta header encoding
From: Junio C Hamano @ 2005-06-29 5:35 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Nicolas Pitre, git
In-Reply-To: <Pine.LNX.4.58.0506282217010.19755@ppc970.osdl.org>
>>>>> "LT" == Linus Torvalds <torvalds@osdl.org> writes:
LT> Ahh. I see it. "packed_delta_info()". And it looks like "diff" uses it
LT> too. Oh, for the copy and rename detection. I don't think I tested that
LT> part, nope.
OK, not too much damage done. I'll fix the rest up.
^ permalink raw reply
* Re: kernel.org and GIT tree rebuilding
From: Linus Torvalds @ 2005-06-29 5:43 UTC (permalink / raw)
To: Nicolas Pitre; +Cc: Git Mailing List
In-Reply-To: <Pine.LNX.4.63.0506290111250.1667@localhost.localdomain>
On Wed, 29 Jun 2005, Nicolas Pitre wrote:
>
> Of course by the time I sent the above you already rewrote the ting to
> be streamable.
And by the time you sent me a new version, I'd already taken part of your
old one by hand ;)
Linus
^ permalink raw reply
* Re: [PATCH] denser delta header encoding
From: Linus Torvalds @ 2005-06-29 5:44 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Nicolas Pitre, git
In-Reply-To: <7virzxk9nd.fsf@assigned-by-dhcp.cox.net>
On Tue, 28 Jun 2005, Junio C Hamano wrote:
>
> OK, not too much damage done. I'll fix the rest up.
Actually, I already did, and pushed out. And this time I verified it by
doing a "git-cat-file -s" on every object on a packed repo.
Linus
^ permalink raw reply
* Re: [PATCH] denser delta header encoding
From: Nicolas Pitre @ 2005-06-29 5:49 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Junio C Hamano, git
In-Reply-To: <Pine.LNX.4.58.0506282244110.19755@ppc970.osdl.org>
On Tue, 28 Jun 2005, Linus Torvalds wrote:
>
>
> On Tue, 28 Jun 2005, Junio C Hamano wrote:
> >
> > OK, not too much damage done. I'll fix the rest up.
>
> Actually, I already did, and pushed out. And this time I verified it by
> doing a "git-cat-file -s" on every object on a packed repo.
Damn!
And just when I was about to send a new patch with the thing nicely
abstracted to fix the dammage.
OK... let me pull again to see what's left then.
Nicolas
^ permalink raw reply
* Re: kernel.org and GIT tree rebuilding
From: Linus Torvalds @ 2005-06-29 5:54 UTC (permalink / raw)
To: Nicolas Pitre; +Cc: Git Mailing List
In-Reply-To: <Pine.LNX.4.58.0506282243180.19755@ppc970.osdl.org>
On Tue, 28 Jun 2005, Linus Torvalds wrote:
>
> On Wed, 29 Jun 2005, Nicolas Pitre wrote:
> >
> > Of course by the time I sent the above you already rewrote the ting to
> > be streamable.
>
> And by the time you sent me a new version, I'd already taken part of your
> old one by hand ;)
Btw, I have the size/type bits reversed from your setup, but please don't
change that, since that would be yet another incompatible pack format
change, and I'd like to calm things down.
Also, I notice that you decode the sizes really strangely: you have a
"while() { }" loop and two separate loads. It's much nicer to do it with a
"do { } while()" loop and a single load, since not only is it less code,
a do-while loop compiles to better code than a while() loop (unless the
compiler is crazy, which it sometimes is).
Linus
^ permalink raw reply
* Re: [PATCH] denser delta header encoding
From: Junio C Hamano @ 2005-06-29 5:59 UTC (permalink / raw)
To: Nicolas Pitre; +Cc: Linus Torvalds, git
In-Reply-To: <Pine.LNX.4.63.0506290146270.1667@localhost.localdomain>
>>>>> "NP" == Nicolas Pitre <nico@cam.org> writes:
NP> On Tue, 28 Jun 2005, Linus Torvalds wrote:
>>
>>
>> On Tue, 28 Jun 2005, Junio C Hamano wrote:
>> >
>> > OK, not too much damage done. I'll fix the rest up.
>>
>> Actually, I already did, and pushed out. And this time I verified it by
>> doing a "git-cat-file -s" on every object on a packed repo.
NP> Damn!
NP> And just when I was about to send a new patch with the thing nicely
NP> abstracted to fix the dammage.
Double Damn!! Three people working on the same piece of code.
And Linus head still breaks with t5300. Triple damn X-<.
^ permalink raw reply
* Re: [PATCH] denser delta header encoding
From: Linus Torvalds @ 2005-06-29 6:07 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Nicolas Pitre, git
In-Reply-To: <7vacl9k8iz.fsf@assigned-by-dhcp.cox.net>
On Tue, 28 Jun 2005, Junio C Hamano wrote:
>
> Double Damn!! Three people working on the same piece of code.
The good news is that I'm pushing out a linux-2.6.13-rc1 release, and will
go to bed after that, so I'm done for the day. No more races with me ;)
Linus
^ permalink raw reply
* [PATCH] Adjust t5300 test for unpack-objects change
From: Junio C Hamano @ 2005-06-29 6:07 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Nicolas Pitre, git
In-Reply-To: <7vacl9k8iz.fsf@assigned-by-dhcp.cox.net>
It now always read from standard input and rejects non-flag
arguments.
Signed-off-by: Junio C Hamano <junkio@cox.net>
---
cd /opt/packrat/playpen/public/in-place/git/git.junio/
jit-diff
# - linus: Fix packed_delta_info() that was broken by the delta header packing change
# + (working tree)
diff --git a/t/t5300-pack-object.sh b/t/t5300-pack-object.sh
--- a/t/t5300-pack-object.sh
+++ b/t/t5300-pack-object.sh
@@ -45,7 +45,8 @@ test_expect_success \
'GIT_OBJECT_DIRECTORY=.git2/objects &&
export GIT_OBJECT_DIRECTORY &&
git-init-db &&
- git-unpack-objects test-1'
+ git-unpack-objects -n <test-1.pack &&
+ git-unpack-objects <test-1.pack'
unset GIT_OBJECT_DIRECTORY
cd $TRASH/.git2
@@ -75,7 +76,8 @@ test_expect_success \
'GIT_OBJECT_DIRECTORY=.git2/objects &&
export GIT_OBJECT_DIRECTORY &&
git-init-db &&
- git-unpack-objects test-2'
+ git-unpack-objects -n <test-2.pack &&
+ git-unpack-objects <test-2.pack'
unset GIT_OBJECT_DIRECTORY
cd $TRASH/.git2
Compilation finished at Tue Jun 28 23:06:08
^ permalink raw reply
* Re: Mercurial vs Updated git HOWTO for kernel hackers
From: Thomas Arendsen Hein @ 2005-06-29 6:32 UTC (permalink / raw)
To: mercurial; +Cc: Linux Kernel, Git Mailing List
In-Reply-To: <62CF578B-B9DF-4DEA-8BAD-041F357771FD@mac.com>
(repost to all lists who received the original mail)
* Kyle Moffett <mrmacman_g4@mac.com> [20050628 22:28]:
> On Jun 28, 2005, at 14:01:57, Matt Mackall wrote:
> >Everything in Mercurial is an append-only log. A transaction journal
> >records the original length of each log so that it can be restored on
> >failure.
>
> Does this mean that (excepting the "undo" feature) one could set the
> ext3 "append-only" attribute on the repository files to avoid losing
> data due to user account compromise?
This will break Mercurial's journaling. If 'hg pull' fails it
truncates the already appended-to files to the last known state.
Thomas
--
Email: thomas@intevation.de
http://intevation.de/~thomas/
^ permalink raw reply
* [PATCH] assorted delta code cleanup
From: Nicolas Pitre @ 2005-06-29 6:49 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Junio C Hamano, git
This is a wrap-up patch including all the cleanups I've done to the
delta code and its usage. The most important change is the
factorization of the delta header handling code.
Signed-off-by: Nicolas Pitre <nico@cam.org>
diff --git a/count-delta.c b/count-delta.c
--- a/count-delta.c
+++ b/count-delta.c
@@ -6,23 +6,9 @@
#include <stdlib.h>
#include <string.h>
#include <limits.h>
+#include "delta.h"
#include "count-delta.h"
-static unsigned long get_hdr_size(const unsigned char **datap)
-{
- const unsigned char *data = *datap;
- unsigned char cmd = *data++;
- unsigned long size = cmd & ~0x80;
- int i = 7;
- while (cmd & 0x80) {
- cmd = *data++;
- size |= (cmd & ~0x80) << i;
- i += 7;
- }
- *datap = data;
- return size;
-}
-
/*
* NOTE. We do not _interpret_ delta fully. As an approximation, we
* just count the number of bytes that are copied from the source, and
@@ -44,15 +30,14 @@ int count_delta(void *delta_buf, unsigne
unsigned char cmd;
unsigned long src_size, dst_size, out;
- /* the smallest delta size possible is 4 bytes */
- if (delta_size < 4)
+ if (delta_size < DELTA_SIZE_MIN)
return -1;
data = delta_buf;
top = delta_buf + delta_size;
- src_size = get_hdr_size(&data);
- dst_size = get_hdr_size(&data);
+ src_size = get_delta_hdr_size(&data);
+ dst_size = get_delta_hdr_size(&data);
added_literal = copied_from_source = out = 0;
while (data < top) {
diff --git a/delta.h b/delta.h
--- a/delta.h
+++ b/delta.h
@@ -9,4 +9,26 @@ extern void *patch_delta(void *src_buf,
void *delta_buf, unsigned long delta_size,
unsigned long *dst_size);
+/* the smallest possible delta size is 4 bytes */
+#define DELTA_SIZE_MIN 4
+
+/*
+ * This must be called twice on the delta data buffer, first to get the
+ * expected reference buffer size, and again to get the result buffer size.
+ */
+static inline unsigned long get_delta_hdr_size(const unsigned char **datap)
+{
+ const unsigned char *data = *datap;
+ unsigned char cmd = *data++;
+ unsigned long size = cmd & ~0x80;
+ int i = 7;
+ while (cmd & 0x80) {
+ cmd = *data++;
+ size |= (cmd & ~0x80) << i;
+ i += 7;
+ }
+ *datap = data;
+ return size;
+}
+
#endif
diff --git a/diff-delta.c b/diff-delta.c
--- a/diff-delta.c
+++ b/diff-delta.c
@@ -306,12 +306,13 @@ void *diff_delta(void *from_buf, unsigne
*orig = i;
}
- /* next time around the largest possible output is 1 + 4 + 3 */
if (max_size && outpos > max_size) {
free(out);
delta_cleanup(&bdf);
return NULL;
}
+
+ /* next time around the largest possible output is 1 + 4 + 3 */
if (outpos > outsize - 8) {
void *tmp = out;
outsize = outsize * 3 / 2;
diff --git a/pack-objects.c b/pack-objects.c
--- a/pack-objects.c
+++ b/pack-objects.c
@@ -34,7 +34,7 @@ static void *delta_against(void *buf, un
if (!otherbuf)
die("unable to read %s", sha1_to_hex(entry->delta->sha1));
delta_buf = diff_delta(otherbuf, othersize,
- buf, size, &delta_size, ~0UL);
+ buf, size, &delta_size, 0);
if (!delta_buf || delta_size != entry->delta_size)
die("delta size changed");
free(buf);
diff --git a/patch-delta.c b/patch-delta.c
--- a/patch-delta.c
+++ b/patch-delta.c
@@ -20,36 +20,20 @@ void *patch_delta(void *src_buf, unsigne
const unsigned char *data, *top;
unsigned char *dst_buf, *out, cmd;
unsigned long size;
- int i;
- /* the smallest delta size possible is 4 bytes */
- if (delta_size < 4)
+ if (delta_size < DELTA_SIZE_MIN)
return NULL;
data = delta_buf;
top = delta_buf + delta_size;
/* make sure the orig file size matches what we expect */
- cmd = *data++;
- size = cmd & ~0x80;
- i = 7;
- while (cmd & 0x80) {
- cmd = *data++;
- size |= (cmd & ~0x80) << i;
- i += 7;
- }
+ size = get_delta_hdr_size(&data);
if (size != src_size)
return NULL;
/* now the result size */
- cmd = *data++;
- size = cmd & ~0x80;
- i = 7;
- while (cmd & 0x80) {
- cmd = *data++;
- size |= (cmd & ~0x80) << i;
- i += 7;
- }
+ size = get_delta_hdr_size(&data);
dst_buf = malloc(size);
if (!dst_buf)
return NULL;
diff --git a/sha1_file.c b/sha1_file.c
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -592,22 +592,6 @@ void * unpack_sha1_file(void *map, unsig
return unpack_sha1_rest(&stream, hdr, *size);
}
-static unsigned long parse_delta_size(unsigned char **p)
-{
- unsigned char c;
- unsigned long size = 0;
- unsigned shift = 0;
- unsigned char *data = *p;
-
- do {
- c = *data++;
- size += (c & 0x7f) << shift;
- shift += 7;
- } while (c & 0x80);
- *p = data;
- return size;
-}
-
static int packed_delta_info(unsigned char *base_sha1,
unsigned long delta_size,
unsigned long left,
@@ -645,11 +629,12 @@ static int packed_delta_info(unsigned ch
* the result size. Verify the base size while we are at it.
*/
data = delta_head;
- verify_base_size = parse_delta_size(&data);
- result_size = parse_delta_size(&data);
+ verify_base_size = get_delta_hdr_size(&data);
if (verify_base_size != base_size)
die("delta base size mismatch");
+ /* Read the result size */
+ result_size = get_delta_hdr_size(&data);
*sizep = result_size;
return 0;
}
^ permalink raw reply
* Re: [PATCH] assorted delta code cleanup
From: Matthias Urlichs @ 2005-06-29 7:10 UTC (permalink / raw)
To: git
In-Reply-To: <Pine.LNX.4.63.0506290246130.1667@localhost.localdomain>
Hi, Nicolas Pitre wrote:
> This is a wrap-up patch including all the cleanups I've done to the
> delta code and its usage.
FWIW, I think Linus said he'd prefer a do/while loop. In fact this old
code ...
> -static unsigned long parse_delta_size(unsigned char **p)
> -{
> - unsigned char c;
> - unsigned long size = 0;
> - unsigned shift = 0;
> - unsigned char *data = *p;
> -
> - do {
> - c = *data++;
> - size += (c & 0x7f) << shift;
> - shift += 7;
> - } while (c & 0x80);
> - *p = data;
> - return size;
> -}
> -
... is IMHO much more readable than this ...
> +static inline unsigned long get_delta_hdr_size(const unsigned char **datap)
> +{
> + const unsigned char *data = *datap;
> + unsigned char cmd = *data++;
> + unsigned long size = cmd & ~0x80;
> + int i = 7;
> + while (cmd & 0x80) {
> + cmd = *data++;
> + size |= (cmd & ~0x80) << i;
> + i += 7;
> + }
> + *datap = data;
> + return size;
> +}
> +
(except that the first += should be |= ).
Yeah, I know, it's nitpicking. Regard it as a testimony to the code's
quality that I didn't see worse. ;-)
NB, it might be a good idea to exit with an error if the shift exceeds 31.
--
Matthias Urlichs | {M:U} IT Design @ m-u-it.de | smurf@smurf.noris.de
Disclaimer: The quote was selected randomly. Really. | http://smurf.noris.de
- -
When it's fall in New York, the air smells as if someone's been frying
goats in it, and if you are keen to breathe the best plan is to open a
window and stick your head in a building.
-- The Hitch Hiker's Guide to the Galaxy
^ permalink raw reply
* Last mile for 1.0 again
From: Junio C Hamano @ 2005-06-29 7:16 UTC (permalink / raw)
To: git; +Cc: Linus Torvalds
In-Reply-To: <Pine.LNX.4.58.0506282252001.14331@ppc970.osdl.org>
>>>>> "LT" == Linus Torvalds <torvalds@osdl.org> writes:
LT> ..., but please don't
LT> change that, since that would be yet another incompatible pack format
LT> change, and I'd like to calm things down.
Calming things down is good. Some stuff we should be looking at
during calming down period are:
- Ask expert help to verify our use of zlib is OK; Linus asked
for this in a message tonight. I am not qualified to do this
myself.
- There is currently nothing that verifies the SHA1 sums
embedded in idx and pack files. A tool for it is needed
[*1*]. I may be doing this myself.
- Although there is a hook in sha1_file() that allows mapping a
pack file on demand (and keep track of their uses for LRU
eviction), there is currently no code to actually evict a
mapped pack file. We need to add some sort of use-reference
counting to lock down a pack file in use, if somebody is
inclined to do this. I will not be doing this myself
immediately.
- Think about what to do with the "dumb server" pull protocols
regarding packed archive files (think about http-pull). I
will not be doing this myself.
- Design "smart server" pull/push pair. This can independently
and actively be done without impacting "calming down" period,
and I am assuming Dan is already looking into this. This may
result in another "deathmatch" between Dan and Jason
Mcmullan, which would be fun to watch ;-).
Not limited to "calming down the packed GIT", but bigger picture
pre-release preparation items I think we need to look into are:
- Update "Last mile for 1.0" list. I think the only thing that
is left from the one I posted on Jun 5th is the tutorials.
Anybody interested in adding more pages to the tutorials? If
there is no taker, I may start continuing what is already
there, stealing example project outline from "Arch meets
Hello World" [*2*].
- Double check the documentation, usage strings in the code and
what the code does still match. Last time I checked, I think
some files in Documentation/ directory were not reachable
from the main GIT(1) page and were not touched by the build
process from Documentation/Makefile. I will not be doing
this myself.
- Blame/Annotate. Does anybody have a fast and correct one
[*3*]?
- Publicity. Somebody may want to start preparing materials to
have us included in http://better-scm.berlios.de/comparison/
[Footnotes]
*1* One possibility is to add that to fsck-cache when --full is
used, but I am somewhat reluctant to do it that way. It would
require you to place that "suspicious" pack under your
repository's .git/objects/pack in order to verify it, which can
spread the damage before you know it.
In general, idx file is relatively small, so we _could_ check
the SHA1 sum for idx files when we map them at runtime in
check_packed_git_idx(). However, verifying 1MB (kernel archive
case) idx file every time we run a single core GIT command may
be a price we would not want to pay, so I am not too
enthusiastic about doing it. We certainly would not want to do
this for pack files every time a pack is mapped at runtime (60MB
kernel archive case).
*2* http://regexps.srparish.net/www/tutorial/html/arch.html.
I am not talking about stealing the concepts but just talking
about stealing the sample project on which example players work
in the sample sessions. I think its section on update/replay
matches closely to merge/rebase, for example.
*3* I have what I wrote in Perl which does rename/copy/rewrite
and multiple parents correctly, but it is way too slow for
on-demand use.
^ permalink raw reply
* [PATCH] Fixlets on top of Nico's clean-up.
From: Junio C Hamano @ 2005-06-29 7:32 UTC (permalink / raw)
To: Nicolas Pitre; +Cc: Linus Torvalds, git
In-Reply-To: <Pine.LNX.4.63.0506290246130.1667@localhost.localdomain>
Do we care about a corner case where delta_size is not smaller
than 4 bytes, *datap passed to get_delta_hdr_size() points at
some bytes before a mmaped page boundary, and all the bytes til
the end of that page have high-bit set? The said inline
function would step over the page boundary and would presumably
segfault/sigbus.
Other than that, with the attached patch on top of it, I find
your clean-up very pleasant to read. Please consider it acked.
------------
If we prefer 0 as maxsize for diff_delta() to say "unlimited",
let's be consistent about it.
This patch also fixes type mismatch in a call to get_delta_hdr_size()
from packed_delta_info().
Signed-off-by: Junio C Hamano <junkio@cox.net>
---
diffcore-break.c | 2 +-
sha1_file.c | 11 ++++-------
test-delta.c | 2 +-
3 files changed, 6 insertions(+), 9 deletions(-)
diff --git a/diffcore-break.c b/diffcore-break.c
--- a/diffcore-break.c
+++ b/diffcore-break.c
@@ -65,7 +65,7 @@ static int should_break(struct diff_file
delta = diff_delta(src->data, src->size,
dst->data, dst->size,
- &delta_size, ~0UL);
+ &delta_size, 0);
/* Estimate the edit size by interpreting delta. */
if (count_delta(delta, delta_size,
diff --git a/sha1_file.c b/sha1_file.c
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -598,9 +598,9 @@ static int packed_delta_info(unsigned ch
char *type,
unsigned long *sizep)
{
- unsigned char *data;
+ const unsigned char *data;
unsigned char delta_head[64];
- unsigned long data_size, result_size, base_size, verify_base_size;
+ unsigned long result_size, base_size, verify_base_size;
z_stream stream;
int st;
@@ -609,13 +609,10 @@ static int packed_delta_info(unsigned ch
if (sha1_object_info(base_sha1, type, &base_size))
die("cannot get info for delta-pack base");
- data = base_sha1 + 20;
- data_size = left - 20;
-
memset(&stream, 0, sizeof(stream));
- stream.next_in = data;
- stream.avail_in = data_size;
+ data = stream.next_in = base_sha1 + 20;
+ stream.avail_in = left - 20;
stream.next_out = delta_head;
stream.avail_out = sizeof(delta_head);
diff --git a/test-delta.c b/test-delta.c
--- a/test-delta.c
+++ b/test-delta.c
@@ -61,7 +61,7 @@ int main(int argc, char *argv[])
if (argv[1][1] == 'd')
out_buf = diff_delta(from_buf, from_size,
data_buf, data_size,
- &out_size, ~0UL);
+ &out_size, 0);
else
out_buf = patch_delta(from_buf, from_size,
data_buf, data_size,
------------------------------------------------
^ permalink raw reply
* [PATCH] Add git-verify-pack command.
From: Junio C Hamano @ 2005-06-29 9:51 UTC (permalink / raw)
To: Linus Torvalds; +Cc: git
In-Reply-To: <7v4qbhfxad.fsf_-_@assigned-by-dhcp.cox.net>
Given a list of <pack>.idx files, this command validates the
index file and the corresponding .pack file for consistency.
This patch also uses the same validation mechanism in fsck-cache
when the --full flag is used.
During normal operation, sha1_file.c verifies that a given .idx
file matches the .pack file by comparing the SHA1 checksum
stored in .idx file and .pack file as a minimum sanity check.
We may further want to check the pack signature and version when
we map the pack, but that would be a separate patch.
Earlier, errors to map a pack file was not flagged fatal but led
to a random fatal error later. This version explicitly die()s
when such an error is detected.
Signed-off-by: Junio C Hamano <junkio@cox.net>
---
*** This actually covers two of the items I listed in the "Last
*** mile again" message. I ended up doing the LRU myself.
*** Linus, what would be the practical/recommended/sane limit
*** for mmap regions we would want to use? Currently I start
*** throwing away mapped packs when the total size of mapped
*** packs exceeds 64MB.
*** Currently, pack-objects/unpack-objects pair is not
*** documented. Any takers?
*** Oh, again, Linus, what is your preferred way to get a
*** cover-letter material (like this) for a single patch? I
*** always do a separate cover letter for multi-patch series
*** with [PATCH 0/N], but do you prefer to get [PATCH 0/1] and
*** [PATCH 1/1] for a single patch, or do you prefer to see
*** commentaries after the three-dash line like this message
*** does?
Documentation/git-verify-pack.txt | 38 +++++++++++++++++++++
Documentation/git.txt | 3 ++
Makefile | 5 ++-
cache.h | 4 ++
fsck-cache.c | 5 +++
pack.h | 2 +
sha1_file.c | 66 ++++++++++++++++++++++++++++---------
t/t5300-pack-object.sh | 38 +++++++++++++++++++++
verify-pack.c | 26 +++++++++++++++
verify_pack.c | 62 +++++++++++++++++++++++++++++++++++
10 files changed, 231 insertions(+), 18 deletions(-)
create mode 100644 Documentation/git-verify-pack.txt
create mode 100644 verify-pack.c
create mode 100644 verify_pack.c
8ac4f374a587c58b3d2ecc44220b6b866f769350
diff --git a/Documentation/git-verify-pack.txt b/Documentation/git-verify-pack.txt
new file mode 100644
--- /dev/null
+++ b/Documentation/git-verify-pack.txt
@@ -0,0 +1,38 @@
+git-verify-pack(1)
+==================
+v0.1, June 2005
+
+NAME
+----
+git-verify-pack - Validate packed GIT archive files.
+
+
+SYNOPSIS
+--------
+'git-verify-pack' <pack>.idx ...
+
+
+DESCRIPTION
+-----------
+Reads given idx file for packed GIT archive created with
+git-pack-objects command and verifies idx file and the
+corresponding pack file.
+
+OPTIONS
+-------
+<pack>.idx ...::
+ The idx files to verify.
+
+
+Author
+------
+Written by Junio C Hamano <junkio@cox.net>
+
+Documentation
+--------------
+Documentation by Junio C Hamano
+
+GIT
+---
+Part of the link:git.html[git] suite
+
diff --git a/Documentation/git.txt b/Documentation/git.txt
--- a/Documentation/git.txt
+++ b/Documentation/git.txt
@@ -110,6 +110,9 @@ link:git-tar-tree.html[git-tar-tree]::
link:git-unpack-file.html[git-unpack-file]::
Creates a temporary file with a blob's contents
+link:git-verify-pack.html[git-verify-pack]::
+ Validates packed GIT archive files
+
The interrogate commands may create files - and you can force them to
touch the working file set - but in general they don't
diff --git a/Makefile b/Makefile
--- a/Makefile
+++ b/Makefile
@@ -36,7 +36,7 @@ PROG= git-update-cache git-diff-files
git-diff-helper git-tar-tree git-local-pull git-write-blob \
git-get-tar-commit-id git-apply git-stripspace \
git-cvs2git git-diff-stages git-rev-parse git-patch-id \
- git-pack-objects git-unpack-objects
+ git-pack-objects git-unpack-objects git-verify-pack
all: $(PROG)
@@ -45,7 +45,7 @@ install: $(PROG) $(SCRIPTS)
LIB_OBJS=read-cache.o sha1_file.o usage.o object.o commit.o tree.o blob.o \
tag.o date.o index.o diff-delta.o patch-delta.o entry.o \
- epoch.o refs.o csum-file.o
+ epoch.o refs.o csum-file.o verify_pack.o
LIB_FILE=libgit.a
LIB_H=cache.h object.h blob.h tree.h commit.h tag.h delta.h epoch.h csum-file.h pack.h
@@ -124,6 +124,7 @@ git-rev-parse: rev-parse.c
git-patch-id: patch-id.c
git-pack-objects: pack-objects.c
git-unpack-objects: unpack-objects.c
+git-verify-pack: verify-pack.c
git-http-pull: LIBS += -lcurl
git-rev-list: LIBS += -lssl
diff --git a/cache.h b/cache.h
--- a/cache.h
+++ b/cache.h
@@ -246,9 +246,13 @@ extern struct packed_git {
unsigned int *index_base;
void *pack_base;
unsigned int pack_last_used;
+ unsigned int pack_use_cnt;
char pack_name[0]; /* something like ".git/objects/pack/xxxxx.pack" */
} *packed_git;
extern void prepare_packed_git(void);
+extern int use_packed_git(struct packed_git *);
+extern void unuse_packed_git(struct packed_git *);
+extern struct packed_git *add_packed_git(char *, int);
extern int num_packed_objects(const struct packed_git *p);
extern int nth_packed_object_sha1(const struct packed_git *, int, unsigned char*);
diff --git a/fsck-cache.c b/fsck-cache.c
--- a/fsck-cache.c
+++ b/fsck-cache.c
@@ -6,6 +6,7 @@
#include "tree.h"
#include "blob.h"
#include "tag.h"
+#include "pack.h"
#define REACHABLE 0x0001
@@ -437,6 +438,10 @@ int main(int argc, char **argv)
alt_odb[j].name[-1] = '/';
}
prepare_packed_git();
+ for (p = packed_git; p; p = p->next)
+ /* verify gives error messages itself */
+ verify_pack(p);
+
for (p = packed_git; p; p = p->next) {
int num = num_packed_objects(p);
for (i = 0; i < num; i++) {
diff --git a/pack.h b/pack.h
--- a/pack.h
+++ b/pack.h
@@ -27,4 +27,6 @@ struct pack_header {
unsigned int hdr_entries;
};
+extern int verify_pack(struct packed_git *);
+
#endif
diff --git a/sha1_file.c b/sha1_file.c
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -302,7 +302,7 @@ static int check_packed_git_idx(const ch
index = idx_map;
/* check index map */
- if (idx_size < 4*256 + 20)
+ if (idx_size < 4*256 + 20 + 20)
return error("index file too small");
nr = 0;
for (i = 0; i < 256; i++) {
@@ -327,12 +327,29 @@ static int check_packed_git_idx(const ch
return 0;
}
-static void unuse_one_packed_git(void)
+static int unuse_one_packed_git(void)
{
- /* NOTYET */
+ struct packed_git *p, *lru = NULL;
+
+ for (p = packed_git; p; p = p->next) {
+ if (p->pack_use_cnt || !p->pack_base)
+ continue;
+ if (!lru || p->pack_last_used < lru->pack_last_used)
+ lru = p;
+ }
+ if (!lru)
+ return 0;
+ munmap(lru->pack_base, lru->pack_size);
+ lru->pack_base = NULL;
+ return 1;
+}
+
+void unuse_packed_git(struct packed_git *p)
+{
+ p->pack_use_cnt--;
}
-static int use_packed_git(struct packed_git *p)
+int use_packed_git(struct packed_git *p)
{
if (!p->pack_base) {
int fd;
@@ -340,28 +357,36 @@ static int use_packed_git(struct packed_
void *map;
pack_mapped += p->pack_size;
- while (PACK_MAX_SZ < pack_mapped)
- unuse_one_packed_git();
+ while (PACK_MAX_SZ < pack_mapped && unuse_one_packed_git())
+ ; /* nothing */
fd = open(p->pack_name, O_RDONLY);
if (fd < 0)
- return -1;
+ die("packfile %s cannot be opened", p->pack_name);
if (fstat(fd, &st)) {
close(fd);
- return -1;
+ die("packfile %s cannot be opened", p->pack_name);
}
if (st.st_size != p->pack_size)
- return -1;
+ die("packfile %s size mismatch.", p->pack_name);
map = mmap(NULL, p->pack_size, PROT_READ, MAP_PRIVATE, fd, 0);
close(fd);
if (map == MAP_FAILED)
- return -1;
+ die("packfile %s cannot be mapped.", p->pack_name);
p->pack_base = map;
+
+ /* Check if the pack file matches with the index file.
+ * this is cheap.
+ */
+ if (memcmp((char*)(p->index_base) + p->index_size - 40,
+ p->pack_base + p->pack_size - 20, 20))
+ die("packfile %s does not match index.", p->pack_name);
}
p->pack_last_used = pack_used_ctr++;
+ p->pack_use_cnt++;
return 0;
}
-static struct packed_git *add_packed_git(char *path, int path_len)
+struct packed_git *add_packed_git(char *path, int path_len)
{
struct stat st;
struct packed_git *p;
@@ -388,6 +413,7 @@ static struct packed_git *add_packed_git
p->next = NULL;
p->pack_base = NULL;
p->pack_last_used = 0;
+ p->pack_use_cnt = 0;
return p;
}
@@ -671,6 +697,7 @@ static int packed_object_info(struct pac
unsigned long offset, size, left;
unsigned char *pack;
enum object_type kind;
+ int retval;
if (use_packed_git(p))
die("cannot map packed file");
@@ -681,8 +708,9 @@ static int packed_object_info(struct pac
switch (kind) {
case OBJ_DELTA:
- return packed_delta_info(pack, size, left, type, sizep);
- break;
+ retval = packed_delta_info(pack, size, left, type, sizep);
+ unuse_packed_git(p);
+ return retval;
case OBJ_COMMIT:
strcpy(type, "commit");
break;
@@ -699,6 +727,7 @@ static int packed_object_info(struct pac
die("corrupted pack file");
}
*sizep = size;
+ unuse_packed_git(p);
return 0;
}
@@ -785,6 +814,7 @@ static void *unpack_entry(struct pack_en
unsigned long offset, size, left;
unsigned char *pack;
enum object_type kind;
+ void *retval;
if (use_packed_git(p))
die("cannot map packed file");
@@ -794,7 +824,9 @@ static void *unpack_entry(struct pack_en
left = p->pack_size - offset;
switch (kind) {
case OBJ_DELTA:
- return unpack_delta_entry(pack, size, left, type, sizep);
+ retval = unpack_delta_entry(pack, size, left, type, sizep);
+ unuse_packed_git(p);
+ return retval;
case OBJ_COMMIT:
strcpy(type, "commit");
break;
@@ -811,12 +843,14 @@ static void *unpack_entry(struct pack_en
die("corrupted pack file");
}
*sizep = size;
- return unpack_non_delta_entry(pack, size, left);
+ retval = unpack_non_delta_entry(pack, size, left);
+ unuse_packed_git(p);
+ return retval;
}
int num_packed_objects(const struct packed_git *p)
{
- /* See check_packed_git_idx and pack-objects.c */
+ /* See check_packed_git_idx() */
return (p->index_size - 20 - 20 - 4*256) / 24;
}
diff --git a/t/t5300-pack-object.sh b/t/t5300-pack-object.sh
--- a/t/t5300-pack-object.sh
+++ b/t/t5300-pack-object.sh
@@ -127,4 +127,42 @@ test_expect_success \
} >current &&
diff expect current'
+unset GIT_OBJECT_DIRECTORY
+
+test_expect_success \
+ 'verify pack' \
+ 'git-verify-pack test-1.idx test-2.idx'
+
+test_expect_success \
+ 'corrupt a pack and see if verify catches' \
+ 'cp test-1.idx test-3.idx &&
+ cp test-2.pack test-3.pack &&
+ if git-verify-pack test-3.idx
+ then false
+ else :;
+ fi &&
+
+ cp test-1.pack test-3.pack &&
+ dd if=/dev/zero of=test-3.pack count=1 bs=1 conv=notrunc seek=2 &&
+ if git-verify-pack test-3.idx
+ then false
+ else :;
+ fi &&
+
+ cp test-1.pack test-3.pack &&
+ dd if=/dev/zero of=test-3.pack count=1 bs=1 conv=notrunc seek=7 &&
+ if git-verify-pack test-3.idx
+ then false
+ else :;
+ fi &&
+
+ cp test-1.pack test-3.pack &&
+ dd if=/dev/zero of=test-3.pack count=1 bs=1 conv=notrunc seek=12 &&
+ if git-verify-pack test-3.idx
+ then false
+ else :;
+ fi &&
+
+ :'
+
test_done
diff --git a/verify-pack.c b/verify-pack.c
new file mode 100644
--- /dev/null
+++ b/verify-pack.c
@@ -0,0 +1,26 @@
+#include "cache.h"
+#include "pack.h"
+
+static int verify_one_pack(char *arg)
+{
+ struct packed_git *g = add_packed_git(arg, strlen(arg));
+ if (!g)
+ return -1;
+ return verify_pack(g);
+}
+
+int main(int ac, char **av)
+{
+ int errs = 0;
+
+ while (1 < ac) {
+ char path[PATH_MAX];
+ strcpy(path, av[1]);
+ if (verify_one_pack(path))
+ errs++;
+ else
+ printf("%s: OK\n", av[1]);
+ ac--; av++;
+ }
+ return !!errs;
+}
diff --git a/verify_pack.c b/verify_pack.c
new file mode 100644
--- /dev/null
+++ b/verify_pack.c
@@ -0,0 +1,62 @@
+#include "cache.h"
+#include "pack.h"
+
+static int verify_packfile(struct packed_git *p)
+{
+ unsigned long index_size = p->index_size;
+ void *index_base = p->index_base;
+ SHA_CTX ctx;
+ unsigned char sha1[20];
+ unsigned long pack_size = p->pack_size;
+ void *pack_base;
+ struct pack_header *hdr;
+ int nr_objects;
+
+ hdr = p->pack_base;
+ if (hdr->hdr_signature != htonl(PACK_SIGNATURE))
+ return error("Packfile signature mismatch", p->pack_name);
+ if (hdr->hdr_version != htonl(PACK_VERSION))
+ return error("Packfile version %d different from ours %d",
+ ntohl(hdr->hdr_version), PACK_VERSION);
+ nr_objects = ntohl(hdr->hdr_entries);
+ if (num_packed_objects(p) != nr_objects)
+ return error("Packfile claims to have %d objects, "
+ "while idx size expects %d", nr_objects,
+ num_packed_objects(p));
+
+ SHA1_Init(&ctx);
+ pack_base = p->pack_base;
+ SHA1_Update(&ctx, pack_base, pack_size - 20);
+ SHA1_Final(sha1, &ctx);
+ if (memcmp(sha1, index_base + index_size - 40, 20))
+ return error("Packfile %s SHA1 mismatch with idx",
+ p->pack_name);
+ if (memcmp(sha1, pack_base + pack_size - 20, 20))
+ return error("Packfile %s SHA1 mismatch with itself",
+ p->pack_name);
+ return 0;
+}
+
+
+int verify_pack(struct packed_git *p)
+{
+ unsigned long index_size = p->index_size;
+ void *index_base = p->index_base;
+ SHA_CTX ctx;
+ unsigned char sha1[20];
+ int ret;
+
+ /* Verify SHA1 sum of the index file */
+ SHA1_Init(&ctx);
+ SHA1_Update(&ctx, index_base, index_size - 20);
+ SHA1_Final(sha1, &ctx);
+ if (memcmp(sha1, index_base + index_size - 20, 20))
+ return error("Packfile index for %s SHA1 mismatch",
+ p->pack_name);
+
+ /* Verify pack file */
+ use_packed_git(p);
+ ret = verify_packfile(p);
+ unuse_packed_git(p);
+ return ret;
+}
------------
^ permalink raw reply
* Re: Mercurial vs Updated git HOWTO for kernel hackers
From: Vojtech Pavlik @ 2005-06-29 10:27 UTC (permalink / raw)
To: Kyle Moffett
Cc: Sean, Matt Mackall, Petr Baudis, Christopher Li, Jeff Garzik,
Linux Kernel, Git Mailing List, mercurial
In-Reply-To: <12B6F9A5-81F8-46BD-A05D-B9FA1A70A9FF@mac.com>
On Tue, Jun 28, 2005 at 11:53:47PM -0400, Kyle Moffett wrote:
> On Jun 28, 2005, at 20:25:02, Sean wrote:
> >there will be a price to pay if the linux community fragments over
> >choice of scm.
>
> I don't agree. With the current set of SCMs, I don't think it will
> be long before somebody invents a gitweb/Mercurial/whatever gateway,
> such that I can "hg serve" from my Mercurial repository and have
> Linus "git pull" from a multiprotocol bridge.
I hope this will happen sooner than later, since that way the
competition between git and mercurial will give us the best tools while
keeping interoperability.
--
Vojtech Pavlik
SuSE Labs, SuSE CR
^ permalink raw reply
* [PATCH 1/1] Remove aliasing between COUNTED and UNINTERESTING flags used by rev-list.c
From: Jon Seymour @ 2005-06-29 14:16 UTC (permalink / raw)
To: git; +Cc: torvalds, jon.seymour
The COUNTED and UNINTERESTING flags were unintentionally aliased.
This patch makes sure that flags defined in rev-list.c have distinct
values from from those defined by epoch.h.
The aliasing may have caused unexpected behaviour of the git-rev-list
--bisect flag.
Signed-off-by: Jon Seymour <jon.seymour@gmail.com>
---
rev-list.c | 8 ++++----
1 files changed, 4 insertions(+), 4 deletions(-)
fb2676c0c3991666db9a3b0ebe222d8028c12a05
diff --git a/rev-list.c b/rev-list.c
--- a/rev-list.c
+++ b/rev-list.c
@@ -4,10 +4,10 @@
#include "blob.h"
#include "epoch.h"
-#define SEEN (1u << 0)
-#define INTERESTING (1u << 1)
-#define COUNTED (1u << 2)
-#define SHOWN (LAST_EPOCH_FLAG << 2)
+#define SEEN (LAST_EPOCH_FLAG << 1)
+#define INTERESTING (LAST_EPOCH_FLAG << 2)
+#define COUNTED (LAST_EPOCH_FLAG << 3)
+#define SHOWN (LAST_EPOCH_FLAG << 4)
static const char rev_list_usage[] =
"usage: git-rev-list [OPTION] commit-id <commit-id>\n"
------------
^ permalink raw reply
* Re: [PATCH] cvsimport: rewritten in Perl
From: Nicolas Pitre @ 2005-06-29 15:06 UTC (permalink / raw)
To: Matthias Urlichs; +Cc: git
In-Reply-To: <pan.2005.06.28.19.23.08.307486@smurf.noris.de>
On Tue, 28 Jun 2005, Matthias Urlichs wrote:
> I just got my machine blocked from a CVS server which didn't like
> to get hammered with connections.
>
> That was cvs2git's shell script. Which, by the way, is slow as hell.
>
> Appended: a git-cvsimport script, written in Perl, which directly talks
> to the CVS server. If the repository is local, it runs a "cvs server"
> child. It produces the same git repository as Linus' version. It can do
> incremental imports. And it's 20 times faster (on my system, with a
> local CVS repository).
Tried it on the bkcvs repository from
ftp.kernel.org/pub/scm/linux/kernel/bkcvs/linux-2.5/
(it can be retrieved with rsync as well)
Your script died after about 30 seconds with:
[...]
New scripts/lxdialog/Makefile: 0 bytes.
New scripts/lxdialog/checklist.c: 0 bytes.
New scripts/lxdialog/colors.h: 0 bytes.
New scripts/lxdialog/dialog.h: 0 bytes.
New scripts/lxdialog/inputbox.c: 0 bytes.
New scripts/lxdialog/lxdialog.c: 0 bytes.
New scripts/lxdialog/menubox.c: 0 bytes.
New scripts/lxdialog/msgbox.c: 0 bytes.
New scripts/lxdialog/textbox.c: 0 bytes.
New scripts/lxdialog/util.c: 0 bytes.
New scripts/lxdialog/yesno.c: 0 bytes.
Can't exec "git-update-cache": Argument list too long at /home/nico/bin/git-cvsimport-script line 402, <CVS> line 8254.
Cannot add files: -1
The original Linus version, although painfully slow, successfully
converts the whole thing after a couple hours.
Also aren't those "0 bytes" a bit suspicious?
Nicolas
^ permalink raw reply
* gitk and duplicate commits
From: Sven Verdoolaege @ 2005-06-29 15:55 UTC (permalink / raw)
To: Paul Mackerras; +Cc: git
If I pass in the same commit multiple times (possibly
under different names) to gitk,
it will echo the warning from git-rev-list
(sort_list_in_merge_order: duplicate commit 82e3583b9426c193659a4c1315d6a3b3cee11dce ignored)
and then simply stop.
Surely this can be handled more gracefully.
Note that I do want to be able to pass the same commit
multiple times. At least, I don't want to check
whether two "branches" happen to refer to the same commit.
skimo
^ permalink raw reply
* Re: [PATCH] Add git-verify-pack command.
From: Linus Torvalds @ 2005-06-29 16:15 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vvf3xcwyo.fsf_-_@assigned-by-dhcp.cox.net>
On Wed, 29 Jun 2005, Junio C Hamano wrote:
>
> *** Linus, what would be the practical/recommended/sane limit
> *** for mmap regions we would want to use? Currently I start
> *** throwing away mapped packs when the total size of mapped
> *** packs exceeds 64MB.
Hey, anybody who ever used BK had better have had 1GB of memory for any
real development. So 64MB is peanuts, but it sounds like a good guess.
> *** Oh, again, Linus, what is your preferred way to get a
> *** cover-letter material (like this) for a single patch?
I don't want cover-letters for single patches, or necessarily even for
short series (1-3 entries). The cover letter is more interesting for large
series, or even with small series if the early patches don't make much
sense on their own: then the cover-letter ends up being a useful place to
explain what the _sequence_ does, and why patch #2 that seems to be
totally useless and removes a feature is actually good ("we'll
re-implement it better in #5 after we've cleaned the code up").
So generally commentaries after the three dashes is good, if the
commentary is "local", ie related not to a series. Only with non-local
explanations does a separate [0/N] thing make sense to me.
Linus
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox