Re: Merge problems with git-mingw

Previous thread: [StGit PATCH 0/6] Survive capital punishment by Karl on Monday, October 8, 2007 - 1:55 am. (12 messages)

Next thread: Re: git: avoiding merges, rebasing by Benoit SIGOURE on Monday, October 8, 2007 - 6:16 am. (1 message)
From: Peter Karlsson
Date: Monday, October 8, 2007 - 4:06 am

Hi!

I am running the mingw port of Git from a Cygwin environment, because I
had problems with the Cygwin version (not sure what the problem was,
probably a CRLF issue). Everything works fine, except for merging:

  $ git merge master
  usage: git-var [-l | <variable>]
  $

Does anyone have any idea on what could cause that? (I've checked that
git-var is indeed the mingw version and not the Cygwin version).

  $ git --version
  git version 1.5.3.mingw.1

I'm running on top of another version control system, so I need to
switch back to master and update the sources from upstream using that
system's tools, and then merge it back into my branch. Without "git
merge" working, that is a bit problematic, to say the least... :-)

-- 
\\// Peter - http://www.softwolves.pp.se/
-

From: Lars Hjemli
Date: Monday, October 8, 2007 - 5:00 am

What does this command return?

  $ git var GIT_COMMITTER_IDENT

--
larsh
-

From: Peter Karlsson
Date: Monday, October 8, 2007 - 5:33 am

Doesn't seem to work:

  $ git var GIT_COMMITTER_IDENT
  usage: git-var [-l | <variable>]

-- 
\\// Peter - http://www.softwolves.pp.se/
-

From: Lars Hjemli
Date: Monday, October 8, 2007 - 6:10 am

Something is weird with your setup and/or the mingw port, but you can
probably work around the issue by doing this:

$ git config user.name "your name"
$ git config user.email "your email"

Optionally, you can add the --global flag to both commands to make the
config visible in all of your repos.

--
larsh
-

From: Peter Karlsson
Date: Monday, October 8, 2007 - 7:56 am

Both of those are configured properly, but even after configuring them
locally for the repository only, I get the same error with "git var".
Weird.

-- 
\\// Peter - http://www.softwolves.pp.se/
-

From: Lars Hjemli
Date: Monday, October 8, 2007 - 12:59 pm

