threads / patch / 31302

patchFallback on getpwuid if envar HOME is unset

Subject: [PATCH] Fallback on getpwuid if envar HOME is unset

## tl;dr

9 messages between Aug 21, 2012 and Aug 21, 2012. Diffs are folded; open one to read it.

replies: 8people: 4as markdown or json

Conley Owens· Aug 21, 2012, 01:28 UTC · lore
>From f64ba3c908b33a2ea5a5ad1f0e5800af76b82ce9 Mon Sep 17 00:00:00 2001
From: Conley Owens <cco3@android.com>
Date: Mon, 20 Aug 2012 18:23:40 -0700
Subject: [PATCH] Fallback on getpwuid if envar HOME is unset
Signed-off-by: Conley Owens <cco3@android.com>
---
 path.c | 5 +++++
 1 file changed, 5 insertions(+)
Show changes to path.c +5 −0
diff --git a/path.c b/path.c
index 66acd24..60affab 100644
--- a/path.c
+++ b/path.c
@@ -144,6 +144,11 @@ void home_config_paths(char **global, char **xdg,
char *file)
        char *to_free = NULL;

        if (!home) {
+         struct passwd *pw = xgetpwuid_self();
+         home = pw->pw_dir;
+       }
+
+       if (!home) {
                if (global)
                        *global = NULL;
        } else {
-- 
1.7.11-rc3
Jeff King· Aug 21, 2012, 02:30 UTC · re: Conley Owens · lore

Re: [PATCH] Fallback on getpwuid if envar HOME is unset

On Mon, Aug 20, 2012 at 06:28:57PM -0700, Conley Owens wrote:
> From f64ba3c908b33a2ea5a5ad1f0e5800af76b82ce9 Mon Sep 17 00:00:00 2001
> From: Conley Owens <cco3@android.com>
> Date: Mon, 20 Aug 2012 18:23:40 -0700
> Subject: [PATCH] Fallback on getpwuid if envar HOME is unset

Please drop these lines from the message body; they are redundant with your email's headers.

This seems sensible on the surface, but I'm a bit curious: why isn't $HOME set? And are there any reasons that somebody who has unset HOME would not want to fallback? For example, running under Apache, HOME is often unset when calling CGI programs. Would it make sense for us to look in ~www-data/.gitconfig in that case?

Show 17 quoted lines
> diff --git a/path.c b/path.c
> index 66acd24..60affab 100644
> --- a/path.c
> +++ b/path.c
> @@ -144,6 +144,11 @@ void home_config_paths(char **global, char **xdg,
> char *file)
>         char *to_free = NULL;
> 
>         if (!home) {
> +         struct passwd *pw = xgetpwuid_self();
> +         home = pw->pw_dir;
> +       }
> +
> +       if (!home) {
>                 if (global)
>                         *global = NULL;
>         } else {
If we do go this route, it would probably make sense to wrap this like:
  const char *home_directory(void)
  {
          const char *dir = getenv("HOME");
          if (!dir) {
                  struct passwd *pw = xgetpwuid_self();
                  dir = pw->pw_dir;
          }
          return dir;
  }

and then call it consistently everywhere we do getenv("HOME"). You'd want to double-check that each caller only uses the result for a short period (unlike getenv, the results of getpwuid will be overwritten at the next call).

-Peff
Conley Owens· Aug 21, 2012, 17:18 UTC · re: Jeff King · lore

Re: [PATCH] Fallback on getpwuid if envar HOME is unset

On Mon, Aug 20, 2012 at 7:30 PM, Jeff King <peff@peff.net> wrote:
Show 15 quoted lines
> On Mon, Aug 20, 2012 at 06:28:57PM -0700, Conley Owens wrote:
>
>> From f64ba3c908b33a2ea5a5ad1f0e5800af76b82ce9 Mon Sep 17 00:00:00 2001
>> From: Conley Owens <cco3@android.com>
>> Date: Mon, 20 Aug 2012 18:23:40 -0700
>> Subject: [PATCH] Fallback on getpwuid if envar HOME is unset
>
> Please drop these lines from the message body; they are redundant with
> your email's headers.
>
> This seems sensible on the surface, but I'm a bit curious: why isn't
> $HOME set? And are there any reasons that somebody who has unset HOME
> would not want to fallback?  For example, running under Apache, HOME is
> often unset when calling CGI programs. Would it make sense for us to
> look in ~www-data/.gitconfig in that case?

I think it might, but perhaps I'm wrong. As another example, upstart strips all the environment variables, so if you run a job as a particular user, that user's .gitconfig will not be read unless HOME is specified.

Show 37 quoted lines
>
>> diff --git a/path.c b/path.c
>> index 66acd24..60affab 100644
>> --- a/path.c
>> +++ b/path.c
>> @@ -144,6 +144,11 @@ void home_config_paths(char **global, char **xdg,
>> char *file)
>>         char *to_free = NULL;
>>
>>         if (!home) {
>> +         struct passwd *pw = xgetpwuid_self();
>> +         home = pw->pw_dir;
>> +       }
>> +
>> +       if (!home) {
>>                 if (global)
>>                         *global = NULL;
>>         } else {
>
> If we do go this route, it would probably make sense to wrap this like:
>
>   const char *home_directory(void)
>   {
>           const char *dir = getenv("HOME");
>           if (!dir) {
>                   struct passwd *pw = xgetpwuid_self();
>                   dir = pw->pw_dir;
>           }
>           return dir;
>   }
>
> and then call it consistently everywhere we do getenv("HOME"). You'd
> want to double-check that each caller only uses the result for a short
> period (unlike getenv, the results of getpwuid will be overwritten at
> the next call).
>
> -Peff
Junio C Hamano· Aug 21, 2012, 18:22 UTC · re: Conley Owens · lore

Re: [PATCH] Fallback on getpwuid if envar HOME is unset

Conley Owens <cco3@android.com> writes:
Show 20 quoted lines
> On Mon, Aug 20, 2012 at 7:30 PM, Jeff King <peff@peff.net> wrote:
>> On Mon, Aug 20, 2012 at 06:28:57PM -0700, Conley Owens wrote:
>>
>>> From f64ba3c908b33a2ea5a5ad1f0e5800af76b82ce9 Mon Sep 17 00:00:00 2001
>>> From: Conley Owens <cco3@android.com>
>>> Date: Mon, 20 Aug 2012 18:23:40 -0700
>>> Subject: [PATCH] Fallback on getpwuid if envar HOME is unset
>>
>> Please drop these lines from the message body; they are redundant with
>> your email's headers.
>>
>> This seems sensible on the surface, but I'm a bit curious: why isn't
>> $HOME set? And are there any reasons that somebody who has unset HOME
>> would not want to fallback?  For example, running under Apache, HOME is
>> often unset when calling CGI programs. Would it make sense for us to
>> look in ~www-data/.gitconfig in that case?
>
> I think it might, but perhaps I'm wrong.  As another example, upstart strips all
> the environment variables, so if you run a job as a particular user, that user's
> .gitconfig will not be read unless HOME is specified.

Do you mean upstart as the "replacement init.d mechanism"? If that is the case, the responsibility to set up HOME was moved to the scripts by upstart if they rely on having a sane value in $HOME; I do not see it as Git's problem, as it is not the only program that looks at and acts on the value of $HOME [*1*].

Where do shells (e.g. bash and dash) go when you say "cd" without parameter when $HOME is unset, for example? I do not think they magically read from getpwent() and use the value from there to fill the $HOME's place. We should follow suit.

[Footnote]

*1* I further have to suspect that enough scripts would be inconvenienced by such a (mis)feature in upstart that over time the environment scrubbing may have to be rethought in upstart, and at that point, this entire discussion would become moot.

Conley Owens· Aug 21, 2012, 18:33 UTC · re: Junio C Hamano · lore

Re: [PATCH] Fallback on getpwuid if envar HOME is unset

On Tue, Aug 21, 2012 at 11:22 AM, Junio C Hamano <gitster@pobox.com> wrote:
Show 28 quoted lines
> Conley Owens <cco3@android.com> writes:
>
>> On Mon, Aug 20, 2012 at 7:30 PM, Jeff King <peff@peff.net> wrote:
>>> On Mon, Aug 20, 2012 at 06:28:57PM -0700, Conley Owens wrote:
>>>
>>>> From f64ba3c908b33a2ea5a5ad1f0e5800af76b82ce9 Mon Sep 17 00:00:00 2001
>>>> From: Conley Owens <cco3@android.com>
>>>> Date: Mon, 20 Aug 2012 18:23:40 -0700
>>>> Subject: [PATCH] Fallback on getpwuid if envar HOME is unset
>>>
>>> Please drop these lines from the message body; they are redundant with
>>> your email's headers.
>>>
>>> This seems sensible on the surface, but I'm a bit curious: why isn't
>>> $HOME set? And are there any reasons that somebody who has unset HOME
>>> would not want to fallback?  For example, running under Apache, HOME is
>>> often unset when calling CGI programs. Would it make sense for us to
>>> look in ~www-data/.gitconfig in that case?
>>
>> I think it might, but perhaps I'm wrong.  As another example, upstart strips all
>> the environment variables, so if you run a job as a particular user, that user's
>> .gitconfig will not be read unless HOME is specified.
>
> Do you mean upstart as the "replacement init.d mechanism"?  If that
> is the case, the responsibility to set up HOME was moved to the
> scripts by upstart if they rely on having a sane value in $HOME; I
> do not see it as Git's problem, as it is not the only program that
> looks at and acts on the value of $HOME [*1*].

Yes, that's the upstart I'm referring to. This makes sense. However, it's a confusing situation to run into. Would a warning about an unset $HOME be appropriate?

Show 14 quoted lines
>
> Where do shells (e.g. bash and dash) go when you say "cd" without
> parameter when $HOME is unset, for example?  I do not think they
> magically read from getpwent() and use the value from there to fill
> the $HOME's place.  We should follow suit.
>
>
> [Footnote]
>
> *1* I further have to suspect that enough scripts would be
> inconvenienced by such a (mis)feature in upstart that over time the
> environment scrubbing may have to be rethought in upstart, and at
> that point, this entire discussion would become moot.
>
Junio C Hamano· Aug 21, 2012, 19:22 UTC · re: Conley Owens · lore

Re: [PATCH] Fallback on getpwuid if envar HOME is unset

Conley Owens <cco3@android.com> writes:
> Yes, that's the upstart I'm referring to.  This makes sense.  However, it's a
> confusing situation to run into.  Would a warning about an unset $HOME be
> appropriate?

Unsetting HOME is an easy way to skip what is in ~/.gitconfig when helping other people on this list, and I wouldn't mind such a warning while I knowingly unset it, I can imagine other helpful people may find such a warning irritating and complain "I know I do not have $HOME set, as I earlier explicitly did unset it myself!".

So, I am on the fence on this one, but because
 (1) no warning would mean upstart scripts writers need to be aware
     of lack of $HOME, but they need to be aware of it for reasons
     unrelated to Git anyway; and
 (2) a warning while trying vanilla Git behaviour to help others
     might be irritating, it is not an every day use anyway.
I do not think it matters either way in practice.
Jeff King· Aug 21, 2012, 19:40 UTC · re: Junio C Hamano · lore

Re: [PATCH] Fallback on getpwuid if envar HOME is unset

On Tue, Aug 21, 2012 at 12:22:18PM -0700, Junio C Hamano wrote:
Show 22 quoted lines
> Conley Owens <cco3@android.com> writes:
> 
> > Yes, that's the upstart I'm referring to.  This makes sense.  However, it's a
> > confusing situation to run into.  Would a warning about an unset $HOME be
> > appropriate?
> 
> Unsetting HOME is an easy way to skip what is in ~/.gitconfig when
> helping other people on this list, and I wouldn't mind such a
> warning while I knowingly unset it, I can imagine other helpful
> people may find such a warning irritating and complain "I know I do
> not have $HOME set, as I earlier explicitly did unset it myself!".
> 
> So, I am on the fence on this one, but because
> 
>  (1) no warning would mean upstart scripts writers need to be aware
>      of lack of $HOME, but they need to be aware of it for reasons
>      unrelated to Git anyway; and
> 
>  (2) a warning while trying vanilla Git behaviour to help others
>      might be irritating, it is not an every day use anyway.
> 
> I do not think it matters either way in practice.

I do not use upstart, but presumably it logs the stderr of jobs it runs. Which means that a warning about unset $HOME would help debugging for people who care about looking in ~/.gitconfig, but would become a noisy nuisance for people whose jobs did not care.

Personally, I think it would be much friendlier of upstart to give the user's jobs a sane minimal environment. That is what cron has always done, setting HOME, LOGNAME, and SHELL. But that is my uninformed 5-second opinion as a long-time Unix user; I have not looked at upstart at all, so perhaps there is some argument I haven't seen.

-Peff
Andreas Schwab· Aug 21, 2012, 18:41 UTC · re: Junio C Hamano · lore

Re: [PATCH] Fallback on getpwuid if envar HOME is unset

Junio C Hamano <gitster@pobox.com> writes:
> Where do shells (e.g. bash and dash) go when you say "cd" without
> parameter when $HOME is unset, for example?

$ bash -c 'unset HOME; cd' bash: line 0: cd: HOME not set $ dash -c 'unset HOME; cd' [no output and cwd not changed]

POSIX says:
    If no directory operand is given and the HOME environment variable
    is empty or undefined, the default behavior is
    implementation-defined and no further steps shall be taken.

Another data point: bash falls back to getpwuid when expanding ~, dash leaves it alone. (POSIX makes it unspecified.)

Andreas.
-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."
Junio C Hamano· Aug 21, 2012, 03:54 UTC · re: Conley Owens · lore

Re: [PATCH] Fallback on getpwuid if envar HOME is unset

Conley Owens <cco3@android.com> writes:
Show 7 quoted lines
> From f64ba3c908b33a2ea5a5ad1f0e5800af76b82ce9 Mon Sep 17 00:00:00 2001
> From: Conley Owens <cco3@android.com>
> Date: Mon, 20 Aug 2012 18:23:40 -0700
> Subject: [PATCH] Fallback on getpwuid if envar HOME is unset
>
> Signed-off-by: Conley Owens <cco3@android.com>
> ---

We can see you are doing what you claim on the title (modulo "envar" typo) to be doing, but it is unclear why this patch wants to exist in the first place.

If the user for whatever reason "unset HOME", why is it a good idea to read from a place that is found by getpwuid()? What problem does it want to fix? Why does a user want this updated behaviour?

Show 14 quoted lines
>  path.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/path.c b/path.c
> index 66acd24..60affab 100644
> --- a/path.c
> +++ b/path.c
> @@ -144,6 +144,11 @@ void home_config_paths(char **global, char **xdg,
> char *file)
>         char *to_free = NULL;
>
>         if (!home) {
> +         struct passwd *pw = xgetpwuid_self();
> +         home = pw->pw_dir;
One level of indent is a HT, not two spaces.
Show 6 quoted lines
> +       }
> +
> +       if (!home) {
>                 if (global)
>                         *global = NULL;
>         } else {

← back to recent threads