[0/12] a rabbit hole of update-server-info fixes

Message

Jeff King April 4, 2019, 11:21 p.m. UTC

This patch series started with patch 12: I just wanted to drop the
unused "force" parameter from update_info_refs().

But that made me look at its sibling update_info_packs(), and whether it
needs its force parameter. It _does_ in fact do something (though I am
slightly doubtful that anybody actually cares in practice).

However, I did see a memory bug in its parser, which is fixed by patch
6. And a bunch of other cleanups, which are patches 7-11. Fortunately
that buggy parser is not used to look at info/packs files we pull over
dumb-http; it has its own parser. So I looked at that, and of course it
had a bug, too (fixed in patch 5).

And in those cleanups, I needed to get the basename of a pack from its
packed_git. So I looked for other code that did the same, in case we
could benefit from a shared helper. And behold, the midx code does. But
you guessed it, there was a bug (actually 2). Fixed in patches 4 (with
necessary refactoring in patch 3).

And while constructing a test for that bug, I noticed that one of the
other tests in t5319 was buggy. So that's patch 1 (with some cleanup in
patch 2).

And here we are. I present them here in reverse rabbit-hole order (which
is also roughly important fixes first, and minor cleanups last). The
individual chunks are mostly independent, but the server-info cleanups
rely on the shared pack_basename() helper added as part of the midx fix.

  [01/12]: t5319: fix bogus cat-file argument
  [02/12]: t5319: drop useless --buffer from cat-file
  [03/12]: packfile: factor out .pack to .idx name conversion
  [04/12]: packfile: check midx coverage with .idx rather than .pack
  [05/12]: http: simplify parsing of remote objects/info/packs
  [06/12]: server-info: fix blind pointer arithmetic
  [07/12]: server-info: simplify cleanup in parse_pack_def()
  [08/12]: server-info: use strbuf to read old info/packs file
  [09/12]: server-info: drop nr_alloc struct member
  [10/12]: packfile.h: drop extern from function declarations
  [11/12]: server-info: drop objdirlen pointer arithmetic
  [12/12]: update_info_refs(): drop unused force parameter

 http.c                      | 35 ++++++---------
 packfile.c                  | 31 ++++++++++---
 packfile.h                  | 86 ++++++++++++++++++++-----------------
 server-info.c               | 57 +++++++++++-------------
 t/t5319-multi-pack-index.sh | 29 ++++++++++---
 5 files changed, 132 insertions(+), 106 deletions(-)

-Peff

Comments

Junio C Hamano April 5, 2019, 5:46 a.m. UTC | #1

Jeff King <[e-mail]> writes:

> This patch series started with patch 12: I just wanted to drop the
> unused "force" parameter from update_info_refs().
>
> But that made me look at its sibling update_info_packs(), and whether it
> ...
> And here we are. I present them here in reverse rabbit-hole order (which
> is also roughly important fixes first, and minor cleanups last). The
> individual chunks are mostly independent, but the server-info cleanups
> rely on the shared pack_basename() helper added as part of the midx fix.

A kind of cover letter to make readers chuckle.  Well written.

And of course, thanks.  It's a delight to read a nicely constructed
series like this one.

>
>   [01/12]: t5319: fix bogus cat-file argument
>   [02/12]: t5319: drop useless --buffer from cat-file
>   [03/12]: packfile: factor out .pack to .idx name conversion
>   [04/12]: packfile: check midx coverage with .idx rather than .pack
>   [05/12]: http: simplify parsing of remote objects/info/packs
>   [06/12]: server-info: fix blind pointer arithmetic
>   [07/12]: server-info: simplify cleanup in parse_pack_def()
>   [08/12]: server-info: use strbuf to read old info/packs file
>   [09/12]: server-info: drop nr_alloc struct member
>   [10/12]: packfile.h: drop extern from function declarations
>   [11/12]: server-info: drop objdirlen pointer arithmetic
>   [12/12]: update_info_refs(): drop unused force parameter
>
>  http.c                      | 35 ++++++---------
>  packfile.c                  | 31 ++++++++++---
>  packfile.h                  | 86 ++++++++++++++++++++-----------------
>  server-info.c               | 57 +++++++++++-------------
>  t/t5319-multi-pack-index.sh | 29 ++++++++++---
>  5 files changed, 132 insertions(+), 106 deletions(-)
>
> -Peff