[Johannes CC'ed as the mingw.git maintainer]


Does 'git var -l' work as expected? Also, could you try the latest
git-package provided by the cygwin installer? If CRLF-handling was
your problem, take a look at the description of core.autocrlf with
'git help config'.

[This does look like an issue with running mingw.git under Cygwin.
Johannes, is this even supposed to work?]

--
larsh
-

From: Johannes Sixt
Date: Monday, October 8, 2007 - 11:10 pm

Sure, why not? Cygwin's settings as well as core.autocrlf should be 
completely irrelevant. I've no clue what could be wrong here.

-- Hannes
-

From: Peter Karlsson
Date: Tuesday, October 9, 2007 - 12:06 am

Hmm, it also gives an error message:

$ git var -l

I tried it with Git 1.5.3.2 from Cygwin, and the problem I'm having is
not with the CRLF handling in the checked out files, but rather in the
repository database itself. Cloning a repository with Cygwin-Git
produced various errors about the object database being corrupt.

I get the same error running mingw-git from a regular cmd.exe prompt,
so it doesn't seem to be related to Cygwin at all.

-- 
\\// Peter - http://www.softwolves.pp.se/
-

From: Johannes Sixt
Date: Tuesday, October 9, 2007 - 12:36 am

What if you run 'git var -l' outside a repository? From CMD?
Have you tried 'git-var -l' (note the dash)? From CMD?
Are you sure you have only one version of git in the PATH?
What's in your .git/config?

-- Hannes
-

From: Peter Karlsson
Date: Tuesday, October 9, 2007 - 1:56 am

C:\Program Files\Git\bin>git var -l
usage: git-var [-l | <variable>]

C:\Program Files\Git\bin>git-var -l
fatal: Not a git repository

C:\Program Files\Git\bin>git --version


This is the .git/config file for one of the repositories where it
fails:

  [core]
        repositoryformatversion = 0
        filemode = false
        bare = false
        logallrefupdates = true
        symlinks = false
        pager = less -x3 -r

  [instaweb]
        local = true
        httpd = c:/cygwin/usr/sbin/lighttpd.exe -f
        port = 4321
        browser = c:/progra~1/opera/opera.exe
        modulepath = c:/cygwin/usr/lib/apache
  [diff]
        renames = copies
        external = C:/Program Files/tkdiff/tkdiff.exe
  [user]
        name = Peter Karlsson
        email = peter.karlsson@tandbergdata.com

This is my global $HOME/.gitconfig:

  [user]
  name = Peter Karlsson
  email = peter@softwolves.pp.se
  [core]
          editor = c:/cygwin/bin/joe.exe
          pager = less -x3 -r
  [color]
  [color]
          pager = true

-- 
\\// Peter - http://www.softwolves.pp.se/
-

From: Johannes Sixt
Date: Tuesday, October 9, 2007 - 2:03 am

For the time being, install this beast in a path without blanks.

This needs fixing, appearently. :(

-- Hannes

-

From: Steffen Prohaska
Date: Tuesday, October 9, 2007 - 9:33 am

I have the printf 'callstack' below from the v1.5.3.mingw.1 version  
(9c792c5)
Apparently spawnve, which is called in compat/mingw.c, corrupts argv.

I have no idea how to continue debugging. Maybe some one else can  
take over.

	Steffen


$ git var GIT_COMMITTER_IDENT

argv at git.c:429
3
C:\Program Files (x86)\Git\bin\git.exe
var
C:/Program Files (x86)/Git/bin/git-var
GIT_COMMITTER_IDENT

argv at compat/mingw.c:301
C:/Program Files (x86)/Git/bin/git-var
GIT_COMMITTER_IDENT
(null)

argv at var.c:54
4
C:/Program
Files
(x86)/Git/bin/git-var
GIT_COMMITTER_IDENT
usage: git-var [-l | <variable>]



-

From: Johannes Sixt
Date: Tuesday, October 9, 2007 - 10:03 am

Thank you. Here is a quick-fix.

-- Hannes

diff --git a/compat/mingw.c b/compat/mingw.c
index 8bb0dba..361216f 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -303,7 +303,7 @@ void openlog(const char *ident, int option,
  {
  }

-static const char *quote_arg(const char *arg)
+const char *quote_arg(const char *arg)
  {
  	/* count chars to quote */
  	int len = 0, n = 0;
diff --git a/exec_cmd.c b/exec_cmd.c
index bad4843..7ab5a0f 100644
--- a/exec_cmd.c
+++ b/exec_cmd.c
@@ -129,7 +129,8 @@ int execv_git_cmd(const char **argv)
  		 */

  		tmp = argv[0];
-		argv[0] = git_command;
+		extern const char *quote_arg(const char *arg);
+		argv[0] = quote_arg(git_command);

  		trace_argv_printf(argv, -1, "trace: exec:");


-

From: Linus Torvalds
Date: Tuesday, October 9, 2007 - 11:09 am

Extern declarations inside a local scope will not work on more modern gcc 
versions. Don't do it. Do proper prototyping in proper scope.

		Linus
-

From: Steffen Prohaska
Date: Tuesday, October 9, 2007 - 2:39 pm

This patch fixed executing of non-builtins if installation
dir contains spaces. That is 'git var' works now.

The original patch is by Johannes Sixt <j.sixt@viscovery.net>.

Signed-off-by: Steffen Prohaska <prohaska@zib.de>
---
 compat/mingw.c    |    2 +-
 exec_cmd.c        |    2 +-
 git-compat-util.h |    1 +
 3 files changed, 3 insertions(+), 2 deletions(-)

Ok, this is what I created from the quick-fix. It works for
me in msysgit. 

Should the patch be polished that it can be applied to git.git
or will we only apply it to 4msysgit?

If it should be polished, what would be the right way?
ifdef in exec_cmd.c?

	Steffen


diff --git a/compat/mingw.c b/compat/mingw.c
index 6632192..0dd0cb0 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -196,7 +196,7 @@ void openlog(const char *ident, int option, int facility)
 {
 }
 
-static const char *quote_arg(const char *arg)
+const char *quote_arg(const char *arg)
 {
 	/* count chars to quote */
 	int len = 0, n = 0;
diff --git a/exec_cmd.c b/exec_cmd.c
index 2a8e48b..4c7c7ca 100644
--- a/exec_cmd.c
+++ b/exec_cmd.c
@@ -132,7 +132,7 @@ int execv_git_cmd(const char **argv)
 		 */
 
 		tmp = argv[0];
-		argv[0] = git_command;
+		argv[0] = quote_arg(git_command);
 
 		trace_argv_printf(argv, -1, "trace: exec:");
 
diff --git a/git-compat-util.h b/git-compat-util.h
index ba5a1a1..5276221 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -482,6 +482,7 @@ int mingw_socket(int domain, int type, int protocol);
 #define fsync(x) 0
 
 extern void quote_argv(const char **dst, const char **src);
+extern const char *quote_arg(const char *arg);
 extern const char *parse_interpreter(const char *cmd);
 
 extern __attribute__((noreturn)) int git_exit(int code);
-- 
1.5.3.mingw.1.13.g70ed-dirty

-

From: Johannes Sixt
Date: Tuesday, October 9, 2007 - 11:16 pm

I'll apply it to mingw.git, and as long as you don't rebase or pull from 
there, you'd also have to apply it to 4msysgit.


This area needs more polishing, and it will be among the *last* mingw 
patches that flow upstream.

BTW, I think the fix is incomplete anyway: We quote only argv[0], but 
actually all of argv should be quoted. Will test.

-- Hannes
-

From: Johannes Sixt
Date: Wednesday, October 10, 2007 - 12:00 am

If an external git command (not a shell script) was invoked with arguments
that contain spaces, these arguments would be split into separate
arguments. They must be quoted. This also affected installations where
$prefix contained a space, as in "C:\Program Files\GIT". Both errors can
be triggered by invoking

     git hash-object "a b"

where "a b" is an existing file.
---

Here is the proper fix.

Yes, this leaks memory on the error path. We can clean that up later...

-- Hannes

  compat/mingw.c |    7 ++++++-
  1 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/compat/mingw.c b/compat/mingw.c
index 8bb0dba..2554f19 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -405,7 +405,12 @@ void mingw_execve()
  {
  	/* check if git_command is a shell script */
  	if (!try_shell_exec(cmd, argv, env)) {
-		int ret = spawnve(_P_WAIT, cmd, argv, env);
+		const char **qargv;
+		int n;
+		for (n = 0; argv[n];) n++;
+		qargv = xmalloc((n+1)*sizeof(char*));
+		quote_argv(qargv, argv);
+		int ret = spawnve(_P_WAIT, cmd, qargv, env);
  		if (ret != -1)
  			exit(ret);
  	}
-- 
1.5.3.717.g01fc-dirty


-

Previous thread: [StGit PATCH 0/6] Survive capital punishment by Karl on Monday, October 8, 2007 - 1:55 am. (12 messages)

Next thread: Re: git: avoiding merges, rebasing by Benoit SIGOURE on Monday, October 8, 2007 - 6:16 am. (1 message)