* [PATCH 04/11] Allow pooled nodes to be recycled back onto a free list
From: Shawn O. Pearce @ 2007-11-09 11:06 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In some cases and for some node types a caller may have used one of
our allocated nodes for some temporary usage and then wants to return
it back to the available free list. We now define a function that
will thread the object onto the front of a free list, which will be
used only after the current block has been exhausted.
We hold off on looking at the free list until we are sure the current
block is empty. This saves about 1000 tests of the (usually empty)
free list per block.
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
alloc.c | 23 +++++++++++++++++++++--
1 files changed, 21 insertions(+), 2 deletions(-)
diff --git a/alloc.c b/alloc.c
index aa429ec..5737a73 100644
--- a/alloc.c
+++ b/alloc.c
@@ -26,6 +26,7 @@ struct node_block
struct node_pool
{
struct node_block *block_list;
+ void *free_list;
char *base;
size_t nr;
};
@@ -38,15 +39,22 @@ struct node_pool
static void report(const char* name, struct node_pool *p, size_t sz)
{
- unsigned int count = 0;
+ unsigned int count = 0, free = 0;
struct node_block *b;
+ void **f;
for (b = p->block_list; b; b = b->next)
count += BLOCKING;
+ for (f = p->free_list; f; f = *f)
+ free++;
+ free += p->nr;
count -= p->nr;
sz = (count * sz) >> 10;
- fprintf(stderr, "%10s: %8u (" SZ_FMT " kB)\n", name, count, sz);
+ fprintf(stderr, "%10s: %8u (" SZ_FMT " kB)", name, count, sz);
+ if (free)
+ fprintf(stderr, ", %8u on free list", free);
+ fputc('\n', stderr);
}
#undef SZ_FMT
@@ -59,6 +67,12 @@ void *alloc_##name##_node(void) \
\
if (!name##_pool.nr) { \
struct node_block *b; \
+ if (name##_pool.free_list) { \
+ ret = name##_pool.free_list; \
+ name##_pool.free_list = *((void**)ret); \
+ memset(ret, 0, sizeof(t)); \
+ return ret; \
+ } \
b = xmalloc(sizeof(*b) + BLOCKING * sizeof(t)); \
b->next = name##_pool.block_list; \
name##_pool.block_list = b; \
@@ -71,6 +85,11 @@ void *alloc_##name##_node(void) \
memset(ret, 0, sizeof(t)); \
return ret; \
} \
+void free_##name##_node(void *n) \
+{ \
+ *((void**)n) = name##_pool.free_list; \
+ name##_pool.free_list = n; \
+} \
static void report_##name(void) \
{ \
report(#name, &name##_pool, sizeof(t)); \
--
1.5.3.5.1622.g41d10
^ permalink raw reply related
* [PATCH 05/11] Bulk allocate struct commit_list, as we do for struct commit
From: Shawn O. Pearce @ 2007-11-09 11:06 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
Typically every commit has at least one struct commit_list allocated
for its parent. It is also not uncommon for there to be more than
one allocated per commit, especially if merges are common in this
repository.
Running `git rev-list --all` doesn't usually wind up requiring many
commit_list items (about 1024 for git.git right now) as the commits
are output as soon as they are first seen and then the parent list
is freed as it is no longer necessary for traversal. However when
we add --topo-order or --date-order and request sorting the lists
are not deallocated until after sorting is complete. Any memory
they consume during that time just increases our footprint.
In git.git for 15865 commits we need 50354 commit_lists to complete
a --topo-order sort. That's an average of 3 commit_list items per
commit (I blame my local reflogs of Junio's pu branch for the high
ratio of commits to their parents). With that kind of allocation
individually mallocing them is wasting a lot of memory for these
very small and otherwise lightwight objects.
Aside from reducing our memory usage during the --topo-order
sorting case a major benefit from allocating these by blocks is we
can release them enmass when we release the other objects in the
object pool. This reduces the cost of the scrub_commit() function
as it no longer needs to free each parent entry individually.
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
alloc.c | 5 ++++-
builtin-diff-tree.c | 8 ++------
builtin-rev-parse.c | 2 +-
builtin-send-pack.c | 2 +-
builtin-show-branch.c | 2 +-
cache.h | 2 ++
commit.c | 13 ++++++-------
http-push.c | 2 +-
reflog-walk.c | 2 +-
revision.c | 6 +++---
upload-pack.c | 2 +-
11 files changed, 23 insertions(+), 23 deletions(-)
diff --git a/alloc.c b/alloc.c
index 5737a73..b69f613 100644
--- a/alloc.c
+++ b/alloc.c
@@ -51,7 +51,7 @@ static void report(const char* name, struct node_pool *p, size_t sz)
count -= p->nr;
sz = (count * sz) >> 10;
- fprintf(stderr, "%10s: %8u (" SZ_FMT " kB)", name, count, sz);
+ fprintf(stderr, "%15s: %8u (" SZ_FMT " kB)", name, count, sz);
if (free)
fprintf(stderr, ", %8u on free list", free);
fputc('\n', stderr);
@@ -144,6 +144,7 @@ static void scrub_any_object(union any_object *o)
}
}
+DEFINE_ALLOCATOR(commit_list, struct commit_list)
DEFINE_ALLOCATOR(blob, struct blob)
DEFINE_SCRUBBING_ALLOCATOR(tree, struct tree, scrub_tree)
DEFINE_SCRUBBING_ALLOCATOR(commit, struct commit, scrub_commit)
@@ -155,6 +156,7 @@ void alloc_report(void)
report_blob();
report_tree();
report_commit();
+ report_commit_list();
report_tag();
report_object();
}
@@ -164,6 +166,7 @@ void alloc_free_everything(void)
free_all_blob();
free_all_tree();
free_all_commit();
+ free_all_commit_list();
free_all_tag();
free_all_object();
}
diff --git a/builtin-diff-tree.c b/builtin-diff-tree.c
index 0b591c8..eb340d2 100644
--- a/builtin-diff-tree.c
+++ b/builtin-diff-tree.c
@@ -31,14 +31,10 @@ static int diff_tree_stdin(char *line)
if (isspace(line[40]) && !get_sha1_hex(line+41, sha1)) {
/* Graft the fake parents locally to the commit */
int pos = 41;
- struct commit_list **pptr, *parents;
+ struct commit_list **pptr;
/* Free the real parent list */
- for (parents = commit->parents; parents; ) {
- struct commit_list *tmp = parents->next;
- free(parents);
- parents = tmp;
- }
+ free_commit_list(commit->parents);
commit->parents = NULL;
pptr = &(commit->parents);
while (line[pos] && !get_sha1_hex(line + pos, sha1)) {
diff --git a/builtin-rev-parse.c b/builtin-rev-parse.c
index d1038a0..7500814 100644
--- a/builtin-rev-parse.c
+++ b/builtin-rev-parse.c
@@ -200,7 +200,7 @@ static int try_difference(const char *arg)
struct commit_list *n = exclude->next;
show_rev(REVERSED,
exclude->item->object.sha1,NULL);
- free(exclude);
+ free_commit_list_node(exclude);
exclude = n;
}
}
diff --git a/builtin-send-pack.c b/builtin-send-pack.c
index 5a0f5c6..818519e 100644
--- a/builtin-send-pack.c
+++ b/builtin-send-pack.c
@@ -82,7 +82,7 @@ static void unmark_and_free(struct commit_list *list, unsigned int mark)
struct commit_list *temp = list;
temp->item->object.flags &= ~mark;
list = temp->next;
- free(temp);
+ free_commit_list_node(temp);
}
}
diff --git a/builtin-show-branch.c b/builtin-show-branch.c
index 6dc835d..f10377c 100644
--- a/builtin-show-branch.c
+++ b/builtin-show-branch.c
@@ -38,7 +38,7 @@ static struct commit *pop_one_commit(struct commit_list **list_p)
list = *list_p;
commit = list->item;
*list_p = list->next;
- free(list);
+ free_commit_list_node(list);
return commit;
}
diff --git a/cache.h b/cache.h
index 8289de1..a33c8ee 100644
--- a/cache.h
+++ b/cache.h
@@ -586,6 +586,8 @@ extern void *alloc_tree_node(void);
extern void *alloc_commit_node(void);
extern void *alloc_tag_node(void);
extern void *alloc_object_node(void);
+extern void *alloc_commit_list_node(void);
+extern void free_commit_list_node(void *);
extern void alloc_report(void);
extern void alloc_free_everything(void);
diff --git a/commit.c b/commit.c
index 05f3cd6..796fc59 100644
--- a/commit.c
+++ b/commit.c
@@ -320,13 +320,12 @@ int parse_commit(struct commit *item)
void scrub_commit(struct commit *item)
{
- free_commit_list(item->parents);
free(item->buffer);
}
struct commit_list *commit_list_insert(struct commit *item, struct commit_list **list_p)
{
- struct commit_list *new_list = xmalloc(sizeof(struct commit_list));
+ struct commit_list *new_list = alloc_commit_list_node();
new_list->item = item;
new_list->next = *list_p;
*list_p = new_list;
@@ -338,7 +337,7 @@ void free_commit_list(struct commit_list *list)
while (list) {
struct commit_list *temp = list;
list = temp->next;
- free(temp);
+ free_commit_list_node(temp);
}
}
@@ -374,7 +373,7 @@ struct commit *pop_most_recent_commit(struct commit_list **list,
struct commit_list *old = *list;
*list = (*list)->next;
- free(old);
+ free_commit_list_node(old);
while (parents) {
struct commit *commit = parents->item;
@@ -416,7 +415,7 @@ struct commit *pop_commit(struct commit_list **stack)
if (top) {
*stack = top->next;
- free(top);
+ free_commit_list_node(top);
}
return item;
}
@@ -561,7 +560,7 @@ static struct commit_list *merge_bases(struct commit *one, struct commit *two)
commit = list->item;
n = list->next;
- free(list);
+ free_commit_list_node(list);
list = n;
flags = commit->object.flags & (PARENT1 | PARENT2 | STALE);
@@ -592,7 +591,7 @@ static struct commit_list *merge_bases(struct commit *one, struct commit *two)
struct commit_list *n = list->next;
if (!(list->item->object.flags & STALE))
insert_by_date(list->item, &result);
- free(list);
+ free_commit_list_node(list);
list = n;
}
return result;
diff --git a/http-push.c b/http-push.c
index 99328f5..75f1664 100644
--- a/http-push.c
+++ b/http-push.c
@@ -1931,7 +1931,7 @@ static void unmark_and_free(struct commit_list *list, unsigned int mark)
struct commit_list *temp = list;
temp->item->object.flags &= ~mark;
list = temp->next;
- free(temp);
+ free_commit_list_node(temp);
}
}
diff --git a/reflog-walk.c b/reflog-walk.c
index ee1456b..a6cc6ca 100644
--- a/reflog-walk.c
+++ b/reflog-walk.c
@@ -236,7 +236,7 @@ void fake_reflog_parent(struct reflog_walk_info *info, struct commit *commit)
return;
}
- commit->parents = xcalloc(sizeof(struct commit_list), 1);
+ commit->parents = alloc_commit_list_node();
commit->parents->item = commit_info->commit;
commit->object.flags &= ~(ADDED | SEEN | SHOWN);
}
diff --git a/revision.c b/revision.c
index 931f978..d9c3998 100644
--- a/revision.c
+++ b/revision.c
@@ -554,7 +554,7 @@ static int limit_list(struct rev_info *revs)
show_early_output_fn_t show;
list = list->next;
- free(entry);
+ free_commit_list_node(entry);
if (revs->max_age != -1 && (commit->date < revs->max_age))
obj->flags |= UNINTERESTING;
@@ -735,7 +735,7 @@ static void prepare_show_merge(struct rev_info *revs)
while (bases) {
struct commit *it = bases->item;
struct commit_list *n = bases->next;
- free(bases);
+ free_commit_list_node(bases);
bases = n;
it->object.flags |= UNINTERESTING;
add_pending_object(revs, &it->object, "(merge-base)");
@@ -1451,7 +1451,7 @@ static struct commit *get_revision_1(struct rev_info *revs)
struct commit *commit = entry->item;
revs->commits = entry->next;
- free(entry);
+ free_commit_list_node(entry);
if (revs->reflog_info)
fake_reflog_parent(revs->reflog_info, commit);
diff --git a/upload-pack.c b/upload-pack.c
index 7e04311..51796dc 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -331,7 +331,7 @@ static int reachable(struct commit *want)
while (work) {
struct commit_list *list = work->next;
struct commit *commit = work->item;
- free(work);
+ free_commit_list_node(work);
work = list;
if (commit->object.flags & THEY_HAVE) {
--
1.5.3.5.1622.g41d10
^ permalink raw reply related
* [PATCH 03/11] Define free_all_objects to deallocate object pools
From: Shawn O. Pearce @ 2007-11-09 11:06 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
The new API call free_all_objects releases all struct objects that
have been allocated thus far in the process, invalidating all of
those pointers and clearing out the object hash. This resets the
process to before any objects were parsed and allocated, allowing
a future revision walk to start from a clean slate without needing
to clear_commit_marks() first, or worry about parent rewriting.
The release routines support recursively releasing any buffers held
by an active object through object specific scrub functions. These
only need to free any buffers that had been held by the object and
are not yet released as any other clearing activity would be handled
by the next user of the memory.
Performance testing with `git rev-list --objects --all` shows that
calling free_all_objects() after completing the traversal doesn't
cost us any additional significant running time.
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
alloc.c | 60 +++++++++++++++++++++++++++++++++++++++++++++++++++++++-----
cache.h | 1 +
commit.c | 6 ++++++
commit.h | 1 +
object.c | 11 +++++++++++
object.h | 1 +
tag.c | 5 +++++
tag.h | 1 +
tree.c | 5 +++++
tree.h | 1 +
10 files changed, 87 insertions(+), 5 deletions(-)
diff --git a/alloc.c b/alloc.c
index a61ae41..aa429ec 100644
--- a/alloc.c
+++ b/alloc.c
@@ -51,7 +51,7 @@ static void report(const char* name, struct node_pool *p, size_t sz)
#undef SZ_FMT
-#define DEFINE_ALLOCATOR(name, t) \
+#define DEFINE_BASE_ALLOCATOR(name, t) \
static struct node_pool name##_pool; \
void *alloc_##name##_node(void) \
{ \
@@ -76,6 +76,36 @@ static void report_##name(void) \
report(#name, &name##_pool, sizeof(t)); \
}
+#define DEFINE_ALLOCATOR(name, t) \
+DEFINE_BASE_ALLOCATOR(name, t) \
+static void free_all_##name(void) \
+{ \
+ struct node_block *b, *n; \
+ for (b = name##_pool.block_list; b; b = n) { \
+ n = b->next; \
+ free(b); \
+ } \
+ memset(&name##_pool, 0, sizeof(name##_pool)); \
+}
+
+#define DEFINE_SCRUBBING_ALLOCATOR(name, t, scrub) \
+DEFINE_BASE_ALLOCATOR(name, t) \
+static void free_all_##name(void) \
+{ \
+ struct node_block *b, *n; \
+ for (b = name##_pool.block_list; b; b = n) { \
+ size_t nr = BLOCKING; \
+ char *e = (char*)(b + 1); \
+ if (b == name##_pool.block_list) \
+ nr -= name##_pool.nr; \
+ for (; nr--; e += sizeof(t)) \
+ scrub((t*)e); \
+ n = b->next; \
+ free(b); \
+ } \
+ memset(&name##_pool, 0, sizeof(name##_pool)); \
+}
+
union any_object {
struct object object;
struct blob blob;
@@ -84,11 +114,22 @@ union any_object {
struct tag tag;
};
+static void scrub_any_object(union any_object *o)
+{
+ switch (o->object.type) {
+ case OBJ_COMMIT: scrub_commit(&o->commit); break;
+ case OBJ_TREE: scrub_tree(&o->tree); break;
+ case OBJ_BLOB: break;
+ case OBJ_TAG: scrub_tag(&o->tag); break;
+ default: die("unknown type %s", typename(o->object.type));
+ }
+}
+
DEFINE_ALLOCATOR(blob, struct blob)
-DEFINE_ALLOCATOR(tree, struct tree)
-DEFINE_ALLOCATOR(commit, struct commit)
-DEFINE_ALLOCATOR(tag, struct tag)
-DEFINE_ALLOCATOR(object, union any_object)
+DEFINE_SCRUBBING_ALLOCATOR(tree, struct tree, scrub_tree)
+DEFINE_SCRUBBING_ALLOCATOR(commit, struct commit, scrub_commit)
+DEFINE_SCRUBBING_ALLOCATOR(tag, struct tag, scrub_tag)
+DEFINE_SCRUBBING_ALLOCATOR(object, union any_object, scrub_any_object)
void alloc_report(void)
{
@@ -98,3 +139,12 @@ void alloc_report(void)
report_tag();
report_object();
}
+
+void alloc_free_everything(void)
+{
+ free_all_blob();
+ free_all_tree();
+ free_all_commit();
+ free_all_tag();
+ free_all_object();
+}
diff --git a/cache.h b/cache.h
index 7174eb1..8289de1 100644
--- a/cache.h
+++ b/cache.h
@@ -587,6 +587,7 @@ extern void *alloc_commit_node(void);
extern void *alloc_tag_node(void);
extern void *alloc_object_node(void);
extern void alloc_report(void);
+extern void alloc_free_everything(void);
/* trace.c */
extern void trace_printf(const char *format, ...);
diff --git a/commit.c b/commit.c
index f074811..05f3cd6 100644
--- a/commit.c
+++ b/commit.c
@@ -318,6 +318,12 @@ int parse_commit(struct commit *item)
return ret;
}
+void scrub_commit(struct commit *item)
+{
+ free_commit_list(item->parents);
+ free(item->buffer);
+}
+
struct commit_list *commit_list_insert(struct commit *item, struct commit_list **list_p)
{
struct commit_list *new_list = xmalloc(sizeof(struct commit_list));
diff --git a/commit.h b/commit.h
index aa67986..cd0d286 100644
--- a/commit.h
+++ b/commit.h
@@ -39,6 +39,7 @@ struct commit *lookup_commit_reference_gently(const unsigned char *sha1,
int parse_commit_buffer(struct commit *item, void *buffer, unsigned long size);
int parse_commit(struct commit *item);
+void scrub_commit(struct commit *item);
struct commit_list * commit_list_insert(struct commit *item, struct commit_list **list_p);
struct commit_list * insert_by_date(struct commit *item, struct commit_list **list);
diff --git a/object.c b/object.c
index 16793d9..9b49472 100644
--- a/object.c
+++ b/object.c
@@ -43,6 +43,17 @@ int type_from_string(const char *str)
die("invalid object type \"%s\"", str);
}
+void free_all_objects(void)
+{
+ if (obj_hash) {
+ free(obj_hash);
+ obj_hash = NULL;
+ nr_objs = 0;
+ obj_hash_size = 0;
+ alloc_free_everything();
+ }
+}
+
static unsigned int hash_obj(struct object *obj, unsigned int n)
{
unsigned int hash = *(unsigned int *)obj->sha1;
diff --git a/object.h b/object.h
index 397bbfa..642beeb 100644
--- a/object.h
+++ b/object.h
@@ -40,6 +40,7 @@ extern int track_object_refs;
extern const char *typename(unsigned int type);
extern int type_from_string(const char *str);
+extern void free_all_objects(void);
extern unsigned int get_max_object_index(void);
extern struct object *get_indexed_object(unsigned int);
extern struct object_refs *lookup_object_refs(struct object *);
diff --git a/tag.c b/tag.c
index f62bcdd..c39d3f0 100644
--- a/tag.c
+++ b/tag.c
@@ -114,3 +114,8 @@ int parse_tag(struct tag *item)
free(data);
return ret;
}
+
+void scrub_tag(struct tag *item)
+{
+ free(item->tag);
+}
diff --git a/tag.h b/tag.h
index 7a0cb00..933b876 100644
--- a/tag.h
+++ b/tag.h
@@ -15,6 +15,7 @@ struct tag {
extern struct tag *lookup_tag(const unsigned char *sha1);
extern int parse_tag_buffer(struct tag *item, void *data, unsigned long size);
extern int parse_tag(struct tag *item);
+extern void scrub_tag(struct tag *item);
extern struct object *deref_tag(struct object *, const char *, int);
#endif /* TAG_H */
diff --git a/tree.c b/tree.c
index 8c0819f..278dad7 100644
--- a/tree.c
+++ b/tree.c
@@ -281,6 +281,11 @@ int parse_tree(struct tree *item)
return parse_tree_buffer(item, buffer, size);
}
+void scrub_tree(struct tree *tree)
+{
+ free(tree->buffer);
+}
+
struct tree *parse_tree_indirect(const unsigned char *sha1)
{
struct object *obj = parse_object(sha1);
diff --git a/tree.h b/tree.h
index dd25c53..d03cfd7 100644
--- a/tree.h
+++ b/tree.h
@@ -16,6 +16,7 @@ struct tree *lookup_tree(const unsigned char *sha1);
int parse_tree_buffer(struct tree *item, void *buffer, unsigned long size);
int parse_tree(struct tree *tree);
+void scrub_tree(struct tree *tree);
/* Parses and returns the tree in the given ent, chasing tags and commits. */
struct tree *parse_tree_indirect(const unsigned char *sha1);
--
1.5.3.5.1622.g41d10
^ permalink raw reply related
* [PATCH 01/11] Fix memory leak in traverse_commit_list
From: Shawn O. Pearce @ 2007-11-09 11:06 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
If we were listing objects too then the objects were buffered in an
array only reachable from a stack allocated structure. When this
function returns that array would be leaked as nobody would have
a reference to it anymore.
Historically this hasn't been a problem as the primary user of
traverse_commit_list() (the noble git-rev-list) would terminate
as soon as the function was finished, thus allowing the operating
system to cleanup memory. However we have been leaking this data
in git-pack-objects ever since that program learned how to run the
revision listing internally, rather than relying on reading object
names from git-rev-list.
To better facilitate reuse of traverse_commit_list during other
builtin tools (such as git-fetch) we shouldn't leak temporary memory
like this and instead we need to clean up properly after ourselves.
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
list-objects.c | 7 +++++++
1 files changed, 7 insertions(+), 0 deletions(-)
diff --git a/list-objects.c b/list-objects.c
index e5c88c2..713bef9 100644
--- a/list-objects.c
+++ b/list-objects.c
@@ -170,4 +170,11 @@ void traverse_commit_list(struct rev_info *revs,
}
for (i = 0; i < objects.nr; i++)
show_object(&objects.objects[i]);
+ free(objects.objects);
+ if (revs->pending.nr) {
+ revs->pending.nr = 0;
+ revs->pending.alloc = 0;
+ revs->pending.objects = NULL;
+ free(revs->pending.objects);
+ }
}
--
1.5.3.5.1622.g41d10
^ permalink raw reply related
* [PATCH 02/11] Reorganize the object memory pools so we can release them
From: Shawn O. Pearce @ 2007-11-09 11:06 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
This changes the object memory allocators in alloc.c so that we
are storing all information about each allocator within a single
struct, rather than hiding it inside of the allocation function.
By keeping this state information in a struct we are better able to
organize a function that would release the allocated memory blocks
back to the system allocator.
Each block allocated via xmalloc() is chained together into a linked
list so we can free them when if we later need to. A nice side
effect of this change is we can avoid the increment of the number
of allocations and instead compute this after the fact based upon
the number of blocks in the linked lists.
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
alloc.c | 86 ++++++++++++++++++++++++++++++++++++++++----------------------
1 files changed, 55 insertions(+), 31 deletions(-)
diff --git a/alloc.c b/alloc.c
index 216c23a..a61ae41 100644
--- a/alloc.c
+++ b/alloc.c
@@ -18,23 +18,62 @@
#define BLOCKING 1024
-#define DEFINE_ALLOCATOR(name, type) \
-static unsigned int name##_allocs; \
+struct node_block
+{
+ struct node_block *next;
+};
+
+struct node_pool
+{
+ struct node_block *block_list;
+ char *base;
+ size_t nr;
+};
+
+#ifdef NO_C99_FORMAT
+#define SZ_FMT "%5u"
+#else
+#define SZ_FMT "%5zu"
+#endif
+
+static void report(const char* name, struct node_pool *p, size_t sz)
+{
+ unsigned int count = 0;
+ struct node_block *b;
+
+ for (b = p->block_list; b; b = b->next)
+ count += BLOCKING;
+ count -= p->nr;
+
+ sz = (count * sz) >> 10;
+ fprintf(stderr, "%10s: %8u (" SZ_FMT " kB)\n", name, count, sz);
+}
+
+#undef SZ_FMT
+
+#define DEFINE_ALLOCATOR(name, t) \
+static struct node_pool name##_pool; \
void *alloc_##name##_node(void) \
{ \
- static int nr; \
- static type *block; \
void *ret; \
\
- if (!nr) { \
- nr = BLOCKING; \
- block = xmalloc(BLOCKING * sizeof(type)); \
+ if (!name##_pool.nr) { \
+ struct node_block *b; \
+ b = xmalloc(sizeof(*b) + BLOCKING * sizeof(t)); \
+ b->next = name##_pool.block_list; \
+ name##_pool.block_list = b; \
+ name##_pool.base = (char*)(b + 1); \
+ name##_pool.nr = BLOCKING; \
} \
- nr--; \
- name##_allocs++; \
- ret = block++; \
- memset(ret, 0, sizeof(type)); \
+ ret = name##_pool.base; \
+ name##_pool.nr--; \
+ name##_pool.base += sizeof(t); \
+ memset(ret, 0, sizeof(t)); \
return ret; \
+} \
+static void report_##name(void) \
+{ \
+ report(#name, &name##_pool, sizeof(t)); \
}
union any_object {
@@ -51,26 +90,11 @@ DEFINE_ALLOCATOR(commit, struct commit)
DEFINE_ALLOCATOR(tag, struct tag)
DEFINE_ALLOCATOR(object, union any_object)
-#ifdef NO_C99_FORMAT
-#define SZ_FMT "%u"
-#else
-#define SZ_FMT "%zu"
-#endif
-
-static void report(const char* name, unsigned int count, size_t size)
-{
- fprintf(stderr, "%10s: %8u (" SZ_FMT " kB)\n", name, count, size);
-}
-
-#undef SZ_FMT
-
-#define REPORT(name) \
- report(#name, name##_allocs, name##_allocs*sizeof(struct name) >> 10)
-
void alloc_report(void)
{
- REPORT(blob);
- REPORT(tree);
- REPORT(commit);
- REPORT(tag);
+ report_blob();
+ report_tree();
+ report_commit();
+ report_tag();
+ report_object();
}
--
1.5.3.5.1622.g41d10
^ permalink raw reply related
* [PATCH 0/11] Updated 'quickfetch' series
From: Shawn O. Pearce @ 2007-11-09 11:06 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
So what started out as a 1 patch hack has now grown into an 11
patch series. *sigh*
1) Fix memory leak in traverse_commit_list
This one seems obvious to me. No real harm to not having it but
later on we'd see the downside to the leak in git-fetch.
2) Reorganize the object memory pools so we can release them
3) Define free_all_objects to deallocate object pools
4) Allow pooled nodes to be recycled back onto a free list
5) Bulk allocate struct commit_list, as we do for struct commit
This part of the series is only lightly tested thus far. It rewrites
the object allocators in alloc.c to allow complete destruction of
the object hash and its contents in a single call. This way we
can rerun the revision machinary a second time in the same process
without the mess of clear_commit_flags() or worring about parents
having been rewritten.
We also decrease memory usage by moving struct commit_list into the
pool of things managed by alloc.c. Given how many of these suckers
we are allocating (3x as many as commits in a --topo-order!) the
malloc overhead saved per parent pointer is probably worth the pain
of reviewing this series.
6) git-fetch: Release objects used by a prior transport
Makes use of the above to ensure transports that are called directly
in-process (e.g. fetch-pack) don't get confused by a prior invocation
of that same transport. I don't think fetch-pack was meant to be
run twice in the same process...
7) git-fetch: Limit automated tag following to only fetched objects
Rewrites the rules behind automated tag following. The new rule is
simpler to understand, has some measure of safety attached to it, and
actually works when we turn on quickfetch behavior in a transport.
8) run-command: Support sending stderr to /dev/null
9) rev-list: Introduce --quiet to avoid /dev/null redirects
10) git-fetch: avoid local fetching from alternate (again)
11) git-fetch: test avoiding unnecessary copying from alternates
Turn on quickfetch for the native git transport. As I'm sitting
here writing this I'm wondering if the quickfetch thing shouldn't
be higher up, like in git-fetch itself, so that we can also use
it in the HTTP transport. But it should work now with the earlier
stuff out of the way.
--
Shawn.
^ permalink raw reply
* Re: git rebase --skip
From: Johannes Schindelin @ 2007-11-09 10:59 UTC (permalink / raw)
To: Jeff King
Cc: Junio C Hamano, Björn Steinbrink, Andreas Ericsson,
Mike Hommey, git
In-Reply-To: <20071109032227.GA31760@sigill.intra.peff.net>
Hi,
On Thu, 8 Nov 2007, Jeff King wrote:
> So I am fine with the original patch (unconditional reset --hard), but
> it would be nice to see the people who care submit concrete proposals
> for such a safety valve.
Isn't having to say "--skip" instead of "--continue" enough? Some people
might complain that it's too easy to get your fingers wired to type
--skip.
In that case, I might beg to differ for two reasons: --skip is definitely
not the default operation, so the fingers do not get any chance to do
that, and even if, they would get wired to --force --skip just as easily.
Besides, after my patch to rebase on a detached HEAD, it is very easy to
go back to the original state and try again.
Ciao,
Dscho
^ permalink raw reply
* Re: [BUG] git-rebase fails when a commit message contains a diff
From: Andreas Ericsson @ 2007-11-09 10:29 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Junio C Hamano, Jonas Fonseca, git
In-Reply-To: <Pine.LNX.4.64.0711090225110.4362@racer.site>
Johannes Schindelin wrote:
>> + case "$INTERACTIVE" in
>> + t)
>> + git_editor "$TODO" || die "Could not execute editor"
>> + ;;
>> + esac
>
>
> Would that not be easier to read as
>
> test t = "$INTERACTIVE" &&
> git_editor "$TODO" || die "Could not execute editor"
>
Written like that, it would die every time $INTERACTIVE isn't "t". You'd need
curly braces around
git_editor "$TODO" || die "Could not execute editor"
--
Andreas Ericsson andreas.ericsson@op5.se
OP5 AB www.op5.se
Tel: +46 8-230225 Fax: +46 8-230231
^ permalink raw reply
* Re: [PATCH 1/2] Add strchrnul()
From: Andreas Ericsson @ 2007-11-09 10:22 UTC (permalink / raw)
To: René Scharfe
Cc: Junio C Hamano, Pierre Habouzit, Git Mailing List,
Johannes Schindelin
In-Reply-To: <4733AEA0.1060602@lsrfire.ath.cx>
René Scharfe wrote:
> As suggested by Pierre Habouzit, add strchrnul(). It's a useful GNU
> extension and can simplify string parser code. There are several
> places in git that can be converted to strchrnul(); as a trivial
> example, this patch introduces its usage to builtin-fetch--tool.c.
>
> Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
> ---
>
> Makefile | 13 +++++++++++++
> builtin-fetch--tool.c | 8 ++------
> compat/strchrnul.c | 8 ++++++++
> git-compat-util.h | 5 +++++
> 4 files changed, 28 insertions(+), 6 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index 0d5590f..578c999 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -30,6 +30,8 @@ all::
> #
> # Define NO_MEMMEM if you don't have memmem.
> #
> +# Define NO_STRCHRNUL if you don't have strchrnul.
> +#
This seems overly complicated. How about this instead?
From: Andreas Ericsson <ae@op5.se>
Subject: [PATCH] Add strchrnul()
As suggested by Pierre Habouzit, add strchrnul(). It's a useful GNU
extension and can simplify string parser code. There are several
places in git that can be converted to strchrnul(); as a trivial
example, this patch introduces its usage to builtin-fetch--tool.c.
strchrnul() was introduced in glibc in April 1999 and included in
glibc-2.1. Checking for that version means the majority of all git
users would get to use the optimized version in glibc. Of the
remaining few some might get to use a slightly slower version
than necessary but probably not slower than what we have today.
Original patch by Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
Signed-off-by: Andreas Ericsson <ae@op5.se>
---
I'm fairly much against forcing people to know what library
functions they have in order to get software to compile
properly. This is, imo, a neater solution, and also inlines
the function as suggested by Dscho.
diff --git a/git-compat-util.h b/git-compat-util.h
index 7b29d1b..9fedf33 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -105,6 +105,18 @@ extern void set_die_routine(void (*routine)(const char *err
extern void set_error_routine(void (*routine)(const char *err, va_list params))
extern void set_warn_routine(void (*routine)(const char *warn, va_list params))
+
+#if !defined(__GLIBC__) && !__GLIBC_PREREQ(2, 1)
+# define strchrnul(s, c) gitstrchrnul(s, c)
+static inline char *gitstrchrnul(const char *s, int c)
+{
+ while (*s && *s != c)
+ s++;
+
+ return (char *)s;
+}
+#endif
+
#ifdef NO_MMAP
#ifndef PROT_READ
diff --git a/builtin-fetch--tool.c b/builtin-fetch--tool.c
index 6a78517..ed60847 100644
--- a/builtin-fetch--tool.c
+++ b/builtin-fetch--tool.c
@@ -435,9 +435,7 @@ static int pick_rref(int sha1_only, const char *rref, const char *ls_remote_resu
cp++;
if (!*cp)
break;
- np = strchr(cp, '\n');
- if (!np)
- np = cp + strlen(cp);
+ np = strchrnul(cp, '\n');
if (pass) {
lrr_list[i].line = cp;
lrr_list[i].name = cp + 41;
@@ -461,9 +459,7 @@ static int pick_rref(int sha1_only, const char *rref, const char *ls_remote_resu
rref++;
if (!*rref)
break;
- next = strchr(rref, '\n');
- if (!next)
- next = rref + strlen(rref);
+ next = strchrnul(rref, '\n');
rreflen = next - rref;
for (i = 0; i < lrr_count; i++) {
--
Andreas Ericsson andreas.ericsson@op5.se
OP5 AB www.op5.se
Tel: +46 8-230225 Fax: +46 8-230231
^ permalink raw reply related
* [PATCH] tests: git push mirror mode tests V2
From: Andy Whitcroft @ 2007-11-09 10:21 UTC (permalink / raw)
To: git
In-Reply-To: <1194541305.0@pinky>
Add some basic tests for git push --mirror mode.
Signed-off-by: Andy Whitcroft <apw@shadowen.org>
---
Following the discussion on how tests which change directory
should use subshells to prevent loss of CWD and of how
! is not something we can rely on, here is an updates to
the tests.
---
t/t5517-push-mirror.sh | 125 ++++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 125 insertions(+), 0 deletions(-)
diff --git a/t/t5517-push-mirror.sh b/t/t5517-push-mirror.sh
new file mode 100755
index 0000000..a65d2f5
--- /dev/null
+++ b/t/t5517-push-mirror.sh
@@ -0,0 +1,125 @@
+#!/bin/sh
+
+test_description='pushing to a mirror repository'
+
+. ./test-lib.sh
+
+D=`pwd`
+
+invert () {
+ if "$@"; then
+ return 1
+ else
+ return 0
+ fi
+}
+
+mk_repo_pair () {
+ rm -rf master mirror &&
+ mkdir mirror &&
+ (
+ cd mirror &&
+ git init
+ ) &&
+ mkdir master &&
+ (
+ cd master &&
+ git init &&
+ git config remote.up.url ../mirror
+ )
+}
+
+
+test_expect_success 'push mirror does not create new branches' '
+
+ mk_repo_pair &&
+ (
+ cd master &&
+ echo one >foo && git add foo && git commit -m one &&
+ git push --mirror up
+ ) &&
+ master_master=$(cd master && git show-ref -s --verify refs/heads/master) &&
+ mirror_master=$(cd mirror && git show-ref -s --verify refs/heads/master) &&
+ test "$master_master" = "$mirror_master"
+
+'
+
+test_expect_success 'push mirror does not update existing branches' '
+
+ mk_repo_pair &&
+ (
+ cd master &&
+ echo one >foo && git add foo && git commit -m one &&
+ git push --mirror up &&
+ echo two >foo && git add foo && git commit -m two &&
+ git push --mirror up
+ ) &&
+ master_master=$(cd master && git show-ref -s --verify refs/heads/master) &&
+ mirror_master=$(cd mirror && git show-ref -s --verify refs/heads/master) &&
+ test "$master_master" = "$mirror_master"
+
+'
+
+test_expect_success 'push mirror does not force update existing branches' '
+
+ mk_repo_pair &&
+ (
+ cd master &&
+ echo one >foo && git add foo && git commit -m one &&
+ git push --mirror up &&
+ echo two >foo && git add foo && git commit -m two &&
+ git push --mirror up &&
+ git reset --hard HEAD^
+ git push --mirror up
+ ) &&
+ master_master=$(cd master && git show-ref -s --verify refs/heads/master) &&
+ mirror_master=$(cd mirror && git show-ref -s --verify refs/heads/master) &&
+ test "$master_master" = "$mirror_master"
+
+'
+
+test_expect_success 'push mirror does not remove branches' '
+
+ mk_repo_pair &&
+ (
+ cd master &&
+ echo one >foo && git add foo && git commit -m one &&
+ git branch remove master &&
+ git push --mirror up &&
+ git branch -D remove
+ git push --mirror up
+ ) &&
+ (
+ cd mirror &&
+ invert git show-ref -s --verify refs/heads/remove
+ )
+
+'
+
+test_expect_success 'push mirror does not add, update and remove together' '
+
+ mk_repo_pair &&
+ (
+ cd master &&
+ echo one >foo && git add foo && git commit -m one &&
+ git branch remove master &&
+ git push --mirror up &&
+ git branch -D remove &&
+ git branch add master &&
+ echo two >foo && git add foo && git commit -m two &&
+ git push --mirror up
+ ) &&
+ master_master=$(cd master && git show-ref -s --verify refs/heads/master) &&
+ master_add=$(cd master && git show-ref -s --verify refs/heads/add) &&
+ mirror_master=$(cd mirror && git show-ref -s --verify refs/heads/master) &&
+ mirror_add=$(cd mirror && git show-ref -s --verify refs/heads/add) &&
+ test "$master_master" = "$mirror_master" &&
+ test "$master_add" = "$mirror_add" &&
+ (
+ cd mirror &&
+ invert git show-ref -s --verify refs/heads/remove
+ )
+
+'
+
+test_done
^ permalink raw reply related
* [PATCH] git-checkout: Test for relative path use.
From: David Symonds @ 2007-11-09 9:12 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Johannes Schindelin, Andreas Ericsson, Johannes Sixt,
David Symonds
In-Reply-To: <ee77f5c20711090110s5d6c533et5e1e016a95fde943@mail.gmail.com>
Signed-off-by: David Symonds <dsymonds@gmail.com>
---
t/t2008-checkout-subdir.sh | 80 ++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 80 insertions(+), 0 deletions(-)
create mode 100755 t/t2008-checkout-subdir.sh
diff --git a/t/t2008-checkout-subdir.sh b/t/t2008-checkout-subdir.sh
new file mode 100755
index 0000000..98d8eb3
--- /dev/null
+++ b/t/t2008-checkout-subdir.sh
@@ -0,0 +1,80 @@
+#!/bin/sh
+#
+# Copyright (c) 2007 David Symonds
+
+test_description='git checkout from subdirectories'
+
+. ./test-lib.sh
+
+test_expect_success setup '
+
+ echo "base" > file0 &&
+ git add file0 &&
+ mkdir dir1 &&
+ echo "hello" > dir1/file1 &&
+ git add dir1/file1 &&
+ mkdir dir2 &&
+ echo "bonjour" > dir2/file2 &&
+ git add dir2/file2 &&
+ test_tick &&
+ git commit -m "populate tree"
+
+'
+
+test_expect_success 'remove and restore with relative path' '
+
+ (
+ cd dir1 &&
+ rm ../file0 &&
+ git checkout HEAD -- ../file0 &&
+ test "base" = "$(cat ../file0)" &&
+ rm ../dir2/file2 &&
+ git checkout HEAD -- ../dir2/file2 &&
+ test "bonjour" = "$(cat ../dir2/file2)" &&
+ rm ../file0 ./file1 &&
+ git checkout HEAD -- .. &&
+ test "base" = "$(cat ../file0)" &&
+ test "hello" = "$(cat file1)"
+ )
+
+'
+
+test_expect_success 'checkout with empty prefix' '
+
+ rm file0 &&
+ git checkout HEAD -- file0 &&
+ test "base" = "$(cat file0)"
+
+'
+
+test_expect_success 'checkout with simple prefix' '
+
+ rm dir1/file1 &&
+ git checkout HEAD -- dir1 &&
+ test "hello" = "$(cat dir1/file1)" &&
+ rm dir1/file1 &&
+ git checkout HEAD -- dir1/file1 &&
+ test "hello" = "$(cat dir1/file1)"
+
+'
+
+test_expect_success 'checkout with complex relative path' '
+
+ rm file1 &&
+ git checkout HEAD -- ../dir1/../dir1/file1 && test -f ./file1
+
+'
+
+test_expect_failure 'relative path outside tree should fail' \
+ 'git checkout HEAD -- ../../Makefile'
+
+test_expect_failure 'incorrect relative path to file should fail (1)' \
+ 'git checkout HEAD -- ../file0'
+
+test_expect_failure 'incorrect relative path should fail (2)' \
+ '( cd dir1 && git checkout HEAD -- ./file0 )'
+
+test_expect_failure 'incorrect relative path should fail (3)' \
+ '( cd dir1 && git checkout HEAD -- ../../file0 )'
+
+test_done
--
1.5.3.1
^ permalink raw reply related
* Re: [PATCH 2/3] rev-list: Introduce --no-output to avoid /dev/null redirects
From: Shawn O. Pearce @ 2007-11-09 9:12 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Alex Riesen, git
In-Reply-To: <7vsl3fvlrb.fsf@gitster.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> wrote:
> Alex Riesen <raa.lkml@gmail.com> writes:
> > Junio C Hamano, Fri, Nov 09, 2007 08:32:01 +0100:
> >>
> >> The function names noshow_xxx() looked a bit funny, but I do not
> >> offhand have better alternatives to offer.
> >
> > "hide", "skip", "ignore"?
>
> But look at what the functions do. The original show_xxx() was
> to print and then process. Shawn splitted them into show_xxx()
> and noshow_xxx(), leaft the printing part in the former, made
> the former call the latter at the end, and moved the processing
> to the latter. So it is not any of the three words.
In my latest patch series I've gone with "finish_xxx()" as it is
finishing our usage of that object. I'll be resubmitting a new
series soon.
--
Shawn.
^ permalink raw reply
* Re: [PATCH] git-checkout: Test for relative path use.
From: David Symonds @ 2007-11-09 9:10 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Sixt, git, Johannes Schindelin, Andreas Ericsson
In-Reply-To: <7vfxzfvlch.fsf@gitster.siamese.dyndns.org>
On Nov 9, 2007 8:06 PM, Junio C Hamano <gitster@pobox.com> wrote:
> "David Symonds" <dsymonds@gmail.com> writes:
>
> > So what would you prefer? Bracketing the whole test in parentheses
> > looks ugly, but I can do that if that's the only option. If I look at
> > t5510-fetch.sh (one of yours, Junio), there is no directory
> > restoration in the case of test failure, as in my original patch.
>
> Yes, that is what I was referring to as "bad examples". The way
> t4116 goes down to different directory do not look ugly to me.
Okay, thanks -- that's a useful example. I'll resend the patch shortly.
Dave.
^ permalink raw reply
* Re: [BUG] git-rebase fails when a commit message contains a diff
From: Benoit Sigoure @ 2007-11-09 9:07 UTC (permalink / raw)
To: Johannes Sixt
Cc: Johannes Schindelin, Junio C Hamano, Jonas Fonseca,
Git Mailing List
In-Reply-To: <473420FE.7010807@viscovery.net>
[-- Attachment #1: Type: text/plain, Size: 1072 bytes --]
On Nov 9, 2007, at 9:57 AM, Johannes Sixt wrote:
> Benoit Sigoure schrieb:
>> Off topic question: why do you guys always do this instead of
>> doing, say, this:
>> INTERACTIVE=false
>> case $1 in
>> --interactive|-i)
>> INTERACTIVE=:
>> ... ;;
>> esac
>> if $INTERACTIVE; then
>> git_editor "$TODO" || die ...
>> fi
>
> Because in some shells 'false' is not a built-in.
Can you name such a shell? (besides Solaris' brain-damaged, b0rken
and foobared /bin/sh which will most likely not work with Git anyway)
> But then this might do it without the extra process:
>
> INTERACTIVE="! :" # false
`!' is not portable either. In particular, I highly doubt `!' will
work on shells that don't have `false' as a builtin (Hello Mr Solaris).
>
> case $1 in
> --interactive|-i)
> INTERACTIVE=:
> ... ;;
> esac
> if $INTERACTIVE; then
> git_editor "$TODO" || die ...
> fi
Correct me if I'm wrong but I think that some shells don't have
`test' as a builtin either.
--
Benoit Sigoure aka Tsuna
EPITA Research and Development Laboratory
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 186 bytes --]
^ permalink raw reply
* Re: [PATCH] git-checkout: Test for relative path use.
From: Junio C Hamano @ 2007-11-09 9:06 UTC (permalink / raw)
To: David Symonds
Cc: Junio C Hamano, Johannes Sixt, git, Johannes Schindelin,
Andreas Ericsson
In-Reply-To: <ee77f5c20711090014qfed56e7y446c014399e47a82@mail.gmail.com>
"David Symonds" <dsymonds@gmail.com> writes:
> So what would you prefer? Bracketing the whole test in parentheses
> looks ugly, but I can do that if that's the only option. If I look at
> t5510-fetch.sh (one of yours, Junio), there is no directory
> restoration in the case of test failure, as in my original patch.
Yes, that is what I was referring to as "bad examples". The way
t4116 goes down to different directory do not look ugly to me.
^ permalink raw reply
* Re: [BUG] git-rebase fails when a commit message contains a diff
From: Ralf Wildenhues @ 2007-11-09 9:05 UTC (permalink / raw)
To: Johannes Sixt
Cc: Benoit Sigoure, Johannes Schindelin, Junio C Hamano,
Jonas Fonseca, Git Mailing List
In-Reply-To: <473420FE.7010807@viscovery.net>
* Johannes Sixt wrote on Fri, Nov 09, 2007 at 09:57:34AM CET:
> Benoit Sigoure schrieb:
>> Off topic question: why do you guys always do this instead of doing, say,
>> this:
>>
>> INTERACTIVE=false
[...]
>> if $INTERACTIVE; then
>> git_editor "$TODO" || die ...
>> fi
>
> Because in some shells 'false' is not a built-in.
[...]
> INTERACTIVE="! :" # false
Which shells do not have 'false' but do support '!' as process exit
status negation? For them, is it important to save another fork?
Solaris sh has neither (it will error on the '!'), but it's ruled out
for git anyway.
Cheers,
Ralf
^ permalink raw reply
* Re: [PATCH 2/3] rev-list: Introduce --no-output to avoid /dev/null redirects
From: Junio C Hamano @ 2007-11-09 8:58 UTC (permalink / raw)
To: Alex Riesen; +Cc: Shawn O. Pearce, git
In-Reply-To: <20071109081204.GB2794@steel.home>
Alex Riesen <raa.lkml@gmail.com> writes:
> Junio C Hamano, Fri, Nov 09, 2007 08:32:01 +0100:
>> "Shawn O. Pearce" <spearce@spearce.org> writes:
>>
>> > @@ -640,7 +656,9 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)
>> > }
>> > }
>> >
>> > - traverse_commit_list(&revs, show_commit, show_object);
>> > + traverse_commit_list(&revs,
>> > + nooutput ? noshow_commit : show_commit,
>> > + nooutput ? noshow_object : show_object);
>> >
>> > return 0;
>> > }
>>
>> The function names noshow_xxx() looked a bit funny, but I do not
>> offhand have better alternatives to offer.
>
> "hide", "skip", "ignore"?
But look at what the functions do. The original show_xxx() was
to print and then process. Shawn splitted them into show_xxx()
and noshow_xxx(), leaft the printing part in the former, made
the former call the latter at the end, and moved the processing
to the latter. So it is not any of the three words.
^ permalink raw reply
* Re: [BUG] git-rebase fails when a commit message contains a diff
From: Johannes Sixt @ 2007-11-09 8:57 UTC (permalink / raw)
To: Benoit Sigoure
Cc: Johannes Schindelin, Junio C Hamano, Jonas Fonseca,
Git Mailing List
In-Reply-To: <6FCE17E3-9FAA-4676-B12A-369B31743DA6@lrde.epita.fr>
Benoit Sigoure schrieb:
> Off topic question: why do you guys always do this instead of doing,
> say, this:
>
> INTERACTIVE=false
>
> case $1 in
> --interactive|-i)
> INTERACTIVE=:
> ... ;;
> esac
> if $INTERACTIVE; then
> git_editor "$TODO" || die ...
> fi
Because in some shells 'false' is not a built-in.
But then this might do it without the extra process:
INTERACTIVE="! :" # false
case $1 in
--interactive|-i)
INTERACTIVE=:
... ;;
esac
if $INTERACTIVE; then
git_editor "$TODO" || die ...
fi
-- Hannes
^ permalink raw reply
* Re: [PATCH] git-checkout: Test for relative path use.
From: David Symonds @ 2007-11-09 8:14 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Sixt, git, Johannes Schindelin, Andreas Ericsson
In-Reply-To: <7v7ikrx2st.fsf@gitster.siamese.dyndns.org>
On Nov 9, 2007 7:04 PM, Junio C Hamano <gitster@pobox.com> wrote:
> "David Symonds" <dsymonds@gmail.com> writes:
>
> > Looking at the existing tests which, when they change directories,
> > don't cd back to where they were; they "cd .." at the start of the
> > next test. I'll add a "cd .." to the relevant bits of my tests.
>
> Do not follow the bad examples, please.
So what would you prefer? Bracketing the whole test in parentheses
looks ugly, but I can do that if that's the only option. If I look at
t5510-fetch.sh (one of yours, Junio), there is no directory
restoration in the case of test failure, as in my original patch.
Perhaps test_ok_ and test_failure_ in test-lib.sh should restore the directory?
Dave.
^ permalink raw reply
* Re: [PATCH 2/3] rev-list: Introduce --no-output to avoid /dev/null redirects
From: Alex Riesen @ 2007-11-09 8:12 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Shawn O. Pearce, git
In-Reply-To: <7vejezx4b2.fsf@gitster.siamese.dyndns.org>
Junio C Hamano, Fri, Nov 09, 2007 08:32:01 +0100:
> "Shawn O. Pearce" <spearce@spearce.org> writes:
>
> > @@ -640,7 +656,9 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)
> > }
> > }
> >
> > - traverse_commit_list(&revs, show_commit, show_object);
> > + traverse_commit_list(&revs,
> > + nooutput ? noshow_commit : show_commit,
> > + nooutput ? noshow_object : show_object);
> >
> > return 0;
> > }
>
> The function names noshow_xxx() looked a bit funny, but I do not
> offhand have better alternatives to offer.
"hide", "skip", "ignore"?
^ permalink raw reply
* Re: corrupt object on git-gc
From: Alex Riesen @ 2007-11-09 8:10 UTC (permalink / raw)
To: Yossi Leybovich; +Cc: git
In-Reply-To: <6C2C79E72C305246B504CBA17B5500C9029A36A1@mtlexch01.mtl.com>
Yossi Leybovich, Fri, Nov 09, 2007 00:59:47 +0100:
> I wonder if someone can help in this error
> I tried to do git-gc and got error on corrupted object.
>
> I do the following:
>
> $ git-gc
> Generating pack...
> Done counting 3037 objects.
> Deltifying 3037 objects...
> error: corrupt loose object '4b9458b3786228369c63936db65827de3cc06200'
It is loose. Nothing uses it in this repository. What do you need to
repair it for?
> fatal: object 4b9458b3786228369c63936db65827de3cc06200 cannot be read
> error: failed to run repack
>
> sleybo@SLEYBO-LT /w/work/EMC/ib.071030.001/ib
> $ cd .git/objects/4b/
>
> sleybo@SLEYBO-LT /w/work/EMC/ib.071030.001/ib/.git/objects/4b
> $ git-fsck-objects.exe 9458b3786228369c63936db65827de3cc06200
> error: corrupt loose object '4b9458b3786228369c63936db65827de3cc06200'
> error: 4b9458b3786228369c63936db65827de3cc06200: object corrupt or
> missing
> error: invalid parameter: expected sha1, got
> '9458b3786228369c63936db65827de3cc06200'
> missing blob 4b9458b3786228369c63936db65827de3cc06200
the directories directly under .git/objects contain the first bytes of
sha1, to use filesystem in a more efficient way. git-fsck expects an
sha1 (or a reference).
Try running moving the corrupt object (with its *whole* name) some
place else and run git-fsck --all.
^ permalink raw reply
* Re: [PATCH] git-checkout: Test for relative path use.
From: Junio C Hamano @ 2007-11-09 8:04 UTC (permalink / raw)
To: David Symonds
Cc: Johannes Sixt, Junio C Hamano, git, Johannes Schindelin,
Andreas Ericsson
In-Reply-To: <ee77f5c20711082324s39a9d441tc05c5a27e6d39f3e@mail.gmail.com>
"David Symonds" <dsymonds@gmail.com> writes:
> Looking at the existing tests which, when they change directories,
> don't cd back to where they were; they "cd .." at the start of the
> next test. I'll add a "cd .." to the relevant bits of my tests.
Do not follow the bad examples, please.
^ permalink raw reply
* Re: [BUG] git-rebase fails when a commit message contains a diff
From: Benoit Sigoure @ 2007-11-09 7:57 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Junio C Hamano, Jonas Fonseca, git
In-Reply-To: <Pine.LNX.4.64.0711090225110.4362@racer.site>
[-- Attachment #1: Type: text/plain, Size: 562 bytes --]
On Nov 9, 2007, at 3:28 AM, Johannes Schindelin wrote:
> Would that not be easier to read as
>
> test t = "$INTERACTIVE" &&
> git_editor "$TODO" || die "Could not execute editor"
Hmm this will `die' if you're not running interactively.
Off topic question: why do you guys always do this instead of doing,
say, this:
INTERACTIVE=false
case $1 in
.
.
.
--interactive|-i)
INTERACTIVE=:
... ;;
esac
.
.
.
if $INTERACTIVE; then
git_editor "$TODO" || die ...
fi
?
--
Benoit Sigoure aka Tsuna
EPITA Research and Development Laboratory
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 186 bytes --]
^ permalink raw reply
* [PATCH] git-checkout: Test for relative path use.
From: David Symonds @ 2007-11-09 7:37 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Johannes Schindelin, Andreas Ericsson, Johannes Sixt,
David Symonds
In-Reply-To: <ee77f5c20711082324s39a9d441tc05c5a27e6d39f3e@mail.gmail.com>
Signed-off-by: David Symonds <dsymonds@gmail.com>
---
Tests that change directories now change back at the start of the
next test. I don't know what to do about that last test, though.
t/t2008-checkout-subdir.sh | 81 ++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 81 insertions(+), 0 deletions(-)
create mode 100755 t/t2008-checkout-subdir.sh
diff --git a/t/t2008-checkout-subdir.sh b/t/t2008-checkout-subdir.sh
new file mode 100755
index 0000000..41e76c9
--- /dev/null
+++ b/t/t2008-checkout-subdir.sh
@@ -0,0 +1,81 @@
+#!/bin/sh
+#
+# Copyright (c) 2007 David Symonds
+
+test_description='git checkout from subdirectories'
+
+. ./test-lib.sh
+
+test_expect_success setup '
+
+ echo "base" > file0 &&
+ git add file0 &&
+ mkdir dir1 &&
+ echo "hello" > dir1/file1 &&
+ git add dir1/file1 &&
+ mkdir dir2 &&
+ echo "bonjour" > dir2/file2 &&
+ git add dir2/file2 &&
+ test_tick &&
+ git commit -m "populate tree"
+
+'
+
+test_expect_success 'remove and restore with relative path' '
+
+ cd dir1 &&
+ rm ../file0 &&
+ git checkout HEAD -- ../file0 &&
+ test "base" = "$(cat ../file0)" &&
+ rm ../dir2/file2 &&
+ git checkout HEAD -- ../dir2/file2 &&
+ test "bonjour" = "$(cat ../dir2/file2)" &&
+ rm ../file0 ./file1 &&
+ git checkout HEAD -- .. &&
+ test "base" = "$(cat ../file0)" &&
+ test "hello" = "$(cat file1)"
+
+'
+
+# currently in dir1/
+test_expect_success 'checkout with empty prefix' '
+
+ cd .. &&
+ rm file0 &&
+ git checkout HEAD -- file0 &&
+ test "base" = "$(cat file0)"
+
+'
+
+test_expect_success 'checkout with simple prefix' '
+
+ rm dir1/file1 &&
+ git checkout HEAD -- dir1 &&
+ test "hello" = "$(cat dir1/file1)" &&
+ rm dir1/file1 &&
+ git checkout HEAD -- dir1/file1 &&
+ test "hello" = "$(cat dir1/file1)"
+
+'
+
+test_expect_success 'checkout with complex relative path' '
+
+ rm file1 &&
+ git checkout HEAD -- ../dir1/../dir1/file1 && test -f ./file1
+
+'
+
+test_expect_failure 'relative path outside tree should fail' \
+ 'git checkout HEAD -- ../../Makefile'
+
+test_expect_failure 'incorrect relative path to file should fail (1)' \
+ 'git checkout HEAD -- ../file0'
+
+test_expect_failure 'incorrect relative path should fail (2)' \
+ 'cd dir1 && git checkout HEAD -- ./file0'
+
+# currently in dir1/
+test_expect_failure 'incorrect relative path should fail (3)' \
+ 'git checkout HEAD -- ../../file0'
+
+test_done
--
1.5.3.1
^ permalink raw reply related
* Re: [PATCH 2/3] rev-list: Introduce --no-output to avoid /dev/null redirects
From: Junio C Hamano @ 2007-11-09 7:32 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: git
In-Reply-To: <20071108080052.GB16690@spearce.org>
"Shawn O. Pearce" <spearce@spearce.org> writes:
> @@ -640,7 +656,9 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)
> }
> }
>
> - traverse_commit_list(&revs, show_commit, show_object);
> + traverse_commit_list(&revs,
> + nooutput ? noshow_commit : show_commit,
> + nooutput ? noshow_object : show_object);
>
> return 0;
> }
The function names noshow_xxx() looked a bit funny, but I do not
offhand have better alternatives to offer.
This allows "--bisect-vars --no-output" and "--bisect-all
--bisect-vars --no-output" but makes them behave differently. I
do not think --no-output is useful with bisection anyway but
maybe it makes sense to forbid the combination?
^ 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