| Message ID | 20190129193818.8645-13-jeremyhu@apple.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Differences between git-2.20.1 and Apple Git-116 | expand |
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);
> 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. > > > > > > > >> 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);
On Tue, Jan 29, 2019 at 03:10:30PM -0800, Junio C Hamano 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. I think it's relevant in this case if I point out what the gitconfig and gitattributes files contain on macOS. The gitconfig file sets up the default credential helper, and the gitattributes file sets up default diff helpers for Objective C and Swift. To my knowledge, I believe what other distributors (including Homebrew) do is they provide the system configuration file with default options if one is missing, allowing the user to retain or delete these options as the administrator sees fit. Apple may or may not want to do that, but I believe that's probably the way that we'll choose to support. While distributors will of course customize Git in whatever way seems most appropriate, I think it's better if those customizations are editable by the user, since that makes it easier to write per-user configuration files that are consistent across platforms. For a tool like Git, that can be quite helpful.
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. 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); > >
> 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! > 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); >> >>
> 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? > >> 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); >>> >>> >
Hi, Jeremy Huddleston Sequoia wrote: > 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? For the internal deployment at Google, what we've done is to put an [include] path directive in the global gitconfig: [include] path = /usr/share/git-core/config Users can edit the global git config in etc, but the distributed config at /usr/share/git-core/config is read-only as part of the distributed package. We considered making an upstream change to bake in the distributed config in the git binary but decided that this way is a little nicer since it lets people comment out the include.path setting if they want to e.g. for experimentation. It's also more explicit (hence easier to understand). Would a similar approach work for your setup? Can you say a little more about how you'd like things to work from an end-user pov? Thanks, Jonathan
> On Jan 30, 2019, at 16:01, Jonathan Nieder <jrnieder@gmail.com> wrote: > > Hi, > > Jeremy Huddleston Sequoia wrote: > >> 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? > > For the internal deployment at Google, what we've done is to put an > [include] path directive in the global gitconfig: > > [include] > path = /usr/share/git-core/config > > Users can edit the global git config in etc, but the distributed > config at /usr/share/git-core/config is read-only as part of the > distributed package. > > We considered making an upstream change to bake in the distributed > config in the git binary but decided that this way is a little > nicer since it lets people comment out the include.path setting if > they want to e.g. for experimentation. It's also more explicit > (hence easier to understand). > > Would a similar approach work for your setup? Can you say a little > more about how you'd like things to work from an end-user pov? That might work. I could put this in the Xcode.app gitconfig: [include] path = /private/etc/gitconfig Would that result in /private/etc/gitconfig's taking precedence or Xcode.app's? Is there anything analogous I could do for gitattributes?
Jeremy Huddleston Sequoia <jeremyhu@apple.com> writes: >>>> 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 do not think you misunderstood. system_path(ETC_GITCONFIG) may be in <prefix>/etc/gitconfig when building with RUNTIME_PREFIX, but then I do not think /etc/gitconfig (without <prefix>) comes into the picture. So as long as you want to add "a forced by distribution, not editable by end user to set a global policy for the entire box" configuration, that goes against the design of the configuration system, which wants to have three levels (i.e. per repository, per user and per box). I think the arrangement jrnieder illustrates in a message in this thread to use the inclusion of distro-provided file from /etc/gitconfig, which documents what is happening clearly and still allows the user to disable the distro-provided one if needed, is probably the best solution under the current design.
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);
Useful for setting up osxkeychain in Xcode.app's gitconfig Signed-off-by: Jeremy Huddleston Sequoia <jeremyhu@apple.com> --- config.c | 13 +++++++++++++ config.h | 2 ++ 2 files changed, 15 insertions(+)