On Wed, Oct 30, 2024 at 03:32:26PM +0100, Karthik Nayak wrote:
> [...]
>
> We do need to consider that a pack file could be part of the alternates
> of a repository, but considering that we only have one repository struct
> and also that we currently anyways use 'the_repository'. We should be
> OK with this change.
Nicely explained.
> diff --git a/object-store-ll.h b/object-store-ll.h
> index 53b8e693b1..e8a22ab5fc 100644
> --- a/object-store-ll.h
> +++ b/object-store-ll.h
> @@ -10,6 +10,7 @@
> struct oidmap;
> struct oidtree;
> struct strbuf;
> +struct repository;
>
> struct object_directory {
> struct object_directory *next;
> @@ -135,6 +136,10 @@ struct packed_git {
> */
> const uint32_t *mtimes_map;
> size_t mtimes_size;
> +
> + /* repo dentoes the repository this packed file belongs to */
> + struct repository *r;
> +
Hmm. What I meant in my earlier suggestion was that we should leave the
member of the struct called "repo", but change the name only in function
arguments.
Sorry to split hairs, but I am somewhat opposed to having such a short
variable name in a struct. In either event, the comment should be made
consistent with the variable name.
> /* something like ".git/objects/pack/xxxxx.pack" */
> char pack_name[FLEX_ARRAY]; /* more */
> };
> diff --git a/packfile.c b/packfile.c
> index 9560f0a33c..1423f23f57 100644
> --- a/packfile.c
> +++ b/packfile.c
> @@ -217,11 +217,12 @@ uint32_t get_pack_fanout(struct packed_git *p, uint32_t value)
> return ntohl(level1_ofs[value]);
> }
>
> -static struct packed_git *alloc_packed_git(int extra)
> +static struct packed_git *alloc_packed_git(struct repository *r, int extra)
This spot I would leave alone.
> {
> struct packed_git *p = xmalloc(st_add(sizeof(*p), extra));
> memset(p, 0, sizeof(*p));
> p->pack_fd = -1;
> + p->r = r;
And this spot I would change to:
p->repo = r;
The rest is looking good.
Thanks,
Taylor
