* [PATCH 1/8] bundle: split basis discovery into its own function
2010-06-26 6:17 ` [RFC/PATCH 0/8] Re: bug: git-bundle create foo --stdin -> segfault Jonathan Nieder
@ 2010-06-26 6:19 ` Jonathan Nieder
2010-06-26 6:20 ` [PATCH 2/8] bundle: use libified rev-list --boundary Jonathan Nieder
` (6 subsequent siblings)
7 siblings, 0 replies; 19+ messages in thread
From: Jonathan Nieder @ 2010-06-26 6:19 UTC (permalink / raw)
To: Joey Hess; +Cc: git, 554682, Adam Brewster, Johannes Schindelin
create_bundle() is getting a little long. Isolate a small
piece to work on without distractions.
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
bundle.c | 58 ++++++++++++++++++++++++++++++++++------------------------
1 files changed, 34 insertions(+), 24 deletions(-)
diff --git a/bundle.c b/bundle.c
index ff97adc..66948f4 100644
--- a/bundle.c
+++ b/bundle.c
@@ -193,35 +193,14 @@ static int is_tag_in_date_range(struct object *tag, struct rev_info *revs)
(revs->min_age == -1 || revs->min_age > date);
}
-int create_bundle(struct bundle_header *header, const char *path,
- int argc, const char **argv)
+static int list_prerequisites(int bundle_fd, struct rev_info *revs,
+ int argc, const char * const *argv)
{
- static struct lock_file lock;
- int bundle_fd = -1;
- int bundle_to_stdout;
const char **argv_boundary = xmalloc((argc + 4) * sizeof(const char *));
- const char **argv_pack = xmalloc(5 * sizeof(const char *));
- int i, ref_count = 0;
char buffer[1024];
- struct rev_info revs;
struct child_process rls;
FILE *rls_fout;
- bundle_to_stdout = !strcmp(path, "-");
- if (bundle_to_stdout)
- bundle_fd = 1;
- else
- bundle_fd = hold_lock_file_for_update(&lock, path,
- LOCK_DIE_ON_ERROR);
-
- /* write signature */
- write_or_die(bundle_fd, bundle_signature, strlen(bundle_signature));
-
- /* init revs to list objects for pack-objects later */
- save_commit_buffer = 0;
- init_revisions(&revs, NULL);
-
- /* write prerequisites */
memcpy(argv_boundary + 3, argv + 1, argc * sizeof(const char *));
argv_boundary[0] = "rev-list";
argv_boundary[1] = "--boundary";
@@ -241,7 +220,7 @@ int create_bundle(struct bundle_header *header, const char *path,
if (!get_sha1_hex(buffer + 1, sha1)) {
struct object *object = parse_object(sha1);
object->flags |= UNINTERESTING;
- add_pending_object(&revs, object, buffer);
+ add_pending_object(revs, object, buffer);
}
} else if (!get_sha1_hex(buffer, sha1)) {
struct object *object = parse_object(sha1);
@@ -251,6 +230,37 @@ int create_bundle(struct bundle_header *header, const char *path,
fclose(rls_fout);
if (finish_command(&rls))
return error("rev-list died");
+ return 0;
+}
+
+int create_bundle(struct bundle_header *header, const char *path,
+ int argc, const char **argv)
+{
+ static struct lock_file lock;
+ int bundle_fd = -1;
+ int bundle_to_stdout;
+ const char **argv_pack = xmalloc(5 * sizeof(const char *));
+ int i, ref_count = 0;
+ struct rev_info revs;
+ struct child_process rls;
+
+ bundle_to_stdout = !strcmp(path, "-");
+ if (bundle_to_stdout)
+ bundle_fd = 1;
+ else
+ bundle_fd = hold_lock_file_for_update(&lock, path,
+ LOCK_DIE_ON_ERROR);
+
+ /* write signature */
+ write_or_die(bundle_fd, bundle_signature, strlen(bundle_signature));
+
+ /* init revs to list objects for pack-objects later */
+ save_commit_buffer = 0;
+ init_revisions(&revs, NULL);
+
+ /* write prerequisites */
+ if (list_prerequisites(bundle_fd, &revs, argc, argv))
+ return -1;
/* write references */
argc = setup_revisions(argc, argv, &revs, NULL);
--
1.7.1.198.g8d802
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH 2/8] bundle: use libified rev-list --boundary
2010-06-26 6:17 ` [RFC/PATCH 0/8] Re: bug: git-bundle create foo --stdin -> segfault Jonathan Nieder
2010-06-26 6:19 ` [PATCH 1/8] bundle: split basis discovery into its own function Jonathan Nieder
@ 2010-06-26 6:20 ` Jonathan Nieder
2010-06-30 17:57 ` Junio C Hamano
2010-06-26 6:20 ` [PATCH 3/8] bundle: give list_prerequisites() loop body its own function Jonathan Nieder
` (5 subsequent siblings)
7 siblings, 1 reply; 19+ messages in thread
From: Jonathan Nieder @ 2010-06-26 6:20 UTC (permalink / raw)
To: Joey Hess; +Cc: git, 554682, Adam Brewster, Johannes Schindelin
The revision walker produces structured output, which should be a
little easier to work with than the text from rev-list.
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
bundle.c | 69 ++++++++++++++++++++++++++++++++++---------------------------
1 files changed, 38 insertions(+), 31 deletions(-)
diff --git a/bundle.c b/bundle.c
index 66948f4..0dd2acb 100644
--- a/bundle.c
+++ b/bundle.c
@@ -196,40 +196,47 @@ static int is_tag_in_date_range(struct object *tag, struct rev_info *revs)
static int list_prerequisites(int bundle_fd, struct rev_info *revs,
int argc, const char * const *argv)
{
- const char **argv_boundary = xmalloc((argc + 4) * sizeof(const char *));
- char buffer[1024];
- struct child_process rls;
- FILE *rls_fout;
+ const char **argv_boundary = xmalloc((argc + 1) * sizeof(const char *));
+ struct rev_info boundary_revs;
+ struct commit *rev;
- memcpy(argv_boundary + 3, argv + 1, argc * sizeof(const char *));
- argv_boundary[0] = "rev-list";
- argv_boundary[1] = "--boundary";
- argv_boundary[2] = "--pretty=oneline";
- argv_boundary[argc + 2] = NULL;
- memset(&rls, 0, sizeof(rls));
- rls.argv = argv_boundary;
- rls.out = -1;
- rls.git_cmd = 1;
- if (start_command(&rls))
- return -1;
- rls_fout = xfdopen(rls.out, "r");
- while (fgets(buffer, sizeof(buffer), rls_fout)) {
- unsigned char sha1[20];
- if (buffer[0] == '-') {
- write_or_die(bundle_fd, buffer, strlen(buffer));
- if (!get_sha1_hex(buffer + 1, sha1)) {
- struct object *object = parse_object(sha1);
- object->flags |= UNINTERESTING;
- add_pending_object(revs, object, buffer);
- }
- } else if (!get_sha1_hex(buffer, sha1)) {
- struct object *object = parse_object(sha1);
- object->flags |= SHOWN;
+ memcpy(argv_boundary, argv, (argc + 1) * sizeof(const char *));
+
+ init_revisions(&boundary_revs, NULL);
+ boundary_revs.boundary = 1;
+ if (setup_revisions(argc, argv_boundary, &boundary_revs, NULL) > 1)
+ return error("unrecognized argument: %s", argv_boundary[1]);
+ if (prepare_revision_walk(&boundary_revs))
+ return error("revision walk setup failed");
+
+ while ((rev = get_revision(&boundary_revs))) {
+ if (rev->object.flags & BOUNDARY) {
+ struct strbuf buf = STRBUF_INIT;
+ struct pretty_print_context ctx = {0};
+ enum object_type type;
+ unsigned long size;
+
+ /*
+ * The commit buffer is needed
+ * to pretty-print boundary commits.
+ */
+ rev->buffer = read_sha1_file(rev->object.sha1,
+ &type, &size);
+
+ strbuf_addch(&buf, '-');
+ strbuf_add(&buf, sha1_to_hex(rev->object.sha1), 40);
+ strbuf_addch(&buf, ' ');
+ pretty_print_commit(CMIT_FMT_ONELINE, rev, &buf, &ctx);
+ strbuf_addch(&buf, '\n');
+ write_or_die(bundle_fd, buf.buf, buf.len);
+
+ rev->object.flags |= UNINTERESTING;
+ add_pending_object(revs, &rev->object, buf.buf);
+ strbuf_release(&buf);
+ } else {
+ rev->object.flags |= SHOWN;
}
}
- fclose(rls_fout);
- if (finish_command(&rls))
- return error("rev-list died");
return 0;
}
--
1.7.1.198.g8d802
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [PATCH 2/8] bundle: use libified rev-list --boundary
2010-06-26 6:20 ` [PATCH 2/8] bundle: use libified rev-list --boundary Jonathan Nieder
@ 2010-06-30 17:57 ` Junio C Hamano
2010-06-30 20:34 ` Jonathan Nieder
0 siblings, 1 reply; 19+ messages in thread
From: Junio C Hamano @ 2010-06-30 17:57 UTC (permalink / raw)
To: Jonathan Nieder
Cc: Joey Hess, git, 554682, Adam Brewster, Johannes Schindelin
Jonathan Nieder <jrnieder@gmail.com> writes:
> The revision walker produces structured output, which should be a
> little easier to work with than the text from rev-list.
Hmm, doesn't it negatively affect later traversal you would need to do if
you smudged the flag bits by running revision traversal like this?
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 2/8] bundle: use libified rev-list --boundary
2010-06-30 17:57 ` Junio C Hamano
@ 2010-06-30 20:34 ` Jonathan Nieder
0 siblings, 0 replies; 19+ messages in thread
From: Jonathan Nieder @ 2010-06-30 20:34 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Joey Hess, git, 554682, Adam Brewster, Johannes Schindelin
Junio C Hamano wrote:
> Jonathan Nieder <jrnieder@gmail.com> writes:
>> The revision walker produces structured output, which should be a
>> little easier to work with than the text from rev-list.
>
> Hmm, doesn't it negatively affect later traversal you would need to do if
> you smudged the flag bits by running revision traversal like this?
I imagine so. I fear this would be the first git command to use the
revision walker twice, and I am not sure whether we can really make that
work.
The revision walker uses object flags for the following purposes:
- marking objects uninteresting/SYMMETRIC_LEFT. Luckily for us, if
an object is uninteresting or SYMMETRIC_LEFT for the first
--boundary walk, it will be likewise for pack-objects, too.
- history simplification (TREESAME), --cherry-pick (SHOWN),
--merge (SYMMETRIC_LEFT). There are already other reasons to
disallow these features for bundle.
- add_parents_to_list (ADDED, SEEN). This one is really worrisome;
should we walk through again to throw away the added parents?
Should there be a pass through all revisions to clear the ADDED
bit?
I’ll figure out a one-pass solution. :(
Jonathan
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 3/8] bundle: give list_prerequisites() loop body its own function
2010-06-26 6:17 ` [RFC/PATCH 0/8] Re: bug: git-bundle create foo --stdin -> segfault Jonathan Nieder
2010-06-26 6:19 ` [PATCH 1/8] bundle: split basis discovery into its own function Jonathan Nieder
2010-06-26 6:20 ` [PATCH 2/8] bundle: use libified rev-list --boundary Jonathan Nieder
@ 2010-06-26 6:20 ` Jonathan Nieder
2010-06-30 18:04 ` Junio C Hamano
2010-06-26 6:21 ` [PATCH 4/8] bundle: split table of contents output into " Jonathan Nieder
` (4 subsequent siblings)
7 siblings, 1 reply; 19+ messages in thread
From: Jonathan Nieder @ 2010-06-26 6:20 UTC (permalink / raw)
To: Joey Hess; +Cc: git, 554682, Adam Brewster, Johannes Schindelin
No functional change intended.
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
bundle.c | 57 +++++++++++++++++++++++++++++++--------------------------
1 files changed, 31 insertions(+), 26 deletions(-)
diff --git a/bundle.c b/bundle.c
index 0dd2acb..e90b5c5 100644
--- a/bundle.c
+++ b/bundle.c
@@ -193,6 +193,33 @@ static int is_tag_in_date_range(struct object *tag, struct rev_info *revs)
(revs->min_age == -1 || revs->min_age > date);
}
+static void list_prerequisite(int bundle_fd, struct rev_info *revs,
+ struct commit *rev)
+{
+ struct strbuf buf = STRBUF_INIT;
+ struct pretty_print_context ctx = {0};
+ enum object_type type;
+ unsigned long size;
+
+ /*
+ * The commit buffer is needed
+ * to pretty-print boundary commits.
+ */
+ rev->buffer = read_sha1_file(rev->object.sha1, &type, &size);
+
+ strbuf_addch(&buf, '-');
+ strbuf_add(&buf, sha1_to_hex(rev->object.sha1), 40);
+ strbuf_addch(&buf, ' ');
+ pretty_print_commit(CMIT_FMT_ONELINE, rev, &buf, &ctx);
+ strbuf_addch(&buf, '\n');
+
+ write_or_die(bundle_fd, buf.buf, buf.len);
+
+ rev->object.flags |= UNINTERESTING;
+ add_pending_object(revs, &rev->object, buf.buf);
+ strbuf_release(&buf);
+}
+
static int list_prerequisites(int bundle_fd, struct rev_info *revs,
int argc, const char * const *argv)
{
@@ -209,33 +236,11 @@ static int list_prerequisites(int bundle_fd, struct rev_info *revs,
if (prepare_revision_walk(&boundary_revs))
return error("revision walk setup failed");
- while ((rev = get_revision(&boundary_revs))) {
- if (rev->object.flags & BOUNDARY) {
- struct strbuf buf = STRBUF_INIT;
- struct pretty_print_context ctx = {0};
- enum object_type type;
- unsigned long size;
-
- /*
- * The commit buffer is needed
- * to pretty-print boundary commits.
- */
- rev->buffer = read_sha1_file(rev->object.sha1,
- &type, &size);
-
- strbuf_addch(&buf, '-');
- strbuf_add(&buf, sha1_to_hex(rev->object.sha1), 40);
- strbuf_addch(&buf, ' ');
- pretty_print_commit(CMIT_FMT_ONELINE, rev, &buf, &ctx);
- strbuf_addch(&buf, '\n');
- write_or_die(bundle_fd, buf.buf, buf.len);
-
- rev->object.flags |= UNINTERESTING;
- add_pending_object(revs, &rev->object, buf.buf);
- strbuf_release(&buf);
- } else {
+ while ((rev = get_revision(revs))) {
+ if (rev->object.flags & BOUNDARY)
+ list_prerequisite(bundle_fd, revs, rev);
+ else
rev->object.flags |= SHOWN;
- }
}
return 0;
}
--
1.7.1.198.g8d802
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [PATCH 3/8] bundle: give list_prerequisites() loop body its own function
2010-06-26 6:20 ` [PATCH 3/8] bundle: give list_prerequisites() loop body its own function Jonathan Nieder
@ 2010-06-30 18:04 ` Junio C Hamano
2010-06-30 20:37 ` Jonathan Nieder
0 siblings, 1 reply; 19+ messages in thread
From: Junio C Hamano @ 2010-06-30 18:04 UTC (permalink / raw)
To: Jonathan Nieder
Cc: Joey Hess, git, 554682, Adam Brewster, Johannes Schindelin
Jonathan Nieder <jrnieder@gmail.com> writes:
> No functional change intended.
>
> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
> ---
> bundle.c | 57 +++++++++++++++++++++++++++++++--------------------------
> 1 files changed, 31 insertions(+), 26 deletions(-)
>
> diff --git a/bundle.c b/bundle.c
> index 0dd2acb..e90b5c5 100644
> --- a/bundle.c
> +++ b/bundle.c
> @@ -193,6 +193,33 @@ static int is_tag_in_date_range(struct object *tag, struct rev_info *revs)
> (revs->min_age == -1 || revs->min_age > date);
> }
>
> +static void list_prerequisite(int bundle_fd, struct rev_info *revs,
> + struct commit *rev)
> +{
> + struct strbuf buf = STRBUF_INIT;
> + struct pretty_print_context ctx = {0};
> + enum object_type type;
> + unsigned long size;
> +
> + /*
> + * The commit buffer is needed
> + * to pretty-print boundary commits.
> + */
> + rev->buffer = read_sha1_file(rev->object.sha1, &type, &size);
> +
> + strbuf_addch(&buf, '-');
> + strbuf_add(&buf, sha1_to_hex(rev->object.sha1), 40);
> + strbuf_addch(&buf, ' ');
> + pretty_print_commit(CMIT_FMT_ONELINE, rev, &buf, &ctx);
> + strbuf_addch(&buf, '\n');
> +
> + write_or_die(bundle_fd, buf.buf, buf.len);
> +
> + rev->object.flags |= UNINTERESTING;
> + add_pending_object(revs, &rev->object, buf.buf);
> + strbuf_release(&buf);
> +}
> +
> static int list_prerequisites(int bundle_fd, struct rev_info *revs,
> int argc, const char * const *argv)
> {
> @@ -209,33 +236,11 @@ static int list_prerequisites(int bundle_fd, struct rev_info *revs,
> if (prepare_revision_walk(&boundary_revs))
> return error("revision walk setup failed");
>
> - while ((rev = get_revision(&boundary_revs))) {
> - if (rev->object.flags & BOUNDARY) {
> -...
> - } else {
> + while ((rev = get_revision(revs))) {
> + if (rev->object.flags & BOUNDARY)
> + list_prerequisite(bundle_fd, revs, rev);
> + else
> rev->object.flags |= SHOWN;
> - }
You used to walk boundary_revs but now you walk revs that is given by the
caller, exhausting the revs.pending the caller wanted to use later to feed
pack_objects with?
Confused...
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH 3/8] bundle: give list_prerequisites() loop body its own function
2010-06-30 18:04 ` Junio C Hamano
@ 2010-06-30 20:37 ` Jonathan Nieder
0 siblings, 0 replies; 19+ messages in thread
From: Jonathan Nieder @ 2010-06-30 20:37 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Joey Hess, git, 554682, Adam Brewster, Johannes Schindelin
Junio C Hamano wrote:
> Jonathan Nieder <jrnieder@gmail.com> writes:
>
>> @@ -209,33 +236,11 @@ static int list_prerequisites(int bundle_fd, struct rev_info *revs,
>> if (prepare_revision_walk(&boundary_revs))
>> return error("revision walk setup failed");
>>
>> - while ((rev = get_revision(&boundary_revs))) {
>> - if (rev->object.flags & BOUNDARY) {
>> -...
>> - } else {
>> + while ((rev = get_revision(revs))) {
>> + if (rev->object.flags & BOUNDARY)
>> + list_prerequisite(bundle_fd, revs, rev);
>> + else
>> rev->object.flags |= SHOWN;
>> - }
>
> You used to walk boundary_revs but now you walk revs that is given by the
> caller
Agh! Typo. Thanks for catching it.
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 4/8] bundle: split table of contents output into its own function
2010-06-26 6:17 ` [RFC/PATCH 0/8] Re: bug: git-bundle create foo --stdin -> segfault Jonathan Nieder
` (2 preceding siblings ...)
2010-06-26 6:20 ` [PATCH 3/8] bundle: give list_prerequisites() loop body its own function Jonathan Nieder
@ 2010-06-26 6:21 ` Jonathan Nieder
2010-06-26 6:22 ` [PATCH 5/8] bundle: reuse setup_revisions result Jonathan Nieder
` (3 subsequent siblings)
7 siblings, 0 replies; 19+ messages in thread
From: Jonathan Nieder @ 2010-06-26 6:21 UTC (permalink / raw)
To: Joey Hess; +Cc: git, 554682, Adam Brewster, Johannes Schindelin
Isolate another piece of create_bundle() to work on.
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
bundle.c | 78 +++++++++++++++++++++++++++++++++++---------------------------
1 files changed, 44 insertions(+), 34 deletions(-)
diff --git a/bundle.c b/bundle.c
index e90b5c5..8ba6479 100644
--- a/bundle.c
+++ b/bundle.c
@@ -245,45 +245,20 @@ static int list_prerequisites(int bundle_fd, struct rev_info *revs,
return 0;
}
-int create_bundle(struct bundle_header *header, const char *path,
- int argc, const char **argv)
+static int bundle_list_refs(int bundle_fd,
+ int argc, const char **argv, struct rev_info *revs)
{
- static struct lock_file lock;
- int bundle_fd = -1;
- int bundle_to_stdout;
- const char **argv_pack = xmalloc(5 * sizeof(const char *));
int i, ref_count = 0;
- struct rev_info revs;
- struct child_process rls;
- bundle_to_stdout = !strcmp(path, "-");
- if (bundle_to_stdout)
- bundle_fd = 1;
- else
- bundle_fd = hold_lock_file_for_update(&lock, path,
- LOCK_DIE_ON_ERROR);
-
- /* write signature */
- write_or_die(bundle_fd, bundle_signature, strlen(bundle_signature));
-
- /* init revs to list objects for pack-objects later */
- save_commit_buffer = 0;
- init_revisions(&revs, NULL);
-
- /* write prerequisites */
- if (list_prerequisites(bundle_fd, &revs, argc, argv))
- return -1;
-
- /* write references */
- argc = setup_revisions(argc, argv, &revs, NULL);
+ argc = setup_revisions(argc, argv, revs, NULL);
if (argc > 1)
return error("unrecognized argument: %s'", argv[1]);
- object_array_remove_duplicates(&revs.pending);
+ object_array_remove_duplicates(&revs->pending);
- for (i = 0; i < revs.pending.nr; i++) {
- struct object_array_entry *e = revs.pending.objects + i;
+ for (i = 0; i < revs->pending.nr; i++) {
+ struct object_array_entry *e = revs->pending.objects + i;
unsigned char sha1[20];
char *ref;
const char *display_ref;
@@ -298,7 +273,7 @@ int create_bundle(struct bundle_header *header, const char *path,
display_ref = (flag & REF_ISSYMREF) ? e->name : ref;
if (e->item->type == OBJ_TAG &&
- !is_tag_in_date_range(e->item, &revs)) {
+ !is_tag_in_date_range(e->item, revs)) {
e->item->flags |= UNINTERESTING;
continue;
}
@@ -344,7 +319,7 @@ int create_bundle(struct bundle_header *header, const char *path,
*/
obj = parse_object(sha1);
obj->flags |= SHOWN;
- add_pending_object(&revs, obj, e->name);
+ add_pending_object(revs, obj, e->name);
}
free(ref);
continue;
@@ -358,7 +333,42 @@ int create_bundle(struct bundle_header *header, const char *path,
free(ref);
}
if (!ref_count)
- die ("Refusing to create empty bundle.");
+ die("Refusing to create empty bundle.");
+ return 0;
+}
+
+int create_bundle(struct bundle_header *header, const char *path,
+ int argc, const char **argv)
+{
+ static struct lock_file lock;
+ int bundle_fd = -1;
+ int bundle_to_stdout;
+ const char **argv_pack = xmalloc(5 * sizeof(const char *));
+ struct rev_info revs;
+ struct child_process rls;
+ int i;
+
+ bundle_to_stdout = !strcmp(path, "-");
+ if (bundle_to_stdout)
+ bundle_fd = 1;
+ else
+ bundle_fd = hold_lock_file_for_update(&lock, path,
+ LOCK_DIE_ON_ERROR);
+
+ /* write signature */
+ write_or_die(bundle_fd, bundle_signature, strlen(bundle_signature));
+
+ /* init revs to list objects for pack-objects later */
+ save_commit_buffer = 0;
+ init_revisions(&revs, NULL);
+
+ /* write prerequisites */
+ if (list_prerequisites(bundle_fd, &revs, argc, argv))
+ return -1;
+
+ /* write references */
+ if (bundle_list_refs(bundle_fd, argc, argv, &revs))
+ return -1;
/* end header */
write_or_die(bundle_fd, "\n", 1);
--
1.7.1.198.g8d802
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH 5/8] bundle: reuse setup_revisions result
2010-06-26 6:17 ` [RFC/PATCH 0/8] Re: bug: git-bundle create foo --stdin -> segfault Jonathan Nieder
` (3 preceding siblings ...)
2010-06-26 6:21 ` [PATCH 4/8] bundle: split table of contents output into " Jonathan Nieder
@ 2010-06-26 6:22 ` Jonathan Nieder
2010-06-26 6:28 ` [PATCH 6/8] Fix bundle --stdin Jonathan Nieder
` (2 subsequent siblings)
7 siblings, 0 replies; 19+ messages in thread
From: Jonathan Nieder @ 2010-06-26 6:22 UTC (permalink / raw)
To: Joey Hess; +Cc: git, 554682, Adam Brewster, Johannes Schindelin
Avoid reading stdin twice for bundle --stdin.
Reported-by: Joey Hess <joey@kitenet.net>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
bundle.c | 57 +++++++++++++++++++++++++++++++++++++--------------------
1 files changed, 37 insertions(+), 20 deletions(-)
diff --git a/bundle.c b/bundle.c
index 8ba6479..311c554 100644
--- a/bundle.c
+++ b/bundle.c
@@ -193,7 +193,17 @@ static int is_tag_in_date_range(struct object *tag, struct rev_info *revs)
(revs->min_age == -1 || revs->min_age > date);
}
-static void list_prerequisite(int bundle_fd, struct rev_info *revs,
+static void object_array_copy(struct object_array *dest, const struct object_array *src)
+{
+ int i;
+
+ for (i = 0; i < src->nr; i++) {
+ struct object_array_entry *e = src->objects + i;
+ add_object_array_with_mode(e->item, e->name, dest, e->mode);
+ }
+}
+
+static void list_prerequisite(int bundle_fd, struct object_array *pending,
struct commit *rev)
{
struct strbuf buf = STRBUF_INIT;
@@ -216,45 +226,52 @@ static void list_prerequisite(int bundle_fd, struct rev_info *revs,
write_or_die(bundle_fd, buf.buf, buf.len);
rev->object.flags |= UNINTERESTING;
- add_pending_object(revs, &rev->object, buf.buf);
+ add_object_array(&rev->object, buf.buf, pending);
strbuf_release(&buf);
}
static int list_prerequisites(int bundle_fd, struct rev_info *revs,
- int argc, const char * const *argv)
+ int argc, const char **argv)
{
- const char **argv_boundary = xmalloc((argc + 1) * sizeof(const char *));
- struct rev_info boundary_revs;
struct commit *rev;
+ struct object_array pending_copy = { 0, 0, NULL };
- memcpy(argv_boundary, argv, (argc + 1) * sizeof(const char *));
+ revs->simplify_history = 0;
+ argc = setup_revisions(argc, argv, revs, NULL);
+ if (argc > 1)
+ return error("unrecognized argument: %s", argv[1]);
- init_revisions(&boundary_revs, NULL);
- boundary_revs.boundary = 1;
- if (setup_revisions(argc, argv_boundary, &boundary_revs, NULL) > 1)
- return error("unrecognized argument: %s", argv_boundary[1]);
- if (prepare_revision_walk(&boundary_revs))
+ if (revs->reflog_info)
+ return error("bundle does not support --walk-reflogs");
+ if (revs->no_walk)
+ return error("bundle does not support --no-walk");
+ if (revs->simplify_history)
+ return error("bundle and history simplification do not mix");
+
+ object_array_copy(&pending_copy, &revs->pending);
+
+ revs->boundary = 1;
+ if (prepare_revision_walk(revs))
return error("revision walk setup failed");
while ((rev = get_revision(revs))) {
if (rev->object.flags & BOUNDARY)
- list_prerequisite(bundle_fd, revs, rev);
+ list_prerequisite(bundle_fd, &pending_copy, rev);
else
rev->object.flags |= SHOWN;
}
+
+ if (revs->pending.objects != NULL)
+ return error("%u revisions pending after boundary walk",
+ revs->pending.nr);
+ revs->pending = pending_copy;
return 0;
}
-static int bundle_list_refs(int bundle_fd,
- int argc, const char **argv, struct rev_info *revs)
+static int bundle_list_refs(int bundle_fd, struct rev_info *revs)
{
int i, ref_count = 0;
- argc = setup_revisions(argc, argv, revs, NULL);
-
- if (argc > 1)
- return error("unrecognized argument: %s'", argv[1]);
-
object_array_remove_duplicates(&revs->pending);
for (i = 0; i < revs->pending.nr; i++) {
@@ -367,7 +384,7 @@ int create_bundle(struct bundle_header *header, const char *path,
return -1;
/* write references */
- if (bundle_list_refs(bundle_fd, argc, argv, &revs))
+ if (bundle_list_refs(bundle_fd, &revs))
return -1;
/* end header */
--
1.7.1.198.g8d802
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH 6/8] Fix bundle --stdin
2010-06-26 6:17 ` [RFC/PATCH 0/8] Re: bug: git-bundle create foo --stdin -> segfault Jonathan Nieder
` (4 preceding siblings ...)
2010-06-26 6:22 ` [PATCH 5/8] bundle: reuse setup_revisions result Jonathan Nieder
@ 2010-06-26 6:28 ` Jonathan Nieder
2010-06-26 6:29 ` [RFC/PATCH 7/8] bundle: Keep names of basis refs after discovery Jonathan Nieder
2010-06-26 6:31 ` [PATCH 8/8] bundle_create: Do not exit when given no revs to bundle Jonathan Nieder
7 siblings, 0 replies; 19+ messages in thread
From: Jonathan Nieder @ 2010-06-26 6:28 UTC (permalink / raw)
To: Joey Hess; +Cc: git, 554682, Adam Brewster, Johannes Schindelin
To write a bundle’s table of contents, the name passed on the command
line for each ref is needed. Unfortunately, names passed through
stdin are put in a temporary buffer and then freed; the random
gibberish that tends to replace them does not look like a meaningful
ref name to bundle_list_refs(), so no valid toc entries are found and
‘git bundle --stdin’ thinks it has been asked to create an empty
bundle.
So teach the revision walker to keep rev names after reading them from
stdin. This fixes ‘git bundle --stdin’ at the cost of a memory leak.
Reported-by: Joey Hess <joey@kitenet.net>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
revision.c | 2 +-
t/t5704-bundle.sh | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/revision.c b/revision.c
index f4b8b38..cf86af3 100644
--- a/revision.c
+++ b/revision.c
@@ -1022,7 +1022,7 @@ static void read_revisions_from_stdin(struct rev_info *revs, const char ***prune
}
die("options not supported in --stdin mode");
}
- if (handle_revision_arg(sb.buf, revs, 0, 1))
+ if (handle_revision_arg(xstrdup(sb.buf), revs, 0, 1))
die("bad revision '%s'", sb.buf);
}
if (seen_dashdash)
diff --git a/t/t5704-bundle.sh b/t/t5704-bundle.sh
index ddc3dc5..cc463f3 100755
--- a/t/t5704-bundle.sh
+++ b/t/t5704-bundle.sh
@@ -30,7 +30,7 @@ test_expect_success 'tags can be excluded by rev-list options' '
'
-test_expect_failure 'bundle --stdin' '
+test_expect_success 'bundle --stdin' '
echo master | git bundle create stdin-bundle.bdl --stdin &&
git ls-remote stdin-bundle.bdl >output &&
@@ -38,7 +38,7 @@ test_expect_failure 'bundle --stdin' '
'
-test_expect_failure 'bundle --stdin <rev-list options>' '
+test_expect_success 'bundle --stdin <rev-list options>' '
echo master | git bundle create hybrid-bundle.bdl --stdin tag &&
git ls-remote hybrid-bundle.bdl >output &&
--
1.7.1.198.g8d802
^ permalink raw reply related [flat|nested] 19+ messages in thread* [RFC/PATCH 7/8] bundle: Keep names of basis refs after discovery
2010-06-26 6:17 ` [RFC/PATCH 0/8] Re: bug: git-bundle create foo --stdin -> segfault Jonathan Nieder
` (5 preceding siblings ...)
2010-06-26 6:28 ` [PATCH 6/8] Fix bundle --stdin Jonathan Nieder
@ 2010-06-26 6:29 ` Jonathan Nieder
2010-06-26 6:31 ` [PATCH 8/8] bundle_create: Do not exit when given no revs to bundle Jonathan Nieder
7 siblings, 0 replies; 19+ messages in thread
From: Jonathan Nieder @ 2010-06-26 6:29 UTC (permalink / raw)
To: Joey Hess; +Cc: git, 554682, Adam Brewster, Johannes Schindelin
Without this change, attempts to examine revs->pending.objects[i].name
when debugging produce random giberish. On the other hand, it
introduces a small per-basis-ref memory leak.
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
For illustration.
bundle.c | 3 +--
1 files changed, 1 insertions(+), 2 deletions(-)
diff --git a/bundle.c b/bundle.c
index 311c554..7aff369 100644
--- a/bundle.c
+++ b/bundle.c
@@ -226,8 +226,7 @@ static void list_prerequisite(int bundle_fd, struct object_array *pending,
write_or_die(bundle_fd, buf.buf, buf.len);
rev->object.flags |= UNINTERESTING;
- add_object_array(&rev->object, buf.buf, pending);
- strbuf_release(&buf);
+ add_object_array(&rev->object, strbuf_detach(&buf, NULL), pending);
}
static int list_prerequisites(int bundle_fd, struct rev_info *revs,
--
1.7.1.198.g8d802
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH 8/8] bundle_create: Do not exit when given no revs to bundle
2010-06-26 6:17 ` [RFC/PATCH 0/8] Re: bug: git-bundle create foo --stdin -> segfault Jonathan Nieder
` (6 preceding siblings ...)
2010-06-26 6:29 ` [RFC/PATCH 7/8] bundle: Keep names of basis refs after discovery Jonathan Nieder
@ 2010-06-26 6:31 ` Jonathan Nieder
7 siblings, 0 replies; 19+ messages in thread
From: Jonathan Nieder @ 2010-06-26 6:31 UTC (permalink / raw)
To: Joey Hess; +Cc: git, 554682, Adam Brewster, Johannes Schindelin
Return an error instead to help with the libification effort.
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
That’s the end of the series. Thanks for reading.
I was a bit torn about whether to present this as a request for
comment or a patch series ready for application. On one hand it fixes
a bug; on the other hand, I have very little confidence that it works
well in the presence of arbitrary rev-list options. Thoughts and
testing would be very welcome.
Good night,
Jonathan
bundle.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/bundle.c b/bundle.c
index 7aff369..7dd3f65 100644
--- a/bundle.c
+++ b/bundle.c
@@ -349,7 +349,7 @@ static int bundle_list_refs(int bundle_fd, struct rev_info *revs)
free(ref);
}
if (!ref_count)
- die("Refusing to create empty bundle.");
+ return error("Refusing to create empty bundle.");
return 0;
}
--
1.7.1.198.g8d802
^ permalink raw reply related [flat|nested] 19+ messages in thread