Subject: Re: [PATCH v2 1/3] remote: move static variables into per-repository struct

From: Junio C Hamano <[email-masked]>
Date: Wed, 13 Oct 2021 13:21:36 -0700

Glen Choo <[email-masked]> writes:

> remote.c does not work with non-the_repository because it stores its
> state as static variables. To support non-the_repository, we can use a
> per-repository struct for the remotes subsystem.
>
> Prepare for this change by defining a struct remote_state that holds
> the remotes subsystem state and move the static variables of remote.c
> into the_repository->remote_state.

That all sounds very sensible, but ...

> diff --git a/remote.h b/remote.h
> index 5a59198252..184d14af3d 100644
> --- a/remote.h
> +++ b/remote.h
> @@ -23,6 +23,40 @@ enum {
>  	REMOTE_BRANCHES
>  };
>  
> +struct rewrite {
> +	const char *base;
> +	size_t baselen;
> +	struct counted_string *instead_of;
> +	int instead_of_nr;
> +	int instead_of_alloc;
> +};
> +struct rewrites {

Missing a blank line between two type decls.

> +	struct rewrite **rewrite;
> +	int rewrite_alloc;
> +	int rewrite_nr;
> +};

It is a bit sad that we still have to keep "struct rewrites"; if we
see how .remotes and .branches are handled, we probably should have
left the <array, alloc, nr> 3-tuple for "struct rewrite" without an
extra layer.  But this step is about moving things around and
wrapping the set of existing static file-scope-global variables into
a structure, so such a restructuring would not belong here.  OK.

> +struct remote_state {
> +	int config_loaded;
> +
> +	struct remote **remotes;
> +	int remotes_alloc;
> +	int remotes_nr;
> +	struct hashmap remotes_hash;
> +
> +	struct branch **branches;
> +	int branches_alloc;
> +	int branches_nr;
> +
> +	struct branch *current_branch;
> +	const char *pushremote_name;
> +
> +	struct rewrites rewrites;
> +	struct rewrites rewrites_push;
> +};
> +void remote_state_clear(struct remote_state *remote_state);

Missing a blank line after "struct" decl.

Thanks.