| Message ID | 5a8f5afccf6e2b451c76306e04ca9ef300c92fdd.1650547400.git.dyroneteng@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | trace2 output for bitmap decision path | expand |
On Thu, Apr 21, 2022 at 09:26:38PM +0800, Teng Long wrote: > diff --git a/pack-bitmap.c b/pack-bitmap.c > index f13a6bfe3a..1b268f655e 100644 > --- a/pack-bitmap.c > +++ b/pack-bitmap.c > @@ -330,7 +330,8 @@ static int open_midx_bitmap_1(struct bitmap_index *bitmap_git, > struct strbuf buf = STRBUF_INIT; > get_midx_filename(&buf, midx->object_dir); > /* ignore extra bitmap file; we can only handle one */ > - warning("ignoring extra bitmap file: %s", buf.buf); > + warning(_("a normal or midx bitmap already has been opened")); > + warning(_("ignoring extra bitmap file: %s"), buf.buf); I'm glad that the existing warning has been marked for translation. There's no reason that it couldn't have been before, so I'm glad to see it added now. But I'm not sure that the new warning tells the user anything that the old one didn't. The old warning says "ignoring extra bitmap file: ...", leading us to believe that another one has already been opened. The new warning makes the latter part explicit, but I don't think it adds any new information that wasn't already readily available. > diff --git a/t/t5310-pack-bitmaps.sh b/t/t5310-pack-bitmaps.sh > index f775fc1ce6..eb63b71852 100755 > --- a/t/t5310-pack-bitmaps.sh > +++ b/t/t5310-pack-bitmaps.sh > @@ -421,6 +421,7 @@ test_expect_success 'complains about multiple pack bitmaps' ' > test_line_count = 2 bitmaps && > > git rev-list --use-bitmap-index HEAD 2>err && > + grep "a bitmap has been opened" err && This is out of sync with the warning you added, causing t5310.74 to fail. Thanks, Taylor
On Thu, 21 Apr 2022 13:25:58 -0400, Taylor Blau wrote: > I'm glad that the existing warning has been marked for translation. > There's no reason that it couldn't have been before, so I'm glad to see > it added now. Thanks and it's actually suggested by Ævar Arnfjörð Bjarmason. > But I'm not sure that the new warning tells the user anything that the > old one didn't. The old warning says "ignoring extra bitmap file: ...", > leading us to believe that another one has already been opened. The new > warning makes the latter part explicit, but I don't think it adds any > new information that wasn't already readily available. > ... > This is out of sync with the warning you added, causing t5310.74 to > fail. Yeah, I agree with you about this. Will roll back on here and fix the broken test in next patch. Thanks
diff --git a/pack-bitmap.c b/pack-bitmap.c index f13a6bfe3a..1b268f655e 100644 --- a/pack-bitmap.c +++ b/pack-bitmap.c @@ -330,7 +330,8 @@ static int open_midx_bitmap_1(struct bitmap_index *bitmap_git, struct strbuf buf = STRBUF_INIT; get_midx_filename(&buf, midx->object_dir); /* ignore extra bitmap file; we can only handle one */ - warning("ignoring extra bitmap file: %s", buf.buf); + warning(_("a normal or midx bitmap already has been opened")); + warning(_("ignoring extra bitmap file: %s"), buf.buf); close(fd); strbuf_release(&buf); return -1; @@ -387,7 +388,8 @@ static int open_pack_bitmap_1(struct bitmap_index *bitmap_git, struct packed_git if (bitmap_git->pack || bitmap_git->midx) { /* ignore extra bitmap file; we can only handle one */ - warning("ignoring extra bitmap file: %s", packfile->pack_name); + warning(_("a normal or midx bitmap already has been opened ")); + warning(_("ignoring extra bitmap file: %s"), packfile->pack_name); close(fd); return -1; } diff --git a/t/t5310-pack-bitmaps.sh b/t/t5310-pack-bitmaps.sh index f775fc1ce6..eb63b71852 100755 --- a/t/t5310-pack-bitmaps.sh +++ b/t/t5310-pack-bitmaps.sh @@ -421,6 +421,7 @@ test_expect_success 'complains about multiple pack bitmaps' ' test_line_count = 2 bitmaps && git rev-list --use-bitmap-index HEAD 2>err && + grep "a bitmap has been opened" err && grep "ignoring extra bitmap file" err ) '
When calling the "open_midx_bitmap_1()" or "open_pack_bitmap_1()", there will be a warning if a normal bitmap or MIDX bitmap already has been opened, then let's make the warning information more detailed. For example, it makes the error clearer in case of an accidental regression where we start looking for single-pack bitmaps after successfully opening a multi-pack one. At the same time, we made the previous and new warning texts support translation. Discussion in community: 1. https://public-inbox.org/git/YkPGq0mDL4NG6D1o@nand.local/#t 2. https://public-inbox.org/git/220330.868rsrpohm.gmgdl@evledraar.gmail.com/ Helped-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Helped-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Teng Long <dyroneteng@gmail.com> --- pack-bitmap.c | 6 ++++-- t/t5310-pack-bitmaps.sh | 1 + 2 files changed, 5 insertions(+), 2 deletions(-)