Re: [PATCH 2/2] git-p4: fix handling of multi-word P4EDITOR

2015-05-26 Thread Junio C Hamano
Luke Diamand l...@diamand.org writes:

 On 07/05/15 23:16, Junio C Hamano wrote:
 Luke Diamand l...@diamand.org writes:


 [Resurrecting old thread]
 ...

 To me, that seems to imply that for GIT_WINDOWS_NATIVE, we take the
 *second* branch and use sh, so again, the the code as it stands will
 be fine. msysgit uses that path.
 ...

 I don't think we need to do anything. msysgit works fine with the
 origin sh, -c, ... code.

Thanks.  Let's merge what is in 'pu' down to 'next' then.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/2] git-p4: fix handling of multi-word P4EDITOR

2015-05-24 Thread Luke Diamand

On 07/05/15 23:16, Junio C Hamano wrote:

Luke Diamand l...@diamand.org writes:



[Resurrecting old thread]



Looking at run-command.c, GIT_WINDOES_NATIVE and POSIX seems to use
pretty much the same construct, except that they use SHELL_PATH
instead of sh.


I think the state of git on Windows is a bit shaky (I'm happy to be 
proved wrong of course), but I think the only seriously active port is 
the msys one.


That, as far as I can tell, uses an msys version of 'sh', so it will be 
perfectly happy with the sh -c ... construct.


There may be a native windows port in existence, but I can't find how to 
build this, and I assume it's going to need Visual Studio, which makes 
it a lot more complex to get going.


The code you were looking at in run-command.c says this:

#ifndef GIT_WINDOWS_NATIVE
nargv[nargc++] = SHELL_PATH;   !GIT_WINDOWS_NATIVE
#else
nargv[nargc++] = sh; GIT_WINDOWS_NATIVE
#endif
nargv[nargc++] = -c;

To me, that seems to imply that for GIT_WINDOWS_NATIVE, we take the 
*second* branch and use sh, so again, the the code as it stands will 
be fine. msysgit uses that path.


(The next line, trying to use -c has no chance of working if Cmd is 
being used).





So something like this may be sufficient, perhaps?

  Makefile  | 1 +
  git-p4.py | 2 +-
  2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index 20058f1..fda44bf 100644
--- a/Makefile
+++ b/Makefile
@@ -1776,6 +1776,7 @@ $(SCRIPT_PYTHON_GEN): GIT-CFLAGS GIT-PREFIX 
GIT-PYTHON-VARS
  $(SCRIPT_PYTHON_GEN): % : %.py
$(QUIET_GEN)$(RM) $@ $@+  \
sed -e '1s|#!.*python|#!$(PYTHON_PATH_SQ)|' \
+   -e 's|SHELL_PATH|$(SHELL_PATH_SQ)|g' \
$ $@+  \
chmod +x $@+  \
mv $@+ $@
diff --git a/git-p4.py b/git-p4.py
index de06046..eb6d4b1 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -1220,7 +1220,7 @@ class P4Submit(Command, P4UserMap):
  editor = os.environ.get(P4EDITOR)
  else:
  editor = read_pipe(git var GIT_EDITOR).strip()
-system([sh, -c, ('%s $@' % editor), editor, template_file])
+system(['''SHELL_PATH''', -c, ('%s $@' % editor), editor, 
template_file])


This seems to be expanded to '''sh''' which doesn't then work at all. I 
didn't take the time to investigate further though.




  # If the file was not saved, prompt to see if this patch should
  # be skipped.  But skip this verification step if configured so.


I don't think we need to do anything. msysgit works fine with the origin 
sh, -c, ... code.


Thanks!
Luke
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html