Re: [PATCH] Added get sendmail from .mailrc

From: Jeff King <peff@peff.net>
Date: Jan 28, 2014, 01:04 UTC
Message-ID: <20140128010459.GA17945@sigill.intra.peff.net>
In-Reply-To: <20140126091709.GA31073@dcvr.yhbt.net>

On Sun, Jan 26, 2014 at 09:17:09AM +0000, Eric Wong wrote:

Show 13 quoted lines

> > > > +use File::HomeDir;
> > > 
> > > We should probably avoid a new dependency and also remain consistent
> > > with the rest of git handles home directories.
> > > 
> > > Unfortunately, expand_user_path()/git_config_pathname() isn't currently
> > > exposed to scripters right now...
> > 
> > Ok, if new dependency is not allowed I see next ways:
> 
> Not saying it's not allowed.  I meant we should probably expose
> expand_user_path()/git_config_pathname() C functions to script helpers
> (so git-config or git-rev-parse can provide them to sh or perl scripts).

I do not think we need anything so complex. Most of the logic in expand_user_path is about handling "~" and "~user". But here we _just_ want to know the current user's home directory, and for that expand_user_path always just looks in $HOME.

So I think $ENV{HOME} would be fine to match what git does. My understanding is that File::HomeDir does some magic that may work better on non-Unix platforms. I do not know if we even care for this feature, since .mailrc is presumably a Unix thing. But if we do, I think our usual strategy with such things is to optionally use the dependency if available, and fall back to something sane. Like:

  sub homedir {
        if (eval { require File::HomeDir; 1 }) {
                return File::HomeDir->my_home;
        }
        return $ENV{HOME};
  }

Whichever code path is followed, you should probably also check the result for "undef", which the original patch did not do.

-Peff