Re: [PATCH 6/7] submodule-config: reuse config_from_gitmodules in repo_read_gitmodules

[Brandon Williams <bmwill@google.com>]

On 06/22, Antonio Ospite wrote:
> Reuse config_from_gitmodules in repo_read_gitmodules to remove some
> duplication and also have a single point where the .gitmodules file is
> read.
> 
> The change does not introduce any new behavior, the same gitmodules_cb
> config callback is still used which only deals with configuration
> specific to submodules.
> 
> The config_from_gitmodules function is moved up in the file —unchanged—
> before its users to avoid a forward declaration.
> 
> Signed-off-by: Antonio Ospite <ao2@ao2.it>
> ---
>  submodule-config.c | 50 +++++++++++++++++++---------------------------
>  1 file changed, 21 insertions(+), 29 deletions(-)
> 
> diff --git a/submodule-config.c b/submodule-config.c
> index e50c944eb..ce204fb53 100644
> --- a/submodule-config.c
> +++ b/submodule-config.c
> @@ -591,6 +591,23 @@ static void submodule_cache_check_init(struct repository *repo)
>  	submodule_cache_init(repo->submodule_cache);
>  }
>  
> +/*
> + * Note: This function is private for a reason, the '.gitmodules' file should
> + * not be used as as a mechanism to retrieve arbitrary configuration stored in
> + * the repository.
> + *
> + * Runs the provided config function on the '.gitmodules' file found in the
> + * working directory.
> + */
> +static void config_from_gitmodules(config_fn_t fn, struct repository *repo, void *data)
> +{
> +	if (repo->worktree) {
> +		char *file = repo_worktree_path(repo, GITMODULES_FILE);
> +		git_config_from_file(fn, file, data);
> +		free(file);
> +	}
> +}
> +
>  static int gitmodules_cb(const char *var, const char *value, void *data)
>  {
>  	struct repository *repo = data;
> @@ -608,19 +625,11 @@ void repo_read_gitmodules(struct repository *repo)
>  {
>  	submodule_cache_check_init(repo);
>  
> -	if (repo->worktree) {
> -		char *gitmodules;
> -
> -		if (repo_read_index(repo) < 0)
> -			return;
> -
> -		gitmodules = repo_worktree_path(repo, GITMODULES_FILE);
> -
> -		if (!is_gitmodules_unmerged(repo->index))
> -			git_config_from_file(gitmodules_cb, gitmodules, repo);
> +	if (repo_read_index(repo) < 0)
> +		return;
>  
> -		free(gitmodules);
> -	}
> +	if (!is_gitmodules_unmerged(repo->index))
> +		config_from_gitmodules(gitmodules_cb, repo, repo);

So the check for the repo's worktree has been pushed into
config_from_gitmodules().  This looks like the right thing to do in
order to get to a world where you'd rather read the gitmodules file from
the index instead of the worktree.

>  
>  	repo->submodule_cache->gitmodules_read = 1;
>  }
> @@ -672,23 +681,6 @@ void submodule_free(struct repository *r)
>  		submodule_cache_clear(r->submodule_cache);
>  }
>  
> -/*
> - * Note: This function is private for a reason, the '.gitmodules' file should
> - * not be used as as a mechanism to retrieve arbitrary configuration stored in
> - * the repository.
> - *
> - * Runs the provided config function on the '.gitmodules' file found in the
> - * working directory.
> - */
> -static void config_from_gitmodules(config_fn_t fn, struct repository *repo, void *data)
> -{
> -	if (repo->worktree) {
> -		char *file = repo_worktree_path(repo, GITMODULES_FILE);
> -		git_config_from_file(fn, file, data);
> -		free(file);
> -	}
> -}
> -
>  struct fetch_config {
>  	int *max_children;
>  	int *recurse_submodules;
> -- 
> 2.18.0
> 

-- 
Brandon Williams