[PATCH v6 4/4] Do not rely on `getenv ("HOME")`'s path conversion

[Johannes Schindelin]

Hi Corinna,

On Thu, 6 Apr 2023, Corinna Vinschen wrote:

> On Apr  4 17:07, Johannes Schindelin wrote:
> > In the very early code path where `dll_crt0_1 ()` calls
> > `user_shared->initialize ()`, the Cygwin runtime calls `internal_pwsid ()`
> > to initialize the user name in preparation for reading the `fstab` file.
> >
> > In case `db_home: env` is defined in `/etc/nsswitch.conf`, we need to
> > look at the environment variable `HOME` and use it, if set.
>
> I'm a bit puzzled by this.  HOME is not a Windows variable.  You're
> usually not supposed to set it to Windows values, but to the POSIX
> value.  I'm aware that Cygwin makes the conversion, too, for historical
> reasons.
>
> But why on earth would you set a variable you have under your own
> control, and which only makes sense in a POSIX environment, to a Windows
> value?

This is actually a well-documented feature of Git for Windows, which
cannot handle Cygwin paths (at least not `git.exe`, which is a MINGW
program).

And since Git for Windows is also used in third-party software like Visual
Studio, it is quite conceivable that the `HOME` variable is intended to be
used by Git for Windows and Cygwin. I know that I will use it for both,
once a Cygwin runtime version is released with these patches.

And even if it is not Git for Windows, we are talking about system-wide
environment variables. And that system is Windows...

So yes, we absolutely have to expect the `HOME` variable to potentially
contain a Windows path.

> > When all of this happens, though, the `pinfo_init ()` function has had no
> > change to run yet (and therefore, `environ_init ()`). At this stage,
>
>   chance

👍

> > therefore, `getenv ()`'s `findenv_func ()` call still finds `getearly ()`
> > and we get the _verbatim_ value of `HOME`. That is, the Windows form.
> > But we need the "POSIX" form.
> > [...]
> > Let's detect when the `HOME` value is still in Windows format in
> > `fetch_home_env ()`, and convert it in that case.
> >
> > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> > ---
> >  winsup/cygwin/uinfo.cc | 8 +++++++-
> >  1 file changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/winsup/cygwin/uinfo.cc b/winsup/cygwin/uinfo.cc
> > index 5e2d88bcd7..bc9e926159 100644
> > --- a/winsup/cygwin/uinfo.cc
> > +++ b/winsup/cygwin/uinfo.cc
> > @@ -929,7 +929,13 @@ fetch_home_env (void)
> >    /* If `HOME` is set, prefer it */
> >    const char *home = getenv ("HOME");
> >    if (home)
> > -    return strdup (home);
> > +    {
> > +      /* In the very early code path of `user_info::initialize ()`, the value
> > +         of the environment variable `HOME` is still in its Windows form. */
> > +      if (isdrive (home))
>
> While the description is clear on the colon problem, shouldn't this
> catch UNC paths as well?  I. e., just check for strchr(home, '\\')?

Good idea! I do not know off-hand how well things work when `HOME` is an
UNC path, but we can fix those things when (and if) they arrive.

I'll use `isdrive (home) || home[0] == '\\'` as is used elsewhere, okay?

Ciao,
Johannes

>
> > +	return (char *) cygwin_create_path (CCP_WIN_A_TO_POSIX, home);
>
>
> > +      return strdup (home);
> > +    }
> >
> >    /* If `HOME` is unset, fall back to `HOMEDRIVE``HOMEPATH`
> >       (without a directory separator, as `HOMEPATH` starts with one). */
> > --
> > 2.40.0.windows.1
>