Hi Jeremy,
On Tue, 29 Jan 2019, Jeremy Huddleston Sequoia wrote:
> > On Jan 29, 2019, at 3:10 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> >
> > Jeremy Huddleston Sequoia <jeremyhu@xxxxxxxxx> writes:
> >
> >> Useful for setting up osxkeychain in Xcode.app's gitconfig
> >>
> >> Signed-off-by: Jeremy Huddleston Sequoia <jeremyhu@xxxxxxxxx>
> >> ---
> >
> > 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.
>
> This allows for per-distribution configuration and could be useful for
> other applications as well that want customizations specific to their
> install of git. For our specific use case, we do not want to munge the
> system policy when installing Xcode. Prior to doing things this way, we
> were just changing the default in our distributed git binary, but this
> seems a bit more flexible.
I think you misunderstood Junio, thinking that he referred to
/etc/gitconfig. He did not. system_path(ETC_GITCONFIG) refers to
<prefix>/etc/gitconfig, where <prefix> is that runtime prefix when
compiled with RUNTIME_PREFIX.
So you can definitely have your own per-distribution configuration: it
lives in that very <prefix>/etc/gitconfig where the portable Git is
installed.
And since we have that nice facility, I agree with Junio that we probably
do not even need an extra config, certainly not one just introduced for
XCode.
Ciao,
Johannes
>
> >
> >
> >
> >
> >
> >
> >
> >> 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);
>
>
