From: Luke Diamand <luke@diamand.org>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org, Jonathan Nieder <jrnieder@gmail.com>,
Chris Lasell <chrisl@pixar.com>
Subject: Re: [PATCH 2/2] git-p4: fix handling of multi-word P4EDITOR
Date: Sun, 24 May 2015 10:28:02 +0100 [thread overview]
Message-ID: <556199A2.8090802@diamand.org> (raw)
In-Reply-To: <xmqqtwvom3t4.fsf@gitster.dls.corp.google.com>
On 07/05/15 23:16, Junio C Hamano wrote:
> Luke Diamand <luke@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
next prev parent reply other threads:[~2015-05-24 9:29 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-05-07 17:24 [PATCH 0/2] Re: Bug: git-p4 edit_template() and P4EDITOR w/options Luke Diamand
2015-05-07 17:25 ` [PATCH 1/2] git-p4: add failing test for P4EDITOR handling Luke Diamand
2015-05-07 17:25 ` [PATCH 2/2] git-p4: fix handling of multi-word P4EDITOR Luke Diamand
2015-05-07 18:11 ` Junio C Hamano
2015-05-07 19:27 ` Luke Diamand
2015-05-07 20:16 ` Junio C Hamano
2015-05-07 20:42 ` Luke Diamand
2015-05-07 21:06 ` Junio C Hamano
2015-05-07 21:16 ` Junio C Hamano
2015-05-07 22:04 ` Luke Diamand
2015-05-07 22:16 ` Junio C Hamano
2015-05-24 9:28 ` Luke Diamand [this message]
2015-05-26 20:21 ` Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=556199A2.8090802@diamand.org \
--to=luke@diamand.org \
--cc=chrisl@pixar.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=jrnieder@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).