Re: [PATCH (Apple Git) 12/13] Enable support for Xcode.app-bundled gitconfig
Show 44 quoted lines
> On Jan 30, 2019, at 13:09, Jeremy Huddleston Sequoia <jeremyhu@apple.com> wrote:
>
>
>
>> On Jan 30, 2019, at 11:32, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
>>
>> Hi Jeremy,
>>
>> On Tue, 29 Jan 2019, Jeremy Huddleston Sequoia wrote:
>>
>>>> On Jan 29, 2019, at 3:10 PM, Junio C Hamano <gitster@pobox.com> wrote:
>>>>
>>>> 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.
>>>
>>> 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.
>
> Oh! Awesome. I didn't even notice this was a thing. That would exactly solve our use case. I'll give that a whirl. That likely allows us to eliminate these two patches completely!
Unfortunately, I was quick to celebrate. This picks up the bundled file instead of a system-wide file. I'd love it if we could still honor system-wide config/attributes in addition to RUNTIME_PREFIX-relative ones (eg: user overrides system which overrides distribution). I worry that as is, we'd stop referencing the system-wide configs which might confuse users.
Is that something you'd be interested in, or should we just continue to maintain our separate patches?
Show 78 quoted lines
>
>> 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);
>>>
>>>
> Jeremy Huddleston Sequoia <jeremyhu@apple.com>