Subject: Re: [RFC PATCH 00/10] An attempt to move packfile funcs to its own file

From: Junio C Hamano <[email-masked]>
Date: Tue, 08 Aug 2017 13:05:05 -0700

Jonathan Tan <[email-masked]> writes:

> While investigating annotating packfiles and loose objects to support
> connectivity checks in partial clones [1], I decided to make the effort
> to separate packfile-related code from sha1_file.c into its own file, to
> make it easier to both code such changes and review them. Here is the
> beginning of those efforts.
>
> Is this something worth doing, and if yes, is this in the right
> direction?

Overall I think it is a good idea to slim down sha1_file.c *if* we
can keep the exposed surface area small enough.

I wonder if the names "pack.[ch]" communicate well that these are
"object access layer that is about reading from packfiles".  The
writer side is called "pack-objects.[ch]".

This may have to make some symbols that used to be private to the
"object access" layer, which was what sha1_file.c was about, global
symbols.  After moving things around, do we end up exposing too many
implementation details to the world outside the "object access"
layer?  I'd assume they are limited to the resulting pack.h and it
would be OK as long as nobody other than sha1_file.c and pack.c
would inculde it, though.

Thanks.

>
> [1] https://public-inbox.org/git/[email-masked]/
>
> Jonathan Tan (10):
>   pack: move pack name-related functions
>   pack: move static state variables
>   pack: move pack_report()
>   pack: move open_pack_index(), parse_pack_index()
>   pack: move release_pack_memory()
>   pack: move pack-closing functions
>   pack: move use_pack()
>   pack: move unuse_pack()
>   pack: move add_packed_git()
>   pack: move install_packed_git()
>
>  Makefile                 |   1 +
>  builtin/am.c             |   1 +
>  builtin/clone.c          |   1 +
>  builtin/count-objects.c  |   1 +
>  builtin/fetch.c          |   1 +
>  builtin/merge.c          |   1 +
>  builtin/pack-redundant.c |   1 +
>  cache.h                  |  45 ----
>  connected.c              |   1 +
>  git-compat-util.h        |   2 -
>  pack.c                   | 669 +++++++++++++++++++++++++++++++++++++++++++++++
>  pack.h                   |  51 ++++
>  sha1_file.c              | 667 ----------------------------------------------
>  sha1_name.c              |   1 +
>  streaming.c              |   1 +
>  15 files changed, 730 insertions(+), 714 deletions(-)
>  create mode 100644 pack.c