Re: [PATCH (Apple Git) 12/13] Enable support for Xcode.app-bundled gitconfig

[Junio C Hamano <gitster@pobox.com>]

Jeremy Huddleston Sequoia <jeremyhu@apple.com> writes:

> Useful for setting up osxkeychain in Xcode.app's gitconfig
>
> Signed-off-by: Jeremy Huddleston Sequoia <jeremyhu@apple.com>
> ---

A concern shared with 13/13 is this.

While it may not hurt too much to look at one extra location even on
non-Apple platform, it probably is a mistake to have this xcode
specific change in generic part of the system like config.c or
attr.c.  For that matter, would it make sense to force Apple uses to
look at one extra location in the first place?  In other words, we
already have "system wide" location (i.e. system_path(ETC_GITCONFIG))
defined so system owners can give reasonable default to its users.
The value of not using that facility and instead adding yet another
place is dubious.


  




>  config.c | 13 +++++++++++++
>  config.h |  2 ++
>  2 files changed, 15 insertions(+)
>
> diff --git a/config.c b/config.c
> index ff521eb27a..656bfef8ab 100644
> --- a/config.c
> +++ b/config.c
> @@ -1631,6 +1631,14 @@ const char *git_etc_gitconfig(void)
>  	return system_wide;
>  }
>  
> +const char *git_xcode_gitconfig(void)
> +{
> +	static const char *xcode_config;
> +	if (!xcode_config)
> +		xcode_config = system_path("share/git-core/gitconfig");
> +	return xcode_config;
> +}
> +
>  /*
>   * Parse environment variable 'k' as a boolean (in various
>   * possible spellings); if missing, use the default value 'def'.
> @@ -1673,6 +1681,11 @@ static int do_git_config_sequence(const struct config_options *opts,
>  	else
>  		repo_config = NULL;
>  
> +	current_parsing_scope = CONFIG_SCOPE_XCODE;
> +	if (git_config_system() && git_xcode_gitconfig() && !access_or_die(git_xcode_gitconfig(), R_OK, 0))
> +		ret += git_config_from_file(fn, git_xcode_gitconfig(),
> +					    data);
> +
>  	current_parsing_scope = CONFIG_SCOPE_SYSTEM;
>  	if (git_config_system() && !access_or_die(git_etc_gitconfig(), R_OK, 0))
>  		ret += git_config_from_file(fn, git_etc_gitconfig(),
> diff --git a/config.h b/config.h
> index ee5d3fa7b4..f848423d28 100644
> --- a/config.h
> +++ b/config.h
> @@ -115,6 +115,7 @@ extern int git_config_rename_section_in_file(const char *, const char *, const c
>  extern int git_config_copy_section(const char *, const char *);
>  extern int git_config_copy_section_in_file(const char *, const char *, const char *);
>  extern const char *git_etc_gitconfig(void);
> +extern const char *git_xcode_gitconfig(void);
>  extern int git_env_bool(const char *, int);
>  extern unsigned long git_env_ulong(const char *, unsigned long);
>  extern int git_config_system(void);
> @@ -131,6 +132,7 @@ enum config_scope {
>  	CONFIG_SCOPE_GLOBAL,
>  	CONFIG_SCOPE_REPO,
>  	CONFIG_SCOPE_CMDLINE,
> +	CONFIG_SCOPE_XCODE,
>  };
>  
>  extern enum config_scope current_config_scope(void);