From 42e18fbf5f94dd6bd303bf702e030a29fa39d6c4 Mon Sep 17 00:00:00 2001 From: Nicolas Pitre Date: Tue, 16 Oct 2007 21:55:45 -0400 Subject: more compact progress display Each progress can be on a single line instead of two. [sp: Changed "Checking files out" to "Checking out files" at Johannes Sixt's suggestion as it better explains the action that is taking place] Signed-off-by: Nicolas Pitre Signed-off-by: Shawn O. Pearce --- builtin-pack-objects.c | 16 +++++---------- builtin-unpack-objects.c | 2 +- index-pack.c | 4 ++-- progress.c | 53 +++++++++++++++++++++++------------------------- progress.h | 10 ++++----- unpack-trees.c | 4 ++-- 6 files changed, 39 insertions(+), 50 deletions(-) diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c index 0be539ed7f..df69abd514 100644 --- a/builtin-pack-objects.c +++ b/builtin-pack-objects.c @@ -606,7 +606,7 @@ static void write_pack_file(void) uint32_t nr_remaining = nr_result; if (do_progress) - start_progress(&progress_state, "Writing %u objects...", "", nr_result); + start_progress(&progress_state, "Writing objects", nr_result); written_list = xmalloc(nr_objects * sizeof(struct object_entry *)); do { @@ -1717,9 +1717,8 @@ static void prepare_pack(int window, int depth) if (nr_deltas) { unsigned nr_done = 0; if (progress) - start_progress(&progress_state, - "Deltifying %u objects...", "", - nr_deltas); + start_progress(&progress_state, "Deltifying objects", + nr_deltas); qsort(delta_list, n, sizeof(*delta_list), type_size_sort); ll_find_deltas(delta_list, n, window+1, depth, &nr_done); if (progress) @@ -2135,23 +2134,18 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix) prepare_packed_git(); if (progress) - start_progress(&progress_state, "Generating pack...", - "Counting objects: ", 0); + start_progress(&progress_state, "Counting objects", 0); if (!use_internal_rev_list) read_object_list_from_stdin(); else { rp_av[rp_ac] = NULL; get_object_list(rp_ac, rp_av); } - if (progress) { + if (progress) stop_progress(&progress_state); - fprintf(stderr, "Done counting %u objects.\n", nr_objects); - } if (non_empty && !nr_result) return 0; - if (progress && (nr_objects != nr_result)) - fprintf(stderr, "Result has %u objects.\n", nr_result); if (nr_result) prepare_pack(window, depth); write_pack_file(); diff --git a/builtin-unpack-objects.c b/builtin-unpack-objects.c index a6ff62fd8c..2317b8f411 100644 --- a/builtin-unpack-objects.c +++ b/builtin-unpack-objects.c @@ -322,7 +322,7 @@ static void unpack_all(void) use(sizeof(struct pack_header)); if (!quiet) - start_progress(&progress, "Unpacking %u objects...", "", nr_objects); + start_progress(&progress, "Unpacking objects", nr_objects); obj_list = xmalloc(nr_objects * sizeof(*obj_list)); for (i = 0; i < nr_objects; i++) { unpack_one(i); diff --git a/index-pack.c b/index-pack.c index db58e05041..c784decd24 100644 --- a/index-pack.c +++ b/index-pack.c @@ -406,7 +406,7 @@ static void parse_pack_objects(unsigned char *sha1) * - remember base (SHA1 or offset) for all deltas. */ if (verbose) - start_progress(&progress, "Indexing %u objects...", "", nr_objects); + start_progress(&progress, "Indexing objects", nr_objects); for (i = 0; i < nr_objects; i++) { struct object_entry *obj = &objects[i]; data = unpack_raw_entry(obj, &delta->base); @@ -455,7 +455,7 @@ static void parse_pack_objects(unsigned char *sha1) * for some more deltas. */ if (verbose) - start_progress(&progress, "Resolving %u deltas...", "", nr_deltas); + start_progress(&progress, "Resolving deltas", nr_deltas); for (i = 0; i < nr_objects; i++) { struct object_entry *obj = &objects[i]; union delta_base base; diff --git a/progress.c b/progress.c index 4344f4eed5..7629e0572b 100644 --- a/progress.c +++ b/progress.c @@ -35,10 +35,11 @@ static void clear_progress_signal(void) progress_update = 0; } -int display_progress(struct progress *progress, unsigned n) +static int display(struct progress *progress, unsigned n, int done) { + char *eol; + if (progress->delay) { - char buf[80]; if (!progress_update || --progress->delay) return 0; if (progress->total) { @@ -51,60 +52,56 @@ int display_progress(struct progress *progress, unsigned n) return 0; } } - if (snprintf(buf, sizeof(buf), - progress->delayed_title, progress->total)) - fprintf(stderr, "%s\n", buf); } + + progress->last_value = n; + eol = done ? ", done. \n" : " \r"; if (progress->total) { unsigned percent = n * 100 / progress->total; if (percent != progress->last_percent || progress_update) { progress->last_percent = percent; - fprintf(stderr, "%s%4u%% (%u/%u) done\r", - progress->prefix, percent, n, progress->total); + fprintf(stderr, "%s: %3u%% (%u/%u)%s", progress->title, + percent, n, progress->total, eol); progress_update = 0; - progress->need_lf = 1; return 1; } } else if (progress_update) { - fprintf(stderr, "%s%u\r", progress->prefix, n); + fprintf(stderr, "%s: %u%s", progress->title, n, eol); progress_update = 0; - progress->need_lf = 1; return 1; } + return 0; } -void start_progress(struct progress *progress, const char *title, - const char *prefix, unsigned total) +int display_progress(struct progress *progress, unsigned n) { - char buf[80]; - progress->prefix = prefix; - progress->total = total; - progress->last_percent = -1; - progress->delay = 0; - progress->need_lf = 0; - if (snprintf(buf, sizeof(buf), title, total)) - fprintf(stderr, "%s\n", buf); - set_progress_signal(); + return display(progress, n, 0); } void start_progress_delay(struct progress *progress, const char *title, - const char *prefix, unsigned total, - unsigned percent_treshold, unsigned delay) + unsigned total, unsigned percent_treshold, unsigned delay) { - progress->prefix = prefix; + progress->title = title; progress->total = total; + progress->last_value = -1; progress->last_percent = -1; progress->delayed_percent_treshold = percent_treshold; - progress->delayed_title = title; progress->delay = delay; - progress->need_lf = 0; set_progress_signal(); } +void start_progress(struct progress *progress, const char *title, unsigned total) +{ + start_progress_delay(progress, title, total, 0, 0); +} + void stop_progress(struct progress *progress) { + if (progress->last_value != -1) { + /* Force the last update */ + progress_update = 1; + display(progress, progress->last_value, 1); + } clear_progress_signal(); - if (progress->need_lf) - fputc('\n', stderr); } diff --git a/progress.h b/progress.h index a7c17ca7c4..07b56bdbb5 100644 --- a/progress.h +++ b/progress.h @@ -2,21 +2,19 @@ #define PROGRESS_H struct progress { - const char *prefix; + const char *title; + int last_value; unsigned total; unsigned last_percent; unsigned delay; unsigned delayed_percent_treshold; - const char *delayed_title; - int need_lf; }; int display_progress(struct progress *progress, unsigned n); void start_progress(struct progress *progress, const char *title, - const char *prefix, unsigned total); + unsigned total); void start_progress_delay(struct progress *progress, const char *title, - const char *prefix, unsigned total, - unsigned percent_treshold, unsigned delay); + unsigned total, unsigned percent_treshold, unsigned delay); void stop_progress(struct progress *progress); #endif diff --git a/unpack-trees.c b/unpack-trees.c index ccfeb6e245..32251018b2 100644 --- a/unpack-trees.c +++ b/unpack-trees.c @@ -307,8 +307,8 @@ static void check_updates(struct cache_entry **src, int nr, total++; } - start_progress_delay(&progress, "Checking %u files out...", - "", total, 50, 2); + start_progress_delay(&progress, "Checking out files", + total, 50, 2); cnt = 0; } -- cgit v1.2.3 From ed1902ef5c6a30942def50809b52e41fbdcdf13f Mon Sep 17 00:00:00 2001 From: Nicolas Pitre Date: Tue, 16 Oct 2007 21:55:46 -0400 Subject: cope with multiple line breaks within sideband progress messages A single sideband packet may sometimes contain multiple lines of progress messages, but we prepend "remote: " only to the whole buffer which creates a messed up display in that case. Make sure that the "remote: " prefix is applied to every remote lines. Signed-off-by: Nicolas Pitre Signed-off-by: Shawn O. Pearce --- sideband.c | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/sideband.c b/sideband.c index 277fa3c10d..ab8a1e990d 100644 --- a/sideband.c +++ b/sideband.c @@ -17,7 +17,7 @@ int recv_sideband(const char *me, int in_stream, int out, int err) strcpy(buf, "remote:"); while (1) { int band, len; - len = packet_read_line(in_stream, buf+7, LARGE_PACKET_MAX); + len = packet_read_line(in_stream, buf+7, LARGE_PACKET_MAX); if (len == 0) break; if (len < 1) { @@ -35,7 +35,22 @@ int recv_sideband(const char *me, int in_stream, int out, int err) return SIDEBAND_REMOTE_ERROR; case 2: buf[7] = ' '; - safe_write(err, buf, 8+len); + len += 8; + while (1) { + int brk = 8; + while (brk < len) { + brk++; + if (buf[brk-1] == '\n' || + buf[brk-1] == '\r') + break; + } + safe_write(err, buf, brk); + if (brk < len) { + memmove(buf + 8, buf + brk, len - brk); + len = len - brk + 8; + } else + break; + } continue; case 1: safe_write(out, buf+8, len); -- cgit v1.2.3 From 2f8b89472cec19a530cb697c4ee20e0ddfd747f6 Mon Sep 17 00:00:00 2001 From: Nicolas Pitre Date: Tue, 16 Oct 2007 21:55:47 -0400 Subject: pack-objects: no delta possible with only one object in the list ... so don't even try in that case, and save another useless line of progress display. Signed-off-by: Nicolas Pitre Signed-off-by: Shawn O. Pearce --- builtin-pack-objects.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c index df69abd514..d729cb7237 100644 --- a/builtin-pack-objects.c +++ b/builtin-pack-objects.c @@ -1714,7 +1714,7 @@ static void prepare_pack(int window, int depth) delta_list[n++] = entry; } - if (nr_deltas) { + if (nr_deltas && n > 1) { unsigned nr_done = 0; if (progress) start_progress(&progress_state, "Deltifying objects", -- cgit v1.2.3 From 7ba502c47bda21d060844863991083f4c319d436 Mon Sep 17 00:00:00 2001 From: Nicolas Pitre Date: Tue, 16 Oct 2007 21:55:48 -0400 Subject: pack-objects.c: fix some global variable abuse and memory leaks To keep things well layered, sha1close() now returns the file descriptor when it doesn't close the file. An ugly cast was added to the return of write_idx_file() to avoid a warning. A proper fix will come separately. Signed-off-by: Nicolas Pitre Signed-off-by: Shawn O. Pearce --- builtin-pack-objects.c | 29 +++++++++++++++-------------- csum-file.c | 23 ++++++++++++++--------- 2 files changed, 29 insertions(+), 23 deletions(-) diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c index d729cb7237..933c2526f8 100644 --- a/builtin-pack-objects.c +++ b/builtin-pack-objects.c @@ -65,8 +65,6 @@ static int no_reuse_delta, no_reuse_object, keep_unreachable; static int local; static int incremental; static int allow_ofs_delta; -static const char *pack_tmp_name, *idx_tmp_name; -static char tmpname[PATH_MAX]; static const char *base_name; static int progress = 1; static int window = 10; @@ -587,12 +585,6 @@ static off_t write_one(struct sha1file *f, return offset + size; } -static int open_object_dir_tmp(const char *path) -{ - snprintf(tmpname, sizeof(tmpname), "%s/%s", get_object_directory(), path); - return xmkstemp(tmpname); -} - /* forward declaration for write_pack_file */ static int adjust_perm(const char *path, mode_t mode); @@ -611,11 +603,16 @@ static void write_pack_file(void) do { unsigned char sha1[20]; + char *pack_tmp_name = NULL; if (pack_to_stdout) { f = sha1fd(1, ""); } else { - int fd = open_object_dir_tmp("tmp_pack_XXXXXX"); + char tmpname[PATH_MAX]; + int fd; + snprintf(tmpname, sizeof(tmpname), + "%s/tmp_pack_XXXXXX", get_object_directory()); + fd = xmkstemp(tmpname); pack_tmp_name = xstrdup(tmpname); f = sha1fd(fd, pack_tmp_name); } @@ -643,19 +640,21 @@ static void write_pack_file(void) if (pack_to_stdout || nr_written == nr_remaining) { sha1close(f, sha1, 1); } else { - sha1close(f, sha1, 0); - fixup_pack_header_footer(f->fd, sha1, pack_tmp_name, nr_written); - close(f->fd); + int fd = sha1close(f, NULL, 0); + fixup_pack_header_footer(fd, sha1, pack_tmp_name, nr_written); + close(fd); } if (!pack_to_stdout) { mode_t mode = umask(0); + char *idx_tmp_name, tmpname[PATH_MAX]; umask(mode); mode = 0444 & ~mode; - idx_tmp_name = write_idx_file(NULL, - (struct pack_idx_entry **) written_list, nr_written, sha1); + idx_tmp_name = (char *) write_idx_file(NULL, + (struct pack_idx_entry **) written_list, + nr_written, sha1); snprintf(tmpname, sizeof(tmpname), "%s-%s.pack", base_name, sha1_to_hex(sha1)); if (adjust_perm(pack_tmp_name, mode)) @@ -672,6 +671,8 @@ static void write_pack_file(void) if (rename(idx_tmp_name, tmpname)) die("unable to rename temporary index file: %s", strerror(errno)); + free(idx_tmp_name); + free(pack_tmp_name); puts(sha1_to_hex(sha1)); } diff --git a/csum-file.c b/csum-file.c index 9ab997120d..9929991dea 100644 --- a/csum-file.c +++ b/csum-file.c @@ -31,22 +31,27 @@ static void sha1flush(struct sha1file *f, unsigned int count) int sha1close(struct sha1file *f, unsigned char *result, int final) { + int fd; unsigned offset = f->offset; if (offset) { SHA1_Update(&f->ctx, f->buffer, offset); sha1flush(f, offset); f->offset = 0; } - if (!final) - return 0; /* only want to flush (no checksum write, no close) */ - SHA1_Final(f->buffer, &f->ctx); - if (result) - hashcpy(result, f->buffer); - sha1flush(f, 20); - if (close(f->fd)) - die("%s: sha1 file error on close (%s)", f->name, strerror(errno)); + if (final) { + /* write checksum and close fd */ + SHA1_Final(f->buffer, &f->ctx); + if (result) + hashcpy(result, f->buffer); + sha1flush(f, 20); + if (close(f->fd)) + die("%s: sha1 file error on close (%s)", + f->name, strerror(errno)); + fd = 0; + } else + fd = f->fd; free(f); - return 0; + return fd; } int sha1write(struct sha1file *f, void *buf, unsigned int count) -- cgit v1.2.3 From 4049b9cfc082affc6365539138f6f5c546bb5685 Mon Sep 17 00:00:00 2001 From: Nicolas Pitre Date: Tue, 16 Oct 2007 21:55:49 -0400 Subject: fix const issues with some functions Two functions, namely write_idx_file() and open_pack_file(), currently return a const pointer. However that pointer is either a copy of the first argument, or set to a malloc'd buffer when that first argument is null. In the later case it is wrong to qualify that pointer as const since ownership of the buffer is transferred to the caller to dispose of, and obviously the free() function is not meant to be passed const pointers. Making the return pointer not const causes a warning when the first argument is returned since that argument is also marked const. The correct thing to do is therefore to remove the const qualifiers, avoiding the need for ugly casts only to silence some warnings. Signed-off-by: Nicolas Pitre Signed-off-by: Shawn O. Pearce --- builtin-pack-objects.c | 2 +- index-pack.c | 8 ++++---- pack-write.c | 3 ++- pack.h | 2 +- 4 files changed, 8 insertions(+), 7 deletions(-) diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c index 933c2526f8..15d3750616 100644 --- a/builtin-pack-objects.c +++ b/builtin-pack-objects.c @@ -652,7 +652,7 @@ static void write_pack_file(void) umask(mode); mode = 0444 & ~mode; - idx_tmp_name = (char *) write_idx_file(NULL, + idx_tmp_name = write_idx_file(NULL, (struct pack_idx_entry **) written_list, nr_written, sha1); snprintf(tmpname, sizeof(tmpname), "%s-%s.pack", diff --git a/index-pack.c b/index-pack.c index c784decd24..60173d5192 100644 --- a/index-pack.c +++ b/index-pack.c @@ -106,7 +106,7 @@ static void use(int bytes) consumed_bytes += bytes; } -static const char *open_pack_file(const char *pack_name) +static char *open_pack_file(char *pack_name) { if (from_stdin) { input_fd = 0; @@ -686,15 +686,15 @@ static void final(const char *final_pack_name, const char *curr_pack_name, int main(int argc, char **argv) { int i, fix_thin_pack = 0; - const char *curr_pack, *pack_name = NULL; - const char *curr_index, *index_name = NULL; + char *curr_pack, *pack_name = NULL; + char *curr_index, *index_name = NULL; const char *keep_name = NULL, *keep_msg = NULL; char *index_name_buf = NULL, *keep_name_buf = NULL; struct pack_idx_entry **idx_objects; unsigned char sha1[20]; for (i = 1; i < argc; i++) { - const char *arg = argv[i]; + char *arg = argv[i]; if (*arg == '-') { if (!strcmp(arg, "--stdin")) { diff --git a/pack-write.c b/pack-write.c index e59b197e5e..d1ed3abe21 100644 --- a/pack-write.c +++ b/pack-write.c @@ -17,7 +17,8 @@ static int sha1_compare(const void *_a, const void *_b) * the SHA1 hash of sorted object names. The objects array passed in * will be sorted by SHA1 on exit. */ -const char *write_idx_file(const char *index_name, struct pack_idx_entry **objects, int nr_objects, unsigned char *sha1) +char *write_idx_file(char *index_name, struct pack_idx_entry **objects, + int nr_objects, unsigned char *sha1) { struct sha1file *f; struct pack_idx_entry **sorted_by_sha, **list, **last; diff --git a/pack.h b/pack.h index f357c9f428..cab4aa8381 100644 --- a/pack.h +++ b/pack.h @@ -55,7 +55,7 @@ struct pack_idx_entry { off_t offset; }; -extern const char *write_idx_file(const char *index_name, struct pack_idx_entry **objects, int nr_objects, unsigned char *sha1); +extern char *write_idx_file(char *index_name, struct pack_idx_entry **objects, int nr_objects, unsigned char *sha1); extern int verify_pack(struct packed_git *, int); extern void fixup_pack_header_footer(int, unsigned char *, const char *, uint32_t); -- cgit v1.2.3 From c85228ed8f31eb739e19cf8abcff84fad44c1258 Mon Sep 17 00:00:00 2001 From: Nicolas Pitre Date: Tue, 16 Oct 2007 21:55:50 -0400 Subject: fix for more minor memory leaks Now that some pointers have lost their const attribute, we can free their associated memory when done with them. This is more a correctness issue about the rule for freeing those pointers which isn't completely trivial more than the leak itself which didn't matter as the program is exiting anyway. Signed-off-by: Nicolas Pitre Signed-off-by: Shawn O. Pearce --- index-pack.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/index-pack.c b/index-pack.c index 60173d5192..2f149a40fd 100644 --- a/index-pack.c +++ b/index-pack.c @@ -815,6 +815,10 @@ int main(int argc, char **argv) free(objects); free(index_name_buf); free(keep_name_buf); + if (pack_name == NULL) + free(curr_pack); + if (index_name == NULL) + free(curr_index); return 0; } -- cgit v1.2.3 From 9c60a966f2c3784cdff31aa6ad9f2086df64d5ba Mon Sep 17 00:00:00 2001 From: "Shawn O. Pearce" Date: Thu, 18 Oct 2007 20:42:20 -0400 Subject: Change 'Deltifying objects' to 'Compressing objects' Recently I was referred to the Grammar Police as the git-pack-objects progress message 'Deltifying %u objects' is considered to be not proper English to at least some small but vocal segment of the English speaking population. Techncially we are applying delta compression to these objects at this stage, so the new term is slightly more acceptable to the Grammar Police but is also just as correct. Signed-off-by: Shawn O. Pearce --- builtin-pack-objects.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c index 15d3750616..21ba977df3 100644 --- a/builtin-pack-objects.c +++ b/builtin-pack-objects.c @@ -1718,7 +1718,7 @@ static void prepare_pack(int window, int depth) if (nr_deltas && n > 1) { unsigned nr_done = 0; if (progress) - start_progress(&progress_state, "Deltifying objects", + start_progress(&progress_state, "Compressing objects", nr_deltas); qsort(delta_list, n, sizeof(*delta_list), type_size_sort); ll_find_deltas(delta_list, n, window+1, depth, &nr_done); -- cgit v1.2.3 From b5d72f0a4cd3cce945ca0d37e4fa0ebbfcdcdb52 Mon Sep 17 00:00:00 2001 From: "Shawn O. Pearce" Date: Fri, 19 Oct 2007 00:08:37 -0400 Subject: Teach prune-packed to use the standard progress meter Rather than reimplementing the progress meter logic and always showing 100 lines of output while pruning already packed objects we now use a delayed progress meter and only show it if there are enough objects to make us take a little while. Most users won't see the message anymore as it usually doesn't take very long to delete the already packed loose objects. This neatens the output of a git-gc or git-repack execution, which is especially important for a `git gc --auto` triggered from within another command. We perform the display_progress() call from within the very innermost loop in case we spend more than 1 second within any single object directory. This ensures that a progress_update event from the timer will still trigger in a timely fashion and allow the user to see the progress meter. While I'm in here I changed the message to be more descriptive of its actual task. "Removing unused objects" is a little scary for new users as they wonder where these unused objects came from and how they should avoid them. Truth is these objects aren't unused in the sense of what git-prune would call a dangling object, these are used but are just duplicates of things we have already stored in a packfile. Signed-off-by: Shawn O. Pearce --- builtin-prune-packed.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/builtin-prune-packed.c b/builtin-prune-packed.c index 977730064b..015c8bb7cc 100644 --- a/builtin-prune-packed.c +++ b/builtin-prune-packed.c @@ -1,5 +1,6 @@ #include "builtin.h" #include "cache.h" +#include "progress.h" static const char prune_packed_usage[] = "git-prune-packed [-n] [-q]"; @@ -7,6 +8,8 @@ static const char prune_packed_usage[] = #define DRY_RUN 01 #define VERBOSE 02 +static struct progress progress; + static void prune_dir(int i, DIR *dir, char *pathname, int len, int opts) { struct dirent *de; @@ -23,6 +26,8 @@ static void prune_dir(int i, DIR *dir, char *pathname, int len, int opts) if (!has_sha1_pack(sha1, NULL)) continue; memcpy(pathname + len, de->d_name, 38); + if (opts == VERBOSE) + display_progress(&progress, i + 1); if (opts & DRY_RUN) printf("rm -f %s\n", pathname); else if (unlink(pathname) < 0) @@ -39,6 +44,11 @@ void prune_packed_objects(int opts) const char *dir = get_object_directory(); int len = strlen(dir); + if (opts == VERBOSE) + start_progress_delay(&progress, + "Removing duplicate objects", + 256, 95, 2); + if (len > PATH_MAX - 42) die("impossible object directory"); memcpy(pathname, dir, len); @@ -49,16 +59,13 @@ void prune_packed_objects(int opts) sprintf(pathname + len, "%02x/", i); d = opendir(pathname); - if (opts == VERBOSE && (d || i == 255)) - fprintf(stderr, "Removing unused objects %d%%...\015", - ((i+1) * 100) / 256); if (!d) continue; prune_dir(i, d, pathname, len + 3, opts); closedir(d); } if (opts == VERBOSE) - fprintf(stderr, "\nDone.\n"); + stop_progress(&progress); } int cmd_prune_packed(int argc, const char **argv, const char *prefix) -- cgit v1.2.3 From 0e30404370c24047c5fa54b2a6c43e53629c916b Mon Sep 17 00:00:00 2001 From: "Shawn O. Pearce" Date: Fri, 19 Oct 2007 01:01:40 -0400 Subject: Stop displaying "Pack pack-$ID created." during git-gc Discussion on the list tonight came to the conclusion that showing the name of the packfile we just created during git-repack is not a very useful message for any end-user. For the really technical folk who need to have the name of the newest packfile they can use something such as `ls -t .git/objects/pack | head -2` to find the most recently created packfile. Signed-off-by: Shawn O. Pearce --- git-repack.sh | 3 --- 1 file changed, 3 deletions(-) diff --git a/git-repack.sh b/git-repack.sh index e72adc4d91..7220635c96 100755 --- a/git-repack.sh +++ b/git-repack.sh @@ -83,9 +83,6 @@ for name in $names ; do fullbases="$fullbases pack-$name" chmod a-w "$PACKTMP-$name.pack" chmod a-w "$PACKTMP-$name.idx" - if test "$quiet" != '-q'; then - echo "Pack pack-$name created." - fi mkdir -p "$PACKDIR" || exit for sfx in pack idx -- cgit v1.2.3 From 0e549137966feb016927a827fb6e359aec8264a3 Mon Sep 17 00:00:00 2001 From: Nicolas Pitre Date: Tue, 30 Oct 2007 14:57:31 -0400 Subject: prune-packed: don't call display_progress() for every file The progress count is per fanout directory, so it is useless to call it for every file as the count doesn't change that often. Signed-off-by: Nicolas Pitre Signed-off-by: Junio C Hamano --- builtin-prune-packed.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/builtin-prune-packed.c b/builtin-prune-packed.c index 015c8bb7cc..907e36828f 100644 --- a/builtin-prune-packed.c +++ b/builtin-prune-packed.c @@ -15,6 +15,9 @@ static void prune_dir(int i, DIR *dir, char *pathname, int len, int opts) struct dirent *de; char hex[40]; + if (opts == VERBOSE) + display_progress(&progress, i + 1); + sprintf(hex, "%02x", i); while ((de = readdir(dir)) != NULL) { unsigned char sha1[20]; @@ -26,8 +29,6 @@ static void prune_dir(int i, DIR *dir, char *pathname, int len, int opts) if (!has_sha1_pack(sha1, NULL)) continue; memcpy(pathname + len, de->d_name, 38); - if (opts == VERBOSE) - display_progress(&progress, i + 1); if (opts & DRY_RUN) printf("rm -f %s\n", pathname); else if (unlink(pathname) < 0) -- cgit v1.2.3 From dc6a0757c4f966dd124bd85be2adad5a0b7b2167 Mon Sep 17 00:00:00 2001 From: Nicolas Pitre Date: Tue, 30 Oct 2007 14:57:32 -0400 Subject: make struct progress an opaque type This allows for better management of progress "object" existence, as well as making the progress display implementation more independent from its callers. Signed-off-by: Nicolas Pitre Signed-off-by: Junio C Hamano --- builtin-pack-objects.c | 16 ++++++++-------- builtin-prune-packed.c | 7 +++---- builtin-unpack-objects.c | 6 +++--- index-pack.c | 12 ++++++------ progress.c | 33 +++++++++++++++++++++++++++------ progress.h | 18 +++++------------- unpack-trees.c | 10 +++++----- 7 files changed, 57 insertions(+), 45 deletions(-) diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c index 21ba977df3..3ca5cc7800 100644 --- a/builtin-pack-objects.c +++ b/builtin-pack-objects.c @@ -73,7 +73,7 @@ static int depth = 50; static int delta_search_threads = 1; static int pack_to_stdout; static int num_preferred_base; -static struct progress progress_state; +static struct progress *progress_state; static int pack_compression_level = Z_DEFAULT_COMPRESSION; static int pack_compression_seen; @@ -598,7 +598,7 @@ static void write_pack_file(void) uint32_t nr_remaining = nr_result; if (do_progress) - start_progress(&progress_state, "Writing objects", nr_result); + progress_state = start_progress("Writing objects", nr_result); written_list = xmalloc(nr_objects * sizeof(struct object_entry *)); do { @@ -630,7 +630,7 @@ static void write_pack_file(void) break; offset = offset_one; if (do_progress) - display_progress(&progress_state, written); + display_progress(progress_state, written); } /* @@ -854,7 +854,7 @@ static int add_object_entry(const unsigned char *sha1, enum object_type type, object_ix[-1 - ix] = nr_objects; if (progress) - display_progress(&progress_state, nr_objects); + display_progress(progress_state, nr_objects); if (name && no_try_delta(name)) entry->no_try_delta = 1; @@ -1518,7 +1518,7 @@ static void find_deltas(struct object_entry **list, unsigned list_size, progress_lock(); (*processed)++; if (progress) - display_progress(&progress_state, *processed); + display_progress(progress_state, *processed); progress_unlock(); /* @@ -1718,8 +1718,8 @@ static void prepare_pack(int window, int depth) if (nr_deltas && n > 1) { unsigned nr_done = 0; if (progress) - start_progress(&progress_state, "Compressing objects", - nr_deltas); + progress_state = start_progress("Compressing objects", + nr_deltas); qsort(delta_list, n, sizeof(*delta_list), type_size_sort); ll_find_deltas(delta_list, n, window+1, depth, &nr_done); if (progress) @@ -2135,7 +2135,7 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix) prepare_packed_git(); if (progress) - start_progress(&progress_state, "Counting objects", 0); + progress_state = start_progress("Counting objects", 0); if (!use_internal_rev_list) read_object_list_from_stdin(); else { diff --git a/builtin-prune-packed.c b/builtin-prune-packed.c index 907e36828f..c66fb037f6 100644 --- a/builtin-prune-packed.c +++ b/builtin-prune-packed.c @@ -8,7 +8,7 @@ static const char prune_packed_usage[] = #define DRY_RUN 01 #define VERBOSE 02 -static struct progress progress; +static struct progress *progress; static void prune_dir(int i, DIR *dir, char *pathname, int len, int opts) { @@ -16,7 +16,7 @@ static void prune_dir(int i, DIR *dir, char *pathname, int len, int opts) char hex[40]; if (opts == VERBOSE) - display_progress(&progress, i + 1); + display_progress(progress, i + 1); sprintf(hex, "%02x", i); while ((de = readdir(dir)) != NULL) { @@ -46,8 +46,7 @@ void prune_packed_objects(int opts) int len = strlen(dir); if (opts == VERBOSE) - start_progress_delay(&progress, - "Removing duplicate objects", + progress = start_progress_delay("Removing duplicate objects", 256, 95, 2); if (len > PATH_MAX - 42) diff --git a/builtin-unpack-objects.c b/builtin-unpack-objects.c index 2317b8f411..e0ecbc51a7 100644 --- a/builtin-unpack-objects.c +++ b/builtin-unpack-objects.c @@ -311,7 +311,7 @@ static void unpack_one(unsigned nr) static void unpack_all(void) { int i; - struct progress progress; + struct progress *progress = NULL; struct pack_header *hdr = fill(sizeof(struct pack_header)); unsigned nr_objects = ntohl(hdr->hdr_entries); @@ -322,12 +322,12 @@ static void unpack_all(void) use(sizeof(struct pack_header)); if (!quiet) - start_progress(&progress, "Unpacking objects", nr_objects); + progress = start_progress("Unpacking objects", nr_objects); obj_list = xmalloc(nr_objects * sizeof(*obj_list)); for (i = 0; i < nr_objects; i++) { unpack_one(i); if (!quiet) - display_progress(&progress, i + 1); + display_progress(progress, i + 1); } if (!quiet) stop_progress(&progress); diff --git a/index-pack.c b/index-pack.c index 2f149a40fd..b4543a4cc2 100644 --- a/index-pack.c +++ b/index-pack.c @@ -46,7 +46,7 @@ static int nr_resolved_deltas; static int from_stdin; static int verbose; -static struct progress progress; +static struct progress *progress; /* We always read in 4kB chunks. */ static unsigned char input_buffer[4096]; @@ -406,7 +406,7 @@ static void parse_pack_objects(unsigned char *sha1) * - remember base (SHA1 or offset) for all deltas. */ if (verbose) - start_progress(&progress, "Indexing objects", nr_objects); + progress = start_progress("Indexing objects", nr_objects); for (i = 0; i < nr_objects; i++) { struct object_entry *obj = &objects[i]; data = unpack_raw_entry(obj, &delta->base); @@ -419,7 +419,7 @@ static void parse_pack_objects(unsigned char *sha1) sha1_object(data, obj->size, obj->type, obj->idx.sha1); free(data); if (verbose) - display_progress(&progress, i+1); + display_progress(progress, i+1); } objects[i].idx.offset = consumed_bytes; if (verbose) @@ -455,7 +455,7 @@ static void parse_pack_objects(unsigned char *sha1) * for some more deltas. */ if (verbose) - start_progress(&progress, "Resolving deltas", nr_deltas); + progress = start_progress("Resolving deltas", nr_deltas); for (i = 0; i < nr_objects; i++) { struct object_entry *obj = &objects[i]; union delta_base base; @@ -487,7 +487,7 @@ static void parse_pack_objects(unsigned char *sha1) } free(data); if (verbose) - display_progress(&progress, nr_resolved_deltas); + display_progress(progress, nr_resolved_deltas); } } @@ -595,7 +595,7 @@ static void fix_unresolved_deltas(int nr_unresolved) append_obj_to_pack(d->base.sha1, data, size, type); free(data); if (verbose) - display_progress(&progress, nr_resolved_deltas); + display_progress(progress, nr_resolved_deltas); } free(sorted_by_pos); } diff --git a/progress.c b/progress.c index 7629e0572b..c342e39c5d 100644 --- a/progress.c +++ b/progress.c @@ -1,6 +1,15 @@ #include "git-compat-util.h" #include "progress.h" +struct progress { + const char *title; + int last_value; + unsigned total; + unsigned last_percent; + unsigned delay; + unsigned delayed_percent_treshold; +}; + static volatile sig_atomic_t progress_update; static void progress_interval(int signum) @@ -76,12 +85,18 @@ static int display(struct progress *progress, unsigned n, int done) int display_progress(struct progress *progress, unsigned n) { - return display(progress, n, 0); + return progress ? display(progress, n, 0) : 0; } -void start_progress_delay(struct progress *progress, const char *title, - unsigned total, unsigned percent_treshold, unsigned delay) +struct progress *start_progress_delay(const char *title, unsigned total, + unsigned percent_treshold, unsigned delay) { + struct progress *progress = malloc(sizeof(*progress)); + if (!progress) { + /* unlikely, but here's a good fallback */ + fprintf(stderr, "%s...\n", title); + return NULL; + } progress->title = title; progress->total = total; progress->last_value = -1; @@ -89,19 +104,25 @@ void start_progress_delay(struct progress *progress, const char *title, progress->delayed_percent_treshold = percent_treshold; progress->delay = delay; set_progress_signal(); + return progress; } -void start_progress(struct progress *progress, const char *title, unsigned total) +struct progress *start_progress(const char *title, unsigned total) { - start_progress_delay(progress, title, total, 0, 0); + return start_progress_delay(title, total, 0, 0); } -void stop_progress(struct progress *progress) +void stop_progress(struct progress **p_progress) { + struct progress *progress = *p_progress; + if (!progress) + return; + *p_progress = NULL; if (progress->last_value != -1) { /* Force the last update */ progress_update = 1; display(progress, progress->last_value, 1); } clear_progress_signal(); + free(progress); } diff --git a/progress.h b/progress.h index 07b56bdbb5..4c6d53524b 100644 --- a/progress.h +++ b/progress.h @@ -1,20 +1,12 @@ #ifndef PROGRESS_H #define PROGRESS_H -struct progress { - const char *title; - int last_value; - unsigned total; - unsigned last_percent; - unsigned delay; - unsigned delayed_percent_treshold; -}; +struct progress; int display_progress(struct progress *progress, unsigned n); -void start_progress(struct progress *progress, const char *title, - unsigned total); -void start_progress_delay(struct progress *progress, const char *title, - unsigned total, unsigned percent_treshold, unsigned delay); -void stop_progress(struct progress *progress); +struct progress *start_progress(const char *title, unsigned total); +struct progress *start_progress_delay(const char *title, unsigned total, + unsigned percent_treshold, unsigned delay); +void stop_progress(struct progress **progress); #endif diff --git a/unpack-trees.c b/unpack-trees.c index 32251018b2..6776c29cde 100644 --- a/unpack-trees.c +++ b/unpack-trees.c @@ -297,7 +297,7 @@ static void check_updates(struct cache_entry **src, int nr, { unsigned short mask = htons(CE_UPDATE); unsigned cnt = 0, total = 0; - struct progress progress; + struct progress *progress = NULL; char last_symlink[PATH_MAX]; if (o->update && o->verbose_update) { @@ -307,8 +307,8 @@ static void check_updates(struct cache_entry **src, int nr, total++; } - start_progress_delay(&progress, "Checking out files", - total, 50, 2); + progress = start_progress_delay("Checking out files", + total, 50, 2); cnt = 0; } @@ -318,7 +318,7 @@ static void check_updates(struct cache_entry **src, int nr, if (total) if (!ce->ce_mode || ce->ce_flags & mask) - display_progress(&progress, ++cnt); + display_progress(progress, ++cnt); if (!ce->ce_mode) { if (o->update) unlink_entry(ce->name, last_symlink); @@ -333,7 +333,7 @@ static void check_updates(struct cache_entry **src, int nr, } } if (total) - stop_progress(&progress);; + stop_progress(&progress); } int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options *o) -- cgit v1.2.3 From 4d4fcc5451d9d653bebcc8afa18543cb426abeed Mon Sep 17 00:00:00 2001 From: Nicolas Pitre Date: Tue, 30 Oct 2007 14:57:33 -0400 Subject: relax usage of the progress API Since it is now OK to pass a null pointer to display_progress() and stop_progress() resulting in a no-op, then we can simplify the code and remove a bunch of lines by not making those calls conditional all the time. Signed-off-by: Nicolas Pitre Signed-off-by: Junio C Hamano --- builtin-pack-objects.c | 18 ++++++------------ builtin-prune-packed.c | 6 ++---- builtin-unpack-objects.c | 6 ++---- index-pack.c | 20 +++++++------------- unpack-trees.c | 8 +++----- 5 files changed, 20 insertions(+), 38 deletions(-) diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c index 3ca5cc7800..52a26a28f4 100644 --- a/builtin-pack-objects.c +++ b/builtin-pack-objects.c @@ -629,8 +629,7 @@ static void write_pack_file(void) if (!offset_one) break; offset = offset_one; - if (do_progress) - display_progress(progress_state, written); + display_progress(progress_state, written); } /* @@ -684,8 +683,7 @@ static void write_pack_file(void) } while (nr_remaining && i < nr_objects); free(written_list); - if (do_progress) - stop_progress(&progress_state); + stop_progress(&progress_state); if (written != nr_result) die("wrote %u objects while expecting %u", written, nr_result); /* @@ -853,8 +851,7 @@ static int add_object_entry(const unsigned char *sha1, enum object_type type, else object_ix[-1 - ix] = nr_objects; - if (progress) - display_progress(progress_state, nr_objects); + display_progress(progress_state, nr_objects); if (name && no_try_delta(name)) entry->no_try_delta = 1; @@ -1517,8 +1514,7 @@ static void find_deltas(struct object_entry **list, unsigned list_size, progress_lock(); (*processed)++; - if (progress) - display_progress(progress_state, *processed); + display_progress(progress_state, *processed); progress_unlock(); /* @@ -1722,8 +1718,7 @@ static void prepare_pack(int window, int depth) nr_deltas); qsort(delta_list, n, sizeof(*delta_list), type_size_sort); ll_find_deltas(delta_list, n, window+1, depth, &nr_done); - if (progress) - stop_progress(&progress_state); + stop_progress(&progress_state); if (nr_done != nr_deltas) die("inconsistency with delta count"); } @@ -2142,8 +2137,7 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix) rp_av[rp_ac] = NULL; get_object_list(rp_ac, rp_av); } - if (progress) - stop_progress(&progress_state); + stop_progress(&progress_state); if (non_empty && !nr_result) return 0; diff --git a/builtin-prune-packed.c b/builtin-prune-packed.c index c66fb037f6..f4287dad10 100644 --- a/builtin-prune-packed.c +++ b/builtin-prune-packed.c @@ -15,8 +15,7 @@ static void prune_dir(int i, DIR *dir, char *pathname, int len, int opts) struct dirent *de; char hex[40]; - if (opts == VERBOSE) - display_progress(progress, i + 1); + display_progress(progress, i + 1); sprintf(hex, "%02x", i); while ((de = readdir(dir)) != NULL) { @@ -64,8 +63,7 @@ void prune_packed_objects(int opts) prune_dir(i, d, pathname, len + 3, opts); closedir(d); } - if (opts == VERBOSE) - stop_progress(&progress); + stop_progress(&progress); } int cmd_prune_packed(int argc, const char **argv, const char *prefix) diff --git a/builtin-unpack-objects.c b/builtin-unpack-objects.c index e0ecbc51a7..1e51865c52 100644 --- a/builtin-unpack-objects.c +++ b/builtin-unpack-objects.c @@ -326,11 +326,9 @@ static void unpack_all(void) obj_list = xmalloc(nr_objects * sizeof(*obj_list)); for (i = 0; i < nr_objects; i++) { unpack_one(i); - if (!quiet) - display_progress(progress, i + 1); + display_progress(progress, i + 1); } - if (!quiet) - stop_progress(&progress); + stop_progress(&progress); if (delta_list) die("unresolved deltas left after unpacking"); diff --git a/index-pack.c b/index-pack.c index b4543a4cc2..879ea15485 100644 --- a/index-pack.c +++ b/index-pack.c @@ -418,12 +418,10 @@ static void parse_pack_objects(unsigned char *sha1) } else sha1_object(data, obj->size, obj->type, obj->idx.sha1); free(data); - if (verbose) - display_progress(progress, i+1); + display_progress(progress, i+1); } objects[i].idx.offset = consumed_bytes; - if (verbose) - stop_progress(&progress); + stop_progress(&progress); /* Check pack integrity */ flush(); @@ -486,8 +484,7 @@ static void parse_pack_objects(unsigned char *sha1) obj->size, obj->type); } free(data); - if (verbose) - display_progress(progress, nr_resolved_deltas); + display_progress(progress, nr_resolved_deltas); } } @@ -594,8 +591,7 @@ static void fix_unresolved_deltas(int nr_unresolved) die("local object %s is corrupt", sha1_to_hex(d->base.sha1)); append_obj_to_pack(d->base.sha1, data, size, type); free(data); - if (verbose) - display_progress(progress, nr_resolved_deltas); + display_progress(progress, nr_resolved_deltas); } free(sorted_by_pos); } @@ -774,8 +770,7 @@ int main(int argc, char **argv) deltas = xmalloc(nr_objects * sizeof(struct delta_entry)); parse_pack_objects(sha1); if (nr_deltas == nr_resolved_deltas) { - if (verbose) - stop_progress(&progress); + stop_progress(&progress); /* Flush remaining pack final 20-byte SHA1. */ flush(); } else { @@ -788,11 +783,10 @@ int main(int argc, char **argv) (nr_objects + nr_unresolved + 1) * sizeof(*objects)); fix_unresolved_deltas(nr_unresolved); - if (verbose) { - stop_progress(&progress); + stop_progress(&progress); + if (verbose) fprintf(stderr, "%d objects were added to complete this thin pack.\n", nr_objects - nr_objects_initial); - } fixup_pack_header_footer(output_fd, sha1, curr_pack, nr_objects); } diff --git a/unpack-trees.c b/unpack-trees.c index 6776c29cde..c527d7d049 100644 --- a/unpack-trees.c +++ b/unpack-trees.c @@ -316,9 +316,8 @@ static void check_updates(struct cache_entry **src, int nr, while (nr--) { struct cache_entry *ce = *src++; - if (total) - if (!ce->ce_mode || ce->ce_flags & mask) - display_progress(progress, ++cnt); + if (!ce->ce_mode || ce->ce_flags & mask) + display_progress(progress, ++cnt); if (!ce->ce_mode) { if (o->update) unlink_entry(ce->name, last_symlink); @@ -332,8 +331,7 @@ static void check_updates(struct cache_entry **src, int nr, } } } - if (total) - stop_progress(&progress); + stop_progress(&progress); } int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options *o) -- cgit v1.2.3 From cf84d51c43fa05cce416bfa3f5db3ad70773abdf Mon Sep 17 00:00:00 2001 From: Nicolas Pitre Date: Tue, 30 Oct 2007 14:57:34 -0400 Subject: add throughput to progress display This adds the ability for the progress code to also display transfer throughput when that makes sense. The math was inspired by commit c548cf4ee0737a321ffe94f6a97c65baf87281be from Linus. Signed-off-by: Nicolas Pitre Signed-off-by: Junio C Hamano --- progress.c | 80 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---- progress.h | 1 + 2 files changed, 77 insertions(+), 4 deletions(-) diff --git a/progress.c b/progress.c index c342e39c5d..15197fbe6c 100644 --- a/progress.c +++ b/progress.c @@ -1,6 +1,19 @@ #include "git-compat-util.h" #include "progress.h" +#define TP_IDX_MAX 8 + +struct throughput { + struct timeval prev_tv; + unsigned long count; + unsigned long avg_bytes; + unsigned long last_bytes[TP_IDX_MAX]; + unsigned int avg_misecs; + unsigned int last_misecs[TP_IDX_MAX]; + unsigned int idx; + char display[20]; +}; + struct progress { const char *title; int last_value; @@ -8,6 +21,7 @@ struct progress { unsigned last_percent; unsigned delay; unsigned delayed_percent_treshold; + struct throughput *throughput; }; static volatile sig_atomic_t progress_update; @@ -46,7 +60,7 @@ static void clear_progress_signal(void) static int display(struct progress *progress, unsigned n, int done) { - char *eol; + char *eol, *tp; if (progress->delay) { if (!progress_update || --progress->delay) @@ -64,18 +78,20 @@ static int display(struct progress *progress, unsigned n, int done) } progress->last_value = n; + tp = (progress->throughput) ? progress->throughput->display : ""; eol = done ? ", done. \n" : " \r"; if (progress->total) { unsigned percent = n * 100 / progress->total; if (percent != progress->last_percent || progress_update) { progress->last_percent = percent; - fprintf(stderr, "%s: %3u%% (%u/%u)%s", progress->title, - percent, n, progress->total, eol); + fprintf(stderr, "%s: %3u%% (%u/%u)%s%s", + progress->title, percent, n, + progress->total, tp, eol); progress_update = 0; return 1; } } else if (progress_update) { - fprintf(stderr, "%s: %u%s", progress->title, n, eol); + fprintf(stderr, "%s: %u%s%s", progress->title, n, tp, eol); progress_update = 0; return 1; } @@ -83,6 +99,60 @@ static int display(struct progress *progress, unsigned n, int done) return 0; } +void display_throughput(struct progress *progress, unsigned long n) +{ + struct throughput *tp; + struct timeval tv; + unsigned int misecs; + + if (!progress) + return; + tp = progress->throughput; + + gettimeofday(&tv, NULL); + + if (!tp) { + progress->throughput = tp = calloc(1, sizeof(*tp)); + if (tp) + tp->prev_tv = tv; + return; + } + + tp->count += n; + + /* + * We have x = bytes and y = microsecs. We want z = KiB/s: + * + * z = (x / 1024) / (y / 1000000) + * z = x / y * 1000000 / 1024 + * z = x / (y * 1024 / 1000000) + * z = x / y' + * + * To simplify things we'll keep track of misecs, or 1024th of a sec + * obtained with: + * + * y' = y * 1024 / 1000000 + * y' = y / (1000000 / 1024) + * y' = y / 977 + */ + misecs = (tv.tv_sec - tp->prev_tv.tv_sec) * 1024; + misecs += (int)(tv.tv_usec - tp->prev_tv.tv_usec) / 977; + + if (misecs > 512) { + tp->prev_tv = tv; + tp->avg_bytes += tp->count; + tp->avg_misecs += misecs; + snprintf(tp->display, sizeof(tp->display), + ", %lu KiB/s", tp->avg_bytes / tp->avg_misecs); + tp->avg_bytes -= tp->last_bytes[tp->idx]; + tp->avg_misecs -= tp->last_misecs[tp->idx]; + tp->last_bytes[tp->idx] = tp->count; + tp->last_misecs[tp->idx] = misecs; + tp->idx = (tp->idx + 1) % TP_IDX_MAX; + tp->count = 0; + } +} + int display_progress(struct progress *progress, unsigned n) { return progress ? display(progress, n, 0) : 0; @@ -103,6 +173,7 @@ struct progress *start_progress_delay(const char *title, unsigned total, progress->last_percent = -1; progress->delayed_percent_treshold = percent_treshold; progress->delay = delay; + progress->throughput = NULL; set_progress_signal(); return progress; } @@ -124,5 +195,6 @@ void stop_progress(struct progress **p_progress) display(progress, progress->last_value, 1); } clear_progress_signal(); + free(progress->throughput); free(progress); } diff --git a/progress.h b/progress.h index 4c6d53524b..61cb68dfa5 100644 --- a/progress.h +++ b/progress.h @@ -3,6 +3,7 @@ struct progress; +void display_throughput(struct progress *progress, unsigned long n); int display_progress(struct progress *progress, unsigned n); struct progress *start_progress(const char *title, unsigned total); struct progress *start_progress_delay(const char *title, unsigned total, -- cgit v1.2.3 From 29e63ed3f6962cd96f7153e9f492f36797d25c33 Mon Sep 17 00:00:00 2001 From: Nicolas Pitre Date: Tue, 30 Oct 2007 14:57:35 -0400 Subject: add throughput display to index-pack ... and call it "Receiving objects" when over stdin to look clearer to end users. Signed-off-by: Nicolas Pitre Signed-off-by: Junio C Hamano --- index-pack.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/index-pack.c b/index-pack.c index 879ea15485..61ea7621be 100644 --- a/index-pack.c +++ b/index-pack.c @@ -87,6 +87,8 @@ static void *fill(int min) die("early EOF"); die("read error on input: %s", strerror(errno)); } + if (from_stdin) + display_throughput(progress, ret); input_len += ret; } while (input_len < min); return input_buffer; @@ -406,7 +408,9 @@ static void parse_pack_objects(unsigned char *sha1) * - remember base (SHA1 or offset) for all deltas. */ if (verbose) - progress = start_progress("Indexing objects", nr_objects); + progress = start_progress( + from_stdin ? "Receiving objects" : "Indexing objects", + nr_objects); for (i = 0; i < nr_objects; i++) { struct object_entry *obj = &objects[i]; data = unpack_raw_entry(obj, &delta->base); -- cgit v1.2.3 From 74b6792f7b2d4b410459cf96977a2007ddea8675 Mon Sep 17 00:00:00 2001 From: Nicolas Pitre Date: Tue, 30 Oct 2007 15:41:13 -0400 Subject: add some copyright notice to the progress display code Some self patting on the back to keep my ego alive. Signed-off-by: Nicolas Pitre Signed-off-by: Junio C Hamano --- progress.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/progress.c b/progress.c index 15197fbe6c..34a59611fb 100644 --- a/progress.c +++ b/progress.c @@ -1,3 +1,13 @@ +/* + * Simple text-based progress display module for GIT + * + * Copyright (c) 2007 by Nicolas Pitre + * + * This code is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + */ + #include "git-compat-util.h" #include "progress.h" -- cgit v1.2.3 From 2a128d63dc140e8daeaf68659d9263b7389b7d1b Mon Sep 17 00:00:00 2001 From: Nicolas Pitre Date: Tue, 30 Oct 2007 17:06:21 -0400 Subject: add throughput display to git-push This one triggers only when git-pack-objects is called with --all-progress and --stdout which is the combination used by git-push. Signed-off-by: Nicolas Pitre Signed-off-by: Junio C Hamano --- builtin-pack-objects.c | 2 +- csum-file.c | 8 ++++++++ csum-file.h | 4 ++++ 3 files changed, 13 insertions(+), 1 deletion(-) diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c index 52a26a28f4..25ec65d0f0 100644 --- a/builtin-pack-objects.c +++ b/builtin-pack-objects.c @@ -606,7 +606,7 @@ static void write_pack_file(void) char *pack_tmp_name = NULL; if (pack_to_stdout) { - f = sha1fd(1, ""); + f = sha1fd_throughput(1, "", progress_state); } else { char tmpname[PATH_MAX]; int fd; diff --git a/csum-file.c b/csum-file.c index 9929991dea..3729e73e19 100644 --- a/csum-file.c +++ b/csum-file.c @@ -8,6 +8,7 @@ * able to verify hasn't been messed with afterwards. */ #include "cache.h" +#include "progress.h" #include "csum-file.h" static void sha1flush(struct sha1file *f, unsigned int count) @@ -17,6 +18,7 @@ static void sha1flush(struct sha1file *f, unsigned int count) for (;;) { int ret = xwrite(f->fd, buf, count); if (ret > 0) { + display_throughput(f->tp, ret); buf = (char *) buf + ret; count -= ret; if (count) @@ -79,6 +81,11 @@ int sha1write(struct sha1file *f, void *buf, unsigned int count) } struct sha1file *sha1fd(int fd, const char *name) +{ + return sha1fd_throughput(fd, name, NULL); +} + +struct sha1file *sha1fd_throughput(int fd, const char *name, struct progress *tp) { struct sha1file *f; unsigned len; @@ -94,6 +101,7 @@ struct sha1file *sha1fd(int fd, const char *name) f->fd = fd; f->error = 0; f->offset = 0; + f->tp = tp; f->do_crc = 0; SHA1_Init(&f->ctx); return f; diff --git a/csum-file.h b/csum-file.h index c3c792f1b5..4d1b231292 100644 --- a/csum-file.h +++ b/csum-file.h @@ -1,11 +1,14 @@ #ifndef CSUM_FILE_H #define CSUM_FILE_H +struct progress; + /* A SHA1-protected file */ struct sha1file { int fd, error; unsigned int offset, namelen; SHA_CTX ctx; + struct progress *tp; char name[PATH_MAX]; int do_crc; uint32_t crc32; @@ -13,6 +16,7 @@ struct sha1file { }; extern struct sha1file *sha1fd(int fd, const char *name); +extern struct sha1file *sha1fd_throughput(int fd, const char *name, struct progress *tp); extern int sha1close(struct sha1file *, unsigned char *, int); extern int sha1write(struct sha1file *, void *, unsigned int); extern void crc32_begin(struct sha1file *); -- cgit v1.2.3 From 93ff3f6a5311ebb6347b817d1c57c94dbbe4de73 Mon Sep 17 00:00:00 2001 From: Nicolas Pitre Date: Thu, 1 Nov 2007 16:59:55 -0400 Subject: return the prune-packed progress display to the inner loop This reverts commit 0e549137966feb016927a827fb6e359aec8264a3 so to return to the same state as commit b5d72f0a4cd3cce945ca0d37e4fa0ebbfcdcdb52. On Wed, 31 Oct 2007, Shawn O. Pearce wrote: > During my testing with a 40,000 loose object case (yea, I fully > unpacked a git.git clone I had laying around) my system stalled > hard in the first object directory. A *lot* longer than 1 second. > So I got no progress meter for a long time, and then a progress > meter appeared on the second directory. Signed-off-by: Nicolas Pitre Signed-off-by: Junio C Hamano --- builtin-prune-packed.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/builtin-prune-packed.c b/builtin-prune-packed.c index f4287dad10..23faf3129f 100644 --- a/builtin-prune-packed.c +++ b/builtin-prune-packed.c @@ -15,8 +15,6 @@ static void prune_dir(int i, DIR *dir, char *pathname, int len, int opts) struct dirent *de; char hex[40]; - display_progress(progress, i + 1); - sprintf(hex, "%02x", i); while ((de = readdir(dir)) != NULL) { unsigned char sha1[20]; @@ -32,6 +30,7 @@ static void prune_dir(int i, DIR *dir, char *pathname, int len, int opts) printf("rm -f %s\n", pathname); else if (unlink(pathname) < 0) error("unable to unlink %s", pathname); + display_progress(progress, i + 1); } pathname[len] = 0; rmdir(pathname); -- cgit v1.2.3 From 3e935d19822db08cc0dedd8764135771ffd6ec7b Mon Sep 17 00:00:00 2001 From: Nicolas Pitre Date: Thu, 1 Nov 2007 16:59:56 -0400 Subject: make sure throughput display gets updated even if progress doesn't move Currently the progress/throughput display update happens only through display_progress(). If the progress based on object count remains unchanged because a large object is being received, the latest throughput won't be displayed. The display update should occur through display_throughput() as well. Signed-off-by: Nicolas Pitre Signed-off-by: Junio C Hamano --- progress.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/progress.c b/progress.c index 34a59611fb..39d5d2c9f2 100644 --- a/progress.c +++ b/progress.c @@ -160,6 +160,9 @@ void display_throughput(struct progress *progress, unsigned long n) tp->last_misecs[tp->idx] = misecs; tp->idx = (tp->idx + 1) % TP_IDX_MAX; tp->count = 0; + + if (progress->last_value != -1 && progress_update) + display(progress, progress->last_value, 0); } } -- cgit v1.2.3 From 81f6654a47075a345ba63a394921f77fc87b6500 Mon Sep 17 00:00:00 2001 From: Nicolas Pitre Date: Thu, 1 Nov 2007 16:59:57 -0400 Subject: Show total transferred as part of throughput progress Right now it is infeasible to offer to the user a reasonable concept of when a clone will be complete as we aren't able to come up with the final pack size until after we have actually transferred the entire thing to the client. However in many cases users can work with a rough rule-of-thumb; for example it is somewhat well known that git.git is about 16 MiB today and that linux-2.6.git is over 120 MiB. We now show the total amount of data we have transferred over the network as part of the throughput meter, organizing it in "human friendly" terms like `ls -h` would do. Users can glance at this, see that the total transferred size is about 3 MiB, see the throughput of X KiB/sec, and determine a reasonable figure of about when the clone will be complete, assuming they know the rough size of the source repository or are able to obtain it. This is also a helpful indicator that there is progress being made even if we stall on a very large object. The thoughput meter may remain relatively constant and the percentage complete and object count won't be changing, but the total transferred will be increasing as additional data is received for this object. [from an initial proposal from Shawn O. Pearce] Signed-off-by: Nicolas Pitre Signed-off-by: Junio C Hamano --- progress.c | 29 ++++++++++++++++++++++++++--- 1 file changed, 26 insertions(+), 3 deletions(-) diff --git a/progress.c b/progress.c index 39d5d2c9f2..3f6a602a53 100644 --- a/progress.c +++ b/progress.c @@ -15,13 +15,14 @@ struct throughput { struct timeval prev_tv; + off_t total; unsigned long count; unsigned long avg_bytes; unsigned long last_bytes[TP_IDX_MAX]; unsigned int avg_misecs; unsigned int last_misecs[TP_IDX_MAX]; unsigned int idx; - char display[20]; + char display[32]; }; struct progress { @@ -128,6 +129,7 @@ void display_throughput(struct progress *progress, unsigned long n) return; } + tp->total += n; tp->count += n; /* @@ -149,11 +151,32 @@ void display_throughput(struct progress *progress, unsigned long n) misecs += (int)(tv.tv_usec - tp->prev_tv.tv_usec) / 977; if (misecs > 512) { + int l = sizeof(tp->display); tp->prev_tv = tv; tp->avg_bytes += tp->count; tp->avg_misecs += misecs; - snprintf(tp->display, sizeof(tp->display), - ", %lu KiB/s", tp->avg_bytes / tp->avg_misecs); + + if (tp->total > 1 << 30) { + l -= snprintf(tp->display, l, ", %u.%2.2u GiB", + (int)(tp->total >> 30), + (int)(tp->total & ((1 << 30) - 1)) / 10737419); + } else if (tp->total > 1 << 20) { + l -= snprintf(tp->display, l, ", %u.%2.2u MiB", + (int)(tp->total >> 20), + ((int)(tp->total & ((1 << 20) - 1)) + * 100) >> 20); + } else if (tp->total > 1 << 10) { + l -= snprintf(tp->display, l, ", %u.%2.2u KiB", + (int)(tp->total >> 10), + ((int)(tp->total & ((1 << 10) - 1)) + * 100) >> 10); + } else { + l -= snprintf(tp->display, l, ", %u bytes", + (int)tp->total); + } + snprintf(tp->display + sizeof(tp->display) - l, l, + " | %lu KiB/s", tp->avg_bytes / tp->avg_misecs); + tp->avg_bytes -= tp->last_bytes[tp->idx]; tp->avg_misecs -= tp->last_misecs[tp->idx]; tp->last_bytes[tp->idx] = tp->count; -- cgit v1.2.3