Local unset override global options

Local unset override global options

[Stefan Hajnoczi]

I have default settings for sending email in the global git config.
However, for one repo different email settings are necessary.  That
specific SMTP server does not want authentication, therefore smtpuser
should be unset.

The issue is that global smtpuser is set for the default SMTP server
and unsetting local smtpuser results in the global smtpuser being
inherited.  What I'd like is smtpuser to be unset for this particular
repo.

Is there any way to "unset override" options locally?  If not, is this
something that could and should be implemented?

Stefan

Re: Local unset override global options

[Michael J Gruber]

Stefan Hajnoczi venit, vidit, dixit 10.04.2010 14:54:
> I have default settings for sending email in the global git config.
> However, for one repo different email settings are necessary.  That
> specific SMTP server does not want authentication, therefore smtpuser
> should be unset.
> 
> The issue is that global smtpuser is set for the default SMTP server
> and unsetting local smtpuser results in the global smtpuser being
> inherited.  What I'd like is smtpuser to be unset for this particular
> repo.
> 
> Is there any way to "unset override" options locally?  If not, is this
> something that could and should be implemented?
> 
> Stefan

have you tried setting it to an empty value?

Michael

Re: Local unset override global options

[Stefan Hajnoczi]

On Mon, Apr 12, 2010 at 9:13 AM, Michael J Gruber
<git@drmicha.warpmail.net> wrote:
> have you tried setting it to an empty value?

Yes I have tried it.  An empty string causes the SMTP auth Perl module
to choke.  It expects a non-empty username and exits with an error.

On my system I have patched git-send-email to perform this check:

