On Sun, 7 Jun 2009, Nick Edelen wrote:
> how does this look?
Comments below.
> Signed-off-by: Nick Edelen <sirnot@xxxxxxxxx>
>
> ---
> t/t5530-upload-pack-error.sh | 2 +-
> upload-pack.c | 50 +++++++++++++++++++++++++++++++++--------
> 2 files changed, 41 insertions(+), 11 deletions(-)
>
> diff --git a/t/t5530-upload-pack-error.sh b/t/t5530-upload-pack-error.sh
> index f5102b9..26bcd1e 100755
> --- a/t/t5530-upload-pack-error.sh
> +++ b/t/t5530-upload-pack-error.sh
> @@ -51,7 +51,7 @@ test_expect_success 'fsck fails' '
> test_expect_success 'upload-pack fails due to error in rev-list' '
>
> ! echo "0032want $(git rev-parse HEAD)
> -00000009done
> +0034shallow $(git rev-parse HEAD^)00000009done
Why did you modify this?
> 0000" | git upload-pack . > /dev/null 2> output.err &&
> grep "waitpid (async) failed" output.err
> '
> diff --git a/upload-pack.c b/upload-pack.c
> index edc7861..c8f2dca 100644
> --- a/upload-pack.c
> +++ b/upload-pack.c
> @@ -29,6 +29,7 @@ static unsigned long oldest_have;
> static int multi_ack, nr_our_refs;
> static int use_thin_pack, use_ofs_delta, use_include_tag;
> static int no_progress;
> +static int shallow_nr;
> static struct object_array have_obj;
> static struct object_array want_obj;
> static unsigned int timeout;
> @@ -107,8 +108,6 @@ static int do_rev_list(int fd, void *create_full_pack)
> struct rev_info revs;
>
> pack_pipe = fdopen(fd, "w");
> - if (create_full_pack)
> - use_thin_pack = 0; /* no point doing it */
> init_revisions(&revs, NULL);
> revs.tag_objects = 1;
> revs.tree_objects = 1;
> @@ -155,13 +154,22 @@ static void create_pack_file(void)
> const char *argv[10];
> int arg = 0;
>
> - rev_list.proc = do_rev_list;
> - /* .data is just a boolean: any non-NULL value will do */
> - rev_list.data = create_full_pack ? &rev_list : NULL;
I'm glad you got rid of that.
> - if (start_async(&rev_list))
> - die("git upload-pack: unable to fork git-rev-list");
> + if (shallow_nr) {
> + rev_list.proc = do_rev_list;
> + rev_list.data = 0;
> + if (start_async(&rev_list))
> + die("git upload-pack: unable to fork git-rev-list");
> + argv[arg++] = "pack-objects";
> + } else {
> + argv[arg++] = "pack-objects";
> + argv[arg++] = "--revs";
> + argv[arg++] = "--include-tag";
Why this unconditional --include-tags here? Isn't it handled already a
couple lines down already?
> + if (create_full_pack)
> + argv[arg++] = "--all";
> + if (use_thin_pack)
> + argv[arg++] = "--thin";
Please turn this "if (use_thin_pack)" into an "else if (use_thin_pack)"
instead. No point using --thin for a full pack.
The rest looks fine to me.
Nicolas
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html