Git development
 help / color / mirror / Atom feed
* Re: [PATCH] gitk: Use git-difftool for external diffs
From: Junio C Hamano @ 2009-12-30  7:49 UTC (permalink / raw)
  To: Nanako Shiraishi; +Cc: David Aguilar, peff, sam, git, paulus
In-Reply-To: <20091230121319.6117@nanako3.lavabit.com>

Nanako Shiraishi <nanako3@lavabit.com> writes:

> Junio, could you tell us what happened to this thread?

See http://thread.gmane.org/gmane.comp.version-control.git/132983/focus=133414

In short, the particular way to call difftool this patch implements was
found to be inadequate to support existing external diff support by gitk
and a small difftool update will happen first, followed by a patch to gitk
to use the updated difftool, to avoid regression.

^ permalink raw reply

* Re: [PATCH] Makefile: determine the list of header files using a glob
From: Mike Hommey @ 2009-12-30  8:00 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Johannes Sixt, Git Mailing List
In-Reply-To: <7v7htbom95.fsf@alter.siamese.dyndns.org>

(Nanako's bunch of reminders reminded me of this thread)

On Fri, Nov 27, 2009 at 10:28:38AM -0800, Junio C Hamano wrote:
> Mike Hommey <mh@glandium.org> writes:
> 
> > On Fri, Nov 27, 2009 at 09:50:47AM +0100, Johannes Sixt wrote:
> >> Mike Hommey schrieb:
> >> > I don't know if the current Makefile works with Solaris' make,...
> >> 
> >> No, it doesn't. You have to use GNU make anyway.
> >
> > Then it's fine. But shouldn't that be noted somewhere, like in the
> > INSTALL file ?
> 
> Surely.  Please make it so.

I had another idea in the interim. If GNU make is necessary, why not
make the Makefile an explicit GNU make only Makefile, by renaming it
GNUmakefile ?

That wouldn't remove the need to add a note in the INSTALL file, though.

Mike

^ permalink raw reply

* Re: Giving command line parameter to textconv command?
From: Junio C Hamano @ 2009-12-30  8:05 UTC (permalink / raw)
  To: Nanako Shiraishi; +Cc: git, Jeff King
In-Reply-To: <20091230121325.6117@nanako3.lavabit.com>

Nanako Shiraishi <nanako3@lavabit.com> writes:

> Junio, could you tell us what happened to this thread?
>
> I use this patch myself, and it works really well.

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.

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

http://thread.gmane.org/gmane.comp.version-control.git/135250/focus=135312

^ permalink raw reply

* Re: Feature suggestion: support arguments for GIT_PROXY_COMMAND &    core.gitproxy
From: Junio C Hamano @ 2009-12-30  8:06 UTC (permalink / raw)
  To: Nanako Shiraishi; +Cc: Jeff King, Joey Korkames, git
In-Reply-To: <20091230121329.6117@nanako3.lavabit.com>

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.

^ permalink raw reply

* Re: [PATCH] Fix archive format with -- on the command line
From: Junio C Hamano @ 2009-12-30  8:11 UTC (permalink / raw)
  To: Nanako Shiraishi; +Cc: René Scharfe, Miklos Vajna, Ilari Liusvaara, git
In-Reply-To: <20091230121309.6117@nanako3.lavabit.com>

Nanako Shiraishi <nanako3@lavabit.com> writes:

> Junio, could you tell us what happened to this thread?
>
> "git archive HEAD non-existing-path" doesn't complain like "git
> add" does, and the patch is to fix it.  No discussion.

It walks the tree twice, once for the checking and then another for doing
its real work.  Doing that way obviously looks stupid and inefficient but
on the other hand it can fail before doing anything, which may be a big
plus.  I couldn't decide the pros-and-cons at the moment.

Probably it is worth queuing the patch as is, and if there are motivated
people who want to, let them "optimize" it by rolling that check into the
main loop.

I dunno.

^ permalink raw reply

* Re: [PATCH] Add --path-prefix option to git-fast-import
From: Gisle Aas @ 2009-12-30  8:17 UTC (permalink / raw)
  To: Sverre Rabbelier; +Cc: git, gitster, Gisle Aas
In-Reply-To: <fabb9a1e0912290606ib1f2e46y1ddbae9aa68e5194@mail.gmail.com>

On Dec 29, 2009, at 15:06, Sverre Rabbelier wrote:

> Heya,
> 
> On Tue, Dec 29, 2009 at 06:51, Gisle Aas <gisle.aas@it.uib.no> wrote:
>> +static const char *path_prefix_prepend(struct strbuf *sb, const char *p)
>> +{
>> +       if (p != sb->buf) {
>> +           strbuf_reset(sb);
>> +           strbuf_addstr(sb, p);
>> +       }
>> +       strbuf_insert(sb, 0, path_prefix, path_prefix_len);
>> +       return sb->buf;
>> +}
>> +
>>  static void file_change_m(struct branch *b)
>>  {
>>        const char *p = command_buf.buf + 2;
>> @@ -1909,6 +1921,8 @@ static void file_change_m(struct branch *b)
>>                        die("Garbage after path in: %s", command_buf.buf);
>>                p = uq.buf;
>>        }
>> +       if (path_prefix)
>> +           p = path_prefix_prepend(&uq, p);
> 
> You could reduce the size of this change by having path_prefix_prepend
> check for path_prefix and just do nothing if it is not set.

I felt the explict test was better style -- more obvious that nothing happens to p
when there is no path_prefix.

--Gisle

^ permalink raw reply

* Re: [PATCH] Makefile: determine the list of header files using a glob
From: Junio C Hamano @ 2009-12-30  8:45 UTC (permalink / raw)
  To: Mike Hommey; +Cc: Johannes Sixt, Git Mailing List
In-Reply-To: <20091230080002.GA31088@glandium.org>

Mike Hommey <mh@glandium.org> writes:

> On Fri, Nov 27, 2009 at 10:28:38AM -0800, Junio C Hamano wrote:
>> Mike Hommey <mh@glandium.org> writes:
>> ...
>> > Then it's fine. But shouldn't that be noted somewhere, like in the
>> > INSTALL file ?
>> 
>> Surely.  Please make it so.
>
> I had another idea in the interim. If GNU make is necessary, why not
> make the Makefile an explicit GNU make only Makefile, by renaming it
> GNUmakefile ?
>
> That wouldn't remove the need to add a note in the INSTALL file, though.

I think you answered the question yourself.

Personally, when I am dealing with other people's projects, I dislike ones
that do not name their own Makefile "Makefile", as I traditionally have
done my local customization by having a higher precedence Makefile
("makefile" in the old world order, or "GNUmakefile" for gmake) that sets
things up and then call into upstream "Makefile".  For this project, since
I am the upstream, I do want to keep the file named "Makefile".

^ permalink raw reply

* [PATCH] reset: unbreak hard resets with GIT_WORK_TREE
From: Jeff King @ 2009-12-30  8:47 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Fyn Fynn, Nguyen Thai Ngoc Duy, Nanako Shiraishi, git
In-Reply-To: <20091229215015.GA1529@coredump.intra.peff.net>

Commit 952dfc6 tried to tighten the safety valves for doing
a "reset --hard" in a bare repository or outside the work
tree, but accidentally broke the case for GIT_WORK_TREE.
This patch unbreaks it.

Most git commands which need a work tree simply use
NEED_WORK_TREE in git.c to die before they get to their
cmd_* function. Reset, however, only needs a work tree in
some cases, and so must handle the work tree itself. The
error that 952dfc6 made was to simply forbid certain
operations if the work tree was not set up; instead, we need
to do the same thing that NEED_WORK_TREE does, which is to
call setup_work_tree(). We no longer have to worry about dying
in the non-worktree case, as setup_work_tree handles that
for us.

Signed-off-by: Jeff King <peff@peff.net>
---
Junio, this should probably go onto maint. I based it directly on
952dfc6, which is in v1.6.6 and v1.6.5.5.

 builtin-reset.c       |    6 ++----
 t/t7103-reset-bare.sh |    6 ++++++
 2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/builtin-reset.c b/builtin-reset.c
index 11d1c6e..e4418bc 100644
--- a/builtin-reset.c
+++ b/builtin-reset.c
@@ -286,10 +286,8 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
 	if (reset_type == NONE)
 		reset_type = MIXED; /* by default */
 
-	if ((reset_type == HARD || reset_type == MERGE)
-	    && !is_inside_work_tree())
-		die("%s reset requires a work tree",
-		    reset_type_names[reset_type]);
+	if (reset_type == HARD || reset_type == MERGE)
+		setup_work_tree();
 
 	/* Soft reset does not touch the index file nor the working tree
 	 * at all, but requires them in a good order.  Other resets reset
diff --git a/t/t7103-reset-bare.sh b/t/t7103-reset-bare.sh
index 68041df..afb55b3 100755
--- a/t/t7103-reset-bare.sh
+++ b/t/t7103-reset-bare.sh
@@ -29,6 +29,12 @@ test_expect_success 'soft reset is ok' '
 	(cd .git && git reset --soft)
 '
 
+test_expect_success 'hard reset works with GIT_WORK_TREE' '
+	mkdir worktree &&
+	GIT_WORK_TREE=$PWD/worktree GIT_DIR=$PWD/.git git reset --hard &&
+	test_cmp file worktree/file
+'
+
 test_expect_success 'setup bare' '
 	git clone --bare . bare.git &&
 	cd bare.git
-- 
1.6.6.320.g7c9b3

^ permalink raw reply related

* [PATCH] textconv: stop leaking file descriptors
From: Jeff King @ 2009-12-30  9:02 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

We read the output from textconv helpers over a pipe, but we
never actually closed our end of the pipe after using it.

Signed-off-by: Jeff King <peff@peff.net>
---
This should go onto 'maint'. Now I'll just go find a brown paper bag...

 diff.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/diff.c b/diff.c
index d14a575..aad4b39 100644
--- a/diff.c
+++ b/diff.c
@@ -3823,11 +3823,13 @@ static char *run_textconv(const char *pgm, struct diff_filespec *spec,
 	if (start_command(&child) != 0 ||
 	    strbuf_read(&buf, child.out, 0) < 0 ||
 	    finish_command(&child) != 0) {
+		close(child.out);
 		strbuf_release(&buf);
 		remove_tempfile();
 		error("error running textconv command '%s'", pgm);
 		return NULL;
 	}
+	close(child.out);
 	remove_tempfile();
 
 	return strbuf_detach(&buf, outsize);
-- 
1.6.6.320.g7c9b3

^ permalink raw reply related

* [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 +++---
 pager.c     |    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;
diff --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


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox