8 messages, 2 authors, 2016-06-15 · open the first message on its own page
From: Nguyen Thai Ngoc Duy <hidden>
Date: 2016-06-15 22:52:22
On git.git $ /usr/bin/time git fsck 333.25user 4.28system 5:37.59elapsed 99%CPU (0avgtext+0avgdata 420080maxresident)k 0inputs+0outputs (0major+726560minor)pagefaults 0swaps That's really long time, perhaps we should print progress so users know it's still running? -- Duy
From: Nguyen Thai Ngoc Duy <hidden>
Date: 2016-06-15 22:52:22
On Wed, Nov 2, 2011 at 7:06 PM, Nguyen Thai Ngoc Duy [off-list ref] wrote:
On git.git $ /usr/bin/time git fsck 333.25user 4.28system 5:37.59elapsed 99%CPU (0avgtext+0avgdata 420080maxresident)k 0inputs+0outputs (0major+726560minor)pagefaults 0swaps That's really long time, perhaps we should print progress so users know it's still running?
Ahh.. --verbose. Sorry for the noise. Still good to show the number of checked objects though. -- Duy
From: Jeff King <hidden>
Date: 2016-06-15 22:52:22
On Wed, Nov 02, 2011 at 07:10:26PM +0700, Nguyen Thai Ngoc Duy wrote:
On Wed, Nov 2, 2011 at 7:06 PM, Nguyen Thai Ngoc Duy [off-list ref] wrote:quoted
On git.git $ /usr/bin/time git fsck 333.25user 4.28system 5:37.59elapsed 99%CPU (0avgtext+0avgdata 420080maxresident)k 0inputs+0outputs (0major+726560minor)pagefaults 0swaps That's really long time, perhaps we should print progress so users know it's still running?Ahh.. --verbose. Sorry for the noise. Still good to show the number of checked objects though.
fsck --verbose is _really_ verbose. It could probably stand to have some
progress meters sprinkled throughout. The patch below produces this on
my git.git repo:
$ git fsck
Checking object directories: 100% (256/256), done.
Verifying packs: 100% (7/7), done.
Checking objects (pack 1/7): 100% (241/241), done.
Checking objects (pack 2/7): 100% (176/176), done.
Checking objects (pack 3/7): 100% (312/312), done.
Checking objects (pack 4/7): 100% (252/252), done.
Checking objects (pack 5/7): 100% (353/353), done.
Checking objects (pack 6/7): 100% (375/375), done.
Checking objects (pack 7/7): 100% (171079/171079), done.
which gives reasonably smooth progress. The longest hang is that
"Verifying pack" 7 is slow (I believe it's doing a sha1 over the whole
thing). If you really wanted to get fancy, you could probably do a
throughput meter as we sha1 the whole contents.
Patch is below. It would need --{no-,}progress support on the command
line, and to check isatty(2) before it would be acceptable.
---diff --git a/builtin/fsck.c b/builtin/fsck.c
index df1a88b..481de4e 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c@@ -11,6 +11,7 @@ #include "fsck.h" #include "parse-options.h" #include "dir.h" +#include "progress.h" #define REACHABLE 0x0001 #define SEEN 0x0002
@@ -512,15 +513,19 @@ static void get_default_heads(void) static void fsck_object_dir(const char *path) { int i; + struct progress *progress; if (verbose) fprintf(stderr, "Checking object directory\n"); + progress = start_progress("Checking object directories", 256); for (i = 0; i < 256; i++) { static char dir[4096]; sprintf(dir, "%s/%02x", path, i); fsck_dir(i, dir); + display_progress(progress, i+1); } + stop_progress(&progress); fsck_sha1_list(); }
@@ -622,19 +627,36 @@ int cmd_fsck(int argc, const char **argv, const char *prefix) if (check_full) { struct packed_git *p; + int i, nr_packs = 0; + struct progress *progress; prepare_packed_git(); for (p = packed_git; p; p = p->next) + nr_packs++; + + progress = start_progress("Verifying packs", nr_packs); + for (i = 1, p = packed_git; p; p = p->next, i++) { /* verify gives error messages itself */ verify_pack(p); + display_progress(progress, i); + } + stop_progress(&progress); - for (p = packed_git; p; p = p->next) { + for (i = 1, p = packed_git; p; p = p->next, i++) { + char buf[32]; uint32_t j, num; if (open_pack_index(p)) continue; num = p->num_objects; - for (j = 0; j < num; j++) + + snprintf(buf, sizeof(buf), "Checking objects (pack %d/%d)", + i, nr_packs); + progress = start_progress(buf, num); + for (j = 0; j < num; j++) { fsck_sha1(nth_packed_object_sha1(p, j)); + display_progress(progress, j+1); + } + stop_progress(&progress); } }
From: Nguyen Thai Ngoc Duy <hidden>
Date: 2016-06-15 22:52:22
2011/11/3 Jeff King [off-list ref]:
On Wed, Nov 02, 2011 at 07:10:26PM +0700, Nguyen Thai Ngoc Duy wrote:quoted
On Wed, Nov 2, 2011 at 7:06 PM, Nguyen Thai Ngoc Duy [off-list ref] wrote:quoted
On git.git $ /usr/bin/time git fsck 333.25user 4.28system 5:37.59elapsed 99%CPU (0avgtext+0avgdata 420080maxresident)k 0inputs+0outputs (0major+726560minor)pagefaults 0swaps That's really long time, perhaps we should print progress so users know it's still running?Ahh.. --verbose. Sorry for the noise. Still good to show the number of checked objects though.fsck --verbose is _really_ verbose. It could probably stand to have some progress meters sprinkled throughout. The patch below produces this on my git.git repo:
Yes, I wanted something like this.
$ git fsck Checking object directories: 100% (256/256), done. Verifying packs: 100% (7/7), done. Checking objects (pack 1/7): 100% (241/241), done. Checking objects (pack 2/7): 100% (176/176), done. Checking objects (pack 3/7): 100% (312/312), done. Checking objects (pack 4/7): 100% (252/252), done. Checking objects (pack 5/7): 100% (353/353), done. Checking objects (pack 6/7): 100% (375/375), done. Checking objects (pack 7/7): 100% (171079/171079), done.
Would be better if we only output one "Checking objects" line.
which gives reasonably smooth progress. The longest hang is that "Verifying pack" 7 is slow (I believe it's doing a sha1 over the whole thing). If you really wanted to get fancy, you could probably do a throughput meter as we sha1 the whole contents.
I'll give it a try.
Patch is below. It would need --{no-,}progress support on the command
line, and to check isatty(2) before it would be acceptable.Agreed on isatty(), though I think this output should be default (with maybe --quiet to silence it on tty). Other messages may be prepended with severity to indicate they are not progress output. -- Duy
From: Nguyễn Thái Ngọc Duy <hidden>
Date: 2016-06-15 22:52:22
fsck is usually a long process and it would be nice if it prints progress from time to time. Signed-off-by: Nguyễn Thái Ngọc Duy <redacted> --- Documentation/git-fsck.txt | 12 ++++++++- builtin/fsck.c | 57 +++++++++++++++++++++++++++++++++++++++++-- pack-check.c | 11 ++++++-- pack.h | 4 ++- 4 files changed, 76 insertions(+), 8 deletions(-)
diff --git a/Documentation/git-fsck.txt b/Documentation/git-fsck.txt
index a2a508d..5245101 100644
--- a/Documentation/git-fsck.txt
+++ b/Documentation/git-fsck.txt@@ -10,7 +10,8 @@ SYNOPSIS -------- [verse] 'git fsck' [--tags] [--root] [--unreachable] [--cache] [--no-reflogs] - [--[no-]full] [--strict] [--verbose] [--lost-found] [<object>*] + [--[no-]full] [--strict] [--verbose] [--lost-found] + [--[no-]progress] [<object>*] DESCRIPTION -----------
@@ -72,6 +73,15 @@ index file, all SHA1 references in .git/refs/*, and all reflogs (unless a blob, the contents are written into the file, rather than its object name. +--progress:: +--no-progress:: + When fsck is run in a terminal, it will show the progress. + These options can force progress to be shown or not + regardless terminal check. ++ +Progress is not shown when --verbose is used. --progress is ignored +in this case. + It tests SHA1 and general object sanity, and it does full tracking of the resulting reachability and everything else. It prints out any corruption it finds (missing or bad objects), and if you use the
diff --git a/builtin/fsck.c b/builtin/fsck.c
index df1a88b..dc6a6fc 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c@@ -11,6 +11,7 @@ #include "fsck.h" #include "parse-options.h" #include "dir.h" +#include "progress.h" #define REACHABLE 0x0001 #define SEEN 0x0002
@@ -27,6 +28,8 @@ static const char *head_points_at; static int errors_found; static int write_lost_and_found; static int verbose; +static int show_progress = -1; + #define ERROR_OBJECT 01 #define ERROR_REACHABLE 02
@@ -512,15 +515,20 @@ static void get_default_heads(void) static void fsck_object_dir(const char *path) { int i; + struct progress *progress; if (verbose) fprintf(stderr, "Checking object directory\n"); + if (show_progress) + progress = start_progress("Checking object directories", 256); for (i = 0; i < 256; i++) { static char dir[4096]; sprintf(dir, "%s/%02x", path, i); fsck_dir(i, dir); + display_progress(progress, i+1); } + stop_progress(&progress); fsck_sha1_list(); }
@@ -591,6 +599,7 @@ static struct option fsck_opts[] = { OPT_BOOLEAN(0, "strict", &check_strict, "enable more strict checking"), OPT_BOOLEAN(0, "lost-found", &write_lost_and_found, "write dangling objects in .git/lost-found"), + OPT_BOOL (0, "progress", &show_progress, "show progress"), OPT_END(), };
@@ -603,6 +612,12 @@ int cmd_fsck(int argc, const char **argv, const char *prefix) read_replace_refs = 0; argc = parse_options(argc, argv, prefix, fsck_opts, fsck_usage, 0); + + if (show_progress == -1) + show_progress = isatty(2); + if (verbose) + show_progress = 0; + if (write_lost_and_found) { check_full = 1; include_reflogs = 0;
@@ -622,20 +637,56 @@ int cmd_fsck(int argc, const char **argv, const char *prefix) if (check_full) { struct packed_git *p; + int i, nr_packs = 0; + struct progress *progress = NULL; prepare_packed_git(); + for (p = packed_git; p; p = p->next) + nr_packs++; + + for (i = 1, p = packed_git; p; p = p->next, i++) { + if (show_progress) { + char buf[32]; + snprintf(buf, sizeof(buf), "Verifying pack %d/%d", + i, nr_packs); + if (open_pack_index(p)) + continue; + progress = start_progress(buf, p->num_objects); + } /* verify gives error messages itself */ - verify_pack(p); + verify_pack(p, progress); + + /* + * we do not stop progress here, let the next + * progress line overwrite the current one for + * the next pack. + */ + } + stop_progress(&progress); - for (p = packed_git; p; p = p->next) { + for (i = 1, p = packed_git; p; p = p->next, i++) { + char buf[64]; uint32_t j, num; if (open_pack_index(p)) continue; num = p->num_objects; - for (j = 0; j < num; j++) + + snprintf(buf, sizeof(buf), "Checking objects on pack %d/%d", + i, nr_packs); + if (show_progress) + progress = start_progress(buf, num); + for (j = 0; j < num; j++) { fsck_sha1(nth_packed_object_sha1(p, j)); + display_progress(progress, j+1); + } + /* + * we do not stop progress here, let the next + * progress line overwrite the current one for + * the next pack. + */ } + stop_progress(&progress); } heads = 0;
diff --git a/pack-check.c b/pack-check.c
index 0c19b6e..e30c18c 100644
--- a/pack-check.c
+++ b/pack-check.c@@ -1,6 +1,7 @@ #include "cache.h" #include "pack.h" #include "pack-revindex.h" +#include "progress.h" struct idx_entry { off_t offset;
@@ -42,7 +43,8 @@ int check_pack_crc(struct packed_git *p, struct pack_window **w_curs, } static int verify_packfile(struct packed_git *p, - struct pack_window **w_curs) + struct pack_window **w_curs, + struct progress *progress) { off_t index_size = p->index_size; const unsigned char *index_base = p->index_data;
@@ -126,7 +128,10 @@ static int verify_packfile(struct packed_git *p, break; } free(data); + if ((i & 1023) == 0) + display_progress(progress, i); } + display_progress(progress, i); free(entries); return err;
@@ -155,7 +160,7 @@ int verify_pack_index(struct packed_git *p) return err; } -int verify_pack(struct packed_git *p) +int verify_pack(struct packed_git *p, struct progress *progress) { int err = 0; struct pack_window *w_curs = NULL;
@@ -164,7 +169,7 @@ int verify_pack(struct packed_git *p) if (!p->index_data) return -1; - err |= verify_packfile(p, &w_curs); + err |= verify_packfile(p, &w_curs, progress); unuse_pack(&w_curs); return err;
diff --git a/pack.h b/pack.h
index 722a54e..572794f 100644
--- a/pack.h
+++ b/pack.h@@ -70,10 +70,12 @@ struct pack_idx_entry { off_t offset; }; +struct progress; + extern const char *write_idx_file(const char *index_name, struct pack_idx_entry **objects, int nr_objects, const struct pack_idx_option *, unsigned char *sha1); extern int check_pack_crc(struct packed_git *p, struct pack_window **w_curs, off_t offset, off_t len, unsigned int nr); extern int verify_pack_index(struct packed_git *); -extern int verify_pack(struct packed_git *); +extern int verify_pack(struct packed_git *, struct progress *); extern void fixup_pack_header_footer(int, unsigned char *, const char *, uint32_t, unsigned char *, off_t); extern char *index_pack_lockfile(int fd); extern int encode_in_pack_object_header(enum object_type, uintmax_t, unsigned char *);
--
1.7.3.1.256.g2539c.dirty
From: Jeff King <hidden>
Date: 2016-06-15 22:52:22
On Thu, Nov 03, 2011 at 10:21:53AM +0700, Nguyen Thai Ngoc Duy wrote:
fsck is usually a long process and it would be nice if it prints progress from time to time.
The output looks good to me. Code looks sane overall, with one comment:
+ for (i = 1, p = packed_git; p; p = p->next, i++) {
+ if (show_progress) {
+ char buf[32];
+ snprintf(buf, sizeof(buf), "Verifying pack %d/%d",
+ i, nr_packs);
+ if (open_pack_index(p))
+ continue;
+ progress = start_progress(buf, p->num_objects);
+ }
/* verify gives error messages itself */
- verify_pack(p);
+ verify_pack(p, progress);
+
+ /*
+ * we do not stop progress here, let the next
+ * progress line overwrite the current one for
+ * the next pack.
+ */
+ }
+ stop_progress(&progress);We're actually leaking some memory here, since stop_progress will also free() the progress object and any associated resources. It's not a lot, but it's kind of ugly. Perhaps there should be a special version of stop_progress that handles this better? Or perhaps we could even come up with a total object count before starting. I guess it would involve mapping each pack index simultaneously, though by my reading of the code, I think we do that anyway (the opened index is cached in the pack object). -Peff
From: Nguyễn Thái Ngọc Duy <hidden>
Date: 2016-06-15 22:52:22
fsck is usually a long process and it would be nice if it prints progress from time to time. Signed-off-by: Nguyễn Thái Ngọc Duy <redacted> --- 2011/11/3 Jeff King [off-list ref]: > We're actually leaking some memory here, since stop_progress will also > free() the progress object and any associated resources. It's not a lot, > but it's kind of ugly. Fixed by always calling stop_progress_msg() but make sure no newline is printed (actually "done\n" is not) Also fixed "progress" initialization in fsck_object_dir() > Or perhaps we could even come up with a total object count > before starting. I guess it would involve mapping each pack index > simultaneously, though by my reading of the code, I think we do that > anyway (the opened index is cached in the pack object). I think this way is better because we can count two numbers at a time, nr. packs and nr. objects of current pack. Total object (I assume you mean of all packs) may be less informative. Documentation/git-fsck.txt | 12 +++++++++- builtin/fsck.c | 52 +++++++++++++++++++++++++++++++++++++++++-- pack-check.c | 11 ++++++-- pack.h | 4 ++- progress.c | 2 +- 5 files changed, 72 insertions(+), 9 deletions(-)
diff --git a/Documentation/git-fsck.txt b/Documentation/git-fsck.txt
index a2a508d..5245101 100644
--- a/Documentation/git-fsck.txt
+++ b/Documentation/git-fsck.txt@@ -10,7 +10,8 @@ SYNOPSIS -------- [verse] 'git fsck' [--tags] [--root] [--unreachable] [--cache] [--no-reflogs] - [--[no-]full] [--strict] [--verbose] [--lost-found] [<object>*] + [--[no-]full] [--strict] [--verbose] [--lost-found] + [--[no-]progress] [<object>*] DESCRIPTION -----------
@@ -72,6 +73,15 @@ index file, all SHA1 references in .git/refs/*, and all reflogs (unless a blob, the contents are written into the file, rather than its object name. +--progress:: +--no-progress:: + When fsck is run in a terminal, it will show the progress. + These options can force progress to be shown or not + regardless terminal check. ++ +Progress is not shown when --verbose is used. --progress is ignored +in this case. + It tests SHA1 and general object sanity, and it does full tracking of the resulting reachability and everything else. It prints out any corruption it finds (missing or bad objects), and if you use the
diff --git a/builtin/fsck.c b/builtin/fsck.c
index df1a88b..e19b78c 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c@@ -11,6 +11,7 @@ #include "fsck.h" #include "parse-options.h" #include "dir.h" +#include "progress.h" #define REACHABLE 0x0001 #define SEEN 0x0002
@@ -27,6 +28,8 @@ static const char *head_points_at; static int errors_found; static int write_lost_and_found; static int verbose; +static int show_progress = -1; + #define ERROR_OBJECT 01 #define ERROR_REACHABLE 02
@@ -512,15 +515,20 @@ static void get_default_heads(void) static void fsck_object_dir(const char *path) { int i; + struct progress *progress = NULL; if (verbose) fprintf(stderr, "Checking object directory\n"); + if (show_progress) + progress = start_progress("Checking object directories", 256); for (i = 0; i < 256; i++) { static char dir[4096]; sprintf(dir, "%s/%02x", path, i); fsck_dir(i, dir); + display_progress(progress, i+1); } + stop_progress(&progress); fsck_sha1_list(); }
@@ -591,6 +599,7 @@ static struct option fsck_opts[] = { OPT_BOOLEAN(0, "strict", &check_strict, "enable more strict checking"), OPT_BOOLEAN(0, "lost-found", &write_lost_and_found, "write dangling objects in .git/lost-found"), + OPT_BOOL (0, "progress", &show_progress, "show progress"), OPT_END(), };
@@ -603,6 +612,12 @@ int cmd_fsck(int argc, const char **argv, const char *prefix) read_replace_refs = 0; argc = parse_options(argc, argv, prefix, fsck_opts, fsck_usage, 0); + + if (show_progress == -1) + show_progress = isatty(2); + if (verbose) + show_progress = 0; + if (write_lost_and_found) { check_full = 1; include_reflogs = 0;
@@ -622,20 +637,51 @@ int cmd_fsck(int argc, const char **argv, const char *prefix) if (check_full) { struct packed_git *p; + int i, nr_packs = 0; + struct progress *progress = NULL; prepare_packed_git(); + for (p = packed_git; p; p = p->next) + nr_packs++; + + for (i = 1, p = packed_git; p; p = p->next, i++) { + if (show_progress) { + char buf[32]; + snprintf(buf, sizeof(buf), "Verifying pack %d/%d", + i, nr_packs); + if (open_pack_index(p)) + continue; + progress = start_progress(buf, p->num_objects); + } /* verify gives error messages itself */ - verify_pack(p); + verify_pack(p, progress); - for (p = packed_git; p; p = p->next) { + if (p->next) + stop_progress_msg(&progress, NULL); + } + stop_progress(&progress); + + for (i = 1, p = packed_git; p; p = p->next, i++) { + char buf[64]; uint32_t j, num; if (open_pack_index(p)) continue; num = p->num_objects; - for (j = 0; j < num; j++) + + snprintf(buf, sizeof(buf), "Checking objects on pack %d/%d", + i, nr_packs); + if (show_progress) + progress = start_progress(buf, num); + for (j = 0; j < num; j++) { fsck_sha1(nth_packed_object_sha1(p, j)); + display_progress(progress, j+1); + } + + if (p->next) + stop_progress_msg(&progress, NULL); } + stop_progress(&progress); } heads = 0;
diff --git a/pack-check.c b/pack-check.c
index 0c19b6e..e30c18c 100644
--- a/pack-check.c
+++ b/pack-check.c@@ -1,6 +1,7 @@ #include "cache.h" #include "pack.h" #include "pack-revindex.h" +#include "progress.h" struct idx_entry { off_t offset;
@@ -42,7 +43,8 @@ int check_pack_crc(struct packed_git *p, struct pack_window **w_curs, } static int verify_packfile(struct packed_git *p, - struct pack_window **w_curs) + struct pack_window **w_curs, + struct progress *progress) { off_t index_size = p->index_size; const unsigned char *index_base = p->index_data;
@@ -126,7 +128,10 @@ static int verify_packfile(struct packed_git *p, break; } free(data); + if ((i & 1023) == 0) + display_progress(progress, i); } + display_progress(progress, i); free(entries); return err;
@@ -155,7 +160,7 @@ int verify_pack_index(struct packed_git *p) return err; } -int verify_pack(struct packed_git *p) +int verify_pack(struct packed_git *p, struct progress *progress) { int err = 0; struct pack_window *w_curs = NULL;
@@ -164,7 +169,7 @@ int verify_pack(struct packed_git *p) if (!p->index_data) return -1; - err |= verify_packfile(p, &w_curs); + err |= verify_packfile(p, &w_curs, progress); unuse_pack(&w_curs); return err;
diff --git a/pack.h b/pack.h
index 722a54e..572794f 100644
--- a/pack.h
+++ b/pack.h@@ -70,10 +70,12 @@ struct pack_idx_entry { off_t offset; }; +struct progress; + extern const char *write_idx_file(const char *index_name, struct pack_idx_entry **objects, int nr_objects, const struct pack_idx_option *, unsigned char *sha1); extern int check_pack_crc(struct packed_git *p, struct pack_window **w_curs, off_t offset, off_t len, unsigned int nr); extern int verify_pack_index(struct packed_git *); -extern int verify_pack(struct packed_git *); +extern int verify_pack(struct packed_git *, struct progress *); extern void fixup_pack_header_footer(int, unsigned char *, const char *, uint32_t, unsigned char *, off_t); extern char *index_pack_lockfile(int fd); extern int encode_in_pack_object_header(enum object_type, uintmax_t, unsigned char *);
diff --git a/progress.c b/progress.c
index 3971f49..fc96a5f 100644
--- a/progress.c
+++ b/progress.c@@ -245,7 +245,7 @@ void stop_progress_msg(struct progress **p_progress, const char *msg) if (!progress) return; *p_progress = NULL; - if (progress->last_value != -1) { + if (progress->last_value != -1 && msg) { /* Force the last update */ char buf[128], *bufp; size_t len = strlen(msg) + 5;
--
1.7.3.1.256.g2539c.dirty
From: Jeff King <hidden>
Date: 2016-06-15 22:52:22
On Thu, Nov 03, 2011 at 03:50:34PM +0700, Nguyen Thai Ngoc Duy wrote:
2011/11/3 Jeff King [off-list ref]: > We're actually leaking some memory here, since stop_progress will also > free() the progress object and any associated resources. It's not a lot, > but it's kind of ugly. Fixed by always calling stop_progress_msg() but make sure no newline is printed (actually "done\n" is not)
Thanks, that sounds reasonable, and the output looks good.
> Or perhaps we could even come up with a total object count > before starting. I guess it would involve mapping each pack index > simultaneously, though by my reading of the code, I think we do that > anyway (the opened index is cached in the pack object). I think this way is better because we can count two numbers at a time, nr. packs and nr. objects of current pack. Total object (I assume you mean of all packs) may be less informative.
Yeah, I meant all of the packs. It's a little more accurate as a progress meter, because you don't otherwise know what's in each of the packs. For example, if I see that we are halfway through "pack 1/2", does that mean we are a quarter of the way done? Not really. Pack 2/2 may be much smaller or much bigger. Finishing pack 1 may make us 99% done, or it may make us 10% done. Showing all of the objects in a flat list gives a more accurate representation (though it's still not entirely accurate; even one gigantic blob may dwarf the earlier objects. But it's the best we can do). In practice, though, I think people tend to have one really big pack and some small ones. And the packs are sorted in recency order, so we'll quickly go through the early little ones, and then spend all of our time on the big old one. Unless they have .keep files. So I don't really care that much. But it would also make the newline stuff go away.
+ for (i = 1, p = packed_git; p; p = p->next, i++) {
+ if (show_progress) {
+ char buf[32];
+ snprintf(buf, sizeof(buf), "Verifying pack %d/%d",
+ i, nr_packs);
+ if (open_pack_index(p))
+ continue;
+ progress = start_progress(buf, p->num_objects);
+ }
/* verify gives error messages itself */
+ verify_pack(p, progress);
+ if (p->next)
+ stop_progress_msg(&progress, NULL);
+ }
+ stop_progress(&progress);
You start the progress in the loop, but stop the final one outside the
loop. Which means that if there are no packs, we'll call stop_progress
even though we didn't start one. I think the progress code will handle
the NULL fine, but it took me a minute to convince myself it's right.
I would find this:
if (p->next)
stop_progress_msg(&progress, NULL);
else
stop_progress(&progress);
a little more readable, or even:
stop_progress_msg(&progress, p->next ? NULL : "done\n");
But I almost wonder if it is worth factoring out the concept of a
"progress group", where you would call it like:
progress = progress_group_start("Checking objects in pack", nr_packs);
for (p = packed_git; p; p = p->next) {
progress_group_next(progress, p->num_objects);
for (j = 0; j < num; j++) {
fsck_sha1(p, j);
display_progress(progress, j+1);
}
}
stop_progress(&progress);
and progress_set_next would take care of formatting the %d/%d counter,
and would not output a newline before writing the new description line.
+ snprintf(buf, sizeof(buf), "Checking objects on pack %d/%d", + i, nr_packs);
s/on/in/ -Peff