* [PATCH 0/9] more robustness against pack corruptions
From: Nicolas Pitre @ 2008-10-29 23:02 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
A few months ago I produced a set of patches to allow git to work even
in the presence of pack corruption given that the corrupted objects have
a good duplicate in the object store. Turns out that this work was
rather incomplete and covered only a limited set of cases.
This series extend coverage to all cases I could think about, and make
repack-objects able to create a good pack in such conditions to "fix"
the corruption without having to perform a full repack.
Yes, this is all about the small and trivial patch I posted a while ago
that I intended to repost with a test case. Well, the test failed
miserably, resulting in this series before it finally all passed. ;-)
builtin-pack-objects.c | 79 +++++++++++++++++-----
builtin-unpack-objects.c | 2 +
cache.h | 2 +-
index-pack.c | 2 +-
pack-revindex.c | 3 +-
sha1_file.c | 85 ++++++++++++++++++-----
t/t5302-pack-index.sh | 3 +-
t/t5303-pack-corruption-resilience.sh | 96 +++++++++++++++++++++++++--
8 files changed, 223 insertions(+), 49 deletions(-)
Nicolas
^ permalink raw reply
* [PATCH 1/9] close another possibility for propagating pack corruption
From: Nicolas Pitre @ 2008-10-29 23:02 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <1225321372-6570-1-git-send-email-nico@cam.org>
Abstract
--------
With index v2 we have a per object CRC to allow quick and safe reuse of
pack data when repacking. this, however, doesn't currently prevent a
stealth corruption from being propagated into a new pack when _not_
reusing pack data as demonstrated by the modification to t5302 included
here.
The Context
-----------
The Git database is all checksumed with SHA1 hashes. Any kind of
corruption can be confirmed by verifying this per object hash against
corresponding data. However this can be costly to perform systematically
and therefore this check is often not performed at run time when
accessing the object database.
First, the loose object format is entirely compressed with zlib which
already provide a CRC verification of its own when inflating data. Any
disk corruption would be caught already in this case.
Then, packed objects are also compressed with zlib but only for their
actual payload. The object headers and delta base references are not
deflated for obvious performance reasons, however this leave them
vulnerable to potentially undetected disk corruptions. Object types
are often validated against the expected type when they're requested,
and deflated size must always match the size recorded in the object header,
so those cases are pretty much covered as well.
Where corruptions could go unnoticed is in the delta base reference.
Of course, in the OBJ_REF_DELTA case, the odds for a SHA1 reference to
get corrupted so it actually matches the SHA1 of another object with the
same size (the delta header stores the expected size of the base object
to apply against) are virtually zero. In the OBJ_OFS_DELTA case, the
reference is a pack offset which would have to match the start boundary
of a different base object but still with the same size, and although this
is relatively much more "probable" than in the OBJ_REF_DELTA case, the
probability is also about zero in absolute terms. Still, the possibility
exists as demonstrated in t5302 and is certainly greater than a SHA1
collision, especially in the OBJ_OFS_DELTA case which is now the default
when repacking.
Again, repacking by reusing existing pack data is OK since the per object
CRC provided by index v2 guards against any such corruptions. What t5302
failed to test is a full repack in such case.
The Solution
------------
As unlikely as this kind of stealth corruption can be in practice, it
certainly isn't acceptable to propagate it into a freshly created pack.
But, because this is so unlikely, we don't want to pay the run time cost
associated with extra validation checks all the time either. Furthermore,
consequences of such corruption in anything but repacking should be rather
visible, and even if it could be quite unpleasant, it still has far less
severe consequences than actively creating bad packs.
So the best compromize is to check packed object CRC when unpacking
objects, and only during the compression/writing phase of a repack, and
only when not streaming the result. The cost of this is minimal (less
than 1% CPU time), and visible only with a full repack.
Someone with a stats background could provide an objective evaluation of
this, but I suspect that it's bad RAM that has more potential for data
corruptions at this point, even in those cases where this extra check
is not performed. Still, it is best to prevent a known hole for
corruption when recreating object data into a new pack.
What about the streamed pack case? Well, any client receiving a pack
must always consider that pack as untrusty and perform full validation
anyway, hence no such stealth corruption could be propagated to remote
repositoryes already. It is therefore worthless doing local validation
in that case.
Signed-off-by: Nicolas Pitre <nico@cam.org>
---
builtin-pack-objects.c | 12 ++++++++++++
sha1_file.c | 15 +++++++++++++++
t/t5302-pack-index.sh | 3 ++-
3 files changed, 29 insertions(+), 1 deletions(-)
diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c
index 59c30d1..0366277 100644
--- a/builtin-pack-objects.c
+++ b/builtin-pack-objects.c
@@ -1689,6 +1689,8 @@ static int add_ref_tag(const char *path, const unsigned char *sha1, int flag, vo
return 0;
}
+extern int do_check_packed_object_crc;
+
static void prepare_pack(int window, int depth)
{
struct object_entry **delta_list;
@@ -1697,6 +1699,16 @@ static void prepare_pack(int window, int depth)
get_object_details();
+ /*
+ * If we're locally repacking then we need to be doubly careful
+ * from now on in order to make sure no stealth corruption gets
+ * propagated to the new pack. Clients receiving streamed packs
+ * should validate everything they get anyway so no need to incure
+ * the additional cost here in that case.
+ */
+ if (!pack_to_stdout)
+ do_check_packed_object_crc = 1;
+
if (!nr_objects || !window || !depth)
return;
diff --git a/sha1_file.c b/sha1_file.c
index ab2b520..88d9cf3 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -1694,6 +1694,8 @@ static void *unpack_delta_entry(struct packed_git *p,
return result;
}
+int do_check_packed_object_crc;
+
void *unpack_entry(struct packed_git *p, off_t obj_offset,
enum object_type *type, unsigned long *sizep)
{
@@ -1701,6 +1703,19 @@ void *unpack_entry(struct packed_git *p, off_t obj_offset,
off_t curpos = obj_offset;
void *data;
+ if (do_check_packed_object_crc && p->index_version > 1) {
+ struct revindex_entry *revidx = find_pack_revindex(p, obj_offset);
+ unsigned long len = revidx[1].offset - obj_offset;
+ if (check_pack_crc(p, &w_curs, obj_offset, len, revidx->nr)) {
+ const unsigned char *sha1 =
+ nth_packed_object_sha1(p, revidx->nr);
+ error("bad packed object CRC for %s",
+ sha1_to_hex(sha1));
+ mark_bad_packed_object(p, sha1);
+ return NULL;
+ }
+ }
+
*type = unpack_object_header(p, &w_curs, &curpos, sizep);
switch (*type) {
case OBJ_OFS_DELTA:
diff --git a/t/t5302-pack-index.sh b/t/t5302-pack-index.sh
index b0b0fda..884e242 100755
--- a/t/t5302-pack-index.sh
+++ b/t/t5302-pack-index.sh
@@ -196,7 +196,8 @@ test_expect_success \
test_expect_success \
'[index v2] 5) pack-objects refuses to reuse corrupted data' \
- 'test_must_fail git pack-objects test-5 <obj-list'
+ 'test_must_fail git pack-objects test-5 <obj-list &&
+ test_must_fail git pack-objects --no-reuse-object test-6 <obj-list'
test_expect_success \
'[index v2] 6) verify-pack detects CRC mismatch' \
--
1.6.0.3.757.g01be.dirty
^ permalink raw reply related
* [PATCH 2/9] better validation on delta base object offsets
From: Nicolas Pitre @ 2008-10-29 23:02 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <1225321372-6570-2-git-send-email-nico@cam.org>
In one case, it was possible to have a bad offset equal to 0 effectively
pointing a delta onto itself and crashing git after too many recursions.
In the other cases, a negative offset could result due to off_t being
signed. Catch those.
Signed-off-by: Nicolas Pitre <nico@cam.org>
---
builtin-pack-objects.c | 4 ++--
builtin-unpack-objects.c | 2 ++
index-pack.c | 2 +-
sha1_file.c | 2 +-
4 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c
index 0366277..d4c721b 100644
--- a/builtin-pack-objects.c
+++ b/builtin-pack-objects.c
@@ -1038,10 +1038,10 @@ static void check_object(struct object_entry *entry)
c = buf[used_0++];
ofs = (ofs << 7) + (c & 127);
}
- if (ofs >= entry->in_pack_offset)
+ ofs = entry->in_pack_offset - ofs;
+ if (ofs <= 0 || ofs >= entry->in_pack_offset)
die("delta base offset out of bound for %s",
sha1_to_hex(entry->idx.sha1));
- ofs = entry->in_pack_offset - ofs;
if (reuse_delta && !entry->preferred_base) {
struct revindex_entry *revidx;
revidx = find_pack_revindex(p, ofs);
diff --git a/builtin-unpack-objects.c b/builtin-unpack-objects.c
index 9f4bdd3..47ed610 100644
--- a/builtin-unpack-objects.c
+++ b/builtin-unpack-objects.c
@@ -370,6 +370,8 @@ static void unpack_delta_entry(enum object_type type, unsigned long delta_size,
base_offset = (base_offset << 7) + (c & 127);
}
base_offset = obj_list[nr].offset - base_offset;
+ if (base_offset <= 0 || base_offset >= obj_list[nr].offset)
+ die("offset value out of bound for delta base object");
delta_data = get_data(delta_size);
if (dry_run || !delta_data) {
diff --git a/index-pack.c b/index-pack.c
index fe75332..60ed41a 100644
--- a/index-pack.c
+++ b/index-pack.c
@@ -338,7 +338,7 @@ static void *unpack_raw_entry(struct object_entry *obj, union delta_base *delta_
base_offset = (base_offset << 7) + (c & 127);
}
delta_base->offset = obj->idx.offset - base_offset;
- if (delta_base->offset >= obj->idx.offset)
+ if (delta_base->offset <= 0 || delta_base->offset >= obj->idx.offset)
bad_object(obj->idx.offset, "delta base offset is out of bound");
break;
case OBJ_COMMIT:
diff --git a/sha1_file.c b/sha1_file.c
index 88d9cf3..e57949b 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -1355,7 +1355,7 @@ static off_t get_delta_base(struct packed_git *p,
base_offset = (base_offset << 7) + (c & 127);
}
base_offset = delta_obj_offset - base_offset;
- if (base_offset >= delta_obj_offset)
+ if (base_offset <= 0 || base_offset >= delta_obj_offset)
return 0; /* out of bound */
*curpos += used;
} else if (type == OBJ_REF_DELTA) {
--
1.6.0.3.757.g01be.dirty
^ permalink raw reply related
* [PATCH 3/9] make unpack_object_header() non fatal
From: Nicolas Pitre @ 2008-10-29 23:02 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <1225321372-6570-3-git-send-email-nico@cam.org>
It is possible to have pack corruption in the object header. Currently
unpack_object_header() simply die() on them instead of letting the caller
deal with that gracefully.
So let's have unpack_object_header() return an error instead, and find
a better name for unpack_object_header_gently() in that context. All
callers of unpack_object_header() are ready for it.
Signed-off-by: Nicolas Pitre <nico@cam.org>
---
builtin-pack-objects.c | 2 +-
cache.h | 2 +-
sha1_file.c | 20 +++++++++++---------
3 files changed, 13 insertions(+), 11 deletions(-)
diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c
index d4c721b..9e249c9 100644
--- a/builtin-pack-objects.c
+++ b/builtin-pack-objects.c
@@ -1002,7 +1002,7 @@ static void check_object(struct object_entry *entry)
* We want in_pack_type even if we do not reuse delta
* since non-delta representations could still be reused.
*/
- used = unpack_object_header_gently(buf, avail,
+ used = unpack_object_header_buffer(buf, avail,
&entry->in_pack_type,
&entry->size);
diff --git a/cache.h b/cache.h
index a3c77f0..1a9edf3 100644
--- a/cache.h
+++ b/cache.h
@@ -751,7 +751,7 @@ extern const unsigned char *nth_packed_object_sha1(struct packed_git *, uint32_t
extern off_t nth_packed_object_offset(const struct packed_git *, uint32_t);
extern off_t find_pack_entry_one(const unsigned char *, struct packed_git *);
extern void *unpack_entry(struct packed_git *, off_t, enum object_type *, unsigned long *);
-extern unsigned long unpack_object_header_gently(const unsigned char *buf, unsigned long len, enum object_type *type, unsigned long *sizep);
+extern unsigned long unpack_object_header_buffer(const unsigned char *buf, unsigned long len, enum object_type *type, unsigned long *sizep);
extern unsigned long get_size_from_delta(struct packed_git *, struct pack_window **, off_t);
extern const char *packed_object_info_detail(struct packed_git *, off_t, unsigned long *, unsigned long *, unsigned int *, unsigned char *);
extern int matches_pack_name(struct packed_git *p, const char *name);
diff --git a/sha1_file.c b/sha1_file.c
index e57949b..7698177 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -1110,7 +1110,8 @@ static int legacy_loose_object(unsigned char *map)
return 0;
}
-unsigned long unpack_object_header_gently(const unsigned char *buf, unsigned long len, enum object_type *type, unsigned long *sizep)
+unsigned long unpack_object_header_buffer(const unsigned char *buf,
+ unsigned long len, enum object_type *type, unsigned long *sizep)
{
unsigned shift;
unsigned char c;
@@ -1122,10 +1123,10 @@ unsigned long unpack_object_header_gently(const unsigned char *buf, unsigned lon
size = c & 15;
shift = 4;
while (c & 0x80) {
- if (len <= used)
- return 0;
- if (sizeof(long) * 8 <= shift)
+ if (len <= used || sizeof(long) * 8 <= shift) {
+ error("bad object header");
return 0;
+ }
c = buf[used++];
size += (c & 0x7f) << shift;
shift += 7;
@@ -1164,7 +1165,7 @@ static int unpack_sha1_header(z_stream *stream, unsigned char *map, unsigned lon
* really worth it and we don't write it any longer. But we
* can still read it.
*/
- used = unpack_object_header_gently(map, mapsize, &type, &size);
+ used = unpack_object_header_buffer(map, mapsize, &type, &size);
if (!used || !valid_loose_object_type[type])
return -1;
map += used;
@@ -1411,10 +1412,11 @@ static int unpack_object_header(struct packed_git *p,
* insane, so we know won't exceed what we have been given.
*/
base = use_pack(p, w_curs, *curpos, &left);
- used = unpack_object_header_gently(base, left, &type, sizep);
- if (!used)
- die("object offset outside of pack file");
- *curpos += used;
+ used = unpack_object_header_buffer(base, left, &type, sizep);
+ if (!used) {
+ type = OBJ_BAD;
+ } else
+ *curpos += used;
return type;
}
--
1.6.0.3.757.g01be.dirty
^ permalink raw reply related
* [PATCH 4/9] make packed_object_info() resilient to pack corruptions
From: Nicolas Pitre @ 2008-10-29 23:02 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <1225321372-6570-4-git-send-email-nico@cam.org>
In the same spirit as commit 8eca0b47ff, let's try to survive a pack
corruption by making packed_object_info() able to fall back to alternate
packs or loose objects.
Signed-off-by: Nicolas Pitre <nico@cam.org>
---
sha1_file.c | 36 ++++++++++++++++++++++++++++++------
1 files changed, 30 insertions(+), 6 deletions(-)
diff --git a/sha1_file.c b/sha1_file.c
index 7698177..384a430 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -1314,8 +1314,10 @@ unsigned long get_size_from_delta(struct packed_git *p,
} while ((st == Z_OK || st == Z_BUF_ERROR) &&
stream.total_out < sizeof(delta_head));
inflateEnd(&stream);
- if ((st != Z_STREAM_END) && stream.total_out != sizeof(delta_head))
- die("delta data unpack-initial failed");
+ if ((st != Z_STREAM_END) && stream.total_out != sizeof(delta_head)) {
+ error("delta data unpack-initial failed");
+ return 0;
+ }
/* Examine the initial part of the delta to figure out
* the result size.
@@ -1382,15 +1384,29 @@ static int packed_delta_info(struct packed_git *p,
off_t base_offset;
base_offset = get_delta_base(p, w_curs, &curpos, type, obj_offset);
+ if (!base_offset)
+ return OBJ_BAD;
type = packed_object_info(p, base_offset, NULL);
+ if (type <= OBJ_NONE) {
+ struct revindex_entry *revidx = find_pack_revindex(p, base_offset);
+ const unsigned char *base_sha1 =
+ nth_packed_object_sha1(p, revidx->nr);
+ mark_bad_packed_object(p, base_sha1);
+ type = sha1_object_info(base_sha1, NULL);
+ if (type <= OBJ_NONE)
+ return OBJ_BAD;
+ }
/* We choose to only get the type of the base object and
* ignore potentially corrupt pack file that expects the delta
* based on a base with a wrong size. This saves tons of
* inflate() calls.
*/
- if (sizep)
+ if (sizep) {
*sizep = get_size_from_delta(p, w_curs, curpos);
+ if (*sizep == 0)
+ type = OBJ_BAD;
+ }
return type;
}
@@ -1500,8 +1516,9 @@ static int packed_object_info(struct packed_git *p, off_t obj_offset,
*sizep = size;
break;
default:
- die("pack %s contains unknown object type %d",
- p->pack_name, type);
+ error("unknown object type %i at offset %"PRIuMAX" in %s",
+ type, (uintmax_t)obj_offset, p->pack_name);
+ type = OBJ_BAD;
}
unuse_pack(&w_curs);
return type;
@@ -1971,7 +1988,14 @@ int sha1_object_info(const unsigned char *sha1, unsigned long *sizep)
if (!find_pack_entry(sha1, &e, NULL))
return status;
}
- return packed_object_info(e.p, e.offset, sizep);
+
+ status = packed_object_info(e.p, e.offset, sizep);
+ if (status < 0) {
+ mark_bad_packed_object(e.p, sha1);
+ status = sha1_object_info(sha1, sizep);
+ }
+
+ return status;
}
static void *read_packed_sha1(const unsigned char *sha1,
--
1.6.0.3.757.g01be.dirty
^ permalink raw reply related
* [PATCH 5/9] make check_object() resilient to pack corruptions
From: Nicolas Pitre @ 2008-10-29 23:02 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <1225321372-6570-5-git-send-email-nico@cam.org>
The check_object() function tries to get away with the least amount of
pack access possible when it already has partial information on given
object rather than calling the more costly packed_object_info().
When things don't look right, it should just give up and fall back to
packed_object_info() directly instead of die()'ing.
Signed-off-by: Nicolas Pitre <nico@cam.org>
---
builtin-pack-objects.c | 23 +++++++++++++++++------
1 files changed, 17 insertions(+), 6 deletions(-)
diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c
index 9e249c9..b595d04 100644
--- a/builtin-pack-objects.c
+++ b/builtin-pack-objects.c
@@ -1005,6 +1005,8 @@ static void check_object(struct object_entry *entry)
used = unpack_object_header_buffer(buf, avail,
&entry->in_pack_type,
&entry->size);
+ if (used == 0)
+ goto give_up;
/*
* Determine if this is a delta and if so whether we can
@@ -1016,6 +1018,8 @@ static void check_object(struct object_entry *entry)
/* Not a delta hence we've already got all we need. */
entry->type = entry->in_pack_type;
entry->in_pack_header_size = used;
+ if (entry->type < OBJ_COMMIT || entry->type > OBJ_BLOB)
+ goto give_up;
unuse_pack(&w_curs);
return;
case OBJ_REF_DELTA:
@@ -1032,16 +1036,20 @@ static void check_object(struct object_entry *entry)
ofs = c & 127;
while (c & 128) {
ofs += 1;
- if (!ofs || MSB(ofs, 7))
- die("delta base offset overflow in pack for %s",
- sha1_to_hex(entry->idx.sha1));
+ if (!ofs || MSB(ofs, 7)) {
+ error("delta base offset overflow in pack for %s",
+ sha1_to_hex(entry->idx.sha1));
+ goto give_up;
+ }
c = buf[used_0++];
ofs = (ofs << 7) + (c & 127);
}
ofs = entry->in_pack_offset - ofs;
- if (ofs <= 0 || ofs >= entry->in_pack_offset)
- die("delta base offset out of bound for %s",
- sha1_to_hex(entry->idx.sha1));
+ if (ofs <= 0 || ofs >= entry->in_pack_offset) {
+ error("delta base offset out of bound for %s",
+ sha1_to_hex(entry->idx.sha1));
+ goto give_up;
+ }
if (reuse_delta && !entry->preferred_base) {
struct revindex_entry *revidx;
revidx = find_pack_revindex(p, ofs);
@@ -1078,6 +1086,8 @@ static void check_object(struct object_entry *entry)
*/
entry->size = get_size_from_delta(p, &w_curs,
entry->in_pack_offset + entry->in_pack_header_size);
+ if (entry->size == 0)
+ goto give_up;
unuse_pack(&w_curs);
return;
}
@@ -1087,6 +1097,7 @@ static void check_object(struct object_entry *entry)
* with sha1_object_info() to find about the object type
* at this point...
*/
+ give_up:
unuse_pack(&w_curs);
}
--
1.6.0.3.757.g01be.dirty
^ permalink raw reply related
* [PATCH 6/9] make find_pack_revindex() aware of the nasty world
From: Nicolas Pitre @ 2008-10-29 23:02 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <1225321372-6570-6-git-send-email-nico@cam.org>
It currently calls die() whenever given offset is not found thinking
that such thing should never happen. But this offset may come from a
corrupted pack whych _could_ happen and not be found. Callers should
deal with this possibility gracefully instead.
Signed-off-by: Nicolas Pitre <nico@cam.org>
---
builtin-pack-objects.c | 2 ++
pack-revindex.c | 3 ++-
sha1_file.c | 18 ++++++++++++------
3 files changed, 16 insertions(+), 7 deletions(-)
diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c
index b595d04..963b432 100644
--- a/builtin-pack-objects.c
+++ b/builtin-pack-objects.c
@@ -1053,6 +1053,8 @@ static void check_object(struct object_entry *entry)
if (reuse_delta && !entry->preferred_base) {
struct revindex_entry *revidx;
revidx = find_pack_revindex(p, ofs);
+ if (!revidx)
+ goto give_up;
base_ref = nth_packed_object_sha1(p, revidx->nr);
}
entry->in_pack_header_size = used + used_0;
diff --git a/pack-revindex.c b/pack-revindex.c
index 6096b62..1de53c8 100644
--- a/pack-revindex.c
+++ b/pack-revindex.c
@@ -140,7 +140,8 @@ struct revindex_entry *find_pack_revindex(struct packed_git *p, off_t ofs)
else
lo = mi + 1;
} while (lo < hi);
- die("internal error: pack revindex corrupt");
+ error("bad offset for revindex");
+ return NULL;
}
void discard_revindex(void)
diff --git a/sha1_file.c b/sha1_file.c
index 384a430..9ce1df0 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -1388,9 +1388,12 @@ static int packed_delta_info(struct packed_git *p,
return OBJ_BAD;
type = packed_object_info(p, base_offset, NULL);
if (type <= OBJ_NONE) {
- struct revindex_entry *revidx = find_pack_revindex(p, base_offset);
- const unsigned char *base_sha1 =
- nth_packed_object_sha1(p, revidx->nr);
+ struct revindex_entry *revidx;
+ const unsigned char *base_sha1;
+ revidx = find_pack_revindex(p, base_offset);
+ if (!revidx)
+ return OBJ_BAD;
+ base_sha1 = nth_packed_object_sha1(p, revidx->nr);
mark_bad_packed_object(p, base_sha1);
type = sha1_object_info(base_sha1, NULL);
if (type <= OBJ_NONE)
@@ -1682,9 +1685,12 @@ static void *unpack_delta_entry(struct packed_git *p,
* This is costly but should happen only in the presence
* of a corrupted pack, and is better than failing outright.
*/
- struct revindex_entry *revidx = find_pack_revindex(p, base_offset);
- const unsigned char *base_sha1 =
- nth_packed_object_sha1(p, revidx->nr);
+ struct revindex_entry *revidx;
+ const unsigned char *base_sha1;
+ revidx = find_pack_revindex(p, base_offset);
+ if (!revidx)
+ return NULL;
+ base_sha1 = nth_packed_object_sha1(p, revidx->nr);
error("failed to read delta base object %s"
" at offset %"PRIuMAX" from %s",
sha1_to_hex(base_sha1), (uintmax_t)base_offset,
--
1.6.0.3.757.g01be.dirty
^ permalink raw reply related
* [PATCH 7/9] pack-objects: allow "fixing" a corrupted pack without a full repack
From: Nicolas Pitre @ 2008-10-29 23:02 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <1225321372-6570-7-git-send-email-nico@cam.org>
When the pack data to be reused is found to be bad, let's fall back to
full object access through the generic path which has its own strategies
to find alternate object sources in that case. This allows for "fixing"
a corrupted pack simply by copying either another pack containing the
object(s) found to be bad, or the loose object itself, into the object
store and launch a repack without the need for -f.
Signed-off-by: Nicolas Pitre <nico@cam.org>
---
builtin-pack-objects.c | 28 +++++++++++++++++++---------
1 files changed, 19 insertions(+), 9 deletions(-)
diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c
index 963b432..826c762 100644
--- a/builtin-pack-objects.c
+++ b/builtin-pack-objects.c
@@ -277,6 +277,7 @@ static unsigned long write_object(struct sha1file *f,
*/
if (!to_reuse) {
+ no_reuse:
if (!usable_delta) {
buf = read_sha1_file(entry->idx.sha1, &type, &size);
if (!buf)
@@ -358,20 +359,30 @@ static unsigned long write_object(struct sha1file *f,
struct revindex_entry *revidx;
off_t offset;
- if (entry->delta) {
+ if (entry->delta)
type = (allow_ofs_delta && entry->delta->idx.offset) ?
OBJ_OFS_DELTA : OBJ_REF_DELTA;
- reused_delta++;
- }
hdrlen = encode_header(type, entry->size, header);
+
offset = entry->in_pack_offset;
revidx = find_pack_revindex(p, offset);
datalen = revidx[1].offset - offset;
if (!pack_to_stdout && p->index_version > 1 &&
- check_pack_crc(p, &w_curs, offset, datalen, revidx->nr))
- die("bad packed object CRC for %s", sha1_to_hex(entry->idx.sha1));
+ check_pack_crc(p, &w_curs, offset, datalen, revidx->nr)) {
+ error("bad packed object CRC for %s", sha1_to_hex(entry->idx.sha1));
+ unuse_pack(&w_curs);
+ goto no_reuse;
+ }
+
offset += entry->in_pack_header_size;
datalen -= entry->in_pack_header_size;
+ if (!pack_to_stdout && p->index_version == 1 &&
+ check_pack_inflate(p, &w_curs, offset, datalen, entry->size)) {
+ error("corrupt packed object for %s", sha1_to_hex(entry->idx.sha1));
+ unuse_pack(&w_curs);
+ goto no_reuse;
+ }
+
if (type == OBJ_OFS_DELTA) {
off_t ofs = entry->idx.offset - entry->delta->idx.offset;
unsigned pos = sizeof(dheader) - 1;
@@ -383,21 +394,19 @@ static unsigned long write_object(struct sha1file *f,
sha1write(f, header, hdrlen);
sha1write(f, dheader + pos, sizeof(dheader) - pos);
hdrlen += sizeof(dheader) - pos;
+ reused_delta++;
} else if (type == OBJ_REF_DELTA) {
if (limit && hdrlen + 20 + datalen + 20 >= limit)
return 0;
sha1write(f, header, hdrlen);
sha1write(f, entry->delta->idx.sha1, 20);
hdrlen += 20;
+ reused_delta++;
} else {
if (limit && hdrlen + datalen + 20 >= limit)
return 0;
sha1write(f, header, hdrlen);
}
-
- if (!pack_to_stdout && p->index_version == 1 &&
- check_pack_inflate(p, &w_curs, offset, datalen, entry->size))
- die("corrupt packed object for %s", sha1_to_hex(entry->idx.sha1));
copy_pack_data(f, p, &w_curs, offset, datalen);
unuse_pack(&w_curs);
reused++;
@@ -1074,6 +1083,7 @@ static void check_object(struct object_entry *entry)
*/
entry->type = entry->in_pack_type;
entry->delta = base_entry;
+ entry->delta_size = entry->size;
entry->delta_sibling = base_entry->delta_child;
base_entry->delta_child = entry;
unuse_pack(&w_curs);
--
1.6.0.3.757.g01be.dirty
^ permalink raw reply related
* [PATCH 8/9] extend test coverage for latest pack corruption resilience improvements
From: Nicolas Pitre @ 2008-10-29 23:02 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <1225321372-6570-8-git-send-email-nico@cam.org>
Signed-off-by: Nicolas Pitre <nico@cam.org>
---
t/t5303-pack-corruption-resilience.sh | 96 ++++++++++++++++++++++++++++++---
1 files changed, 89 insertions(+), 7 deletions(-)
diff --git a/t/t5303-pack-corruption-resilience.sh b/t/t5303-pack-corruption-resilience.sh
index 31b20b2..ac181ea 100755
--- a/t/t5303-pack-corruption-resilience.sh
+++ b/t/t5303-pack-corruption-resilience.sh
@@ -41,11 +41,17 @@ create_new_pack() {
git verify-pack -v ${pack}.pack
}
+do_repack() {
+ pack=`printf "$blob_1\n$blob_2\n$blob_3\n" |
+ git pack-objects $@ .git/objects/pack/pack` &&
+ pack=".git/objects/pack/pack-${pack}"
+}
+
do_corrupt_object() {
ofs=`git show-index < ${pack}.idx | grep $1 | cut -f1 -d" "` &&
ofs=$(($ofs + $2)) &&
chmod +w ${pack}.pack &&
- dd if=/dev/zero of=${pack}.pack count=1 bs=1 conv=notrunc seek=$ofs &&
+ dd of=${pack}.pack count=1 bs=1 conv=notrunc seek=$ofs &&
test_must_fail git verify-pack ${pack}.pack
}
@@ -60,7 +66,7 @@ test_expect_success \
test_expect_success \
'create corruption in header of first object' \
- 'do_corrupt_object $blob_1 0 &&
+ 'do_corrupt_object $blob_1 0 < /dev/zero &&
test_must_fail git cat-file blob $blob_1 > /dev/null &&
test_must_fail git cat-file blob $blob_2 > /dev/null &&
test_must_fail git cat-file blob $blob_3 > /dev/null'
@@ -119,7 +125,7 @@ test_expect_success \
'create corruption in header of first delta' \
'create_new_pack &&
git prune-packed &&
- do_corrupt_object $blob_2 0 &&
+ do_corrupt_object $blob_2 0 < /dev/zero &&
git cat-file blob $blob_1 > /dev/null &&
test_must_fail git cat-file blob $blob_2 > /dev/null &&
test_must_fail git cat-file blob $blob_3 > /dev/null'
@@ -134,6 +140,15 @@ test_expect_success \
git cat-file blob $blob_3 > /dev/null'
test_expect_success \
+ '... and then a repack "clears" the corruption' \
+ 'do_repack &&
+ git prune-packed &&
+ git verify-pack ${pack}.pack &&
+ git cat-file blob $blob_1 > /dev/null &&
+ git cat-file blob $blob_2 > /dev/null &&
+ git cat-file blob $blob_3 > /dev/null'
+
+test_expect_success \
'create corruption in data of first delta' \
'create_new_pack &&
git prune-packed &&
@@ -153,10 +168,19 @@ test_expect_success \
git cat-file blob $blob_3 > /dev/null'
test_expect_success \
+ '... and then a repack "clears" the corruption' \
+ 'do_repack &&
+ git prune-packed &&
+ git verify-pack ${pack}.pack &&
+ git cat-file blob $blob_1 > /dev/null &&
+ git cat-file blob $blob_2 > /dev/null &&
+ git cat-file blob $blob_3 > /dev/null'
+
+test_expect_success \
'corruption in delta base reference of first delta (OBJ_REF_DELTA)' \
'create_new_pack &&
git prune-packed &&
- do_corrupt_object $blob_2 2 &&
+ do_corrupt_object $blob_2 2 < /dev/zero &&
git cat-file blob $blob_1 > /dev/null &&
test_must_fail git cat-file blob $blob_2 > /dev/null &&
test_must_fail git cat-file blob $blob_3 > /dev/null'
@@ -171,17 +195,75 @@ test_expect_success \
git cat-file blob $blob_3 > /dev/null'
test_expect_success \
- 'corruption in delta base reference of first delta (OBJ_OFS_DELTA)' \
+ '... and then a repack "clears" the corruption' \
+ 'do_repack &&
+ git prune-packed &&
+ git verify-pack ${pack}.pack &&
+ git cat-file blob $blob_1 > /dev/null &&
+ git cat-file blob $blob_2 > /dev/null &&
+ git cat-file blob $blob_3 > /dev/null'
+
+test_expect_success \
+ 'corruption #0 in delta base reference of first delta (OBJ_OFS_DELTA)' \
'create_new_pack --delta-base-offset &&
git prune-packed &&
- do_corrupt_object $blob_2 2 &&
+ do_corrupt_object $blob_2 2 < /dev/zero &&
git cat-file blob $blob_1 > /dev/null &&
test_must_fail git cat-file blob $blob_2 > /dev/null &&
test_must_fail git cat-file blob $blob_3 > /dev/null'
test_expect_success \
- '... and a redundant pack allows for full recovery too' \
+ '... but having a loose copy allows for full recovery' \
'mv ${pack}.idx tmp &&
+ git hash-object -t blob -w file_2 &&
+ mv tmp ${pack}.idx &&
+ git cat-file blob $blob_1 > /dev/null &&
+ git cat-file blob $blob_2 > /dev/null &&
+ git cat-file blob $blob_3 > /dev/null'
+
+test_expect_success \
+ '... and then a repack "clears" the corruption' \
+ 'do_repack --delta-base-offset &&
+ git prune-packed &&
+ git verify-pack ${pack}.pack &&
+ git cat-file blob $blob_1 > /dev/null &&
+ git cat-file blob $blob_2 > /dev/null &&
+ git cat-file blob $blob_3 > /dev/null'
+
+test_expect_success \
+ 'corruption #1 in delta base reference of first delta (OBJ_OFS_DELTA)' \
+ 'create_new_pack --delta-base-offset &&
+ git prune-packed &&
+ printf "\x01" | do_corrupt_object $blob_2 2 &&
+ git cat-file blob $blob_1 > /dev/null &&
+ test_must_fail git cat-file blob $blob_2 > /dev/null &&
+ test_must_fail git cat-file blob $blob_3 > /dev/null'
+
+test_expect_success \
+ '... but having a loose copy allows for full recovery' \
+ 'mv ${pack}.idx tmp &&
+ git hash-object -t blob -w file_2 &&
+ mv tmp ${pack}.idx &&
+ git cat-file blob $blob_1 > /dev/null &&
+ git cat-file blob $blob_2 > /dev/null &&
+ git cat-file blob $blob_3 > /dev/null'
+
+test_expect_success \
+ '... and then a repack "clears" the corruption' \
+ 'do_repack --delta-base-offset &&
+ git prune-packed &&
+ git verify-pack ${pack}.pack &&
+ git cat-file blob $blob_1 > /dev/null &&
+ git cat-file blob $blob_2 > /dev/null &&
+ git cat-file blob $blob_3 > /dev/null'
+
+test_expect_success \
+ '... and a redundant pack allows for full recovery too' \
+ 'do_corrupt_object $blob_2 2 < /dev/zero &&
+ git cat-file blob $blob_1 > /dev/null &&
+ test_must_fail git cat-file blob $blob_2 > /dev/null &&
+ test_must_fail git cat-file blob $blob_3 > /dev/null &&
+ mv ${pack}.idx tmp &&
git hash-object -t blob -w file_1 &&
git hash-object -t blob -w file_2 &&
printf "$blob_1\n$blob_2\n" | git pack-objects .git/objects/pack/pack &&
--
1.6.0.3.757.g01be.dirty
^ permalink raw reply related
* [PATCH 9/9] pack-objects: don't leak pack window reference when splitting packs
From: Nicolas Pitre @ 2008-10-29 23:02 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <1225321372-6570-9-git-send-email-nico@cam.org>
Signed-off-by: Nicolas Pitre <nico@cam.org>
---
builtin-pack-objects.c | 12 +++++++++---
1 files changed, 9 insertions(+), 3 deletions(-)
diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c
index 826c762..c93d69a 100644
--- a/builtin-pack-objects.c
+++ b/builtin-pack-objects.c
@@ -389,22 +389,28 @@ static unsigned long write_object(struct sha1file *f,
dheader[pos] = ofs & 127;
while (ofs >>= 7)
dheader[--pos] = 128 | (--ofs & 127);
- if (limit && hdrlen + sizeof(dheader) - pos + datalen + 20 >= limit)
+ if (limit && hdrlen + sizeof(dheader) - pos + datalen + 20 >= limit) {
+ unuse_pack(&w_curs);
return 0;
+ }
sha1write(f, header, hdrlen);
sha1write(f, dheader + pos, sizeof(dheader) - pos);
hdrlen += sizeof(dheader) - pos;
reused_delta++;
} else if (type == OBJ_REF_DELTA) {
- if (limit && hdrlen + 20 + datalen + 20 >= limit)
+ if (limit && hdrlen + 20 + datalen + 20 >= limit) {
+ unuse_pack(&w_curs);
return 0;
+ }
sha1write(f, header, hdrlen);
sha1write(f, entry->delta->idx.sha1, 20);
hdrlen += 20;
reused_delta++;
} else {
- if (limit && hdrlen + datalen + 20 >= limit)
+ if (limit && hdrlen + datalen + 20 >= limit) {
+ unuse_pack(&w_curs);
return 0;
+ }
sha1write(f, header, hdrlen);
}
copy_pack_data(f, p, &w_curs, offset, datalen);
--
1.6.0.3.757.g01be.dirty
^ permalink raw reply related
* Pull request for sub-tree merge into /contrib/gitstats
From: Sverre Rabbelier @ 2008-10-29 23:06 UTC (permalink / raw)
To: Junio C Hamano, Git Mailinglist
My work is available in the git repository at:
git://repo.or.cz/git-stats.git master
Please use a subtree merge to put this in contrib/gitstats.
David Symonds (2):
Insert the git_stats path to the start of os.sys.path, not 1 element in.
Fix some spelling mistakes.
Sverre Rabbelier (237):
Created a script to setup a repo to test metrics on.
Converted the setupRepo script to python to allow creating the same repo.
Added the use cases in text form.
Added a README.
Initial commit for notes branch
Changelog for 28-05-2008
Changelog for 29-05-2008
Changelog for 30-05-2008
Changelog for 31-05-2008
Created a script that aggregates author activity in one part of
the content.
Updated the script to handle merges and to treat '-' as 0.
Modified author.activityInArea to take a list of strings instead
of a file.
Added a module containing the wrappers for the commands.
Replaced funny characters in use-cases.txt with whitespace.
Created a script that handles index related activities.
Added a script to handle data mining related to commit history.
Created a 'touched' command in the wrapper module.
Added pathsTouched to the history module.
Created a commitTouched function and a prettyPrint helper function
Wrapped the use-cases.txt document at ~72 characters.
Created a list of metrics that can be used to indentify fixes.
Added three more data mining functions to the history module
Gave the setupRepo script some major TLC.
Added commitdiffEqual functionality to the history module.
Creating the repository in a temp directory, use tempfile.gettempdir().
Improved the metrics documentation.
Use 'os.path.join(a, b)' rather than 'a + os.sep + b'.
Removed try/except from setupRepo but added a manual raise
Use the same configurations as test-lib.sh in setupRepo
Copied the test-lib.sh script from git.git/t
Modified test-lib.sh to use ~/code/git as git dir.
Added a basic test that sets up our test repository.
Added a test that verifies the HEAD revision hash
There is now a testcase to check HEAD, don't do it in setupRepo
Added a basic dispatcher to stats.py.
Moved things around a lot, and made commitTouches fixup nonrelative paths
Added basic dispatching functionality for diff
Added a __init__.py file since every module needs one
Made stats.py add the directory containing git_stats to the path
Moved stats.py up one directory
We no longer require running from the root of the repository
Prefixed private methods with _ and and made parseFileDiff
accept larger diffs
Change 'PATHS' to 'COMMIT COMMIT' in the usage text
Finished the dispatching function for commit.
Moved stats.py back to the git_stats directory
Removed the $GIT_STATS_PATH related code
Removed extra newlines and replaced import with 'from .. import ..'
Moved stats.py back up again.
Moved diff.diffContains to commit.logContains
Removed the executable bit on author.py and index.py
Added dispatching to branch and finished branchList
Added dispatching to the author module
Added dispatching to the index module
Started on testing author.py and found the first bug so far
Bugfix for the author filter in author.py
Created a test case to point out an unimplemented function
Added a testcase to point out wrong output for 'stats.py branch -c'
Bugfix for the branchList output in branch.py
Finished the branch testcases, but no test for -r switch yet
The testcases require two patches to git-python to run successfully
Changelog for 01-06-2008
Changelog for 02-06-2008
Changelog for 02-06-2008 (2)
Changelog for 03-06-2008
Changelog for 04-06-2008
Changelog for 05-06-2008
Changelog for 06-06-2008
Changelog for 07-06-2008
Changelog for 08-06-2008
Changelog for 09-06-2008
Changelog for 10-06-2008
Made GitStats compatible with the latest version of git-python
Patches to git-python, since its latest changes, are no longer required
Added testcases for 'commit'
Changelog for 10-06-2008 (2)
Changelog for 11-06-2008
Bugfix for commitdiffMatches
Usability enhancement: print help msg on 'stats.py subcmd'
Merge branch 'notes'
Moved the changelog into the doc subdirectory
Changelog for 12-06-2008
Added testcases for diff and pointed out a bug and some output problems
Bugfix for diff -i, also removed debug output
Improved the dispatching in index.py
Added tests for index.py
Improved the test descriptions
Expanded the setupRepo script to generate a more elaborate tree
Made use of with_keep_cwd now that git-python runs from the top dir
Added tests for "commit -r" and pointed out a bug
Added a todo to check the file passed to "stats.py commit"
Stripped down test-lib.sh to the bits we need
Changelog for 13-06-2008
Changelog for 14-06-2008
Refactored the custom parser into it's own file
Bugfix for commit -t, now checks if the specified file is sane
Changelog for 15-06-2008
Bugfix, created a missing test for "diff -n"
Slight usability improvement
Expanded setupRepo and refactored 'addFile' into it's own function
Bugfix for the testsuite, added tests for 'index -a'
Changelog for 16-06-2008
Created a remote branch in the setupRepo script
Bugfix, added a testcase for "branch -r -c"
Changelog for 17-06-2008
Extended the description of the 'branch contains' metric
Refactored setupRepo a bit to make room for a metrics repo
Added a little more documentation on the branch metric
Extended setupRepo to create the repo as specified in metrics.txt
Updated the metrics repo to have two commits on the topic branch
Initial draft of the 'belongs to' metric
Typo, 'dilate' should be 'dilute'
Changelog for 19-06-2008
Described a more complex tree in metrics.txt
Added a bumpTime to make the commit history look a bit more sane
Changelog for 20-06-2008
Added with_exceptions=False to 'git rev-parse --verify'
Added a GIT_STATS_PRETTY_PRINT environmental variable
Added a '-v' option to stats.py
Added more tags to the generated test repo
Bail out when the testrepo couldn't be set up
Use the new GIT_STATS_PRETTY_PRINT and added tags features
When specifying kwargs they should be in quotes when not part of
a function call.
Use Repo(".").git instead of Git(".")
Updated the metrics repo to match the documented tree
Changelog for 24-06-2008
Changelog for 27-06-2008
Made the 'belongs to' metric recursive
Added tests for the 'belongs to' metric
First gather all parentage information, then run metrics
Changelog for 28-06-2008
Moved isUnique into parse.py so that other modules may use it
Added a "author -f" aggregation, and a test case.
Changelog for 29-06-2008
Minor cleanups, added documentation
Added a switch that lists only reverts of a specified commit
Changelog for 02-07-2008
Changelog for 02-07-2008
When parsing the parent listing take into account parentless commits
Print the usage help sorted and added some documentation
Use a stack-based approach instead of a recursive algorithm
Added a simple 'stats.py bug' command
Disable debug output, print minimum dilution
Optimizations to the 'belongs to' metric
Bugfix to only ignore if dilution was 'worse'
Optimion to the 'belongs to' metric
Don't filter out subsets in the belongs to metric
Optimization in the 'belongs to' metric
Changelog for 06-07-2008
Improved the output of 'branch -b' with '-d'
Added aggregation and showing all activity
Add a config parser and an example configuration
Bugfix to _parseFileDiff so that it handles diffs with mode changes only
Made use of the new config parser in bug.py
Changelog for 10-07-2008
When the specified path was not found, return an empty dict
Don't die when an option is not specified at all
Moved the commit information into a seperate class
Restructured the commit and diff module so that diff depends on commit
Refactored bug.py to allow for a trivial aggregation function
Refactored stats.py to use a main function
Allow for checking deleted files in commitsThatTouched
Check for empty diffs
Add a diff memory to diff.py and use it in bug.py
Don't look before we leap in getting the commit diff
Add a 'commits that touch' memory to commit.py
Two more memories were added and some refactoring
Print authors sorted in 'author -e'
Added an option to disable line numbering in diff parsing
Added a proof of concept matcher
Changelog for 12-07-2008
Add an option to ignore parent in the 'belongs to' metric
Added an option to limit the amount of commits checked
Allow specifying True, False, or None in the config file
Allow specifying True, False, or None on the command line
General cleanups in bug.py
Bugfix for branchContains Fix the branch
Make use of the ignore_parents option to belongsTo
Changelog for 14-07-2008
Provide a default empty dict to pretty names
Refactored bug.py to use a Memory and Options class
Make config.py more versatile
Teach config.py to read multiple verses
Introduce a bugfixRating to bug.py
Changelog for 15-07-2008
Refactoring, cleanups and documentation
Changelog for 16-07-2008
Updated the README to include specific installation instructions
Added a 'net loc' to 'author -a'
Updated the tests to match the new output of 'author -a'
Official 0.1.4 release of git-python
Removed the redundant setupRepo.sh script
Added an option to print the path for setupRepo.py
Added a way to specify the path to use instead of the default
Don't assume the temp path is /tmp
Make use of the 'sorted' built-in
Changelog for 17-07-2008
Added a unit-test framework for GitStats
Use 'key in dict' instead of 'dict.has_key(key)'
Renamed fileDiff to FileDiff
Bugfix for diff.py, don't die on empty diffs
Make use of getattr instead of just trying
Use format specifiers instead of appending to a string
Replace backslashes with parens to do line continuation
Don't asume setupRepo.py is executable
Remove unneeded executionable bit on setupRepo.py
Replace check_file with checkFile
Use dashed_form for variable names instead of camelCase
Changelog for 23-07-2008
Made config.read take a bunch of strings instead of a path
Improved testing.py output and removed manual parsing of arguments
Added a module to dispatch unit-testing commands
Hooked up the test suite in stats.py
Do not require lines to end with a '\n' in the config parser
Refactor config.py to make it more testable
Wrote unittests for the config module and hooked them up
Changelog for 24-07-2008
Don't require stats.py to be in $PATH when running the regression tests
Renamed README to INSTALL
Expanded the INSTALL file to include usage information
Added a README describing GitStats purpose
Convert matcher.py to use optparse
Add a description about the branch module
Add a description about the author module
Add a description about the commit module
Add a general note about the 'stats.py'
Add a description about the diff module
Add a description about the index module
Add a description about the matcher module
Add a description about the tests module
Renamed the GitStats to include a gitstats- prefix
Add a description about the bug module
Renamed Memory->GitCache and Options->OptionList
Expanded the documentation of the bug module
Ran ispell on all non gitstats-* files in doc/
Ran ispell on all gitstats-* files in doc/
Added some example values to gitstats-bug.txt
Bugfix for the config parsing mechanism
INSTALL | 33 ++
README | 59 ++++
doc/changelog.txt | 335 +++++++++++++++++++++
doc/gitstats-author.txt | 30 ++
doc/gitstats-branch.txt | 35 +++
doc/gitstats-bug.txt | 109 +++++++
doc/gitstats-commit.txt | 44 +++
doc/gitstats-diff.txt | 24 ++
doc/gitstats-index.txt | 19 ++
doc/gitstats-matcher.txt | 19 ++
doc/gitstats-stats.txt | 36 +++
doc/gitstats-tests.txt | 8 +
doc/metrics.txt | 83 ++++++
doc/use-cases.txt | 160 ++++++++++
src/git_stats/author.py | 315 ++++++++++++++++++++
src/git_stats/branch.py | 420 ++++++++++++++++++++++++++
src/git_stats/bug.py | 306 +++++++++++++++++++
src/git_stats/commit.py | 343 ++++++++++++++++++++++
src/git_stats/config | 5 +
src/git_stats/config.py | 219 ++++++++++++++
src/git_stats/config_tests.py | 245 +++++++++++++++
src/git_stats/diff.py | 515 ++++++++++++++++++++++++++++++++
src/git_stats/index.py | 102 +++++++
src/git_stats/matcher.py | 130 ++++++++
src/git_stats/parse.py | 109 +++++++
src/git_stats/testing.py | 212 +++++++++++++
src/git_stats/tests.py | 96 ++++++
src/scripts/setupRepo.py | 652 +++++++++++++++++++++++++++++++++++++++++
src/scripts/setupRepo.sh | 31 --
src/stats.py | 112 +++++++
src/t/t8100-stats.sh | 333 +++++++++++++++++++++
src/t/t8101-metrics.sh | 163 ++++++++++
src/t/test-lib.sh | 372 +++++++++++++++++++++++
33 files changed, 5643 insertions(+), 31 deletions(-)
create mode 100644 INSTALL
create mode 100644 README
create mode 100644 doc/changelog.txt
create mode 100644 doc/gitstats-author.txt
create mode 100644 doc/gitstats-branch.txt
create mode 100644 doc/gitstats-bug.txt
create mode 100644 doc/gitstats-commit.txt
create mode 100644 doc/gitstats-diff.txt
create mode 100644 doc/gitstats-index.txt
create mode 100644 doc/gitstats-matcher.txt
create mode 100644 doc/gitstats-stats.txt
create mode 100644 doc/gitstats-tests.txt
create mode 100644 doc/metrics.txt
create mode 100644 doc/use-cases.txt
create mode 100644 src/git_stats/__init__.py
create mode 100644 src/git_stats/author.py
create mode 100644 src/git_stats/branch.py
create mode 100644 src/git_stats/bug.py
create mode 100644 src/git_stats/commit.py
create mode 100644 src/git_stats/config
create mode 100644 src/git_stats/config.py
create mode 100644 src/git_stats/config_tests.py
create mode 100644 src/git_stats/diff.py
create mode 100644 src/git_stats/index.py
create mode 100644 src/git_stats/matcher.py
create mode 100644 src/git_stats/parse.py
create mode 100644 src/git_stats/testing.py
create mode 100755 src/git_stats/tests.py
create mode 100644 src/scripts/setupRepo.py
delete mode 100755 src/scripts/setupRepo.sh
create mode 100755 src/stats.py
create mode 100755 src/t/t8100-stats.sh
create mode 100755 src/t/t8101-metrics.sh
create mode 100644 src/t/test-lib.sh
^ permalink raw reply
* Re: jgit as a jira plugin
From: Shawn O. Pearce @ 2008-10-29 23:08 UTC (permalink / raw)
To: J. Longman; +Cc: git
In-Reply-To: <5915DAE3-7BDF-4296-9DB3-6FBEE504A317@xiplink.com>
"J. Longman" <longman@xiplink.com> wrote:
> I've integrated jgit into a plugin for the Jira Issue tracking system.
> There is more information here:
> http://confluence.atlassian.com/display/JIRAEXT/Jira+Git+Plugin
Cool!
> 1) I noticed that there is a maven pom file. Are you present in a maven
> repository? Also any problem with embedding a working snapshot in my
> plugin?
No, we aren't hosted in any repository yet. The pom file exists to
make it easier for people who prefer maven to build, but its not the
primary build system for jgit.
> 2) I'd like to find out the jgit way to achieve the equivalent of 'svn
> update'. I understand that fetch can do this but being new to git, I
> don't really understand quite what I need yet. The goal is to have git
> the latest commits from the origin before indexing.
Use a Transport instance to execute a default fetch (no args) on say
the "remote" origin. That will download the objects to the local
database, but it won't update a working directory. But I'm not sure
you would care about the working directory in the backend of Jira.
> Thanks for jgit - it took me a day or two to wrap my head around getting
> the list of files changed in a commit but otherwise its great to have
> something that can be integrated into jira.
Yea, about that, we wanted to write more tutorials on the API... ;-)
--
Shawn.
^ permalink raw reply
* Re: Pull request for sub-tree merge into /contrib/gitstats
From: Shawn O. Pearce @ 2008-10-29 23:12 UTC (permalink / raw)
To: sverre; +Cc: Junio C Hamano, Git Mailinglist
In-Reply-To: <bd6139dc0810291606o2efe4254me378335b76861340@mail.gmail.com>
Sverre Rabbelier <alturin@gmail.com> wrote:
> Please use a subtree merge to put this in contrib/gitstats.
Yea, about that...
> Sverre Rabbelier (237):
> Created a script to setup a repo to test metrics on.
> Converted the setupRepo script to python to allow creating the same repo.
> Added the use cases in text form.
> Added a README.
> Initial commit for notes branch
> Changelog for 28-05-2008
> Changelog for 29-05-2008
> Changelog for 30-05-2008
> Changelog for 31-05-2008
...
How is this going to look in the "What's in git.git" email?
We don't use ChangeLog files in git.git and we don't have
notes branches, and we already have a README.
Most stuff in contrib/ has its commit messages with a prefix string
to make it more clear when looking at the shortlog what is being
impacted. Maybe this should be re-written with filter-branch to
include a prefix before it merges.
--
Shawn.
^ permalink raw reply
* Re: Pull request for sub-tree merge into /contrib/gitstats
From: Nicolas Pitre @ 2008-10-29 23:31 UTC (permalink / raw)
To: sverre; +Cc: Junio C Hamano, Git Mailinglist
In-Reply-To: <bd6139dc0810291606o2efe4254me378335b76861340@mail.gmail.com>
On Thu, 30 Oct 2008, Sverre Rabbelier wrote:
> My work is available in the git repository at:
>
> git://repo.or.cz/git-stats.git master
>
> Please use a subtree merge to put this in contrib/gitstats.
[...]
Why do you have commits such as:
> Changelog for 01-06-2008
> Changelog for 02-06-2008
> Changelog for 02-06-2008 (2)
> Changelog for 03-06-2008
> Changelog for 04-06-2008
> Changelog for 05-06-2008
> Changelog for 06-06-2008
> Changelog for 07-06-2008
> Changelog for 08-06-2008
> Changelog for 09-06-2008
> Changelog for 10-06-2008
?
If those are not significant enough to have a proper description, then
I'd suggest you use 'git rebase -i' and its "squash" command to fold
them into the appropriate commit.
Nicolas
^ permalink raw reply
* Re: Pull request for sub-tree merge into /contrib/gitstats
From: Sverre Rabbelier @ 2008-10-29 23:38 UTC (permalink / raw)
To: Nicolas Pitre; +Cc: Junio C Hamano, Git Mailinglist
In-Reply-To: <alpine.LFD.2.00.0810291922320.13034@xanadu.home>
On Thu, Oct 30, 2008 at 00:31, Nicolas Pitre <nico@cam.org> wrote:
> If those are not significant enough to have a proper description, then
> I'd suggest you use 'git rebase -i' and its "squash" command to fold
> them into the appropriate commit.
They are there because the work was done incrementally, having them as
seperate commits shows when the changelog was modified. Having a
description for a change to the changelog is pretty senseless, since
it would be the same text as what is in the diff.
That said, sure, I can squash those commits no problem.
--
Cheers,
Sverre Rabbelier
^ permalink raw reply
* Re: Pull request for sub-tree merge into /contrib/gitstats
From: Sverre Rabbelier @ 2008-10-29 23:39 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: Junio C Hamano, Git Mailinglist
In-Reply-To: <20081029231251.GB31926@spearce.org>
On Thu, Oct 30, 2008 at 00:12, Shawn O. Pearce <spearce@spearce.org> wrote:
> Most stuff in contrib/ has its commit messages with a prefix string
> to make it more clear when looking at the shortlog what is being
> impacted. Maybe this should be re-written with filter-branch to
> include a prefix before it merges.
Sure, I'm fine with rewriting all commit messages to have a "gitstats:" prefix.
--
Cheers,
Sverre Rabbelier
^ permalink raw reply
* Re: jgit as a jira plugin
From: J. Longman @ 2008-10-29 23:49 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: git
In-Reply-To: <20081029230816.GA31926@spearce.org>
On 29-Oct-08, at 7:08 PM, Shawn O. Pearce wrote:
> "J. Longman" <longman@xiplink.com> wrote:
>> 2) I'd like to find out the jgit way to achieve the equivalent of
>> 'svn
>> update'. I understand that fetch can do this but being new to git, I
>> don't really understand quite what I need yet. The goal is to have
>> git
>> the latest commits from the origin before indexing.
> Use a Transport instance to execute a default fetch (no args) on say
> the "remote" origin. That will download the objects to the local
> database, but it won't update a working directory. But I'm not sure
> you would care about the working directory in the backend of Jira.
Basically I stole the pgm.Fetch code:
Transport tn = Transport.open(repository, "origin");
final FetchResult r;
List<RefSpec> toget = new ArrayList<RefSpec>();
try {
r = tn.fetch(new TextProgressMonitor(), toget);
} finally {
tn.close();
}
Can I assume that this enough to update the database? If so I think
I'm doing what you're suggesting. After this (and not shown) is some
logging code taken from Fetch, which results in the following:
From /Users/longman/workspace2/work/../masterRepo/
131dcf5..078d43f master -> origin/master
but there doesn't appear to be any specific mention of the incoming
changes.
>> Thanks for jgit - it took me a day or two to wrap my head around
>> getting
>> the list of files changed in a commit but otherwise its great to have
>> something that can be integrated into jira.
> Yea, about that, we wanted to write more tutorials on the API... ;-)
Well, the egit does provide some examples, just there's another API
involved which can be confusing. The code in the jira git plugin the
key classes are GitManagerImpl and RevisionIndexer, but the
RevisionIndexer has some Lucene (text search engine) API mixed-in.
Plus I'm still even learning about git much less the jgit api so I
can't vouch for quality or correctness ;-). I could snip some code
out and send it to you off-list for inclusion in the wiki maybe.
later, j
--
J. Longman
longman@xiplink.com
The information transmitted is intended only for the person or entity to
which it is addressed and may contain confidential and/or privileged
material. If you have received this in error, please contact the sender
and delete this communication and any copy immediately. Thank you.
^ permalink raw reply
* Re: jgit as a jira plugin
From: Shawn O. Pearce @ 2008-10-29 23:53 UTC (permalink / raw)
To: J. Longman; +Cc: git
In-Reply-To: <D7D18CE8-BDBD-430A-BCB6-D1BEFD21C949@xiplink.com>
"J. Longman" <longman@xiplink.com> wrote:
> Basically I stole the pgm.Fetch code:
>
> Transport tn = Transport.open(repository, "origin");
> final FetchResult r;
> List<RefSpec> toget = new ArrayList<RefSpec>();
> try {
> r = tn.fetch(new TextProgressMonitor(), toget);
> } finally {
> tn.close();
> }
>
> Can I assume that this enough to update the database?
Yes
> After this (and not shown) is some
> logging code taken from Fetch, which results in the following:
>
> From /Users/longman/workspace2/work/../masterRepo/
> 131dcf5..078d43f master -> origin/master
>
> but there doesn't appear to be any specific mention of the incoming
> changes.
Well, the new changes are what "git log 131dcf5..078d43f" outputs.
--
Shawn.
^ permalink raw reply
* [PATCH] git-filter-branch: Add an example on how to remove empty commits
From: Petr Baudis @ 2008-10-30 0:33 UTC (permalink / raw)
To: git; +Cc: Sverre Rabbelier
From: Sverre Rabbelier <srabbelier@gmail.com>
Signed-off-by: Sverre Rabbelier <srabbelier@gmail.com>
Signed-off-by: Petr Baudis <pasky@suse.cz>
---
Documentation/git-filter-branch.txt | 15 +++++++++++++++
1 files changed, 15 insertions(+), 0 deletions(-)
diff --git a/Documentation/git-filter-branch.txt b/Documentation/git-filter-branch.txt
index fed6de6..2565244 100644
--- a/Documentation/git-filter-branch.txt
+++ b/Documentation/git-filter-branch.txt
@@ -246,6 +246,21 @@ git filter-branch --commit-filter '
fi' HEAD
------------------------------------------------------------------------------
+To remove commits that are empty (do not introduce any change):
+
+------------------------------------------------------------------------------
+git rev-list HEAD | while read c; do [ -n "$(git diff-tree --root $c)" ] || echo $c; done > revs
+
+git filter-branch --commit-filter '
+ if grep -q "$GIT_COMMIT" '"$(pwd)/"revs';
+ then
+ skip_commit "$@";
+ else
+ git commit-tree "$@";
+ fi' HEAD
+
+------------------------------------------------------------------------------
+
The function 'skip_commit' is defined as follows:
--------------------------
--
1.5.6.3.536.g61aad
^ permalink raw reply related
* Re: Pull request for sub-tree merge into /contrib/gitstats
From: Sverre Rabbelier @ 2008-10-30 0:38 UTC (permalink / raw)
To: Junio C Hamano, Git Mailinglist
In-Reply-To: <bd6139dc0810291606o2efe4254me378335b76861340@mail.gmail.com>
On Thu, Oct 30, 2008 at 00:06, Sverre Rabbelier <alturin@gmail.com> wrote:
> My work is available in the git repository at:
Please use instead:
> git://repo.or.cz/git-stats.git for-junio
Which has the "gitstats:" prefix to all commit messages, and does not
have the changelog file.
--
Cheers,
Sverre Rabbelier
^ permalink raw reply
* Re: [PATCH] git-filter-branch: Add an example on how to remove empty commits
From: Johannes Schindelin @ 2008-10-30 0:56 UTC (permalink / raw)
To: Petr Baudis; +Cc: git, Sverre Rabbelier
In-Reply-To: <1225326833-15210-1-git-send-email-pasky@suse.cz>
Hi,
On Wed, 29 Oct 2008, Petr Baudis wrote:
> +To remove commits that are empty (do not introduce any change):
> +
> +------------------------------------------------------------------------------
> +git rev-list HEAD | while read c; do [ -n "$(git diff-tree --root $c)" ] || echo $c; done > revs
> +
> +git filter-branch --commit-filter '
> + if grep -q "$GIT_COMMIT" '"$(pwd)/"revs';
> + then
> + skip_commit "$@";
> + else
> + git commit-tree "$@";
> + fi' HEAD
You would not need to use the temporary "revs" file by using something
(totally untested, of course):
git filter-branch --commit-filter '
if git diff-tree --exit-status -q "$GIT_COMMIT";
then
git commit-tree "$@";
else
skip_commit "$@";
fi' HEAD
Of course, you could also mention that you could use
git log --cherry-pick -p --pretty=format: ..<branch>@{1}
to verify that all skipped commits had empty diffs. That one is also
totally untested.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH] git-filter-branch: Add an example on how to remove empty commits
From: Sam Vilain @ 2008-10-30 0:39 UTC (permalink / raw)
To: Petr Baudis; +Cc: git, Sverre Rabbelier
In-Reply-To: <1225326833-15210-1-git-send-email-pasky@suse.cz>
On Wed, 2008-10-29 at 17:33 -0700, Petr Baudis wrote:
> +To remove commits that are empty (do not introduce any change):
> +
> +------------------------------------------------------------------------------
> +git rev-list HEAD | while read c; do [ -n "$(git diff-tree --root $c)" ] || echo $c; done > revs
> +
> +git filter-branch --commit-filter '
> + if grep -q "$GIT_COMMIT" '"$(pwd)/"revs';
> + then
Why not put the git diff-tree in the commit filter?
Is this tested? It doesn't look like it does what the comment says...
surely you have to compare with the previous commit, not the null
commit?
Sam.
^ permalink raw reply
* Re: Encoding problems using git-svn
From: James North @ 2008-10-30 3:28 UTC (permalink / raw)
To: git
In-Reply-To: <8b168cfb0810282014r789ac01dnec51824de1078f0@mail.gmail.com>
Ok, I made a quick change in git-svn script and seems like is working
now in my system with locale set to iso-8859-1.
Dunno if this is the right place to post this, but I hope someone
knowledgeable see this and tells if this would work as a general fix.
This patch is against 1.6.0.2
--- git-svn 2008-09-15 13:04:46.000000000 +0200
+++ git-svn.mine 2008-10-30 04:21:09.000000000 +0100
@@ -43,6 +43,7 @@
use Getopt::Long qw/:config gnu_getopt no_ignore_case auto_abbrev/;
use IPC::Open3;
use Git;
+use Encode;
BEGIN {
# import functions from Git into our packages, en masse
@@ -1061,6 +1062,7 @@
&& !$saw_from) {
$msgbuf .= "\n\nFrom: $author";
}
+ $msgbuf = encode("utf8", $msgbuf);
print $log_fh $msgbuf or croak $!;
command_close_pipe($msg_fh, $ctx);
}
On Wed, Oct 29, 2008 at 4:14 AM, James North <tocapicha@gmail.com> wrote:
> Hi,
>
> I'm using git-svn on a system with ISO-8859-1 encoding. The problem is
> when I try to use "git svn dcommit" to send changes to a remote svn
> (also ISO-8859-1).
>
> Seems like git-svn is sending commit messages with utf-8 (just a
> guessing...) and they look bad on the remote svn log. E.g. "Ca?\241a
> de cami?\243n"
>
> I have tried using i18n.commitencoding=ISO-8859-1 as suggested by the
> warning when doing "git svn dcommit" but messages still are sent with
> wrong encoding.
>
> I'm mising something?
>
> Thanks everyone
>
^ permalink raw reply
* [PATCH] Documentation: add a planning document for the next CLI revamp
From: Sam Vilain @ 2008-10-30 3:48 UTC (permalink / raw)
To: git, git; +Cc: Sam Vilain, Sam Vilain
From: Sam Vilain <samv@vilain.net>
For cross-command CLI changes to be effective, they need to be
cohesively planned. Add a planning document for this next set of
changes.
Signed-off-by: Sam Vilain <sam@vilain.net>
---
Some suggestions, which have been briefly scanned over by some of the
(remaining @4pm) GitTogether attendees.
Please keep it constructive! :)
Documentation/cli-revamp.txt | 135 ++++++++++++++++++++++++++++++++++++++++++
1 files changed, 135 insertions(+), 0 deletions(-)
create mode 100644 Documentation/cli-revamp.txt
diff --git a/Documentation/cli-revamp.txt b/Documentation/cli-revamp.txt
new file mode 100644
index 0000000..980ea07
--- /dev/null
+++ b/Documentation/cli-revamp.txt
@@ -0,0 +1,135 @@
+GIT command line revamp
+=======================
+
+This design document is designed for review and critique over planned
+direction for changing the command set used by git, rather than
+reviewing and critiquing individual changes.
+
+In general, old commands will be grandfathered for a year or longer,
+and all plumbing commands will still work as originally designed.
+
+Please bear in mind when critiquing that each of these changes might
+themselves have a progressive implementation, for instance the new
+behaviour being optional initially.
+
+Please try to be positive with your comments; let's try to come up
+with solutions and not argue about the details of the solutions
+presented until those details are submitted. In particular, critical
+comments that do not acknowledge the presence of a problem are
+worthless at this stage.
+
+Add/rm/reset/checkout/revert
+----------------------------
+
+Many find these confusing.
+
+ * 'git stage' would do what 'git add' does now.
+
+ * 'git unstage' would do what 'git reset --' does now
+
+ * 'git status' would encourage the user to use
+ 'git diff --staged' to see staged changes as a patch
+
+ * 'git commit' with no changes should give useful information about
+ using 'git stage', 'git commit -a' or 'git commit filename ...'
+
+ * 'git add' and 'git rm': no change
+
+ * 'git update-index' considered plumbing, not changed
+
+ * 'git revert' deprecated in favour of 'git cherry-pick --revert'
+
+ * 'git undo' would do what 'git checkout HEAD --' does now
+
+ * 'git checkout branch' would, if there is a remote branch called
+ 'branch' on exactly one remote, do what
+ 'git checkout -b branch thatremote/branch' does now. If it is
+ ambiguous, it would be an error, forcing the explicit notation.
+
+ * 'git branch --switch' : alternative to checkout
+
+
+Push/pull
+---------
+
+These commands are asymmetric, and this seems mostly historical.
+
+ * 'git push --matching' does what 'git push' does today (without
+ explicit configuration)
+
+ * 'git push' with no ref args and no 'push =' configuration does
+ what:
+ 'git push origin $(git symbolic-ref HEAD | sed "s!refs/heads/!!")'
+ does today. ie, it only pushes the current branch.
+ If a branch was defined in branch.<name>.push, push to that ref
+ instead of the matching one. If there is no matching ref, and
+ there is a branch.<name>.merge, push back there.
+
+ * 'git pull' behaviour unchanged
+
+ * 'git push' to checked out branch of non-bare repository not
+ allowed without special configuration. Configuration available
+ that allows working directory to be updated, known caveats
+ notwithstanding. Ideally, it would refuse only in situations
+ where a broken working copy would be left (because you couldn't
+ fix it), and work when it can be known to be safe.
+
+
+Informational
+-------------
+
+ * 'git branch' should default to '--color=auto -v'
+
+ * 'git tag -l' should show more information
+
+
+Working with patches
+--------------------
+
+ * 'git send-email' should prompt for all SMTP-related information
+ about sending e-mail when it is running with no configuration.
+ Because these days /usr/lib/sendmail is rarely configured
+ correctly.
+
+ * other git send-email functionality which has bitten people -
+ particularly building the recipient list - should prompt for
+ confirmation until configured to be automatic.
+
+ * 'git am -3' the default; with global option to make it not the
+ default for those that prefer the speed of -2
+
+
+Submodules
+----------
+
+ * submodules should be able to refer to symbolic ref names, svn
+ style - in the .gitmodules file. The actual commit used is still
+ recorded in the index.
+
+ * when switching branches, if the checked out revision of a submodule
+ changes, then it should be switched as well
+
+ * 'git submodule update' should be able to be triggered when
+ switching branches (but not be the default behaviour)
+
+
+Others
+------
+
+ * 'git export' command that does what
+ 'git archive --format=tar --prefix=dir | tar x' does now
+
+ * conflicted merges should point the user immediately to
+ 'git mergetool' and mention you need to use 'git stage' to mark
+ resolved files and 'git commit' when done.
+
+ * 'git init --server' (or similar) should do everything required for
+ exporting::
+----
+chmod -R a+rX
+touch git-daemon-export-ok
+git gc
+git update-server-info
+chmod u+x .git/hooks/post-update
+git config core.sharedrepository=1
+----
--
debian.1.5.6.1
^ permalink raw reply related
* Re: Using the --track option when creating a branch
From: Sam Vilain @ 2008-10-30 5:12 UTC (permalink / raw)
To: Bill Lear; +Cc: git
In-Reply-To: <18696.32778.842933.486171@lisa.zopyra.com>
On Wed, 2008-10-29 at 09:23 -0600, Bill Lear wrote:
> We use git in a way that makes it desirable for us to only push/pull
> to the same remote branch. So, if I'm in branch X, I want 'git push'
> to push to origin/X, and 'git pull' to fetch into origin/X and then
> merge into X from origin/X.
>
> In other words, we want git push/pull to behave in branches other than
> master the same way it does when in master.
>
> I have discovered the '--track' option when creating a local branch,
> and this appears to me to be the thing that gives us the desired
> behavior.
As things currently stand this is not achievable behaviour. The
behaviour of 'git push' is to push all matching refs. If you are lucky
this is what you intended, but it also pushes any changes to *other*
branches that you have made.
I have tabled a change proposal to make it work as you suggest in a
separate thread.
Sam
^ 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