* [PATCH v3] builtin-push: add --delete as syntactic sugar for :foo
From: Jan Krüger @ 2009-12-30 9:52 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Nanako Shiraishi, Git ML, Sverre Rabbelier
In-Reply-To: <7vvdfpg1je.fsf@alter.siamese.dyndns.org>
Refspecs without a source side have been reported as confusing by many.
As an alternative, this adds support for commands like:
git push origin --delete somebranch
git push origin --delete tag sometag
Specifically, --delete will prepend a colon to all colon-less refspecs
given on the command line, and will refuse to accept refspecs with
colons to prevent undue confusion.
Signed-off-by: Jan Krüger <jk@jk.gs>
---
Junio C Hamano <gitster@pobox.com> wrote:
> namely, (1) barf and abort if src:dst is given; (2) touch only refs
> given from the command line, "push there --delete" without any
> refspec is an error; (3) be careful about "git push there tag v1.0.0"
> form.
>
> So if Jan or Sverre want to resurrect the topic, I am all for it.
Alrighty. I assume by (3) you meant that it should be possible to use
something like "push there --delete tag v1.0.0". Version 3 of the patch
adds this, and it also includes updated tests and (brief) documentation.
Documentation/git-push.txt | 4 ++++
builtin-push.c | 26 +++++++++++++++++++++++---
t/t5516-fetch-push.sh | 16 ++++++++++++++++
3 files changed, 43 insertions(+), 3 deletions(-)
diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
index 52c0538..e3eb1e8 100644
--- a/Documentation/git-push.txt
+++ b/Documentation/git-push.txt
@@ -91,6 +91,10 @@ nor in any Push line of the corresponding remotes file---see below).
will be tab-separated and sent to stdout instead of stderr. The full
symbolic names of the refs will be given.
+--delete::
+ All listed refs are deleted from the remote repository. This is
+ the same as prefixing all refs with a colon.
+
--tags::
All refs under `$GIT_DIR/refs/tags` are pushed, in
addition to refspecs explicitly listed on the command
diff --git a/builtin-push.c b/builtin-push.c
index dcfb53f..f7661d2 100644
--- a/builtin-push.c
+++ b/builtin-push.c
@@ -15,6 +15,7 @@ static const char * const push_usage[] = {
};
static int thin;
+static int deleterefs;
static const char *receivepack;
static const char **refspec;
@@ -39,11 +40,24 @@ static void set_refspecs(const char **refs, int nr)
if (nr <= ++i)
die("tag shorthand without <tag>");
len = strlen(refs[i]) + 11;
- tag = xmalloc(len);
- strcpy(tag, "refs/tags/");
+ if (deleterefs) {
+ tag = xmalloc(len+1);
+ strcpy(tag, ":refs/tags/");
+ } else {
+ tag = xmalloc(len);
+ strcpy(tag, "refs/tags/");
+ }
strcat(tag, refs[i]);
ref = tag;
- }
+ } else if (deleterefs && !strchr(ref, ':')) {
+ char *delref;
+ int len = strlen(ref)+1;
+ delref = xmalloc(len);
+ strcpy(delref, ":");
+ strcat(delref, ref);
+ ref = delref;
+ } else if (deleterefs)
+ die("--delete only accepts plain target ref names");
add_refspec(ref);
}
}
@@ -196,6 +210,7 @@ int cmd_push(int argc, const char **argv, const char *prefix)
OPT_BIT( 0 , "all", &flags, "push all refs", TRANSPORT_PUSH_ALL),
OPT_BIT( 0 , "mirror", &flags, "mirror all refs",
(TRANSPORT_PUSH_MIRROR|TRANSPORT_PUSH_FORCE)),
+ OPT_BOOLEAN( 0, "delete", &deleterefs, "delete refs"),
OPT_BOOLEAN( 0 , "tags", &tags, "push tags (can't be used with --all or --mirror)"),
OPT_BIT('n' , "dry-run", &flags, "dry run", TRANSPORT_PUSH_DRY_RUN),
OPT_BIT( 0, "porcelain", &flags, "machine-readable output", TRANSPORT_PUSH_PORCELAIN),
@@ -209,6 +224,11 @@ int cmd_push(int argc, const char **argv, const char *prefix)
git_config(git_default_config, NULL);
argc = parse_options(argc, argv, prefix, options, push_usage, 0);
+ if (deleterefs && (tags || (flags & (TRANSPORT_PUSH_ALL | TRANSPORT_PUSH_MIRROR))))
+ die("--delete is incompatible with --all, --mirror and --tags");
+ if (deleterefs && argc < 2)
+ die("--delete doesn't make sense without any refs");
+
if (tags)
add_refspec("refs/tags/*");
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index 516127b..a17666c 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -547,6 +547,22 @@ test_expect_success 'allow deleting an invalid remote ref' '
'
+test_expect_success 'allow deleting a ref using --delete' '
+ mk_test heads/master &&
+ (cd testrepo && git config receive.denyDeleteCurrent warn) &&
+ git push testrepo --delete master &&
+ (cd testrepo && test_must_fail git rev-parse --verify refs/heads/master)
+'
+
+test_expect_success 'allow deleting a tag using --delete' '
+ mk_test heads/master &&
+ git tag -a -m dummy_message deltag heads/master &&
+ git push testrepo --tags &&
+ (cd testrepo && git rev-parse --verify -q refs/tags/deltag) &&
+ git push testrepo --delete tag deltag &&
+ (cd testrepo && test_must_fail git rev-parse --verify refs/tags/deltag)
+'
+
test_expect_success 'warn on push to HEAD of non-bare repository' '
mk_test heads/master
(cd testrepo &&
--
1.6.6.60.gc2ff1
^ permalink raw reply related
* Re: Giving command line parameter to textconv command?
From: Jeff King @ 2009-12-30 9:56 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Nanako Shiraishi, git
In-Reply-To: <7vr5qc51kv.fsf@alter.siamese.dyndns.org>
On Wed, Dec 30, 2009 at 12:05:20AM -0800, Junio C Hamano wrote:
> I stopped promoting the patch after Peff mentioned he was planning a
> rework of textconv, but now I re-read it, I think his rework would be
> orthogonal to the patch.
Actually, I thought I volunteered to produce a patch series converting
textconv and diff.
> Also Peff gives a good hint about borrowing how launch_editor() calls out
> to the shell. I think the codepath we fork&exec user-defined commands
> (not hooks, but filters like smudge/clean/textconv and EDITOR/PAGER that
> take a command line) need to be first enumerated, then we need to see if
> we can refactor what launch_editor() does into a common helper function.
I think the helper function is not too hard. Whereas the patch you
posted earlier actually runs { "sh", "-c", "%s %s" } for textconv, it is
a bit simpler to do { "sh", "-c", "%s \"$@\"", ... } as launch_editor
does. And then we don't have to worry about quoting the arguments (which
your patch gets wrong).
> I felt it was unclear what we would want to do with GIT_EXTERNAL_DIFF,
> diff.external, and diff.<driver>.command trio, but I tend to agree that we
> should run things the same way, honoring $IFS and shell everywhere.
I thought it was left at "maybe for 1.7.0", but perhaps we don't have
time now to issue sufficient warning.
Anyway, I'm working on a series which will hopefully be done in a few
minutes.
-Peff
^ permalink raw reply
* Re: signing tags: user.name assumed to be gpg uid?
From: Peter Vereshagin @ 2009-12-30 10:47 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vaax1bi3e.fsf@alter.siamese.dyndns.org>
Oh Junio want you buy me a mersedes benz?
2009/12/29 13:12:05 -0800 Junio C Hamano <gitster@pobox.com> => To Peter Vereshagin :
JCH> > I notice that after I init and config user.name and user.email the
JCH> > user.comment gets taken from ~/.gpg automatically.
JCH> What is "user.comment"?
My probable suggestion about where the gpg key's comment field should be written for config --global. Sorry, it's not from ~/.gnupg
JCH> > Is it a correct behaviour?
JCH> Yes, matching with user.name/user.email (actually committer information)
JCH> is often the most convenient thing to do.
Is it necessary the pgp comment field to be present on the user.name? It isn't for From: in e-mail messages.
JCH> You can use user.signingkey configuration to use a key that doesn't match
JCH> your name, too.
Thanks!
73! Peter pgp: A0E26627 (4A42 6841 2871 5EA7 52AB 12F8 0CE1 4AAC A0E2 6627)
--
http://vereshagin.org
^ permalink raw reply
* [Updated PATCH 0/2] Improve remote helpers exec error reporting
From: Ilari Liusvaara @ 2009-12-30 10:52 UTC (permalink / raw)
To: git
This reroll fixes the following from previous round:
- Split loop-trying-to-close to its own inline function.
- Don't rely on pipe(2) preserving fd array in case of failure.
- Don't try to use partially received error codes.
- Don't send error about partial write as it would go to who knows where.
- Add a testcase (ENOENT is detected correctly).
Ilari Liusvaara (2):
Report exec errors from run-command
Improve transport helper exec failure reporting
Makefile | 1 +
run-command.c | 79 ++++++++++++++++++++++++++++++++++++++++++++++--
t/t0061-run-command.sh | 13 ++++++++
test-run-command.c | 35 +++++++++++++++++++++
transport-helper.c | 14 ++++++--
5 files changed, 135 insertions(+), 7 deletions(-)
create mode 100755 t/t0061-run-command.sh
create mode 100644 test-run-command.c
^ permalink raw reply
* [Updated PATCH 1/2] Report exec errors from run-command
From: Ilari Liusvaara @ 2009-12-30 10:52 UTC (permalink / raw)
To: git
In-Reply-To: <1262170338-11574-1-git-send-email-ilari.liusvaara@elisanet.fi>
Previously run-command was unable to report errors happening in exec
call. Change it to pass errno from failed exec to errno value at
return.
The errno value passing can be done by opening close-on-exec pipe and
piping the error code through in case of failure. In case of success,
close-on-exec closes the pipe on successful exec and parent process
gets end of file on read.
Signed-off-by: Ilari Liusvaara <ilari.liusvaara@elisanet.fi>
---
Makefile | 1 +
run-command.c | 79 ++++++++++++++++++++++++++++++++++++++++++++++--
t/t0061-run-command.sh | 13 ++++++++
test-run-command.c | 35 +++++++++++++++++++++
4 files changed, 125 insertions(+), 3 deletions(-)
create mode 100755 t/t0061-run-command.sh
create mode 100644 test-run-command.c
diff --git a/Makefile b/Makefile
index 4a1e5bc..452ad50 100644
--- a/Makefile
+++ b/Makefile
@@ -1725,6 +1725,7 @@ TEST_PROGRAMS += test-parse-options$X
TEST_PROGRAMS += test-path-utils$X
TEST_PROGRAMS += test-sha1$X
TEST_PROGRAMS += test-sigchain$X
+TEST_PROGRAMS += test-run-command$X
all:: $(TEST_PROGRAMS)
diff --git a/run-command.c b/run-command.c
index cf2d8f7..34e5af4 100644
--- a/run-command.c
+++ b/run-command.c
@@ -15,6 +15,12 @@ static inline void dup_devnull(int to)
close(fd);
}
+static inline void force_close(int fd)
+{
+ while (close(fd) < 0 && errno != EBADF)
+ ; /* No-op */
+}
+
int start_command(struct child_process *cmd)
{
int need_in, need_out, need_err;
@@ -76,9 +82,64 @@ fail_pipe:
trace_argv_printf(cmd->argv, "trace: run_command:");
#ifndef WIN32
+{
+ int report_pipe[2] = {-1, -1};
+
+ if (pipe(report_pipe) < 0) {
+ report_pipe[0] = -1;
+ report_pipe[1] = -1;
+ warning("Can't open pipe for exec status report: %s\n",
+ strerror(errno));
+ }
+
fflush(NULL);
cmd->pid = fork();
- if (!cmd->pid) {
+ if (cmd->pid > 0) {
+ int r = 0, ret;
+ force_close(report_pipe[1]);
+read_again:
+ if (report_pipe[0] > 0)
+ r = read(report_pipe[0], &ret, sizeof(ret));
+ if (r < 0 && (errno == EAGAIN || errno == EINTR ||
+ errno == EWOULDBLOCK))
+ goto read_again;
+ else if (r < 0)
+ warning("Can't read exec status report: %s\n",
+ strerror(errno));
+ else if (r == 0)
+ ;
+ else if (r < sizeof(ret)) {
+ warning("Received incomplete exec status report.\n");
+ errno = EBADMSG;
+ } else {
+ failed_errno = ret;
+ /*
+ * Clean up the process that did the failed execution
+ * so no zombies remain.
+ */
+wait_again:
+ r = waitpid(cmd->pid, &ret, 0);
+ if (r < 0 && errno != ECHILD)
+ goto wait_again;
+ cmd->pid = -1;
+ }
+ } else if (!cmd->pid) {
+ int r = 0;
+ int flags;
+ force_close(report_pipe[0]);
+
+ flags = fcntl(report_pipe[1], F_GETFD);
+ if (flags < 0)
+ r = -1;
+ flags |= FD_CLOEXEC;
+ r = (r || fcntl(report_pipe[1], F_SETFD, flags));
+ if (r) {
+ warning("Can't FD_CLOEXEC pipe for exec status "
+ "report: %s\n", strerror(errno));
+ force_close(report_pipe[1]);
+ report_pipe[1] = -1;
+ }
+
if (cmd->no_stdin)
dup_devnull(0);
else if (need_in) {
@@ -126,13 +187,25 @@ fail_pipe:
} else {
execvp(cmd->argv[0], (char *const*) cmd->argv);
}
+ failed_errno = errno;
+
trace_printf("trace: exec '%s' failed: %s\n", cmd->argv[0],
strerror(errno));
+
+ r = 0;
+write_again:
+ if (report_pipe[1] >= 0)
+ r = write(report_pipe[1], &failed_errno,
+ sizeof(failed_errno));
+ if (r < 0 && (errno == EAGAIN || errno == EINTR ||
+ errno == EWOULDBLOCK))
+ goto write_again;
+
exit(127);
- }
- if (cmd->pid < 0)
+ } else if (cmd->pid < 0)
error("cannot fork() for %s: %s", cmd->argv[0],
strerror(failed_errno = errno));
+}
#else
{
int s0 = -1, s1 = -1, s2 = -1; /* backups of stdin, stdout, stderr */
diff --git a/t/t0061-run-command.sh b/t/t0061-run-command.sh
new file mode 100755
index 0000000..1d9e82a
--- /dev/null
+++ b/t/t0061-run-command.sh
@@ -0,0 +1,13 @@
+#!/bin/sh
+#
+# Copyright (c) 2009 Ilari Liusvaara
+#
+
+test_description='Test run command'
+
+. ./test-lib.sh
+
+test_expect_success "reporting ENOENT" \
+"test-run-command 1"
+
+test_done
diff --git a/test-run-command.c b/test-run-command.c
new file mode 100644
index 0000000..6297785
--- /dev/null
+++ b/test-run-command.c
@@ -0,0 +1,35 @@
+/*
+ * test-run-command.c: test run command API.
+ *
+ * (C) 2009 Ilari Liusvaara <ilari.liusvaara@elisanet.fi>
+ *
+ * This code is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include "git-compat-util.h"
+#include "run-command.h"
+#include <string.h>
+#include <errno.h>
+
+int main(int argc, char **argv)
+{
+ char* procs[2];
+ struct child_process proc;
+ memset(&proc, 0, sizeof(proc));
+
+ if(argc < 2)
+ return 1;
+
+ if (argv[1][1] == '1')
+ procs[0] = "does-not-exist-62896869286";
+ procs[1] = NULL;
+ proc.argv = (const char **)procs;
+
+ if (!run_command(&proc))
+ return 1;
+ if (errno != ENOENT)
+ return 1;
+ return 0;
+}
--
1.6.6.3.gaa2e1
^ permalink raw reply related
* [Updated PATCH 2/2] Improve transport helper exec failure reporting
From: Ilari Liusvaara @ 2009-12-30 10:52 UTC (permalink / raw)
To: git
In-Reply-To: <1262170338-11574-1-git-send-email-ilari.liusvaara@elisanet.fi>
Previously transport-helper exec failure error reporting was pretty
much useless as it didn't report errors from execve, only from pipe
and fork. Now that run-command passes errno from exec, use the
improved support to actually print useful errors if execution fails.
Signed-off-by: Ilari Liusvaara <ilari.liusvaara@elisanet.fi>
---
transport-helper.c | 14 ++++++++++----
1 files changed, 10 insertions(+), 4 deletions(-)
diff --git a/transport-helper.c b/transport-helper.c
index 5078c71..0965c9b 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -31,13 +31,19 @@ static struct child_process *get_helper(struct transport *transport)
helper->out = -1;
helper->err = 0;
helper->argv = xcalloc(4, sizeof(*helper->argv));
- strbuf_addf(&buf, "remote-%s", data->name);
+ strbuf_addf(&buf, "git-remote-%s", data->name);
helper->argv[0] = strbuf_detach(&buf, NULL);
helper->argv[1] = transport->remote->name;
helper->argv[2] = transport->url;
- helper->git_cmd = 1;
- if (start_command(helper))
- die("Unable to run helper: git %s", helper->argv[0]);
+ helper->git_cmd = 0;
+ if (start_command(helper)) {
+ if (errno == ENOENT)
+ die("Unable to find remote helper for \"%s\"",
+ data->name);
+ else
+ die("Unable to run helper %s: %s", helper->argv[0],
+ strerror(errno));
+ }
data->helper = helper;
write_str_in_full(helper->in, "capabilities\n");
--
1.6.6.3.gaa2e1
^ permalink raw reply related
* [PATCH 1/6] run-command: add "use shell" option
From: Jeff King @ 2009-12-30 10:53 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Nanako Shiraishi, git
In-Reply-To: <20091230095634.GA16349@coredump.intra.peff.net>
Many callsites run "sh -c $CMD" to run $CMD. We can make it
a little simpler for them by factoring out the munging of
argv.
For simple cases with no arguments, this doesn't help much, but:
1. For cases with arguments, we save the caller from
having to build the appropriate shell snippet.
2. We can later optimize to avoid the shell when
there are no metacharacters in the program.
Signed-off-by: Jeff King <peff@peff.net>
---
I made the matching tweak to the Windows half of run-command, but I
don't actually have a box to test it on.
I modeled this after execv_git_cmd. Like that function, I try to release
the allocated argv on error. However, we do actually leak the strbuf
memory in one case. I'm not sure how much we care. On unix, this will
always happen in a forked process which will either exec or die. On
Windows, we seem to already be leaking the prepared argv for the git_cmd
case (and now we leak the shell_cmd case, too).
run-command.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
run-command.h | 2 ++
2 files changed, 47 insertions(+), 0 deletions(-)
diff --git a/run-command.c b/run-command.c
index cf2d8f7..dc65903 100644
--- a/run-command.c
+++ b/run-command.c
@@ -15,6 +15,46 @@ static inline void dup_devnull(int to)
close(fd);
}
+static const char **prepare_shell_cmd(const char **argv)
+{
+ int argc, nargc = 0;
+ const char **nargv;
+
+ for (argc = 0; argv[argc]; argc++)
+ ; /* just counting */
+ /* +1 for NULL, +3 for "sh -c" plus extra $0 */
+ nargv = xmalloc(sizeof(*nargv) * (argc + 1 + 3));
+
+ if (argc < 1)
+ die("BUG: shell command is empty");
+
+ nargv[nargc++] = "sh";
+ nargv[nargc++] = "-c";
+
+ if (argc < 2)
+ nargv[nargc++] = argv[0];
+ else {
+ struct strbuf arg0 = STRBUF_INIT;
+ strbuf_addf(&arg0, "%s \"$@\"", argv[0]);
+ nargv[nargc++] = strbuf_detach(&arg0, NULL);
+ }
+
+ for (argc = 0; argv[argc]; argc++)
+ nargv[nargc++] = argv[argc];
+ nargv[nargc] = NULL;
+
+ return nargv;
+}
+
+static int execv_shell_cmd(const char **argv)
+{
+ const char **nargv = prepare_shell_cmd(argv);
+ trace_argv_printf(nargv, "trace: exec:");
+ execvp(nargv[0], (char **)nargv);
+ free(nargv);
+ return -1;
+}
+
int start_command(struct child_process *cmd)
{
int need_in, need_out, need_err;
@@ -123,6 +163,8 @@ fail_pipe:
cmd->preexec_cb();
if (cmd->git_cmd) {
execv_git_cmd(cmd->argv);
+ } else if (cmd->use_shell) {
+ execv_shell_cmd(cmd->argv);
} else {
execvp(cmd->argv[0], (char *const*) cmd->argv);
}
@@ -179,6 +221,8 @@ fail_pipe:
if (cmd->git_cmd) {
cmd->argv = prepare_git_cmd(cmd->argv);
+ } else if (cmd->use_shell) {
+ cmd->argv = prepare_shell_cmd(cmd->argv);
}
cmd->pid = mingw_spawnvpe(cmd->argv[0], cmd->argv, env);
@@ -297,6 +341,7 @@ static void prepare_run_command_v_opt(struct child_process *cmd,
cmd->git_cmd = opt & RUN_GIT_CMD ? 1 : 0;
cmd->stdout_to_stderr = opt & RUN_COMMAND_STDOUT_TO_STDERR ? 1 : 0;
cmd->silent_exec_failure = opt & RUN_SILENT_EXEC_FAILURE ? 1 : 0;
+ cmd->use_shell = opt & RUN_USING_SHELL ? 1 : 0;
}
int run_command_v_opt(const char **argv, int opt)
diff --git a/run-command.h b/run-command.h
index fb34209..967ba8c 100644
--- a/run-command.h
+++ b/run-command.h
@@ -33,6 +33,7 @@ struct child_process {
unsigned git_cmd:1; /* if this is to be git sub-command */
unsigned silent_exec_failure:1;
unsigned stdout_to_stderr:1;
+ unsigned use_shell:1;
void (*preexec_cb)(void);
};
@@ -46,6 +47,7 @@ extern int run_hook(const char *index_file, const char *name, ...);
#define RUN_GIT_CMD 2 /*If this is to be git sub-command */
#define RUN_COMMAND_STDOUT_TO_STDERR 4
#define RUN_SILENT_EXEC_FAILURE 8
+#define RUN_USING_SHELL 16
int run_command_v_opt(const char **argv, int opt);
/*
--
1.6.6.65.g050d2.dirty
^ permalink raw reply related
* [PATCH 2/6] run-command: convert simple callsites to use_shell
From: Jeff King @ 2009-12-30 10:53 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Nanako Shiraishi, git
In-Reply-To: <20091230095634.GA16349@coredump.intra.peff.net>
Now that we have the use_shell feature, these callsites can
all be converted with small changes.
Signed-off-by: Jeff King <peff@peff.net>
---
These should be non-controversial.
convert.c | 3 ++-
imap-send.c | 8 ++------
ll-merge.c | 6 +++---
| 5 +++--
4 files changed, 10 insertions(+), 12 deletions(-)
diff --git a/convert.c b/convert.c
index 491e714..950b1f9 100644
--- a/convert.c
+++ b/convert.c
@@ -249,10 +249,11 @@ static int filter_buffer(int fd, void *data)
struct child_process child_process;
struct filter_params *params = (struct filter_params *)data;
int write_err, status;
- const char *argv[] = { "sh", "-c", params->cmd, NULL };
+ const char *argv[] = { params->cmd, NULL };
memset(&child_process, 0, sizeof(child_process));
child_process.argv = argv;
+ child_process.use_shell = 1;
child_process.in = -1;
child_process.out = fd;
diff --git a/imap-send.c b/imap-send.c
index de8114b..51f371b 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -965,17 +965,13 @@ static struct store *imap_open_store(struct imap_server_conf *srvc)
/* open connection to IMAP server */
if (srvc->tunnel) {
- const char *argv[4];
+ const char *argv[] = { srvc->tunnel, NULL };
struct child_process tunnel = {0};
imap_info("Starting tunnel '%s'... ", srvc->tunnel);
- argv[0] = "sh";
- argv[1] = "-c";
- argv[2] = srvc->tunnel;
- argv[3] = NULL;
-
tunnel.argv = argv;
+ tunnel.use_shell = 1;
tunnel.in = -1;
tunnel.out = -1;
if (start_command(&tunnel))
diff --git a/ll-merge.c b/ll-merge.c
index 2d6b6d6..18511e2 100644
--- a/ll-merge.c
+++ b/ll-merge.c
@@ -175,7 +175,7 @@ static int ll_ext_merge(const struct ll_merge_driver *fn,
{ "B", temp[2] },
{ NULL }
};
- const char *args[] = { "sh", "-c", NULL, NULL };
+ const char *args[] = { NULL, NULL };
int status, fd, i;
struct stat st;
@@ -190,8 +190,8 @@ static int ll_ext_merge(const struct ll_merge_driver *fn,
strbuf_expand(&cmd, fn->cmdline, strbuf_expand_dict_cb, &dict);
- args[2] = cmd.buf;
- status = run_command_v_opt(args, 0);
+ args[0] = cmd.buf;
+ status = run_command_v_opt(args, RUN_USING_SHELL);
fd = open(temp[1], O_RDONLY);
if (fd < 0)
goto bad;
--git a/pager.c b/pager.c
index 92c03f6..2c7e8ec 100644
--- a/pager.c
+++ b/pager.c
@@ -28,7 +28,7 @@ static void pager_preexec(void)
}
#endif
-static const char *pager_argv[] = { "sh", "-c", NULL, NULL };
+static const char *pager_argv[] = { NULL, NULL };
static struct child_process pager_process;
static void wait_for_pager(void)
@@ -81,7 +81,8 @@ void setup_pager(void)
spawned_pager = 1; /* means we are emitting to terminal */
/* spawn the pager */
- pager_argv[2] = pager;
+ pager_argv[0] = pager;
+ pager_process.use_shell = 1;
pager_process.argv = pager_argv;
pager_process.in = -1;
if (!getenv("LESS")) {
--
1.6.6.65.g050d2.dirty
^ permalink raw reply related
* [PATCH 3/6] run-command: optimize out useless shell calls
From: Jeff King @ 2009-12-30 10:55 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Nanako Shiraishi, git
In-Reply-To: <20091230095634.GA16349@coredump.intra.peff.net>
If there are no metacharacters in the program to be run, we
can just skip running the shell entirely and directly exec
the program.
The metacharacter test is pulled verbatim from
launch_editor, which already implements this optimization.
Signed-off-by: Jeff King <peff@peff.net>
---
Something inside me feels wrong with a catch-known-metacharacter test
instead of an allow-known-good check. But this is the same test we have
been using with launch_editor for some time, so I decided not to mess
with it.
run-command.c | 18 ++++++++++--------
1 files changed, 10 insertions(+), 8 deletions(-)
diff --git a/run-command.c b/run-command.c
index dc65903..22e2777 100644
--- a/run-command.c
+++ b/run-command.c
@@ -28,15 +28,17 @@ static const char **prepare_shell_cmd(const char **argv)
if (argc < 1)
die("BUG: shell command is empty");
- nargv[nargc++] = "sh";
- nargv[nargc++] = "-c";
-
- if (argc < 2)
- nargv[nargc++] = argv[0];
- else {
- struct strbuf arg0 = STRBUF_INIT;
- strbuf_addf(&arg0, "%s \"$@\"", argv[0]);
- nargv[nargc++] = strbuf_detach(&arg0, NULL);
+ if (strcspn(argv[0], "|&;<>()$`\\\"' \t\n*?[#~=%") != strlen(argv[0])) {
+ nargv[nargc++] = "sh";
+ nargv[nargc++] = "-c";
+
+ if (argc < 2)
+ nargv[nargc++] = argv[0];
+ else {
+ struct strbuf arg0 = STRBUF_INIT;
+ strbuf_addf(&arg0, "%s \"$@\"", argv[0]);
+ nargv[nargc++] = strbuf_detach(&arg0, NULL);
+ }
}
for (argc = 0; argv[argc]; argc++)
--
1.6.6.65.g050d2.dirty
^ permalink raw reply related
* [PATCH 4/6] editor: use run_command's shell feature
From: Jeff King @ 2009-12-30 10:56 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Nanako Shiraishi, git
In-Reply-To: <20091230095634.GA16349@coredump.intra.peff.net>
Now that run_command implements the same code in a more
general form, we can make use of it.
Signed-off-by: Jeff King <peff@peff.net>
---
Should also be non-controversial, and the diffstat shows that this is
the payoff for all of the added code earlier in the series. :)
editor.c | 21 ++-------------------
1 files changed, 2 insertions(+), 19 deletions(-)
diff --git a/editor.c b/editor.c
index 615f575..d834003 100644
--- a/editor.c
+++ b/editor.c
@@ -36,26 +36,9 @@ int launch_editor(const char *path, struct strbuf *buffer, const char *const *en
return error("Terminal is dumb, but EDITOR unset");
if (strcmp(editor, ":")) {
- size_t len = strlen(editor);
- int i = 0;
- int failed;
- const char *args[6];
- struct strbuf arg0 = STRBUF_INIT;
+ const char *args[] = { editor, path, NULL };
- if (strcspn(editor, "|&;<>()$`\\\"' \t\n*?[#~=%") != len) {
- /* there are specials */
- strbuf_addf(&arg0, "%s \"$@\"", editor);
- args[i++] = "sh";
- args[i++] = "-c";
- args[i++] = arg0.buf;
- }
- args[i++] = editor;
- args[i++] = path;
- args[i] = NULL;
-
- failed = run_command_v_opt_cd_env(args, 0, NULL, env);
- strbuf_release(&arg0);
- if (failed)
+ if (run_command_v_opt_cd_env(args, RUN_USING_SHELL, NULL, env))
return error("There was a problem with the editor '%s'.",
editor);
}
--
1.6.6.65.g050d2.dirty
^ permalink raw reply related
* [PATCH 5/6] textconv: use shell to run helper
From: Jeff King @ 2009-12-30 11:01 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Nanako Shiraishi, git
In-Reply-To: <20091230095634.GA16349@coredump.intra.peff.net>
Currently textconv helpers are run directly. Running through
the shell is useful because the user can provide a program
with command line arguments, like "antiword -f".
It also makes textconv more consistent with other parts of
git, most of which run their helpers using the shell.
The downside is that textconv helpers with shell
metacharacters (like space) in the filename will be broken.
Signed-off-by: Jeff King <peff@peff.net>
---
This is the first actual change in behavior. This patch and 6/6 should
perhaps be held back with a warning period. I am inclined to consider it
a bug that they didn't use the shell, and this is fixing it.
diff.c | 1 +
t/t4030-diff-textconv.sh | 2 +-
t/t4031-diff-rewrite-binary.sh | 2 +-
3 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/diff.c b/diff.c
index d14a575..2794238 100644
--- a/diff.c
+++ b/diff.c
@@ -3818,6 +3818,7 @@ static char *run_textconv(const char *pgm, struct diff_filespec *spec,
*arg = NULL;
memset(&child, 0, sizeof(child));
+ child.use_shell = 1;
child.argv = argv;
child.out = -1;
if (start_command(&child) != 0 ||
diff --git a/t/t4030-diff-textconv.sh b/t/t4030-diff-textconv.sh
index a3f0897..c16d538 100755
--- a/t/t4030-diff-textconv.sh
+++ b/t/t4030-diff-textconv.sh
@@ -48,7 +48,7 @@ test_expect_success 'file is considered binary by plumbing' '
test_expect_success 'setup textconv filters' '
echo file diff=foo >.gitattributes &&
- git config diff.foo.textconv "$PWD"/hexdump &&
+ git config diff.foo.textconv "\"$PWD\""/hexdump &&
git config diff.fail.textconv false
'
diff --git a/t/t4031-diff-rewrite-binary.sh b/t/t4031-diff-rewrite-binary.sh
index a894c60..27fb31b 100755
--- a/t/t4031-diff-rewrite-binary.sh
+++ b/t/t4031-diff-rewrite-binary.sh
@@ -54,7 +54,7 @@ chmod +x dump
test_expect_success 'setup textconv' '
echo file diff=foo >.gitattributes &&
- git config diff.foo.textconv "$PWD"/dump
+ git config diff.foo.textconv "\"$PWD\""/dump
'
test_expect_success 'rewrite diff respects textconv' '
--
1.6.6.65.g050d2.dirty
^ permalink raw reply related
* [PATCH 6/6] diff: run external diff helper with shell
From: Jeff King @ 2009-12-30 11:03 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Nanako Shiraishi, git
In-Reply-To: <20091230095634.GA16349@coredump.intra.peff.net>
This is mostly to make it more consistent with the rest of
git, which uses the shell to exec helpers.
Signed-off-by: Jeff King <peff@peff.net>
---
Consistency is the main advantage here. Though I think you can actually
do craziness like setting
GIT_EXTERNAL_DIFF='my-command $2 $3 && true' git diff
to pick out some subset of the parameters, I can't imagine it is
actually a good idea.
diff.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/diff.c b/diff.c
index 2794238..9bb40bc 100644
--- a/diff.c
+++ b/diff.c
@@ -2294,7 +2294,7 @@ static void run_external_diff(const char *pgm,
}
*arg = NULL;
fflush(NULL);
- retval = run_command_v_opt(spawn_arg, 0);
+ retval = run_command_v_opt(spawn_arg, RUN_USING_SHELL);
remove_tempfile();
if (retval) {
fprintf(stderr, "external diff died, stopping at %s.\n", name);
--
1.6.6.65.g050d2.dirty
^ permalink raw reply related
* Re: Feature suggestion: support arguments for GIT_PROXY_COMMAND & core.gitproxy
From: Jeff King @ 2009-12-30 11:05 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Nanako Shiraishi, Joey Korkames, git
In-Reply-To: <7vmy1051ik.fsf@alter.siamese.dyndns.org>
On Wed, Dec 30, 2009 at 12:06:43AM -0800, Junio C Hamano wrote:
> Nanako Shiraishi <nanako3@lavabit.com> writes:
>
> > Junio, could you tell us what happened to this thread?
> >
> > I think this is probably related to the "textconv" change.
>
> Yeah, you guessed right. I think the unified way to launch user specified
> programs would come first and then spawning proxy the same way as others
> would become trivial.
As a "7/6" on top of my other series, this should just be:
diff --git a/connect.c b/connect.c
index db965c9..838146c 100644
--- a/connect.c
+++ b/connect.c
@@ -432,6 +432,7 @@ static void git_proxy_connect(int fd[2], char *host)
argv[2] = port;
argv[3] = NULL;
memset(&proxy, 0, sizeof(proxy));
+ proxy.use_shell = 1;
proxy.argv = argv;
proxy.in = -1;
proxy.out = -1;
But I will leave it to somebody who actually uses the proxy feature to
test and submit a patch.
-Peff
^ permalink raw reply related
* Re: [PATCH 1/2] MinGW: Use pid_t more consequently, introduce uid_t for greater compatibility
From: Johannes Schindelin @ 2009-12-30 11:11 UTC (permalink / raw)
To: Sebastian Schuberth; +Cc: kusmabite, Johannes Sixt, git
In-Reply-To: <bdca99240912291716g74c35f22r7599f2c254fb1b09@mail.gmail.com>
Hi,
On Wed, 30 Dec 2009, Sebastian Schuberth wrote:
> Anyway, IMHO the correct declaration of e.g. getuid() is not "int
> getuid()", but "uid_t getuid()" etc. So even if the uid_t type was not
> required, it's a good change I think.
FWIW I concur with this reasoning. Even if in this particular case, we
could work around the issue, it is basically a non-intrusive, minimal
janitorial patch. Especially since the original author of the wrong
signature concurs.
Ciao,
Dscho
^ permalink raw reply
* Re: Minor bug in bash completion
From: SZEDER Gábor @ 2009-12-30 11:22 UTC (permalink / raw)
To: Sylvain RABOT; +Cc: spearce, git
In-Reply-To: <4B3A140A.60302@steek.com>
Hi Sylvain,
On Tue, Dec 29, 2009 at 03:36:58PM +0100, Sylvain RABOT wrote:
> I found a bug in the git bash completion.
> It occurs when I press tab to complete branch name when I want to pull
> from the origin.
> Instead of completing the branch name it prompts me directly for my
> password on the origin remote.
I don't think it's a bug. The completion script should offer the
currently available refs in the remote repository after a 'git pull
<remote> <TAB>'. In order to do that it contacts the remote
repository for the list of refs available there. Depending on the
access method, it might need to authenticate, in your case via ssh.
To silence the password prompts you should change your ssh
configuration to use key-based authentication when logging in to the
remote repository's server (just google for 'ssh login without
password').
Best,
Gábor
^ permalink raw reply
* Re: [PATCH v3] builtin-push: add --delete as syntactic sugar for :foo
From: Sverre Rabbelier @ 2009-12-30 12:14 UTC (permalink / raw)
To: Jan Krüger; +Cc: Junio C Hamano, Nanako Shiraishi, Git ML
In-Reply-To: <20091230105244.67f5969e@perceptron>
Heya,
On Wed, Dec 30, 2009 at 03:52, Jan Krüger <jk@jk.gs> wrote:
>> So if Jan or Sverre want to resurrect the topic, I am all for it.
>
> Alrighty. I assume by (3) you meant that it should be possible to use
> something like "push there --delete tag v1.0.0". Version 3 of the patch
> adds this, and it also includes updated tests and (brief) documentation.
Thanks for picking this up.
--
Cheers,
Sverre Rabbelier
^ permalink raw reply
* Re: [PATCH/resend] CVS Server: Support reading base and roots from environment
From: Nanako Shiraishi @ 2009-12-30 13:41 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git Mailing List, Phil Miller
In-Reply-To: <7vocmwvmvr.fsf@alter.siamese.dyndns.org>
Junio, could you tell us what happened to this thread?
Phil Miller's patch to help gitosis installation. No response from
git-cvsserver users.
^ permalink raw reply
* Re: [PATCH] t9700-perl-git.sh: Fix a test failure on Cygwin
From: Nanako Shiraishi @ 2009-12-30 13:40 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Ramsay Jones, GIT Mailing-list
In-Reply-To: <4B059150.5050303@ramsay1.demon.co.uk>
Junio, could you tell us what happened to this thread?
^ permalink raw reply
* Re: [PATCH v2] cvsserver: make the output of 'update' more compatible with cvs.
From: Nanako Shiraishi @ 2009-12-30 13:41 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Sergei Organov, git
In-Reply-To: <87bpibdonj.fsf@osv.gnss.ru>
Junio, could you tell us what happened to this thread?
^ permalink raw reply
* Re: [PATCH] git-update-index: report(...) now flushes stdout after printing the report line
From: Nanako Shiraishi @ 2009-12-30 13:41 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Sebastian Thiel, git
In-Reply-To: <loom.20091119T221732-624@post.gmane.org>
Junio, could you tell us what happened to this thread?
^ permalink raw reply
* Re: [PATCH v2] Let core.excludesfile default to ~/.gitexcludes.
From: Nanako Shiraishi @ 2009-12-30 13:41 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Matthieu Moy, Matthieu Moy, git
In-Reply-To: <1258840832-22130-1-git-send-email-Matthieu.Moy@imag.fr>
Junio, could you tell us what happened to this thread?
^ permalink raw reply
* Re: [PATCH] bash completion: add space between branch name and status flags
From: Nanako Shiraishi @ 2009-12-30 13:41 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Shawn O. Pearce, git, Roman Fietze
In-Reply-To: <7v4oopxeuf.fsf@alter.siamese.dyndns.org>
Junio, could you tell us what happened to this thread?
In response to a patch from Roman Fietze to outline a better way
to do it. Nothing happened.
^ permalink raw reply
* Re: [Updated PATCH 1/2] Report exec errors from run-command
From: Erik Faye-Lund @ 2009-12-30 13:47 UTC (permalink / raw)
To: Ilari Liusvaara; +Cc: git
In-Reply-To: <1262170338-11574-2-git-send-email-ilari.liusvaara@elisanet.fi>
On Wed, Dec 30, 2009 at 11:52 AM, Ilari Liusvaara
<ilari.liusvaara@elisanet.fi> wrote:
> +static inline void force_close(int fd)
> +{
> + while (close(fd) < 0 && errno != EBADF)
> + ; /* No-op */
> +}
> +
According to http://linux.die.net/man/2/close, close can set errno to
EBADF, EINTR, or EIO. Currently, you're retrying on EINTR and EIO.
When we get EIO, are you sure it makes sense to retry? I'd imagine
that error would most likely just repeat itself, leading to an
infinite loop. How about "while (close(fd) < 0 && errno == EINTR)"
instead? I've seen other functions (like xread in wrapper.c) only
retry on those errors that it expects. In xreads case, it's not
retrying on EIO.
Perhaps it's OK still, since force_close() is only used on pipes. I
don't know if closing a pipe can generate EIO or not.
--
Erik "kusma" Faye-Lund
^ permalink raw reply
* Re: [PATCH 1/6] run-command: add "use shell" option
From: Erik Faye-Lund @ 2009-12-30 13:55 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, Nanako Shiraishi, git
In-Reply-To: <20091230105316.GA22959@coredump.intra.peff.net>
On Wed, Dec 30, 2009 at 11:53 AM, Jeff King <peff@peff.net> wrote:
> Many callsites run "sh -c $CMD" to run $CMD. We can make it
> a little simpler for them by factoring out the munging of
> argv.
>
> For simple cases with no arguments, this doesn't help much, but:
>
> 1. For cases with arguments, we save the caller from
> having to build the appropriate shell snippet.
>
> 2. We can later optimize to avoid the shell when
> there are no metacharacters in the program.
>
Nice. This could be helpful if we ever decide not to depend on a
sh-compatible shell on Windows (since one isn't included by default no
Windows, and installing msys/cygwin has some compatibility-issues with
previous installations).
--
Erik "kusma" Faye-Lund
^ permalink raw reply
* Re: [PATCH] git-update-index: report(...) now flushes stdout after printing the report line
From: Sebastian Thiel @ 2009-12-30 13:56 UTC (permalink / raw)
To: git
In-Reply-To: <loom.20091119T221732-624@post.gmane.org>
I'd like to add that since version 1.6.5, non-tty's do not receive any progress
information anymore. The patch causing this says it wants to, in short words,
unify the push and fetch handling regarding the way progress messages are sent.
Now third-party wrappers, such as git-python, are unable to provide any progress
information anymore for possibly lengthy operations.
This is why I clearly recommend to add some kind of a "progress-force" flag that
turns progress messages on again for send-pack and receive-pack.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox