Re: [msysGit] Re: [PATCH] Add a Windows-specific fallback to getenv("HOME");

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Karsten,

On Thu, 5 Jun 2014, Karsten Blees wrote:

> After a bit of digging in the history and the old googlegroups issue
> tracker, I think this patch is completely unrelated to the non-ASCII
> problems.

Actually, the non-ASCII problems were the trigger for my patch.

> In summary, this patch fixes 'git config' for the portable version only,
> and it only does so partially.

Care to elaborate?

> Am 04.06.2014 17:46, schrieb Johannes Schindelin:
> 
> > I would be strongly in favor of fixing the problem by the root:
> > avoiding to have Git rely on the HOME environment variable to be set,
> > but instead add a clean API call that even says what it is supposed to
> > do: gimme the user's home directory's path. And that is exactly what
> > the patch does.
> 
> By that argument we'd have to introduce API abstractions for every
> environment variable that could possibly resemble a path (PATH, TMPDIR,
> GIT_DIR, GIT_WORK_DIR, GIT_TRACE* etc.).

But of course you are mixing things here. GIT_* are purely Git-specific
constructs, so there is no possibility for confusion. PATH and TMPDIR need
to be handled specially (as does HOME) because we are reusing environment
variable concepts that pose their own set of problems on Windows because
of the separator, the path separator and the encoding problems.

I understand that it is easy to confuse my want for a API function for the
home variable with handling for other environment variables. But that HOME
is an environment variable is not the point at all! It just *happens* to
be an environment variable on Linux/Unix.

> We already have similar fallback logic for TMPDIR that is completely
> non-intrusive to core git code (fully encapsulated in mingw.c, see
> mingw_getenv (upstream) or mingw_startup (msysgit)). IMO such a solution
> would be hugely preferable over adding an additional
> get_home_directory() API (and continuously checking that no new upstream
> code accidentally introduces another 'getenv("HOME")').

Well, since you mention that TMPDIR hack: this is a hack. We are bending
over in order for upstream not having to accomodate non-POSIX operating
systems. But how much cleaner would it be if there was an API call with
varargs. After all, by the reasoning "TMPDIR is a standard on Unix" you
would also have to special case "/tmp/" in all the open/opendir/etc
functions because the temporary directory is /tmp/ on Linux/Unix, right?

Render me even more convinced that the API call is the cleanest way to go,
Dscho
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]