Re: [Outreachy PATCH 1/6] notes: store the repository in struct notes_tree

[Patrick Steinhardt <ps@pks.im>]

On Thu, Nov 05, 2026 at 09:33:39PM +0100, Okhuomon Ajayi wrote:
> diff --git a/notes.c b/notes.c
> index ec9c2cb150..4ab3d45d7c 100644
> --- a/notes.c
> +++ b/notes.c
> @@ -1012,8 +1012,9 @@ char *default_notes_ref(struct repository *repo)
>  	return notes_ref;
>  }
>  
> -void init_notes(struct notes_tree *t, const char *notes_ref,
> -		combine_notes_fn combine_notes, int flags)
> +void init_notes(struct repository *r, struct notes_tree *t,
> +		const char *notes_ref, combine_notes_fn combine_notes,
> +		int flags)
>  {
>  	struct object_id oid, object_oid;
>  	unsigned short mode;

I think this function should continue to receive `struct notes_tree` as
first parameter, as this is the subsystem it belongs to. While at it,
and as we already have to adjust all callers anyway, we might rename the
function to `notes_tree_init()`.

> @@ -1057,7 +1059,8 @@ void init_notes(struct notes_tree *t, const char *notes_ref,
>  	free(to_free);
>  }
>  
> -struct notes_tree **load_notes_trees(struct string_list *refs, int flags)
> +struct notes_tree **load_notes_trees(struct repository *r,
> +				     struct string_list *refs, int flags)
>  {
>  	struct string_list_item *item;
>  	int counter = 0;

Okay, here the arguments make sense. We could likewise rename to
`notes_tree_load()` though.

> diff --git a/notes.h b/notes.h
> index 6dc6d7b265..0f50020816 100644
> --- a/notes.h
> +++ b/notes.h
> @@ -50,6 +50,7 @@ int combine_notes_cat_sort_uniq(struct object_id *cur_oid,
>   * simultaneously.
>   */
>  extern struct notes_tree {
> +	struct repository *repo;
>  	struct int_node *root;
>  	struct non_note *first_non_note, *prev_non_note;
>  	char *ref;

Oof, this interface is cursed, it really should be refactored to not
have implicit global state. Arguably, this default notes tree should be
either dropped, or it should be stored as part of a repository.

Anyway, as far as I can see we always initialize this default notes tree
via `init_notes()`. So it's cursed, but I think it should at least be
correct.

Patrick