if (defined $smtp_authuser and length $smtp_authuser) {

This works but feels like a hack.  I think unset override could be
useful for any git config option, not just sendemail.smtpuser.

I'm not familiar with git internals; do you have other suggestions for
solving this issue?

Stefan

Re: Local unset override global options

[Michael J Gruber]

Stefan Hajnoczi venit, vidit, dixit 12.04.2010 16:23:
> On Mon, Apr 12, 2010 at 9:13 AM, Michael J Gruber
> <git@drmicha.warpmail.net> wrote:
>> have you tried setting it to an empty value?
> 
> Yes I have tried it.  An empty string causes the SMTP auth Perl module
> to choke.  It expects a non-empty username and exits with an error.
> 
> On my system I have patched git-send-email to perform this check:
> 
> if (defined $smtp_authuser and length $smtp_authuser) {
> 
> This works but feels like a hack.  I think unset override could be
> useful for any git config option, not just sendemail.smtpuser.
> 
> I'm not familiar with git internals; do you have other suggestions for
> solving this issue?

I don't think it's possible to ignore/unset a specific global config
value right now, you can only change the path where that is looked for.
I see two ways to go forward:

- Change users of the config (such as git-send-email) to treat empty
values as unset values.

- Introduce a special value "unset" for config options.

Michael

Re: Local unset override global options

[Jakub Narebski]

Michael J Gruber <git@drmicha.warpmail.net> writes:
> Stefan Hajnoczi venit, vidit, dixit 12.04.2010 16:23:
>> On Mon, Apr 12, 2010 at 9:13 AM, Michael J Gruber
>> <git@drmicha.warpmail.net> wrote:
>>
>>> have you tried setting it to an empty value?
>> 
>> Yes I have tried it.  An empty string causes the SMTP auth Perl module
>> to choke.  It expects a non-empty username and exits with an error.
>> 
>> On my system I have patched git-send-email to perform this check:
>> 
>> if (defined $smtp_authuser and length $smtp_authuser) {
>> 
>> This works but feels like a hack.  I think unset override could be
>> useful for any git config option, not just sendemail.smtpuser.
>> 
>> I'm not familiar with git internals; do you have other suggestions for
>> solving this issue?
> 
> I don't think it's possible to ignore/unset a specific global config
> value right now, you can only change the path where that is looked for.
> I see two ways to go forward:
> 
> - Change users of the config (such as git-send-email) to treat empty
> values as unset values.
> 
> - Introduce a special value "unset" for config options.

How does special case of no value, i.e.

   [sendemail]
        smtpuser

rather than using empty value

   [sendemail]
        smtpuser = ""

work?

Also if we assume that nobody needs to have support for
sendemail.smtpuser = 0, the condition could be simplified to

  if ($smtp_authuser) {

-- 
Jakub Narebski
Poland
ShadeHawk on #git

Re: Local unset override global options

[Junio C Hamano]

Jakub Narebski <jnareb@gmail.com> writes:

> How does special case of no value, i.e.
>
>    [sendemail]
>         smtpuser

That is not a special case; that is "boolean true".

Re: Local unset override global options

[Jakub Narebski]

Junio C Hamano wrote:
> Jakub Narebski <jnareb@gmail.com> writes:
> 
> > How does special case of no value, i.e.
> >
> >    [sendemail]
> >         smtpuser
> 
> That is not a special case; that is "boolean true".

Well, it is "boolean true" *if* git-config is used with '--bool'.

If git-send-email.perl used "git config -l -z" to read all its config 
variables at once into hash, like gitweb.perl does in the 
git_parse_project_config() subroutine, it would be able to distinguish 
between those the case of no value:

    [sendemail]
         smtpuser

(in which case "exists $config{'sendemail.smptuser'}" but "not defined 
$config{'sendemail.smtpuser'}", and of empty value

    [sendemail]
         smtpuser = ""

(in which case it is "defined $config{'sendemail.smtpuser'}" but not
true "$config{'sendemail.smtpuser'}).


But as git-send-email.perl uses Git::config to read config variables
one by one (equivalent to "git config --get <variable>"), I think that
also in the case of no value you would end up with $smtp_authuser
defined but empty (and false).


P.S. I wonder if 'sendemail.<identity>' section and 'sendemail.identity'
could help with original problem...
-- 
Jakub Narebski
Poland

Re: Local unset override global options

[Junio C Hamano]

Jakub Narebski <jnareb@gmail.com> writes:

> Junio C Hamano wrote:
>> Jakub Narebski <jnareb@gmail.com> writes:
>> 
>> > How does special case of no value, i.e.
>> >
>> >    [sendemail]
>> >         smtpuser
>> 
>> That is not a special case; that is "boolean true".
>
> Well, it is "boolean true" *if* git-config is used with '--bool'.
>
> If git-send-email.perl used "git config -l -z" to read all its config 
> variables at once into hash, like gitweb.perl does in the 
> git_parse_project_config() subroutine, it would be able to distinguish 
> between those the case of no value:
>
>     [sendemail]
>          smtpuser

If sendemail is ever re-written in C and use git_config() API, the above
will become boolean.  Besides, even if you are sticking to sendemail.perl
forever, you are still inventing another convention that "boolean true
means no I don't want it".  And it is clearly a lot less intuitive than
"empty means no I don't want it", isn't it?

Re: Local unset override global options

[Erik Faye-Lund]

On Mon, Apr 12, 2010 at 8:18 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Jakub Narebski <jnareb@gmail.com> writes:
>
>> Junio C Hamano wrote:
>>> Jakub Narebski <jnareb@gmail.com> writes:
>>>
>>> > How does special case of no value, i.e.
>>> >
>>> >    [sendemail]
>>> >         smtpuser
>>>
>>> That is not a special case; that is "boolean true".
>>
>> Well, it is "boolean true" *if* git-config is used with '--bool'.
>>
>> If git-send-email.perl used "git config -l -z" to read all its config
>> variables at once into hash, like gitweb.perl does in the
>> git_parse_project_config() subroutine, it would be able to distinguish
>> between those the case of no value:
>>
>>     [sendemail]
>>          smtpuser
>
> If sendemail is ever re-written in C and use git_config() API, the above
> will become boolean.  Besides, even if you are sticking to sendemail.perl

Actually, I have a very very (very) rudimentary port of send-email in
C. I worked on that mostly for Windows compatibility, but we already
solved the issue for msysGit by supplying msmtp instead, so I haven't
been working at it for quite some time. It's based on an older version
of send-email, and is incomplete in many ways (no alias-support, for
one), but most of the "tricky" parts are done AFAICT (SMTP with SSL if
OpenSSL is available).

Is there any interest in this?

I think in the long run, it'd be beneficial for Git for Windows not to
strictly depend on perl, but I don't really see that happening any
time soon as there's just too many essential tools written in perl.
Send-email is one of them (possibly one of the less essential tools
for most Windows users, though).

-- 
Erik "kusma" Faye-Lund

Re: Local unset override global options

[Jakub Narebski]

Junio C Hamano wrote:
> Jakub Narebski <jnareb@gmail.com> writes:
> 
> > Junio C Hamano wrote:
> >> Jakub Narebski <jnareb@gmail.com> writes:
> >> 
> >> > How does special case of no value, i.e.
> >> >
> >> >    [sendemail]
> >> >         smtpuser
> >> 
> >> That is not a special case; that is "boolean true".
> >
> > Well, it is "boolean true" *if* git-config is used with '--bool'.
> >
> > If git-send-email.perl used "git config -l -z" to read all its config 
> > variables at once into hash, like gitweb.perl does in the 
> > git_parse_project_config() subroutine, it would be able to distinguish 
> > between those the case of no value:
> >
> >     [sendemail]
> >          smtpuser
> 
> If sendemail is ever re-written in C and use git_config() API, the above
> will become boolean.

Isn't this only for git_config_bool and the like... oh, I see that
git_config_string does not consider no value case proper string value,
and warns that it is suitable only for boolean variables:

   if (!value)
                return config_error_nonbool(var);

> Besides, even if you are sticking to sendemail.perl 
> forever, you are still inventing another convention that "boolean true
> means no I don't want it".  And it is clearly a lot less intuitive than
> "empty means no I don't want it", isn't it?

True, after thinking a bit about this using no value to unset is 
a horrible, horrible hack.  git-send-email should be corrected to not
only check that there is value from config or command line option, but
also that it is sane (i.e. non-empty, or simply true-ish if we say
that smtpuser = "0" is not something we need to worry about supporting).


About ORIGINAL problem: I think it can be done by setting different
identities, and just switching them using sendemail.identity; for some
of those identities sendemail.<identity>.smtpuser can be not set.

-- 
Jakub Narebski
Poland

Re: Local unset override global options

[Junio C Hamano]

Michael J Gruber <git@drmicha.warpmail.net> writes:

> have you tried setting it to an empty value?

That sounds like a useful thing in general even outside override context.
I however suspect that the code is not written carefully to check for
empty string (which cannot possibly mean any valid username).

Perhaps we would need something like this.

 git-send-email.perl |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/git-send-email.perl b/git-send-email.perl
index ce569a9..7c3f7c1 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -1043,7 +1043,7 @@ X-Mailer: git-send-email $gitversion
 			    defined $smtp_server_port ? "port=$smtp_server_port" : "";
 		}
 
-		if (defined $smtp_authuser) {
+		if (defined $smtp_authuser && $smtp_authuser ne '') {
 
 			if (!defined $smtp_authpass) {
 

Re: Local unset override global options

[Stefan Hajnoczi]

On Mon, Apr 12, 2010 at 9:35 AM, Junio C Hamano <gitster@pobox.com> wrote:
> @@ -1043,7 +1043,7 @@ X-Mailer: git-send-email $gitversion
>                            defined $smtp_server_port ? "port=$smtp_server_port" : "";
>                }
>
> -               if (defined $smtp_authuser) {
> +               if (defined $smtp_authuser && $smtp_authuser ne '') {
>
>                        if (!defined $smtp_authpass) {
>

That would work but introduces a special case for smtpuser.  Do you
think users may wish to unset override other options too?

Stefan

Re: Local unset override global options

[Junio C Hamano]

Stefan Hajnoczi <stefanha@gmail.com> writes:

> That would work but introduces a special case for smtpuser.  Do you
> think users may wish to unset override other options too?

I would indeed agree "users may _wish_ to", but that does not matter as
much as "users _need_ to, otherwise they cannot do X and Y and Z".

You seem to think from the beginning of this thread that "empty means I
don't want it" is a hacky special case that is limited to this smtpuser
variable, but I do not share that view.  Not at all.

In fact, "empty means I don't want it" is a very common convention even
outside the context of git.  Make's use of its variables is a good
example.  Many people's shell scripts tend to treat an empty variable and
an unset variable the same way.  The language itself does distinguish an
empty and an unset variable, but the colon form of ${var:-something} was
invented in addition to the more old fashioned ${var-something} exactly
because "empty means unset" is so common.  Perl (run without -w/strict) is
the same way and undef becomes empty string by default and you need to
actively check defined($var) if you care about the difference.  Etc. etc.

There are two rather big reasons we would want to stick to the current
format without introducing "unset".  At least we would want to try to
until we find real need that justifies such a change.

 - We could introduce a backward incompatible change to the configuration
   file format to say "This variable is not set at all", but once somebody
   starts using the syntax in his .git/config (or worse $HOME/.gitconfig),
   that repository cannot be used by older version of git anymore.

 - Also internally our variables are never "unset".  Behaviour of a
   plumbing or Porcelain command implemented in C is controlled by
   variables in C, and the contents of the configuration files overwrite
   these variables as they are parsed from the least specific (system) to
   the most specific.  The best you can do is "reset to default" (not
   "unset"), and even that involves fair amount of change.

Re: Local unset override global options

[Stefan Hajnoczi]

On Mon, Apr 12, 2010 at 4:48 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Stefan Hajnoczi <stefanha@gmail.com> writes:
>
>> That would work but introduces a special case for smtpuser.  Do you
>> think users may wish to unset override other options too?
>
> I would indeed agree "users may _wish_ to", but that does not matter as
> much as "users _need_ to, otherwise they cannot do X and Y and Z".
>
> You seem to think from the beginning of this thread that "empty means I
> don't want it" is a hacky special case that is limited to this smtpuser
> variable, but I do not share that view.  Not at all.

> There are two rather big reasons we would want to stick to the current
> format without introducing "unset".  At least we would want to try to
> until we find real need that justifies such a change.
>
>  - We could introduce a backward incompatible change to the configuration
>   file format to say "This variable is not set at all", but once somebody
>   starts using the syntax in his .git/config (or worse $HOME/.gitconfig),
>   that repository cannot be used by older version of git anymore.
>
>  - Also internally our variables are never "unset".  Behaviour of a
>   plumbing or Porcelain command implemented in C is controlled by
>   variables in C, and the contents of the configuration files overwrite
>   these variables as they are parsed from the least specific (system) to
>   the most specific.  The best you can do is "reset to default" (not
>   "unset"), and even that involves fair amount of change.

Knowing that variables are never unset internally gives me a different
perspective.  Thanks for the explanation.

Stefan