All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] Add a Windows-specific fallback to getenv("HOME");
@ 2014-06-04 11:47 Stepan Kasal
  2014-06-04 13:47 ` Duy Nguyen
  0 siblings, 1 reply; 31+ messages in thread
From: Stepan Kasal @ 2014-06-04 11:47 UTC (permalink / raw)
  To: GIT Mailing-list; +Cc: Thomas Braun, msysgit

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Wed, 2 Jun 2010 00:41:33 +0200

If HOME is not set, use $HOMEDRIVE/$HOMEPATH

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Stepan Kasal <kasal@ucw.cz>
---
Hi,
   this patch is present in msysGit for 4 years.
Stepan

 compat/mingw.c    | 18 ++++++++++++++++++
 compat/mingw.h    |  3 +++
 git-compat-util.h |  4 ++++
 path.c            |  4 ++--
 shell.c           |  2 +-
 5 files changed, 28 insertions(+), 3 deletions(-)

diff --git a/compat/mingw.c b/compat/mingw.c
index a0e13bc..8eb21dc 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -1847,3 +1847,21 @@ int mingw_offset_1st_component(const char *path)
 
 	return offset + is_dir_sep(path[offset]);
 }
+
+const char *get_windows_home_directory(void)
+{
+	static const char *home_directory = NULL;
+	struct strbuf buf = STRBUF_INIT;
+
+	if (home_directory)
+		return home_directory;
+
+	home_directory = getenv("HOME");
+	if (home_directory && *home_directory)
+		return home_directory;
+
+	strbuf_addf(&buf, "%s/%s", getenv("HOMEDRIVE"), getenv("HOMEPATH"));
+	home_directory = strbuf_detach(&buf, NULL);
+
+	return home_directory;
+}
diff --git a/compat/mingw.h b/compat/mingw.h
index 3eaf822..a88a7ab 100644
--- a/compat/mingw.h
+++ b/compat/mingw.h
@@ -386,3 +386,6 @@ static int mingw_main(c,v)
  * Used by Pthread API implementation for Windows
  */
 extern int err_win_to_posix(DWORD winerr);
+
+extern const char *get_windows_home_directory();
+#define get_home_directory() get_windows_home_directory()
diff --git a/git-compat-util.h b/git-compat-util.h
index b6f03b3..409e644 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -740,4 +740,8 @@ struct tm *git_gmtime_r(const time_t *, struct tm *);
 #define gmtime_r git_gmtime_r
 #endif
 
+#ifndef get_home_directory
+#define get_home_directory() getenv("HOME")
+#endif
+
 #endif
diff --git a/path.c b/path.c
index bc804a3..09b362c 100644
--- a/path.c
+++ b/path.c
@@ -133,7 +133,7 @@ char *git_path(const char *fmt, ...)
 void home_config_paths(char **global, char **xdg, char *file)
 {
 	char *xdg_home = getenv("XDG_CONFIG_HOME");
-	char *home = getenv("HOME");
+	const char *home = get_home_directory();
 	char *to_free = NULL;
 
 	if (!home) {
@@ -274,7 +274,7 @@ char *expand_user_path(const char *path)
 		const char *username = path + 1;
 		size_t username_len = first_slash - username;
 		if (username_len == 0) {
-			const char *home = getenv("HOME");
+			const char *home = get_home_directory();
 			if (!home)
 				goto return_null;
 			strbuf_add(&user_path, home, strlen(home));
diff --git a/shell.c b/shell.c
index 5c0d47a..edd8c3a 100644
--- a/shell.c
+++ b/shell.c
@@ -55,7 +55,7 @@ static char *make_cmd(const char *prog)
 
 static void cd_to_homedir(void)
 {
-	const char *home = getenv("HOME");
+	const char *home = get_home_directory();
 	if (!home)
 		die("could not determine user's home directory; HOME is unset");
 	if (chdir(home) == -1)
-- 
1.9.2.msysgit.0.655.g1a42564

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "msysGit" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* Re: [PATCH] Add a Windows-specific fallback to getenv("HOME");
  2014-06-04 11:47 [PATCH] Add a Windows-specific fallback to getenv("HOME"); Stepan Kasal
@ 2014-06-04 13:47 ` Duy Nguyen
  2014-06-04 14:05   ` Erik Faye-Lund
                     ` (2 more replies)
  0 siblings, 3 replies; 31+ messages in thread
From: Duy Nguyen @ 2014-06-04 13:47 UTC (permalink / raw)
  To: Stepan Kasal; +Cc: GIT Mailing-list, Thomas Braun, msysGit

On Wed, Jun 4, 2014 at 6:47 PM, Stepan Kasal <kasal@ucw.cz> wrote:
> @@ -133,7 +133,7 @@ char *git_path(const char *fmt, ...)
>  void home_config_paths(char **global, char **xdg, char *file)
>  {
>         char *xdg_home = getenv("XDG_CONFIG_HOME");
> -       char *home = getenv("HOME");
> +       const char *home = get_home_directory();
>         char *to_free = NULL;
>
>         if (!home) {

Just checking. Instead of replace the call sites, can we check and
setenv("HOME") if it's missing instead? MinGW port already replaces
main(). Extra initialization should not be a problem. I feel
"getenv("HOME")" a tiny bit more familiar than get_home_directory(),
but that's really weak argument as the number of call sites has not
increased in 4 years.
-- 
Duy

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "msysGit" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: Re: [PATCH] Add a Windows-specific fallback to getenv("HOME");
  2014-06-04 13:47 ` Duy Nguyen
@ 2014-06-04 14:05   ` Erik Faye-Lund
  2014-06-04 14:55     ` Karsten Blees
  2014-06-04 15:14     ` Johannes Schindelin
  2014-06-04 14:53   ` Stepan Kasal
  2014-06-04 15:13   ` Johannes Schindelin
  2 siblings, 2 replies; 31+ messages in thread
From: Erik Faye-Lund @ 2014-06-04 14:05 UTC (permalink / raw)
  To: Duy Nguyen; +Cc: Stepan Kasal, GIT Mailing-list, Thomas Braun, msysGit

On Wed, Jun 4, 2014 at 3:47 PM, Duy Nguyen <pclouds@gmail.com> wrote:
> On Wed, Jun 4, 2014 at 6:47 PM, Stepan Kasal <kasal@ucw.cz> wrote:
>> @@ -133,7 +133,7 @@ char *git_path(const char *fmt, ...)
>>  void home_config_paths(char **global, char **xdg, char *file)
>>  {
>>         char *xdg_home = getenv("XDG_CONFIG_HOME");
>> -       char *home = getenv("HOME");
>> +       const char *home = get_home_directory();
>>         char *to_free = NULL;
>>
>>         if (!home) {
>
> Just checking. Instead of replace the call sites, can we check and
> setenv("HOME") if it's missing instead? MinGW port already replaces
> main(). Extra initialization should not be a problem. I feel
> "getenv("HOME")" a tiny bit more familiar than get_home_directory(),
> but that's really weak argument as the number of call sites has not
> increased in 4 years.

Yeah. But we already set %HOME% to %HOMEDRIVE%%HOMEPATH% in
/etc/profile, git-cmd.bat, gitk.cmd *and* git-wrapper... Do we really
need one more place?

It seems some of these could be dropped...

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "msysGit" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH] Add a Windows-specific fallback to getenv("HOME");
  2014-06-04 13:47 ` Duy Nguyen
  2014-06-04 14:05   ` Erik Faye-Lund
@ 2014-06-04 14:53   ` Stepan Kasal
  2014-06-04 15:13   ` Johannes Schindelin
  2 siblings, 0 replies; 31+ messages in thread
From: Stepan Kasal @ 2014-06-04 14:53 UTC (permalink / raw)
  To: Duy Nguyen; +Cc: GIT Mailing-list, Thomas Braun, msysGit

Hello,

On Wed, Jun 04, 2014 at 08:47:58PM +0700, Duy Nguyen wrote:
> setenv("HOME") if it's missing instead? MinGW port already replaces
> main(). Extra initialization should not be a problem.

well, I would be afraid to modify the environment for subprocesses.
It could hit back in certain situations, like with git filter-branch.

We could replace getenv() with a wrapper though.
But I don't think it's worth it.

> I feel
> "getenv("HOME")" a tiny bit more familiar than get_home_directory(),
> but that's really weak argument as the number of call sites has not
> increased in 4 years.

Actually, the patch had to be updated when msysgit modifications were
rebased.  But yes, the number of call sites has not icreased: it was
called in four places back than, and it is called at three places
now.  There is only one that is in common, though.

Stepan

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "msysGit" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: Re: [PATCH] Add a Windows-specific fallback to getenv("HOME");
  2014-06-04 14:05   ` Erik Faye-Lund
@ 2014-06-04 14:55     ` Karsten Blees
  2014-06-04 15:14     ` Johannes Schindelin
  1 sibling, 0 replies; 31+ messages in thread
From: Karsten Blees @ 2014-06-04 14:55 UTC (permalink / raw)
  To: kusmabite, Duy Nguyen
  Cc: Stepan Kasal, GIT Mailing-list, Thomas Braun, msysGit

Am 04.06.2014 16:05, schrieb Erik Faye-Lund:
> On Wed, Jun 4, 2014 at 3:47 PM, Duy Nguyen <pclouds@gmail.com> wrote:
>> On Wed, Jun 4, 2014 at 6:47 PM, Stepan Kasal <kasal@ucw.cz> wrote:
>>> @@ -133,7 +133,7 @@ char *git_path(const char *fmt, ...)
>>>  void home_config_paths(char **global, char **xdg, char *file)
>>>  {
>>>         char *xdg_home = getenv("XDG_CONFIG_HOME");
>>> -       char *home = getenv("HOME");
>>> +       const char *home = get_home_directory();
>>>         char *to_free = NULL;
>>>
>>>         if (!home) {
>>
>> Just checking. Instead of replace the call sites, can we check and
>> setenv("HOME") if it's missing instead? MinGW port already replaces
>> main(). Extra initialization should not be a problem. I feel
>> "getenv("HOME")" a tiny bit more familiar than get_home_directory(),
>> but that's really weak argument as the number of call sites has not
>> increased in 4 years.
> 

Setting the variable instead of wrapping getenv has the additional benefit that it also affects child processes (read: scripted commands).

> Yeah. But we already set %HOME% to %HOMEDRIVE%%HOMEPATH% in
> /etc/profile, git-cmd.bat, gitk.cmd *and* git-wrapper... Do we really
> need one more place?
> 

...all of these also do the string concatenation correctly (i.e. not "C:/\Users\MyName" as this patch does), fall back to %USERPROFILE% if %HOMEPATH% is not set, and most (except git-wrapper) even check that the directory exists. So IMO this patch has been superseded by more robust solutions and should be dropped.


-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "msysGit" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: Re: [PATCH] Add a Windows-specific fallback to getenv("HOME");
  2014-06-04 13:47 ` Duy Nguyen
  2014-06-04 14:05   ` Erik Faye-Lund
  2014-06-04 14:53   ` Stepan Kasal
@ 2014-06-04 15:13   ` Johannes Schindelin
  2 siblings, 0 replies; 31+ messages in thread
From: Johannes Schindelin @ 2014-06-04 15:13 UTC (permalink / raw)
  To: Duy Nguyen; +Cc: Stepan Kasal, GIT Mailing-list, Thomas Braun, msysGit

Hi Duy,

On Wed, 4 Jun 2014, Duy Nguyen wrote:

> On Wed, Jun 4, 2014 at 6:47 PM, Stepan Kasal <kasal@ucw.cz> wrote:
> > @@ -133,7 +133,7 @@ char *git_path(const char *fmt, ...)
> >  void home_config_paths(char **global, char **xdg, char *file)
> >  {
> >         char *xdg_home = getenv("XDG_CONFIG_HOME");
> > -       char *home = getenv("HOME");
> > +       const char *home = get_home_directory();
> >         char *to_free = NULL;
> >
> >         if (!home) {
> 
> Just checking. Instead of replace the call sites, can we check and
> setenv("HOME") if it's missing instead? MinGW port already replaces
> main(). Extra initialization should not be a problem. I feel
> "getenv("HOME")" a tiny bit more familiar than get_home_directory(),
> but that's really weak argument as the number of call sites has not
> increased in 4 years.

There is a good reason why we did not go for that (noticably cheaper)
solution. In fact, it used to be our solution until too many things got
broken by setting the HOME variable: Git is not the only program making
use of that variable (and IIRC Putty or a merge helper got seriously
confused when we set it).

So I am afraid, no, we cannot simply setenv(HOME).

Ciao,
Johannes

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "msysGit" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: Re: [PATCH] Add a Windows-specific fallback to getenv("HOME");
  2014-06-04 14:05   ` Erik Faye-Lund
  2014-06-04 14:55     ` Karsten Blees
@ 2014-06-04 15:14     ` Johannes Schindelin
  2014-06-04 15:18       ` Erik Faye-Lund
  1 sibling, 1 reply; 31+ messages in thread
From: Johannes Schindelin @ 2014-06-04 15:14 UTC (permalink / raw)
  To: Erik Faye-Lund
  Cc: Duy Nguyen, Stepan Kasal, GIT Mailing-list, Thomas Braun, msysGit

Hi Erik,

On Wed, 4 Jun 2014, Erik Faye-Lund wrote:

> On Wed, Jun 4, 2014 at 3:47 PM, Duy Nguyen <pclouds@gmail.com> wrote:
> > On Wed, Jun 4, 2014 at 6:47 PM, Stepan Kasal <kasal@ucw.cz> wrote:
> >> @@ -133,7 +133,7 @@ char *git_path(const char *fmt, ...)
> >>  void home_config_paths(char **global, char **xdg, char *file)
> >>  {
> >>         char *xdg_home = getenv("XDG_CONFIG_HOME");
> >> -       char *home = getenv("HOME");
> >> +       const char *home = get_home_directory();
> >>         char *to_free = NULL;
> >>
> >>         if (!home) {
> >
> > Just checking. Instead of replace the call sites, can we check and
> > setenv("HOME") if it's missing instead? MinGW port already replaces
> > main(). Extra initialization should not be a problem. I feel
> > "getenv("HOME")" a tiny bit more familiar than get_home_directory(),
> > but that's really weak argument as the number of call sites has not
> > increased in 4 years.
> 
> Yeah. But we already set %HOME% to %HOMEDRIVE%%HOMEPATH% in
> /etc/profile, git-cmd.bat, gitk.cmd *and* git-wrapper... Do we really
> need one more place?
> 
> It seems some of these could be dropped...

No. Git is not always called through Bash or the git-wrapper,
unfortunately.

Ciao,
Dscho

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "msysGit" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: Re: [PATCH] Add a Windows-specific fallback to getenv("HOME");
  2014-06-04 15:14     ` Johannes Schindelin
@ 2014-06-04 15:18       ` Erik Faye-Lund
  2014-06-04 15:27         ` Johannes Schindelin
  0 siblings, 1 reply; 31+ messages in thread
From: Erik Faye-Lund @ 2014-06-04 15:18 UTC (permalink / raw)
  To: Johannes Schindelin
  Cc: Duy Nguyen, Stepan Kasal, GIT Mailing-list, Thomas Braun, msysGit

On Wed, Jun 4, 2014 at 5:14 PM, Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
> Hi Erik,
>
> On Wed, 4 Jun 2014, Erik Faye-Lund wrote:
>
>> On Wed, Jun 4, 2014 at 3:47 PM, Duy Nguyen <pclouds@gmail.com> wrote:
>> > On Wed, Jun 4, 2014 at 6:47 PM, Stepan Kasal <kasal@ucw.cz> wrote:
>> >> @@ -133,7 +133,7 @@ char *git_path(const char *fmt, ...)
>> >>  void home_config_paths(char **global, char **xdg, char *file)
>> >>  {
>> >>         char *xdg_home = getenv("XDG_CONFIG_HOME");
>> >> -       char *home = getenv("HOME");
>> >> +       const char *home = get_home_directory();
>> >>         char *to_free = NULL;
>> >>
>> >>         if (!home) {
>> >
>> > Just checking. Instead of replace the call sites, can we check and
>> > setenv("HOME") if it's missing instead? MinGW port already replaces
>> > main(). Extra initialization should not be a problem. I feel
>> > "getenv("HOME")" a tiny bit more familiar than get_home_directory(),
>> > but that's really weak argument as the number of call sites has not
>> > increased in 4 years.
>>
>> Yeah. But we already set %HOME% to %HOMEDRIVE%%HOMEPATH% in
>> /etc/profile, git-cmd.bat, gitk.cmd *and* git-wrapper... Do we really
>> need one more place?
>>
>> It seems some of these could be dropped...
>
> No. Git is not always called through Bash or the git-wrapper,
> unfortunately.

I'm aware of that. But you said in a previous e-mail that e.g putty
got confused when we set HOME. How is this a problem for git.exe, but
not when we set it in the shell?

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "msysGit" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: Re: [PATCH] Add a Windows-specific fallback to getenv("HOME");
  2014-06-04 15:18       ` Erik Faye-Lund
@ 2014-06-04 15:27         ` Johannes Schindelin
  2014-06-04 15:45           ` Stepan Kasal
  2014-06-04 15:46           ` Johannes Schindelin
  0 siblings, 2 replies; 31+ messages in thread
From: Johannes Schindelin @ 2014-06-04 15:27 UTC (permalink / raw)
  To: Erik Faye-Lund
  Cc: Duy Nguyen, Stepan Kasal, GIT Mailing-list, Thomas Braun, msysGit

Hi kusma,

On Wed, 4 Jun 2014, Erik Faye-Lund wrote:

> On Wed, Jun 4, 2014 at 5:14 PM, Johannes Schindelin
> <Johannes.Schindelin@gmx.de> wrote:
> >
> > On Wed, 4 Jun 2014, Erik Faye-Lund wrote:
> >
> >> On Wed, Jun 4, 2014 at 3:47 PM, Duy Nguyen <pclouds@gmail.com> wrote:
> >> > On Wed, Jun 4, 2014 at 6:47 PM, Stepan Kasal <kasal@ucw.cz> wrote:
> >> >> @@ -133,7 +133,7 @@ char *git_path(const char *fmt, ...)
> >> >>  void home_config_paths(char **global, char **xdg, char *file)
> >> >>  {
> >> >>         char *xdg_home = getenv("XDG_CONFIG_HOME");
> >> >> -       char *home = getenv("HOME");
> >> >> +       const char *home = get_home_directory();
> >> >>         char *to_free = NULL;
> >> >>
> >> >>         if (!home) {
> >> >
> >> > Just checking. Instead of replace the call sites, can we check and
> >> > setenv("HOME") if it's missing instead? MinGW port already replaces
> >> > main(). Extra initialization should not be a problem. I feel
> >> > "getenv("HOME")" a tiny bit more familiar than
> >> > get_home_directory(), but that's really weak argument as the number
> >> > of call sites has not increased in 4 years.
> >>
> >> Yeah. But we already set %HOME% to %HOMEDRIVE%%HOMEPATH% in
> >> /etc/profile, git-cmd.bat, gitk.cmd *and* git-wrapper... Do we really
> >> need one more place?
> >>
> >> It seems some of these could be dropped...
> >
> > No. Git is not always called through Bash or the git-wrapper,
> > unfortunately.
> 
> I'm aware of that. But you said in a previous e-mail that e.g putty got
> confused when we set HOME. How is this a problem for git.exe, but not
> when we set it in the shell?

The problem arises whenever git.exe calls subprocesses. You can pollute
the environment by setting HOME, I do not recall the details, but I
remember that we had to be very careful *not* to do that, hence the patch.
Sorry, has been a long time.

Ciao,
Dscho

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "msysGit" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: Re: [PATCH] Add a Windows-specific fallback to getenv("HOME");
  2014-06-04 15:27         ` Johannes Schindelin
@ 2014-06-04 15:45           ` Stepan Kasal
  2014-06-04 15:56             ` [msysGit] " Johannes Schindelin
  2014-06-04 15:46           ` Johannes Schindelin
  1 sibling, 1 reply; 31+ messages in thread
From: Stepan Kasal @ 2014-06-04 15:45 UTC (permalink / raw)
  To: Johannes Schindelin
  Cc: Erik Faye-Lund, Duy Nguyen, GIT Mailing-list, Thomas Braun, msysGit

Hi dscho,

> > On Wed, Jun 4, 2014 at 5:14 PM, Johannes Schindelin
> > <Johannes.Schindelin@gmx.de> wrote:
> > > No. Git is not always called through Bash or the git-wrapper,
> > > unfortunately.

but you have to admit, that in most cases it is called through bash
or the git wrapper.

> The problem arises whenever git.exe calls subprocesses. You can pollute
> the environment by setting HOME, I do not recall the details, but I
> remember that we had to be very careful *not* to do that, hence the patch.
> Sorry, has been a long time.

Yeah, memories.  Is this experience still valid?  How many users do
profit from this, using c:/Program\ Files \(86\)/bin/git.exe instead of 
c:/Program\ Files \(86\)/cmd/git.exe, either by pure luck or
intentionally?

It seems that we should keep the patch, to minimize surprise if
bin/git.exe is used directly.

But we should probably make it consistent with other places:
- $HOMEDRIVE$HOMEPATH (without the slash)
- $USERPROFILE if the above dir does not exist.
- setenv HOME instead of wrapper

We can make this change for msysGit 2.0.0 only, so that we do not
break 1.9.4 ;-)

Does this make sense?
	Stepan

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "msysGit" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: Re: [PATCH] Add a Windows-specific fallback to getenv("HOME");
  2014-06-04 15:27         ` Johannes Schindelin
  2014-06-04 15:45           ` Stepan Kasal
@ 2014-06-04 15:46           ` Johannes Schindelin
  2014-06-05  1:42             ` Karsten Blees
  1 sibling, 1 reply; 31+ messages in thread
From: Johannes Schindelin @ 2014-06-04 15:46 UTC (permalink / raw)
  To: Erik Faye-Lund
  Cc: Duy Nguyen, Stepan Kasal, GIT Mailing-list, Thomas Braun, msysGit

Hi kusma,

On Wed, 4 Jun 2014, Johannes Schindelin wrote:

> The problem arises whenever git.exe calls subprocesses. You can pollute
> the environment by setting HOME, I do not recall the details, but I
> remember that we had to be very careful *not* to do that, hence the patch.
> Sorry, has been a long time.

Actually, a quick search in my Applegate vaults^W^Wmail archives suggests
that we had tons of troubles with non-ASCII characters in the path.

Given that none of us really has time to recreate the problems, or to take
care of them if there arises a new problem due to setting the HOME
variable again (remember: while we have UTF-8 support in Git, thanks to
Karsten's tireless efforts, and while that seems to fix the biggest bugs
for us, other MinGW software does not have that luxury and will continue
to barf on non-ASCII characters in the HOME variable), 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.

Ciao,
Dscho

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "msysGit" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [msysGit] Re: [PATCH] Add a Windows-specific fallback to getenv("HOME");
  2014-06-04 15:45           ` Stepan Kasal
@ 2014-06-04 15:56             ` Johannes Schindelin
  2014-06-04 16:16               ` Stepan Kasal
  0 siblings, 1 reply; 31+ messages in thread
From: Johannes Schindelin @ 2014-06-04 15:56 UTC (permalink / raw)
  To: Stepan Kasal
  Cc: Erik Faye-Lund, Duy Nguyen, GIT Mailing-list, Thomas Braun, msysGit

Hi Stepan,

On Wed, 4 Jun 2014, Stepan Kasal wrote:

> > > On Wed, Jun 4, 2014 at 5:14 PM, Johannes Schindelin
> > > <Johannes.Schindelin@gmx.de> wrote:
> > > > No. Git is not always called through Bash or the git-wrapper,
> > > > unfortunately.
> 
> but you have to admit, that in most cases it is called through bash or
> the git wrapper.

It would seem so. But the plan was always to make the user experience on
Windows less abysmal than now (I just do not have enough time these days
to pursue that goal myself), which includes the goal to make git.exe the
main entrance point, not Bash nor the git-wrapper.

> > The problem arises whenever git.exe calls subprocesses. You can pollute
> > the environment by setting HOME, I do not recall the details, but I
> > remember that we had to be very careful *not* to do that, hence the patch.
> > Sorry, has been a long time.
> 
> Yeah, memories.

*Very* vague. Sorry.

> Is this experience still valid?  How many users do profit from this,
> using c:/Program\ Files \(86\)/bin/git.exe instead of c:/Program\ Files
> \(86\)/cmd/git.exe, either by pure luck or intentionally?

Keep in mind that the most problems were introduced by the fact that
USERPROFILE disagrees with HOMEDRIVE\HOMEPATH at times.

> It seems that we should keep the patch, to minimize surprise if
> bin/git.exe is used directly.

I am also in favor of keeping the patch because it introduces a bit of
documentation. It says pretty precisely what it wants and allows
platform-specific handling without having to play games with the
environment, as was suggested earlier.

And of course you cannot deny that it had four years of testing. The HOME
problems never came back after we included this patch.

> But we should probably make it consistent with other places:
> - $HOMEDRIVE$HOMEPATH (without the slash)
> - $USERPROFILE if the above dir does not exist.
> - setenv HOME instead of wrapper

Possibly. But again, it is hard to argue with four years of testing. Any
change you make now will lack that kind of vetting.

> We can make this change for msysGit 2.0.0 only, so that we do not
> break 1.9.4 ;-)

So we can break 2.0.0! ;-)

Actually, 2.0.0 for Windows needs to wait a little longer (it is a little
bit unfortunate that we could not coordinate it with upstream) because the
plan is to switch to mingwGitDevEnv for said release. No more msysGit.
Like, bu-bye. Thanks for all the fish.

Ciao,
Dscho

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: Re: [PATCH] Add a Windows-specific fallback to getenv("HOME");
  2014-06-04 15:56             ` [msysGit] " Johannes Schindelin
@ 2014-06-04 16:16               ` Stepan Kasal
  2014-06-04 17:49                 ` [msysGit] " Johannes Schindelin
                                   ` (2 more replies)
  0 siblings, 3 replies; 31+ messages in thread
From: Stepan Kasal @ 2014-06-04 16:16 UTC (permalink / raw)
  To: Johannes Schindelin
  Cc: Erik Faye-Lund, Duy Nguyen, GIT Mailing-list, Thomas Braun, msysGit

Hi dscho,

your arguments seem really strong.  (Especially the four years of
battle testing, with the memories of constant problems with HOME before.)

I hope they are strong enough to convince Junio to accept this patch;
that would help.

Stepan

PS (about mingwGitDevEnv):
> plan is to switch to mingwGitDevEnv for said release. No more msysGit.
> Like, bu-bye. Thanks for all the fish.

Interesting.

With msysgit, there is the "net installer" - first time I installed
msys/mingw sucessfully, it was as easy as Cygwin, perhaps even
easier.

When I go to mingwGitDevEnv home page, I read about chickens, eggs,
and upgrading Perl (which msysGit simply gives up, hinting that it is
almost impossible).
So I decided to wait for their Git 2.0.0 release before I try to
install it (again).

I apologize for being so cheeky, I hope it will help anyway...

PPS: from marketing point of view, mingwGitDevEnv is far from usable
name.  Dscho, if you support the idea, would you mind franchising
msysGit 2.0 for a decent amount?

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "msysGit" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [msysGit] Re: [PATCH] Add a Windows-specific fallback to getenv("HOME");
  2014-06-04 16:16               ` Stepan Kasal
@ 2014-06-04 17:49                 ` Johannes Schindelin
  2014-06-06  9:12                   ` Git for Windows SDK Philip Oakley
  2014-06-04 23:10                 ` Re: [PATCH] Add a Windows-specific fallback to getenv("HOME"); Duy Nguyen
  2014-06-06 19:26                 ` Sebastian Schuberth
  2 siblings, 1 reply; 31+ messages in thread
From: Johannes Schindelin @ 2014-06-04 17:49 UTC (permalink / raw)
  To: Stepan Kasal
  Cc: Erik Faye-Lund, Duy Nguyen, GIT Mailing-list, Thomas Braun, msysGit

Hi Stepan,

On Wed, 4 Jun 2014, Stepan Kasal wrote:

> PS (about mingwGitDevEnv):
> > plan is to switch to mingwGitDevEnv for said release. No more msysGit.
> > Like, bu-bye. Thanks for all the fish.
> 
> Interesting.
> 
> With msysgit, there is the "net installer" - first time I installed
> msys/mingw sucessfully, it was as easy as Cygwin, perhaps even
> easier.
> 
> When I go to mingwGitDevEnv home page, I read about chickens, eggs, and
> upgrading Perl (which msysGit simply gives up, hinting that it is almost
> impossible).  So I decided to wait for their Git 2.0.0 release before I
> try to install it (again).

I understand. And now that upstream Git 2.0.0 is out, it will be very hard
to use that as a deadline to push against. So: don't hold your breath.

> PPS: from marketing point of view, mingwGitDevEnv is far from usable
> name.  Dscho, if you support the idea, would you mind franchising
> msysGit 2.0 for a decent amount?

Make me an offer :-P

Seriously again, I am in favor of calling it the Git for Windows SDK. But
really, it is bikeshedding at this point. There is real work to do, still,
before we can switch. Lots of unaddressed questions. Too little time.
Speaking of which... budget's depleted for today ;-)

Ciao,
Dscho

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: Re: [PATCH] Add a Windows-specific fallback to getenv("HOME");
  2014-06-04 16:16               ` Stepan Kasal
  2014-06-04 17:49                 ` [msysGit] " Johannes Schindelin
@ 2014-06-04 23:10                 ` Duy Nguyen
  2014-06-06 19:26                 ` Sebastian Schuberth
  2 siblings, 0 replies; 31+ messages in thread
From: Duy Nguyen @ 2014-06-04 23:10 UTC (permalink / raw)
  To: Stepan Kasal
  Cc: Johannes Schindelin, Erik Faye-Lund, GIT Mailing-list,
	Thomas Braun, msysGit

On Wed, Jun 4, 2014 at 11:16 PM, Stepan Kasal <kasal@ucw.cz> wrote:
> Hi dscho,
>
> your arguments seem really strong.  (Especially the four years of
> battle testing, with the memories of constant problems with HOME before.)
>
> I hope they are strong enough to convince Junio to accept this patch;
> that would help.

I think you should include the problems Dscho described in the commit
message too.
-- 
Duy

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "msysGit" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: Re: [PATCH] Add a Windows-specific fallback to getenv("HOME");
  2014-06-04 15:46           ` Johannes Schindelin
@ 2014-06-05  1:42             ` Karsten Blees
  2014-06-05  8:03               ` [PATCH v2] " Stepan Kasal
  2014-06-05 12:03               ` Re: [PATCH] " Johannes Schindelin
  0 siblings, 2 replies; 31+ messages in thread
From: Karsten Blees @ 2014-06-05  1:42 UTC (permalink / raw)
  To: Johannes Schindelin, Erik Faye-Lund
  Cc: Duy Nguyen, Stepan Kasal, GIT Mailing-list, Thomas Braun, msysGit

Am 04.06.2014 17:46, schrieb Johannes Schindelin:
> Hi kusma,
> 
> On Wed, 4 Jun 2014, Johannes Schindelin wrote:
> 
>> The problem arises whenever git.exe calls subprocesses. You can pollute
>> the environment by setting HOME, I do not recall the details, but I
>> remember that we had to be very careful *not* to do that, hence the patch.
>> Sorry, has been a long time.
> 
> Actually, a quick search in my Applegate vaults^W^Wmail archives suggests
> that we had tons of troubles with non-ASCII characters in the path.
> 

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.

In summary, this patch fixes 'git config' for the portable version only, and it only does so partially. Thus I don't think its ready for upstream, at least not in its current form. See below for the nasty details.


> 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.).

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")').

Cheers,
Karsten

====


Analysis of $HOME-realted issues:


1. mangled non-ASCII characters in environment variables

E.g. issue 491 [1], reportedly fixed in v1.7.10 ([1] comment #10).

This is actually a bug in msys.dll, and there's nothing that can be done about it from within git.exe. It is also not a problem if git is launched from cmd.exe.

The root cause is that the msys environment is initialized using GetEnvironmentStringsA(), which returns GetOEMCP()-encoded strings (e.g. cp850), rather than GetACP() (e.g. cp1252) as all other *A API functions do [2]. This adds one level of mangling whenever a native Windows program starts an msys program (so e.g. the call chain bash->git->bash->wish would mangle twice, see [1] comment #3).

For the fixed GetEnvironmentStringsA(), see [3] lines 459ff.

(As a side note, $HOMEDRIVE and $HOMEPATH originally did not have this problem, as they were separately initialized from NetUserGetInfoA(). This was changed in v1.6.3, however, at that time etc/profile was still using the broken $USERPROFILE. See [4], [5].)


2. 'git config' doesn't work with disconnected network drives

Issues 259 [6], 497 [7] and 512 [8], fixed in v1.7.0.2 for bash and v1.7.2.3 for cmd.

Apparently, $HOMEDRIVE$HOMEPATH is the home directory on the network, and $USERPROFILE is local. To be able to work offline, we need to check if $HOMEDRIVE$HOMEPATH exists and fall back to $USERPROFILE otherwise.

Note that git-wrapper does _not_ check if $HOMEDRIVE$HOMEPATH actually exists, as the original git.cmd did. This is probably a regression wrt issue 259.


3. HOME is not set when using the portable version

Issue 482 [9], partially fixed in v1.7.2.3 by this patch.

'Partially' because:
- there's no fallback to $USERPROFILE, so it doesn't work with disconnected network drives (see problem 2.)
- it doesn't setenv(HOME) for child processes (at least git-gui accesses $env(HOME) directly, but I haven't checked what happens if HOME is not set)

Incidentally, this patch was first released with v1.7.2.3, which also sets $HOME correctly in both etc/profile and git.cmd. So I suspect that this patch has always been essentially dead code (except perhaps for the portable version, I've never used that).


[1] https://code.google.com/p/msysgit/issues/detail?id=491
[2] http://msdn.microsoft.com/en-us/library/windows/desktop/ms683187%28v=vs.85%29.aspx
[3] https://github.com/msysgit/msysgit/blob/msys/src/rt/patches/0013-msys.dll-basic-Unicode-support.patch
[4] https://github.com/msysgit/msysgit/blob/msys/src/rt/patches/0007-only-override-the-variables-HOMEPATH-and-HOMEDRIVE-i.patch
[5] https://github.com/msysgit/msysgit/commit/6b096c9
[6] https://code.google.com/p/msysgit/issues/detail?id=259
[7] https://code.google.com/p/msysgit/issues/detail?id=497
[8] https://code.google.com/p/msysgit/issues/detail?id=512
[9] https://code.google.com/p/msysgit/issues/detail?id=482

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "msysGit" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [PATCH v2] Add a Windows-specific fallback to getenv("HOME");
  2014-06-05  1:42             ` Karsten Blees
@ 2014-06-05  8:03               ` Stepan Kasal
  2014-06-05  8:32                 ` [msysGit] " Torsten Bögershausen
  2014-06-05  9:40                 ` [PATCH v2] " Karsten Blees
  2014-06-05 12:03               ` Re: [PATCH] " Johannes Schindelin
  1 sibling, 2 replies; 31+ messages in thread
From: Stepan Kasal @ 2014-06-05  8:03 UTC (permalink / raw)
  To: Karsten Blees
  Cc: Johannes Schindelin, Erik Faye-Lund, Duy Nguyen,
	GIT Mailing-list, Thomas Braun, msysGit

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Wed, 2 Jun 2010 00:41:33 +0200

If HOME is not set, use $HOMEDRIVE$HOMEPATH

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Stepan Kasal <kasal@ucw.cz>
---

Hello Karsten,
thanks for your explanation.  There are more things to be done, but
I hope you can ack this patch as a step forward.

Hello Dscho,
I hope you can ack this as well: it is basically equivalent with your
patch, tailored according to current upstream fashion,  ;-)

Stepan

 compat/mingw.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/compat/mingw.c b/compat/mingw.c
index a0e13bc..e108388 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -1181,6 +1181,11 @@ char *mingw_getenv(const char *name)
 		if (!result)
 			result = getenv_cs("TEMP");
 	}
+	if (!result && !strcmp(name, "HOME")) {
+		struct strbuf buf = STRBUF_INIT;
+		strbuf_addf(&buf, "%s%s", getenv_cs("HOMEDRIVE"), getenv_cs("HOMEPATH"));
+		result = strbuf_detach(&buf, NULL);
+	}
 	return result;
 }
 
-- 
2.0.0.9635.g0be03cb

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "msysGit" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* Re: [msysGit] [PATCH v2] Add a Windows-specific fallback to getenv("HOME");
  2014-06-05  8:03               ` [PATCH v2] " Stepan Kasal
@ 2014-06-05  8:32                 ` Torsten Bögershausen
  2014-06-05 10:23                   ` [PATCH v3] " Stepan Kasal
  2014-06-05  9:40                 ` [PATCH v2] " Karsten Blees
  1 sibling, 1 reply; 31+ messages in thread
From: Torsten Bögershausen @ 2014-06-05  8:32 UTC (permalink / raw)
  To: Stepan Kasal, Karsten Blees
  Cc: Johannes Schindelin, Erik Faye-Lund, Duy Nguyen,
	GIT Mailing-list, Thomas Braun, msysGit

On 2014-06-05 10.03, Stepan Kasal wrote:
> From: Johannes Schindelin <johannes.schindelin@gmx.de>
> Date: Wed, 2 Jun 2010 00:41:33 +0200
> 
> If HOME is not set, use $HOMEDRIVE$HOMEPATH
> 
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> Signed-off-by: Stepan Kasal <kasal@ucw.cz>
> ---
> 
> Hello Karsten,
> thanks for your explanation.  There are more things to be done, but
> I hope you can ack this patch as a step forward.
> 
> Hello Dscho,
> I hope you can ack this as well: it is basically equivalent with your
> patch, tailored according to current upstream fashion,  ;-)
> 
> Stepan
> 
>  compat/mingw.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/compat/mingw.c b/compat/mingw.c
> index a0e13bc..e108388 100644
> --- a/compat/mingw.c
> +++ b/compat/mingw.c
> @@ -1181,6 +1181,11 @@ char *mingw_getenv(const char *name)
>  		if (!result)
>  			result = getenv_cs("TEMP");
>  	}
> +	if (!result && !strcmp(name, "HOME")) {
> +		struct strbuf buf = STRBUF_INIT;
> +		strbuf_addf(&buf, "%s%s", getenv_cs("HOMEDRIVE"), getenv_cs("HOMEPATH"));
should we have a NULL pointer check here?
What happens if %HOMEPATH% is not set for any reason ?
getenv_cs will return NULL, and strbuf_addf() does not like that, as far as I know.
And even if it converts a NULL pointer into "<NULL>" or "NULL", the result is not what we want.
If HOMEDRIVE is set, but not HOMEPATH, we can fall back into the root of HOMEDRIVE:

	if (!result && !strcmp(name, "HOME")) {
		const char *homedrive = getenv_cs("HOMEDRIVE");
		const char *homepath = getenv_cs("HOMEPATH");
		if (!homepath)
			homepath = "";
        	if (homedrive) {
			struct strbuf buf = STRBUF_INIT;
			strbuf_addf(&buf, "%s%s", homedrive, homepath);
			result = strbuf_detach(&buf, NULL);
		}
	}
	return result;
}  
 

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH v2] Add a Windows-specific fallback to getenv("HOME");
  2014-06-05  8:03               ` [PATCH v2] " Stepan Kasal
  2014-06-05  8:32                 ` [msysGit] " Torsten Bögershausen
@ 2014-06-05  9:40                 ` Karsten Blees
  2014-06-05  9:58                   ` Erik Faye-Lund
                                     ` (2 more replies)
  1 sibling, 3 replies; 31+ messages in thread
From: Karsten Blees @ 2014-06-05  9:40 UTC (permalink / raw)
  To: Stepan Kasal
  Cc: Johannes Schindelin, Erik Faye-Lund, Duy Nguyen,
	GIT Mailing-list, Thomas Braun, msysGit

Am 05.06.2014 10:03, schrieb Stepan Kasal:
> From: Johannes Schindelin <johannes.schindelin@gmx.de>
> Date: Wed, 2 Jun 2010 00:41:33 +0200
> 
> If HOME is not set, use $HOMEDRIVE$HOMEPATH
> 
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> Signed-off-by: Stepan Kasal <kasal@ucw.cz>
> ---
> 
> Hello Karsten,
> thanks for your explanation.  There are more things to be done, but
> I hope you can ack this patch as a step forward.
> 

No, not really. Its sure better than introducing a special get_home_directory(), but it still increases the diff between upstream and msysgit rather than reducing it. The main critique points still remain:

 * $HOME is usually set up correctly before calling git, so this is essentially dead code (just checked, portable git's git-bash.bat and git-cmd.bat also do this correctly)

 * even if $HOME was empty, git should setenv("HOME") so that child processes can benefit from it (similar to TMPDIR and TERM in current msysgit's mingw_startup()). Not setting $HOME because it may hypothetically break child processes is a very weak argument, as we always did set $HOME in etc/profile (since the initial version back in 2007).

 * no fallback to $USERPROFILE doesn't work with diconnected home share

If you really have time to spare, I suggest you focus on getting the Unicode patches upstream so that we can progress from there (e.g. move $HOME setup to mingw_startup() so that we can get rid of redundant logic in etc/profile, git-wrapper, git-bash.bat, git-cmd.bat etc.).

> Hello Dscho,
> I hope you can ack this as well: it is basically equivalent with your
> patch, tailored according to current upstream fashion,  ;-)
> 
> Stepan
> 
>  compat/mingw.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/compat/mingw.c b/compat/mingw.c
> index a0e13bc..e108388 100644
> --- a/compat/mingw.c
> +++ b/compat/mingw.c
> @@ -1181,6 +1181,11 @@ char *mingw_getenv(const char *name)
>  		if (!result)
>  			result = getenv_cs("TEMP");
>  	}

else?

> +	if (!result && !strcmp(name, "HOME")) {
> +		struct strbuf buf = STRBUF_INIT;
> +		strbuf_addf(&buf, "%s%s", getenv_cs("HOMEDRIVE"), getenv_cs("HOMEPATH"));

No surplus '/', good!

> +		result = strbuf_detach(&buf, NULL);

This leaks memory.

> +	}
>  	return result;
>  }
>  
> 

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "msysGit" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH v2] Add a Windows-specific fallback to getenv("HOME");
  2014-06-05  9:40                 ` [PATCH v2] " Karsten Blees
@ 2014-06-05  9:58                   ` Erik Faye-Lund
  2014-06-05 21:44                     ` Karsten Blees
  2014-06-05 11:23                   ` Stepan Kasal
  2014-06-05 13:39                   ` Johannes Schindelin
  2 siblings, 1 reply; 31+ messages in thread
From: Erik Faye-Lund @ 2014-06-05  9:58 UTC (permalink / raw)
  To: Karsten Blees
  Cc: Stepan Kasal, Johannes Schindelin, Duy Nguyen, GIT Mailing-list,
	Thomas Braun, msysGit

On Thu, Jun 5, 2014 at 11:40 AM, Karsten Blees <karsten.blees@gmail.com> wrote:
> Am 05.06.2014 10:03, schrieb Stepan Kasal:
>> From: Johannes Schindelin <johannes.schindelin@gmx.de>
>> Date: Wed, 2 Jun 2010 00:41:33 +0200
>>
>> If HOME is not set, use $HOMEDRIVE$HOMEPATH
>>
>> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
>> Signed-off-by: Stepan Kasal <kasal@ucw.cz>
>> ---
>>
>> Hello Karsten,
>> thanks for your explanation.  There are more things to be done, but
>> I hope you can ack this patch as a step forward.
>>
>
> No, not really. Its sure better than introducing a special get_home_directory(), but it still increases the diff between upstream and msysgit rather than reducing it. The main critique points still remain:
>
>  * $HOME is usually set up correctly before calling git, so this is essentially dead code (just checked, portable git's git-bash.bat and git-cmd.bat also do this correctly)

What about when tools like TortoiseGit and Git Extensions call git?
We're not guaranteed that they did the $HOME-dance, are we?

>  * even if $HOME was empty, git should setenv("HOME") so that child processes can benefit from it (similar to TMPDIR and TERM in current msysgit's mingw_startup()). Not setting $HOME because it may hypothetically break child processes is a very weak argument, as we always did set $HOME in etc/profile (since the initial version back in 2007).
>
>  * no fallback to $USERPROFILE doesn't work with diconnected home share
>
> If you really have time to spare, I suggest you focus on getting the Unicode patches upstream so that we can progress from there (e.g. move $HOME setup to mingw_startup() so that we can get rid of redundant logic in etc/profile, git-wrapper, git-bash.bat, git-cmd.bat etc.).

Perhaps we can patch up the upstream to better match Git for Windows
without upstreaming the Unicode patches? Don't get me wrong; I think
upstreaming them is a good idea, but in case time is lacking...

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "msysGit" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [PATCH v3] Add a Windows-specific fallback to getenv("HOME");
  2014-06-05  8:32                 ` [msysGit] " Torsten Bögershausen
@ 2014-06-05 10:23                   ` Stepan Kasal
  0 siblings, 0 replies; 31+ messages in thread
From: Stepan Kasal @ 2014-06-05 10:23 UTC (permalink / raw)
  To: Torsten Bögershausen
  Cc: Karsten Blees, Johannes Schindelin, Erik Faye-Lund, Duy Nguyen,
	GIT Mailing-list, Thomas Braun, msysGit

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Wed, 2 Jun 2010 00:41:33 +0200

If HOME is not set, use $HOMEDRIVE$HOMEPATH

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Stepan Kasal <kasal@ucw.cz>
---

On Thu, Jun 05, 2014 at 10:32:44AM +0200, Torsten Bögershausen wrote:
> > +		strbuf_addf(&buf, "%s%s", getenv_cs("HOMEDRIVE"), getenv_cs("HOMEPATH"));
> should we have a NULL pointer check here?

You are right, of course.

> If HOMEDRIVE is set, but not HOMEPATH, we can fall back into the root of HOMEDRIVE:

Indeed, but it means setting homepath="\\";

Updated according to your comments.  Thanks,
	Stepan

 compat/mingw.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/compat/mingw.c b/compat/mingw.c
index a0e13bc..14af013 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -1181,6 +1181,17 @@ char *mingw_getenv(const char *name)
 		if (!result)
 			result = getenv_cs("TEMP");
 	}
+	if (!result && !strcmp(name, "HOME")) {
+		const char *homedrive = getenv_cs("HOMEDRIVE");
+		const char *homepath = getenv_cs("HOMEPATH");
+		if (homedrive) {
+			struct strbuf buf = STRBUF_INIT;
+			if (!homepath)
+				homepath = "\\";
+			strbuf_addf(&buf, "%s%s", homedrive, homepath);
+			result = strbuf_detach(&buf, NULL);
+		}
+	}
 	return result;
 }
 
-- 
2.0.0.9635.g0be03cb

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "msysGit" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* Re: [PATCH v2] Add a Windows-specific fallback to getenv("HOME");
  2014-06-05  9:40                 ` [PATCH v2] " Karsten Blees
  2014-06-05  9:58                   ` Erik Faye-Lund
@ 2014-06-05 11:23                   ` Stepan Kasal
  2014-06-05 13:39                   ` Johannes Schindelin
  2 siblings, 0 replies; 31+ messages in thread
From: Stepan Kasal @ 2014-06-05 11:23 UTC (permalink / raw)
  To: Karsten Blees
  Cc: Johannes Schindelin, Erik Faye-Lund, Duy Nguyen,
	GIT Mailing-list, Thomas Braun, msysGit

Hello,

On Thu, Jun 05, 2014 at 11:40:50AM +0200, Karsten Blees wrote:
> Am 05.06.2014 10:03, schrieb Stepan Kasal:
> > I hope you can ack this patch as a step forward.
> 
> No, not really. It's sure better than introducing a special
> get_home_directory(), but it still increases the diff between
> upstream and msysgit rather than reducing it. [...]

this patch (v3) is a win-win for both sides:

- upstream would get at least $HOMEDRIVE$HOMEPATH
- downstream would get rid of get_home_directory
- it would decrease the diff, in my metric[*]

[*]  The patch with get_home_directory() could be that dropped.
Yes, the patch "84b7969 Win32: patch Windows environment on startup"
would have to be updated, but I can handle that easily.

> The main critique points still remain:
>  * $HOME is usually set up correctly before calling git, [...]
>  * even if $HOME was empty, git should setenv("HOME") [...]
>  * no fallback to $USERPROFILE [...]

This is a plan for further work, but not an argument against the
current version of patch.

> If you really have time to spare, I suggest you focus on getting
> the Unicode patches upstream so that we can progress from there

Not that much time.  That's why I try to push the patches that seem
to be simpler.  Some get discussed, some get ignored, but some get
accepted (or dropped).

Stepan

PS:
tongue in cheek:
If _you_ could find some time, could you please support these:
http://thread.gmane.org/gmane.comp.version-control.msysgit/20324

The first patch of the pair introduces mingw_startup, which is a good
base for other changes.
The second one is my new fix for const warnings, exactly according
to the lines mentioned here: instead of fixing all the consumers and
waiting when it'll break again, I modified the mingw-specific code
to adapt better.

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "msysGit" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: Re: [PATCH] Add a Windows-specific fallback to getenv("HOME");
  2014-06-05  1:42             ` Karsten Blees
  2014-06-05  8:03               ` [PATCH v2] " Stepan Kasal
@ 2014-06-05 12:03               ` Johannes Schindelin
  2014-06-05 12:15                 ` [msysGit] " Stepan Kasal
  2014-06-05 14:33                 ` Karsten Blees
  1 sibling, 2 replies; 31+ messages in thread
From: Johannes Schindelin @ 2014-06-05 12:03 UTC (permalink / raw)
  To: Karsten Blees
  Cc: Erik Faye-Lund, Duy Nguyen, Stepan Kasal, GIT Mailing-list,
	Thomas Braun, msysGit

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

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "msysGit" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [msysGit] Re: [PATCH] Add a Windows-specific fallback to getenv("HOME");
  2014-06-05 12:03               ` Re: [PATCH] " Johannes Schindelin
@ 2014-06-05 12:15                 ` Stepan Kasal
  2014-06-05 14:33                 ` Karsten Blees
  1 sibling, 0 replies; 31+ messages in thread
From: Stepan Kasal @ 2014-06-05 12:15 UTC (permalink / raw)
  To: Johannes Schindelin
  Cc: Karsten Blees, Erik Faye-Lund, Duy Nguyen, GIT Mailing-list,
	Thomas Braun, msysGit

Hi,

On Thu, Jun 05, 2014 at 02:03:39PM +0200, Johannes Schindelin wrote:
> Render me even more convinced that the API call is the cleanest way to go,

But not me.  

In a paralel post, Duy Nguyen wrote:
> Thank you for working on pushing msysgit patches upstream. I don't use
> git on windows, but it's nice to see all windows-specific changes in
> one code base so we can try to workaround it when new patches/features
> are developed.

... but we should not obscure that more than necessary.  If the API
call is   getenv("HOME");  it helps.  And the hack with
mingw_getenv() is not that bad, so we should be pragmatic and accept
it.

Stepan

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH v2] Add a Windows-specific fallback to getenv("HOME");
  2014-06-05  9:40                 ` [PATCH v2] " Karsten Blees
  2014-06-05  9:58                   ` Erik Faye-Lund
  2014-06-05 11:23                   ` Stepan Kasal
@ 2014-06-05 13:39                   ` Johannes Schindelin
  2014-06-05 20:03                     ` Karsten Blees
  2 siblings, 1 reply; 31+ messages in thread
From: Johannes Schindelin @ 2014-06-05 13:39 UTC (permalink / raw)
  To: Karsten Blees
  Cc: Stepan Kasal, Erik Faye-Lund, Duy Nguyen, GIT Mailing-list,
	Thomas Braun, msysGit

Hi Karsten,

On Thu, 5 Jun 2014, Karsten Blees wrote:

> Am 05.06.2014 10:03, schrieb Stepan Kasal:
> 
>  * even if $HOME was empty, git should setenv("HOME") so that child
>  processes can benefit from it (similar to TMPDIR and TERM in current
>  msysgit's mingw_startup()). Not setting $HOME because it may
>  hypothetically break child processes is a very weak argument, as we
>  always did set $HOME in etc/profile (since the initial version back in
>  2007).

I do remember that I tried that first, as I mentioned in this thread.
There must have been a breakage preventing me from going that route.

And in particular with your changes to Unicodify the complete environment,
I am *highly* doubtful that child processes will be able to handle
themselves properly, unless we spend a whole lot of time converting back
and forth the environment when calling children.

Ciao,
Dscho

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "msysGit" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: Re: [PATCH] Add a Windows-specific fallback to getenv("HOME");
  2014-06-05 12:03               ` Re: [PATCH] " Johannes Schindelin
  2014-06-05 12:15                 ` [msysGit] " Stepan Kasal
@ 2014-06-05 14:33                 ` Karsten Blees
  1 sibling, 0 replies; 31+ messages in thread
From: Karsten Blees @ 2014-06-05 14:33 UTC (permalink / raw)
  To: Johannes Schindelin
  Cc: Erik Faye-Lund, Duy Nguyen, Stepan Kasal, GIT Mailing-list,
	Thomas Braun, msysGit

Am 05.06.2014 14:03, schrieb Johannes Schindelin:
> 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.

The commit message explicitly claims that it fixes issue 482, which is: 'git config --global' in the portable version fails with "fatal: $HOME not set" (not "unable to access '...'", which you would get for a mangled path that doesn't exist).

As outlined in the previous mail (analysis 1.), the non-ASCII problem is caused by a bug in msys.dll, and it is in fact impossible to fix in git (even if that was your intention).

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

See previous mail analysis 3. In short: it doesn't work with disconnected home share (issue 259), and it doesn't setenv("HOME") (so child processes such as git-gui will most likely fail).

>> 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.

Exactly. In order to support different platforms, we need to agree on a common abstraction layer to access platform-specific functionality. For the git project, this common abstraction layer happens to be the POSIX standard (actually: the subset of the standard that is used by core git code). And compat/mingw.c implements that abstraction layer for the native Windows platform.

There are cases where conforming to the standard is simply not feasible, e.g. fork() (we don't want to build another cygwin). So we sometimes need special handling for certain functionality in core-git (see run-command.c in case of fork()).

However, getenv("HOME"), getenv("TMPDIR") and getenv("PATH") are all fully POSIX compliant, including the standardised variable names. In this particular case, conforming to the standard (via special handling in mingw_getenv or mingw_startup) is actually even _simpler_ than inventing a new, non-standard, undocumented get_home_directory() API.

> 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?

No, POSIX doesn't specify path names. The standard way to get the temp directory is 'getenv("TMPDIR")'. A hardcoded "/tmp" in core git code would be a bug.


-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "msysGit" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH v2] Add a Windows-specific fallback to getenv("HOME");
  2014-06-05 13:39                   ` Johannes Schindelin
@ 2014-06-05 20:03                     ` Karsten Blees
  0 siblings, 0 replies; 31+ messages in thread
From: Karsten Blees @ 2014-06-05 20:03 UTC (permalink / raw)
  To: Johannes Schindelin
  Cc: Stepan Kasal, Erik Faye-Lund, Duy Nguyen, GIT Mailing-list,
	Thomas Braun, msysGit

Am 05.06.2014 15:39, schrieb Johannes Schindelin:
> And in particular with your changes to Unicodify the complete environment,
> I am *highly* doubtful that child processes will be able to handle
> themselves properly, unless we spend a whole lot of time converting back
> and forth the environment when calling children.

The unicode version _does_ convert back and forth, in mingw_startup and make_environment_block, respectively. However, as the unicode environment is sorted, this is actually much faster than the original version.

To put things in perspective *:

entire mingw_startup: ~450 µs
 * _wgetmainargs: 25 µs
 * allocate+convert args and environment: 25 µs
 * qsort environment: 15 µs
 * winansi_init: 393 µs

entire mingw_spawnve_fd: ~1250 µs
 * make_environment_block: 25 µs
 * CreateProcessW: 690 µs

Now, the unicode mingw_getenv is O(log n) (~0.15 µs per call) and MSVCRT's getenv is O(n) (~3.6 µs per call).

A git command that just launches a script (e.g. git gui) calls getenv ~25 times. (3.6 µs - 0.15 µs) * 25 = 86 µs, i.e. this compensates the additional startup time (including qsort) more than twice.

(*) Measurements done via QueryPerformanceCounter, with 75 environment entries, on a Core i7 960, Windows 7 x64

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "msysGit" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH v2] Add a Windows-specific fallback to getenv("HOME");
  2014-06-05  9:58                   ` Erik Faye-Lund
@ 2014-06-05 21:44                     ` Karsten Blees
  2014-06-06  8:03                       ` Stepan Kasal
  0 siblings, 1 reply; 31+ messages in thread
From: Karsten Blees @ 2014-06-05 21:44 UTC (permalink / raw)
  To: kusmabite
  Cc: Stepan Kasal, Johannes Schindelin, Duy Nguyen, GIT Mailing-list,
	Thomas Braun, msysGit

Am 05.06.2014 11:58, schrieb Erik Faye-Lund:
> On Thu, Jun 5, 2014 at 11:40 AM, Karsten Blees <karsten.blees@gmail.com> wrote:
>> Am 05.06.2014 10:03, schrieb Stepan Kasal:
>>> From: Johannes Schindelin <johannes.schindelin@gmx.de>
>>> Date: Wed, 2 Jun 2010 00:41:33 +0200
>>>
>>> If HOME is not set, use $HOMEDRIVE$HOMEPATH
>>>
>>> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
>>> Signed-off-by: Stepan Kasal <kasal@ucw.cz>
>>> ---
>>>
>>> Hello Karsten,
>>> thanks for your explanation.  There are more things to be done, but
>>> I hope you can ack this patch as a step forward.
>>>
>>
>> No, not really. Its sure better than introducing a special get_home_directory(), but it still increases the diff between upstream and msysgit rather than reducing it. The main critique points still remain:
>>
>>  * $HOME is usually set up correctly before calling git, so this is essentially dead code (just checked, portable git's git-bash.bat and git-cmd.bat also do this correctly)
> 
> What about when tools like TortoiseGit and Git Extensions call git?
> We're not guaranteed that they did the $HOME-dance, are we?
> 

GitExtensions does the same thing, see issue 497. I don't know about TortoiseGit, but I suspect the same.

>>  * even if $HOME was empty, git should setenv("HOME") so that child processes can benefit from it (similar to TMPDIR and TERM in current msysgit's mingw_startup()). Not setting $HOME because it may hypothetically break child processes is a very weak argument, as we always did set $HOME in etc/profile (since the initial version back in 2007).
>>
>>  * no fallback to $USERPROFILE doesn't work with diconnected home share
>>
>> If you really have time to spare, I suggest you focus on getting the Unicode patches upstream so that we can progress from there (e.g. move $HOME setup to mingw_startup() so that we can get rid of redundant logic in etc/profile, git-wrapper, git-bash.bat, git-cmd.bat etc.).
> 
> Perhaps we can patch up the upstream to better match Git for Windows
> without upstreaming the Unicode patches? Don't get me wrong; I think
> upstreaming them is a good idea, but in case time is lacking...
> 

The unicode patch series happens to be one of the first on top of upstream, and its also the longest (~40 patches) and I believe most intrusive one (~1500 lines changed). So I think the most time-preserving option is to send it upstream as unchanged as possible (probably with the bugfix-patches squashed). There's only ~50 lines changed outside of compat, so hopefully there won't be too many additional review-rounds...

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "msysGit" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH v2] Add a Windows-specific fallback to getenv("HOME");
  2014-06-05 21:44                     ` Karsten Blees
@ 2014-06-06  8:03                       ` Stepan Kasal
  0 siblings, 0 replies; 31+ messages in thread
From: Stepan Kasal @ 2014-06-06  8:03 UTC (permalink / raw)
  To: Karsten Blees
  Cc: kusmabite, Johannes Schindelin, Duy Nguyen, GIT Mailing-list,
	Thomas Braun, msysGit

Hi,

On Thu, Jun 05, 2014 at 11:44:22PM +0200, Karsten Blees wrote:
> I think the most time-preserving option is to send it upstream as
> unchanged as possible (probably with the bugfix-patches squashed).

I plan to submit one by one or in a small series.
Agreed about the squashes, I have several of them prepared, thanks to
your hints.

Stepan

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "msysGit" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Git for Windows SDK
  2014-06-04 17:49                 ` [msysGit] " Johannes Schindelin
@ 2014-06-06  9:12                   ` Philip Oakley
  0 siblings, 0 replies; 31+ messages in thread
From: Philip Oakley @ 2014-06-06  9:12 UTC (permalink / raw)
  To: Johannes Schindelin, Stepan Kasal
  Cc: Erik Faye-Lund, Duy Nguyen, GIT Mailing-list, Thomas Braun, msysGit

----- Original Message ----- 
From: "Johannes Schindelin" <Johannes.Schindelin@gmx.de>
was "Re: [msysGit] Re: [PATCH] Add a Windows-specific fallback to 
getenv("HOME");

<snip>

> Seriously again, I am in favor of calling it the Git for Windows SDK. 
> But
> really, it is bikeshedding at this point. There is real work to do, 
> still,
> before we can switch. Lots of unaddressed questions. Too little time.
> Speaking of which... budget's depleted for today ;-)
>
> Ciao,
> Dscho
>
> -- 

I like the "Git for Windows SDK" name.

For me it communicates the intent very well and reflects in my mind the 
wider world naming conventuion for such things. It probably isn't an 
"IDE" in the wider sense, though I'm sure it could be, depending on 
viewpoint.

so +1 on the "G4W SDK".
</bikeshedding>

Philip 

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH] Add a Windows-specific fallback to getenv("HOME");
  2014-06-04 16:16               ` Stepan Kasal
  2014-06-04 17:49                 ` [msysGit] " Johannes Schindelin
  2014-06-04 23:10                 ` Re: [PATCH] Add a Windows-specific fallback to getenv("HOME"); Duy Nguyen
@ 2014-06-06 19:26                 ` Sebastian Schuberth
  2 siblings, 0 replies; 31+ messages in thread
From: Sebastian Schuberth @ 2014-06-06 19:26 UTC (permalink / raw)
  To: Stepan Kasal, Johannes Schindelin
  Cc: Erik Faye-Lund, Duy Nguyen, GIT Mailing-list, Thomas Braun, msysGit

On 04.06.2014 18:16, Stepan Kasal wrote:

>> plan is to switch to mingwGitDevEnv for said release. No more msysGit.
>> Like, bu-bye. Thanks for all the fish.
> 
> Interesting.
> 
> With msysgit, there is the "net installer" - first time I installed
> msys/mingw sucessfully, it was as easy as Cygwin, perhaps even
> easier.

And with mingwGitDevEnv, there's the equivalent installer at [1].

> When I go to mingwGitDevEnv home page, I read about chickens, eggs,
> and upgrading Perl (which msysGit simply gives up, hinting that it is
> almost impossible).

I have absolutely no idea what chickens and eggs that would be. If you care to elaborate, please consider using the mingwGitDevEnv mailing list [2].

> PPS: from marketing point of view, mingwGitDevEnv is far from usable
> name.  Dscho, if you support the idea, would you mind franchising
> msysGit 2.0 for a decent amount?

Doh. Marketing. As if we would sell something. I still believe developers are more interested in getting things done no matter what the tools are called. At east they should be.

[1] http://mingwgitdevenv.cloudapp.net/job/mingwGitDevEnv-build-installer/lastSuccessfulBuild/artifact/download.html
[2] http://groups.google.com/group/mingwGitDevEnv

-- 
Sebastian Schuberth

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "msysGit" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply	[flat|nested] 31+ messages in thread

end of thread, other threads:[~2014-06-06 19:26 UTC | newest]

Thread overview: 31+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-06-04 11:47 [PATCH] Add a Windows-specific fallback to getenv("HOME"); Stepan Kasal
2014-06-04 13:47 ` Duy Nguyen
2014-06-04 14:05   ` Erik Faye-Lund
2014-06-04 14:55     ` Karsten Blees
2014-06-04 15:14     ` Johannes Schindelin
2014-06-04 15:18       ` Erik Faye-Lund
2014-06-04 15:27         ` Johannes Schindelin
2014-06-04 15:45           ` Stepan Kasal
2014-06-04 15:56             ` [msysGit] " Johannes Schindelin
2014-06-04 16:16               ` Stepan Kasal
2014-06-04 17:49                 ` [msysGit] " Johannes Schindelin
2014-06-06  9:12                   ` Git for Windows SDK Philip Oakley
2014-06-04 23:10                 ` Re: [PATCH] Add a Windows-specific fallback to getenv("HOME"); Duy Nguyen
2014-06-06 19:26                 ` Sebastian Schuberth
2014-06-04 15:46           ` Johannes Schindelin
2014-06-05  1:42             ` Karsten Blees
2014-06-05  8:03               ` [PATCH v2] " Stepan Kasal
2014-06-05  8:32                 ` [msysGit] " Torsten Bögershausen
2014-06-05 10:23                   ` [PATCH v3] " Stepan Kasal
2014-06-05  9:40                 ` [PATCH v2] " Karsten Blees
2014-06-05  9:58                   ` Erik Faye-Lund
2014-06-05 21:44                     ` Karsten Blees
2014-06-06  8:03                       ` Stepan Kasal
2014-06-05 11:23                   ` Stepan Kasal
2014-06-05 13:39                   ` Johannes Schindelin
2014-06-05 20:03                     ` Karsten Blees
2014-06-05 12:03               ` Re: [PATCH] " Johannes Schindelin
2014-06-05 12:15                 ` [msysGit] " Stepan Kasal
2014-06-05 14:33                 ` Karsten Blees
2014-06-04 14:53   ` Stepan Kasal
2014-06-04 15:13   ` Johannes Schindelin

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.