Git development
 help / color / mirror / Atom feed
* Re: Possible regression in git-rev-list --header
From: Junio C Hamano @ 2006-12-30 20:20 UTC (permalink / raw)
  To: Marco Costalba; +Cc: git, Johannes Schindelin
In-Reply-To: <Pine.LNX.4.63.0612301955340.19693@wbgn013.biozentrum.uni-wuerzburg.de>

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> On Sat, 30 Dec 2006, Marco Costalba wrote:
>
>> When 'commitencoding' variable is set in config file then git-rev-list
>> called with --header option sends also the encoding information.
>
> As Jakub pointed out, qgit should not expect to know all headers. I am 
> very sorry, since I said I looked at all parsers of the commit header in 
> git, but that was _only_ git, and no porcelains.
>
> Please fix qgit, since I really consider this a bug.

I have to agree with Johannes.  In principle Porcelains should
be prepared to see and ignore unknown headers.

However, this commit created by `commit-tree` certalinly can be
improved.

        $ git rev-list --header -n1 HEAD
        6d751699cb04150abd79a730187d4e2ed6330c05
        tree 70209eebdc59d108948feb15c3c5497f299ef290
        parent 49a8186d7352d0454df79b289fecb18c8e535c32
        author Marco Costalba <mcostalba@gmail.com> 1167500660 +0100
        committer Marco Costalba <mcostalba@gmail.com> 1167500660 +0100
        encoding UTF-8

           Test commit

           Let's see what git-rev-list --header spits out.

           Signed-off-by: Marco Costalba <mcostalba@gmail.com>

-- >8 --
commit-tree: cope with different ways "utf-8" can be spelled.

People can spell config.commitencoding differently from what we
internally have ("utf-8") to mean UTF-8.  Try to accept them and
treat them equally.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---

diff --git a/builtin-commit-tree.c b/builtin-commit-tree.c
index 146aaff..0651e59 100644
--- a/builtin-commit-tree.c
+++ b/builtin-commit-tree.c
@@ -119,8 +119,7 @@ int cmd_commit_tree(int argc, const char **argv, const char *prefix)
 	}
 
 	/* Not having i18n.commitencoding is the same as having utf-8 */
-	encoding_is_utf8 = (!git_commit_encoding ||
-			    !strcmp(git_commit_encoding, "utf-8"));
+	encoding_is_utf8 = is_encoding_utf8(git_commit_encoding);
 
 	init_buffer(&buffer, &size);
 	add_buffer(&buffer, &size, "tree %s\n", sha1_to_hex(tree_sha1));
diff --git a/utf8.c b/utf8.c
index 1eedd8b..7c80eec 100644
--- a/utf8.c
+++ b/utf8.c
@@ -277,6 +277,15 @@ void print_wrapped_text(const char *text, int indent, int indent2, int width)
 	}
 }
 
+int is_encoding_utf8(const char *name)
+{
+	if (!name)
+		return 1;
+	if (!strcasecmp(name, "utf-8") || !strcasecmp(name, "utf8"))
+		return 1;
+	return 0;
+}
+
 /*
  * Given a buffer and its encoding, return it re-encoded
  * with iconv.  If the conversion fails, returns NULL.
diff --git a/utf8.h b/utf8.h
index cae2a8e..a07c5a8 100644
--- a/utf8.h
+++ b/utf8.h
@@ -3,6 +3,8 @@
 
 int utf8_width(const char **start);
 int is_utf8(const char *text);
+int is_encoding_utf8(const char *name);
+
 void print_wrapped_text(const char *text, int indent, int indent2, int len);
 
 #ifndef NO_ICONV

^ permalink raw reply related

* [PATCH] Move commit reencoding parameter parsing to revision.c
From: Junio C Hamano @ 2006-12-30 20:22 UTC (permalink / raw)
  To: Marco Costalba; +Cc: git, Johannes Schindelin
In-Reply-To: <Pine.LNX.4.63.0612301955340.19693@wbgn013.biozentrum.uni-wuerzburg.de>

Also I think we need this patch for completeness on top of what
we have in 'master'.

-- >8 --
[PATCH] Move commit reencoding parameter parsing to revision.c

This way, git-rev-list and git-diff-tree with --pretty can use
it.  Because they are both plumbing, we default not to do any
conversion unless --encoding=<encoding> is explicitly given from
the command line.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---
 Documentation/git-rev-list.txt   |    1 +
 Documentation/pretty-formats.txt |    8 ++++++++
 builtin-diff-tree.c              |    6 ++++++
 builtin-rev-list.c               |    6 ++++++
 revision.c                       |    8 ++++++++
 5 files changed, 29 insertions(+), 0 deletions(-)

diff --git a/Documentation/git-rev-list.txt b/Documentation/git-rev-list.txt
index 9e0dcf8..86c94e7 100644
--- a/Documentation/git-rev-list.txt
+++ b/Documentation/git-rev-list.txt
@@ -21,6 +21,7 @@ SYNOPSIS
 	     [ \--stdin ]
 	     [ \--topo-order ]
 	     [ \--parents ]
+	     [ \--encoding[=<encoding>] ]
 	     [ \--(author|committer|grep)=<pattern> ]
 	     [ [\--objects | \--objects-edge] [ \--unpacked ] ]
 	     [ \--pretty | \--header ]
diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt
index 996f628..8cba13f 100644
--- a/Documentation/pretty-formats.txt
+++ b/Documentation/pretty-formats.txt
@@ -76,3 +76,11 @@ displayed in full, regardless of whether --abbrev or
 --no-abbrev are used, and 'parents' information show the
 true parent commits, without taking grafts nor history
 simplification into account.
+
+
+--encoding[=<encoding>]::
+	The commit objects record the encoding used for the log message
+	in their encoding header; this option can be used to tell the
+	command to re-code the commit log message in the encoding
+	preferred by the user.  For non plumbing commands this
+	defaults to UTF-8.
diff --git a/builtin-diff-tree.c b/builtin-diff-tree.c
index 24cb2d7..6ce2c0f 100644
--- a/builtin-diff-tree.c
+++ b/builtin-diff-tree.c
@@ -67,6 +67,12 @@ int cmd_diff_tree(int argc, const char **argv, const char *prefix)
 	static struct rev_info *opt = &log_tree_opt;
 	int read_stdin = 0;
 
+	/* default to turn the conversion off -- diff-tree is not a
+	 * Porcelain but as low plumbing as it can go, as far as
+	 * two-tree comparison is concerned.
+	 */
+	git_log_output_encoding = "";
+
 	init_revisions(opt, prefix);
 	git_config(git_default_config); /* no "diff" UI options */
 	nr_sha1 = 0;
diff --git a/builtin-rev-list.c b/builtin-rev-list.c
index 1bb3a06..0c3dce6 100644
--- a/builtin-rev-list.c
+++ b/builtin-rev-list.c
@@ -226,6 +226,12 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)
 	int i;
 	int read_from_stdin = 0;
 
+	/* default to turn the conversion off -- rev-list is not a
+	 * Porcelain but as low plumbing as it can go, as far as
+	 * revision traversal is concerned.
+	 */
+	git_log_output_encoding = "";
+
 	init_revisions(&revs, prefix);
 	revs.abbrev = 0;
 	revs.commit_format = CMIT_FMT_UNSPECIFIED;
diff --git a/revision.c b/revision.c
index af9f874..6e4ec46 100644
--- a/revision.c
+++ b/revision.c
@@ -1039,6 +1039,14 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const ch
 				all_match = 1;
 				continue;
 			}
+			if (!strncmp(arg, "--encoding=", 11)) {
+				arg += 11;
+				if (strcmp(arg, "none"))
+					git_log_output_encoding = strdup(arg);
+				else
+					git_log_output_encoding = "";
+				continue;
+			}
 
 			opts = diff_opt_parse(&revs->diffopt, argv+i, argc-i);
 			if (opts > 0) {
-- 
1.5.0.rc0.g6bb1

^ permalink raw reply related

* Re: Possible regression in git-rev-list --header
From: Junio C Hamano @ 2006-12-30 22:19 UTC (permalink / raw)
  To: Marco Costalba; +Cc: git, Johannes Schindelin
In-Reply-To: <7v7iw9jftv.fsf@assigned-by-dhcp.cox.net>

Junio C Hamano <junkio@cox.net> writes:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
>> On Sat, 30 Dec 2006, Marco Costalba wrote:
>>
>>> When 'commitencoding' variable is set in config file then git-rev-list
>>> called with --header option sends also the encoding information.
>>
>> As Jakub pointed out, qgit should not expect to know all headers. I am 
>> very sorry, since I said I looked at all parsers of the commit header in 
>> git, but that was _only_ git, and no porcelains.
>>
>> Please fix qgit, since I really consider this a bug.
>
> I have to agree with Johannes.  In principle Porcelains should
> be prepared to see and ignore unknown headers.
>
> However, this commit created by `commit-tree` certalinly can be
> improved.
> ...

Another thing.  I think it would make sense to remove "encoding"
header after pretty_print_commit successfully re-codes the
buffer.  An alternative is to rewrite "encoding" header to show
which encoding the log now uses (and omit it if it is UTF-8).

The attached patch does the latter, but I think removing the
header altogether in this case would make it easier to use
overall.  If you want to, you can change

	if (is_encoding_utf8(encoding))

in the patch with "if (1)" (the above macro is what was
introduced in my previous patch).

Pro for having "encoding" rewritten as this patch does is that
the output is still self-identifying.  You can tell what
encoding you are supposed to interpret the log messages in.  Con
against it is that the output is after re-coding as the user
asked (either with the config or --encoding=<encoding> option),
and the extra header becomes redundant information.

-- >8 --

 commit.c |   44 ++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 44 insertions(+), 0 deletions(-)

diff --git a/commit.c b/commit.c
index e13b9cb..bf82fe8 100644
--- a/commit.c
+++ b/commit.c
@@ -624,6 +624,48 @@ static char *get_header(const struct commit *commit, const char *key)
 	}
 }
 
+static char *replace_encoding_header(char *buf, char *encoding)
+{
+	char *encoding_header = strstr(buf, "\nencoding ");
+	char *end_of_encoding_header;
+	int encoding_header_pos;
+	int encoding_header_len;
+	int new_len;
+	int need_len;
+	int buflen = strlen(buf) + 1;
+
+	if (!encoding_header)
+		return buf; /* should not happen but be defensive */
+	encoding_header++;
+	end_of_encoding_header = strchr(encoding_header, '\n');
+	if (!end_of_encoding_header)
+		return buf; /* should not happen but be defensive */
+	end_of_encoding_header++;
+
+	encoding_header_len = end_of_encoding_header - encoding_header;
+	encoding_header_pos = encoding_header - buf;
+
+	if (is_encoding_utf8(encoding)) {
+		/* we have re-coded to UTF-8; drop the header */
+		memmove(encoding_header, end_of_encoding_header,
+			buflen - (encoding_header_pos + encoding_header_len));
+		return buf;
+	}
+	new_len = strlen(encoding);
+	need_len = new_len + strlen("encoding \n");
+	if (encoding_header_len < need_len) {
+		buf = xrealloc(buf, buflen + (need_len - encoding_header_len));
+		encoding_header = buf + encoding_header_pos;
+		end_of_encoding_header = encoding_header + encoding_header_len;
+	}
+	memmove(end_of_encoding_header + (need_len - encoding_header_len),
+		end_of_encoding_header,
+		buflen - (encoding_header_pos + encoding_header_len));
+	memcpy(encoding_header + 9, encoding, strlen(encoding));
+	encoding_header[9 + new_len] = '\n';
+	return buf;
+}
+
 static char *logmsg_reencode(const struct commit *commit)
 {
 	char *encoding;
@@ -642,6 +684,8 @@ static char *logmsg_reencode(const struct commit *commit)
 		return NULL;
 	}
 	out = reencode_string(commit->buffer, output_encoding, encoding);
+	out = replace_encoding_header(out, output_encoding);
+
 	free(encoding);
 	if (!out)
 		return NULL;

^ permalink raw reply related

* Re: Possible regression in git-rev-list --header
From: Johannes Schindelin @ 2006-12-31  0:37 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Marco Costalba, git
In-Reply-To: <7v7iw9jftv.fsf@assigned-by-dhcp.cox.net>

Hi,

On Sat, 30 Dec 2006, Junio C Hamano wrote:

> commit-tree: cope with different ways "utf-8" can be spelled.

Does iconv handle these? If not, we should reset git_commit_encoding.

Ciao,
Dscho

^ permalink raw reply

* [PATCH] Fix formatting for urls section of fetch, pull, and push manpages
From: Theodore Ts'o @ 2006-12-31  1:03 UTC (permalink / raw)
  To: git


The line:

[remote "<remote>"]

was getting swallowed up by asciidoc, causing a critical line in the
explanation for how to store the .git/remotes information in .git/config
to go missing from the git-fetch, git-pull, and git-push manpages.  

Put all of the examples into delimited blocks to fix this problem and to
make them look nicer.

Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
---
 Documentation/urls.txt |   23 ++++++++++++++++-------
 1 files changed, 16 insertions(+), 7 deletions(-)

diff --git a/Documentation/urls.txt b/Documentation/urls.txt
index 670827c..870c950 100644
--- a/Documentation/urls.txt
+++ b/Documentation/urls.txt
@@ -40,9 +40,11 @@ In addition to the above, as a short-hand, the name of a
 file in `$GIT_DIR/remotes` directory can be given; the
 named file should be in the following format:
 
-	URL: one of the above URL format
-	Push: <refspec>
-	Pull: <refspec>
+------------
+URL: one of the above URL format
+Push: <refspec>
+Pull: <refspec>
+------------
 
 Then such a short-hand is specified in place of
 <repository> without <refspec> parameters on the command
@@ -54,10 +56,12 @@ be specified for additional branch mappings.
 Or, equivalently, in the `$GIT_DIR/config` (note the use
 of `fetch` instead of `Pull:`):
 
+------------
 [remote "<remote>"]
 	url = <url>
 	push = <refspec>
 	fetch = <refspec>
+------------
 
 The name of a file in `$GIT_DIR/branches` directory can be
 specified as an older notation short-hand; the named
@@ -68,10 +72,15 @@ name of remote head (URL fragment notation).
 without the fragment is equivalent to have this in the
 corresponding file in the `$GIT_DIR/remotes/` directory.
 
-	URL: <url>
-	Pull: refs/heads/master:<remote>
+------------
+URL: <url>
+Pull: refs/heads/master:<remote>
+------------
+
 
 while having `<url>#<head>` is equivalent to
 
-	URL: <url>
-	Pull: refs/heads/<head>:<remote>
+------------
+URL: <url>
+Pull: refs/heads/<head>:<remote>
+------------
-- 
1.5.0.rc0.g1ed48

^ permalink raw reply related

* [PATCH] Fix yet another subtle xdl_merge() bug
From: Johannes Schindelin @ 2006-12-31  1:07 UTC (permalink / raw)
  To: git, junkio

[-- Attachment #1: Type: TEXT/PLAIN, Size: 4239 bytes --]

In very obscure cases, a merge can hit an unexpected code path (where the 
original code went as far as saying that this was a bug). This failing 
merge was noticed by Alexandre Juillard.

The problem is that the original file contains something like this:

-- snip --
two non-empty lines
before two empty lines


after two empty lines
-- snap --

and this snippet is reduced to _one_ empty line in _both_ new files. 
However, it is ambiguous as to which hunk takes the empty line: the first 
or the second one?

Indeed in Alexandre's example files, the xdiff algorithm attributes the 
empty line to the first hunk in one case, and to the second hunk in the 
other case.

(Trimming down the example files _changes_ that behaviour!)

Thus, the call to xdl_merge_cmp_lines() has no chance to realize that the 
change is actually identical in both new files. Therefore, 
xdl_refine_conflicts() finds an empty diff script, which was not expected 
there, because (the original author of xdl_merge() thought) 
xdl_merge_cmp_lines() would catch that case earlier.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---

	Alexandre sent me this set of files where xdl_merge() returns 
	-1 instead of performing as expected. With his permission, I 
	attached them to this mail.

	On Thu, 28 Dec 2006, Alexandre Julliard wrote:
	
	> I'm still unable to complete the merge though, it dies a bit 
	> later with:
	> 
	> ...
	> Auto-merging dlls/ntdll/virtual.c
	> fatal: Failed to execute internal merge
	> Merge with strategy recursive failed.
	> 
	> Apparently xdl_merge() returns -1 for that one. I've attached 
	> the corresponding files.

	I tried for about 20 hours total to concoct a short test case for 
	this, still failing. I give up.

	Trying to find out what happens led me to an interesting hunt for 
	a bug which is not there. The code is even more elegant than I 
	thought: it took me 10 hours of staring into my laptop, scribbling 
	on my notepad and trying several examples, to find out why it 
	works.

	The code in question is where two conflicts overlap, i.e. the 
	original lines affected by both diff scripts overlap. The 
	interesting case is when the overlapping regions are 
	not identical.

	xdl_do_merge() enlarges the smaller region, and adds the same 
	amount of lines _to the corresponding region in the new file_.

	This seems wrong, as there can be modifications in that region, 
	too! Why does the code perform correctly, then?

	It is a feature/obscure behaviour of xdl_append_merge() that it 
	tries to combine conflicts. And if said problem occurs, it _will_ 
	modify the last conflict, but _only_ the end of the regions! Thus, 
	it does not matter if the first line passed to xdl_append_merge() 
	is actually wrong, as long as it is guaranteed to overlap the old 
	conflict (which is the case: if it would not overlap, there would 
	be no modification in that part of the original file!).

	The same holds for the last line: if it is wrong, it will get 
	fixed in a subsequent call to xdl_append_merge().

	If this sounds confused, it is because I thought too long and hard 
	about it, _and_ I just had a lovely poker night (my first one!). 
	But please, if you want to understand it, but my explanation is 
	not satisfactory, you may bug me for a better description, and I 
	will do it. Though it has to wait until 2nd of January.

	Happy new year!

 xdiff/xmerge.c |    7 +++++--
 1 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/xdiff/xmerge.c b/xdiff/xmerge.c
index 294450b..b83b334 100644
--- a/xdiff/xmerge.c
+++ b/xdiff/xmerge.c
@@ -166,6 +166,8 @@ static int xdl_fill_merge_buffer(xdfenv_t *xe1, const char *name1,
 			size += xdl_recs_copy(xe2, m->i2 - m->i1 + i1,
 					m->i1 + m->chg2 - i1, 0,
 					dest ? dest + size : NULL);
+		else
+			continue;
 		i1 = m->i1 + m->chg1;
 	}
 	size += xdl_recs_copy(xe1, i1, xe1->xdf2.nrec - i1, 0,
@@ -213,9 +215,10 @@ static int xdl_refine_conflicts(xdfenv_t *xe1, xdfenv_t *xe2, xdmerge_t *m,
 			return -1;
 		}
 		if (!xscr) {
-			/* If this happens, it's a bug. */
+			/* If this happens, the changes are identical. */
 			xdl_free_env(&xe);
-			return -2;
+			m->mode = 4;
+			continue;
 		}
 		x = xscr;
 		m->i1 = xscr->i1 + i1;


[-- Attachment #2: Type: APPLICATION/X-GUNZIP, Size: 47452 bytes --]

^ permalink raw reply related

* Re: [PATCH] xdl_merge(): fix a segmentation fault when refining conflicts
From: Johannes Schindelin @ 2006-12-31  1:09 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: Shawn Pearce, git
In-Reply-To: <en6fj1$ji5$1@sea.gmane.org>

Hi,

On Sat, 30 Dec 2006, Jakub Narebski wrote:

> Johannes Schindelin wrote:
> 
> > Of course, you can hit mismerges like the illustrated one _without_ 
> > being marked as conflict (e.g. if the chunk of identical code is _not_ 
> > added, but only the increments), but we should at least avoid them 
> > where possible.
> 
> Perhaps you could make it even more conservating merge conflicts option 
> (to tighten merge conflicts even more) to xdl_merge, but not used by 
> default because as it removes accidental conflicts it increases 
> mismerges (falsely not conflicted).

There is no way to do this sanely. If you want to catch these mismerges, 
you have to mark _all_ modifications as conflicting.

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH] Move commit reencoding parameter parsing to revision.c
From: Johannes Schindelin @ 2006-12-31  1:10 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Marco Costalba, git
In-Reply-To: <7v3b6xjfrr.fsf_-_@assigned-by-dhcp.cox.net>

Hi,

this patch makes a lot of sense IMHO.

Ciao,
Dscho

^ permalink raw reply

* Re: Possible regression in git-rev-list --header
From: Johannes Schindelin @ 2006-12-31  1:13 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Marco Costalba, git
In-Reply-To: <7vlkkphvrb.fsf@assigned-by-dhcp.cox.net>

Hi,

On Sat, 30 Dec 2006, Junio C Hamano wrote:

> Another thing.  I think it would make sense to remove "encoding" header 
> after pretty_print_commit successfully re-codes the buffer.  An 
> alternative is to rewrite "encoding" header to show which encoding the 
> log now uses (and omit it if it is UTF-8).

I think it would be wrong. Sure, the output may be encoded differently, 
but the _original_ commit was not. And this is the information I want 
to see when I look at the raw commit.

If you do not want to see all headers, you have to choose a different 
--pretty option.

Ciao,
Dscho
 

^ permalink raw reply

* Re: [PATCH 0/11] Misc. pull/merge/am improvements
From: Shawn Pearce @ 2006-12-31  1:21 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vmz57p6tn.fsf@assigned-by-dhcp.cox.net>

Junio C Hamano <junkio@cox.net> wrote:
> > Junio C Hamano <junkio@cox.net> wrote:
> >> While I was looking at the problem, I noticed something a bit
> >> easier to reproduce and should be lot easier to diagnose.  At
> >> http://userweb.kernel.org/~junio/broken.tar, I have a tarball of
> >> git.git repository.
> >
> 
> I think there is a thinko in the OFS_DELTA arm of that switch
> statement.  You are resetting buf to (in-pack-offset + used), so
> you should fetch the variably encoded length starting from buf[0].

Yes.  Your patch looks correct; that's a really bad thinko on
my part.  Thanks for tracking it down, and fixing it.

-- 
Shawn.

^ permalink raw reply

* Strict ordering by date
From: H. Peter Anvin @ 2006-12-31  1:03 UTC (permalink / raw)
  To: git

Hi all,

I have added a --pretty=rpm option to git in order to automatically 
generate the changelog for an RPM specfile.  So far, so good. 
Unfortunately, rpmbuild is very picky and will simply refuse to run, at 
all, if any of the entries are out of reverse chronological order.  When 
dealing with merges or cherrypicking -- or, for that matter, a commit 
done with a broken time setting -- this doesn't work with either 
--date-order or --topo-order.

(And yes, this is an rpmbuild misfeature.)

I've tried to add a --strict-date-order option, but it's not clear at 
what point -- if any -- we actually have the full list of entries to 
display.  It doesn't seem to happen anywhere inside revision.c.

Any pointers appreciated.

	-hpa

^ permalink raw reply

* Re: SIGSEGV in merge recursive
From: Luben Tuikov @ 2006-12-31  1:42 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Junio C Hamano, git
In-Reply-To: <Pine.LNX.4.63.0612301204280.19693@wbgn013.biozentrum.uni-wuerzburg.de>

--- Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> On Fri, 29 Dec 2006, Luben Tuikov wrote:
> 
> > > > Failed to read a valid object file image from memory.
> > > 
> > > Who says this?
> 
> Again, who says this? I cannot find _anything_ in my local repo (with its 
> own set of modifications, which do not hide such a message).

GDB.  This message is not present when I run it manually from shell prompt.

> Given that there is a fix in master for a segfault, I have to admit that I 
> believe you did not use _that_ version, but a git without that fix.

A-ha!  Since I never run master, after I read your message, I compiled
master, installed it and tried the same merge/pull and it succeeded -- i.e.
didn't segfault, but complained about unresolved conflicts (as it should).

So it appears that the fix is in "master", but not in "next".

Junio, can you confirm this?

    Luben


> Besides, Alexandre hit an interesting bug, which is not at all easy to 
> reproduce (except with three 70k files which I don't want to include in 
> the test set). Since Alexandre provided _examples_ where I can _reproduce_ 
> the problem, I will be working on that bug, and not yours.
> 
> Ciao,
> Dscho
> 
> 

^ permalink raw reply

* Re: Possible regression in git-rev-list --header
From: Junio C Hamano @ 2006-12-31  1:45 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Marco Costalba, git
In-Reply-To: <Pine.LNX.4.63.0612310211300.25709@wbgn013.biozentrum.uni-wuerzburg.de>

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> On Sat, 30 Dec 2006, Junio C Hamano wrote:
>
>> Another thing.  I think it would make sense to remove "encoding" header 
>> after pretty_print_commit successfully re-codes the buffer.  An 
>> alternative is to rewrite "encoding" header to show which encoding the 
>> log now uses (and omit it if it is UTF-8).
>
> I think it would be wrong. Sure, the output may be encoded differently, 
> but the _original_ commit was not. And this is the information I want 
> to see when I look at the raw commit.

When you want to see the raw commit, you would not ask for it to
re-code, so "removal after successfully re-codes" would not kick
in (if you _really_ want to look at the raw commit, I guess
cat-file can help, but let's not go there).  Re-coding the
message but still showing what the original encoding was does
not sound making much sense to me.

I've pushed this out after a small rework.

The rule is:

 - if you ask for re-coding (either by i18n.* configuration or
   an explicit --encoding option from the command line), and if
   re-coding successfully does its job, you do not see
   "encoding" header;

 - if the buffer cannot successfully be re-coded, no re-coding
   is done, and the caller can inspect "encoding" header.

 - if you do not ask for re-coding, "encoding" header is left as
   is, so is the commit log message.  The caller can deal with
   any re-coding itself.

^ permalink raw reply

* What commands can and can not be used with bare repositories?
From: Theodore Ts'o @ 2006-12-31  1:48 UTC (permalink / raw)
  To: git

In order to minimize the amount of files which need to be backed up, I
decided to create an "upstream" repository which contains nothing but
Linus's tree, which I packed into a single pack file, and which I would
then only update every 3-6 months.

The git tree(s) that I would use for Linux hacking would then use the
upstream repository as an alternate source of objects.  That way the
"git gc" command is much faster, and it doesn't end up thrashing the
main pack file found in the "upstream" repository (which is currently
about 135 megs).  This streamlines my backups a tad (every little bit
helps).

The only real problem with this is that I don't really need to have a
working directory in the "upstream" repository, so I decided to try
using a "bare repository".  This is only barely (sorry) documented in
the git Documentation, where in the git-clone manpage there is a
description of how the administrative files end up in the top-level
directory instead of the .git subdirectory.  

What isn't documented is what commands actually can deal with a bare
repository.  At the moment, it looks like a bare repository can be a
target of a git pull, push, and merge commands, and it can be a source
for a git clone, but that seems to be about it.  All other commands,
such as "git log" blow up with the error message "Not a git repository".
This to me seems a bit lame, since why isn't a "bare repository" also a
"git repository"?  All of the information is there for "git log" to
work.  Commands that require a working directory obviously can't work,
but there are plenty of git commands for which there's no reason why
they shouldn't be able to operate on a bare repository.  For example,
"git repack", "git log", "git fetch", etc.

So as a suggestion, it would be good if exactly what you can and can't
use a "bare repository" for were documented.  If it really is push,
pull, fetch, and clone, I'll happily submit a patch to enhance the
documentation accordingly.   

The next obvious question, though, is *should* that be all that works?
A somewhat squinky hack that works quite nicely is to create a symlink
from . to .git in the bare repository.  At this point "git log", "git
fetch", "git repack", etc., all start working.  Of course, commands such
as "git status" that involve a working directory will be pretty
confused, but maybe we could fix that.  What if we were to change "git
clone --bare" to create the .git -> . symlink, and then add a check to
commands that require a working directory to see if ".git" is a symlink
to ., and if so, give an error message, "operation not supported on bare
repository"?

					- Ted

^ permalink raw reply

* Re: What commands can and can not be used with bare repositories?
From: Shawn Pearce @ 2006-12-31  1:57 UTC (permalink / raw)
  To: Theodore Ts'o; +Cc: git
In-Reply-To: <E1H0poE-0000qd-Ee@candygram.thunk.org>

Theodore Ts'o <tytso@mit.edu> wrote:
> What isn't documented is what commands actually can deal with a bare
> repository.  At the moment, it looks like a bare repository can be a
> target of a git pull, push, and merge commands

Sorry but 'git merge' cannot be used in a bare repository (no working
directory to update during the merge) and 'git merge' can only work on
the current repository, which rules out the bare repository.

> and it can be a source
> for a git clone, but that seems to be about it.  All other commands,
> such as "git log" blow up with the error message "Not a git repository".

Try "git --bare log".  Or "git --git-dir=/path/to log".

> This to me seems a bit lame, since why isn't a "bare repository" also a
> "git repository"?  All of the information is there for "git log" to
> work.  Commands that require a working directory obviously can't work,
> but there are plenty of git commands for which there's no reason why
> they shouldn't be able to operate on a bare repository.  For example,
> "git repack", "git log", "git fetch", etc.

Actually most commands work on a bare repository.
Very few don't: the ones that require a working directory.
E.g. status/revert/cherry-pick/commit/am/merge/pull.  (You can
pull from a bare repository, but you cannot run pull *in* a bare
repository.)

> confused, but maybe we could fix that.  What if we were to change "git
> clone --bare" to create the .git -> . symlink, and then add a check to
> commands that require a working directory to see if ".git" is a symlink
> to ., and if so, give an error message, "operation not supported on bare
> repository"?

No.  Better would be to make git's repository setup logic
automatically detect if "." is a Git repository, and if so let the
commands that work without a working directory run.

-- 
Shawn.

^ permalink raw reply

* Re: What commands can and can not be used with bare repositories?
From: J. Bruce Fields @ 2006-12-31  1:57 UTC (permalink / raw)
  To: Theodore Ts'o; +Cc: git
In-Reply-To: <E1H0poE-0000qd-Ee@candygram.thunk.org>

On Sat, Dec 30, 2006 at 08:48:22PM -0500, Theodore Ts'o wrote:
> What isn't documented is what commands actually can deal with a bare
> repository.  At the moment, it looks like a bare repository can be a
> target of a git pull, push, and merge commands, and it can be a source
> for a git clone, but that seems to be about it.  All other commands,
> such as "git log" blow up with the error message "Not a git repository".
> This to me seems a bit lame, since why isn't a "bare repository" also a
> "git repository"?  All of the information is there for "git log" to
> work.  Commands that require a working directory obviously can't work,
> but there are plenty of git commands for which there's no reason why
> they shouldn't be able to operate on a bare repository.  For example,
> "git repack", "git log", "git fetch", etc.

Yup.  Anything that should work actually does; you just need to know to
use one of:

	GIT_DIR=. git log
	git --git-dir=. log
	git --bare log

Why git couldn't figure this out for you, I don't know....

--b.

^ permalink raw reply

* [PATCH 1/2] Teach Git how to parse standard power of 2 suffixes.
From: Shawn O. Pearce @ 2006-12-31  2:02 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

Sometimes its necessary to supply a value as a power of two in a
configuration parameter.  In this case the user may want to use
the standard suffixes such as K, KB, KiB, etc. to indicate that
the numerical value should be multiplied by a constant base before
being used.

The new git_config_datasize function can be used in config file
handler functions to obtain a size_t in bytes.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---

 Applies on top of sp/mmap, but was broken out to make it easier
 to apply earlier in case someone else needed this function.

 cache.h  |    1 +
 config.c |   25 +++++++++++++++++++++++++
 2 files changed, 26 insertions(+), 0 deletions(-)

diff --git a/cache.h b/cache.h
index a5fc232..abbcab3 100644
--- a/cache.h
+++ b/cache.h
@@ -418,6 +418,7 @@ extern int git_config_from_file(config_fn_t fn, const char *);
 extern int git_config(config_fn_t fn);
 extern int git_config_int(const char *, const char *);
 extern int git_config_bool(const char *, const char *);
+extern size_t git_config_datasize(const char *, const char *);
 extern int git_config_set(const char *, const char *);
 extern int git_config_set_multivar(const char *, const char *, const char *, int);
 extern int git_config_rename_section(const char *, const char *);
diff --git a/config.c b/config.c
index 2e0d5a8..07ad2f1 100644
--- a/config.c
+++ b/config.c
@@ -255,6 +255,31 @@ int git_config_bool(const char *name, const char *value)
 	return git_config_int(name, value) != 0;
 }
 
+size_t git_config_datasize(const char *name, const char *value)
+{
+	if (value && *value) {
+		char *end;
+		unsigned long val = strtoul(value, &end, 0);
+		while (isspace(*end))
+			end++;
+		if (!*end)
+			return val;
+		if (!strcasecmp(end, "k")
+			|| !strcasecmp(end, "kb")
+			|| !strcasecmp(end, "kib"))
+			return val * 1024;
+		if (!strcasecmp(end, "m")
+			|| !strcasecmp(end, "mb")
+			|| !strcasecmp(end, "mib"))
+			return val * 1024 * 1024;
+		if (!strcasecmp(end, "g")
+			|| !strcasecmp(end, "gb")
+			|| !strcasecmp(end, "gib"))
+			return val * 1024 * 1024 * 1024;
+	}
+	die("bad config value for '%s' in %s", name, config_file_name);
+}
+
 int git_default_config(const char *var, const char *value)
 {
 	/* This needs a better name */
-- 
1.5.0.rc0.g6bb1

^ permalink raw reply related

* [PATCH 2/2] Support common unit suffixes in packedGitWindowSize.
From: Shawn O. Pearce @ 2006-12-31  2:02 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <bc77965370c83bccc9b04c68911ab7e9d7d83d58.1167530501.git.spearce@spearce.org>

Make use of the new git_config_datasize function to parse the value
for core.packedGitWindowSize and core.packedGitLimit, as these
need to be a bytecount internally but the user will probably want
to specify them with standard units, such as "32 MiB".

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 Applies on top of sp/mmap.

 Documentation/config.txt |   10 ++++++++--
 config.c                 |    6 +++---
 2 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index d71653d..17d5b53 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -123,21 +123,27 @@ core.packedGitWindowSize::
 	single mapping operation.  Larger window sizes may allow
 	your system to process a smaller number of large pack files
 	more quickly.  Smaller window sizes will negatively affect
-	performance due to increased calls to the opreating system's
+	performance due to increased calls to the operating system's
 	memory manager, but may improve performance when accessing
 	a large number of large pack files.  Default is 32 MiB,
 	which should be reasonable for all users/operating systems.
 	You probably do not need to adjust this value.
 
+	Common unit suffixes of 'k', 'kb', 'kib', 'm', 'mb', 'mib',
+	'g', 'gb', 'gib' are supported.
+
 core.packedGitLimit::
 	Maximum number of bytes to map simultaneously into memory
 	from pack files.  If Git needs to access more than this many
 	bytes at once to complete an operation it will unmap existing
 	regions to reclaim virtual address space within the process.
 	Default is 256 MiB, which should be reasonable for all
-	users/operating systems, except on largest Git projects.
+	users/operating systems, except on the largest projects.
 	You probably do not need to adjust this value.
 
+	Common unit suffixes of 'k', 'kb', 'kib', 'm', 'mb', 'mib',
+	'g', 'gb', 'gib' are supported.
+
 alias.*::
 	Command aliases for the gitlink:git[1] command wrapper - e.g.
 	after defining "alias.last = cat-file commit HEAD", the invocation
diff --git a/config.c b/config.c
index 07ad2f1..a9f4497 100644
--- a/config.c
+++ b/config.c
@@ -324,8 +324,8 @@ int git_default_config(const char *var, const char *value)
 	}
 
 	if (!strcmp(var, "core.packedgitwindowsize")) {
-		int pgsz = getpagesize();
-		packed_git_window_size = git_config_int(var, value);
+		unsigned long pgsz = getpagesize();
+		packed_git_window_size = git_config_datasize(var, value);
 		packed_git_window_size /= pgsz;
 		if (packed_git_window_size < 2)
 			packed_git_window_size = 2;
@@ -334,7 +334,7 @@ int git_default_config(const char *var, const char *value)
 	}
 
 	if (!strcmp(var, "core.packedgitlimit")) {
-		packed_git_limit = git_config_int(var, value);
+		packed_git_limit = git_config_datasize(var, value);
 		return 0;
 	}
 
-- 
1.5.0.rc0.g6bb1

^ permalink raw reply related

* Re: What commands can and can not be used with bare repositories?
From: Theodore Tso @ 2006-12-31  2:12 UTC (permalink / raw)
  To: Shawn Pearce; +Cc: git
In-Reply-To: <20061231015732.GB5082@spearce.org>

On Sat, Dec 30, 2006 at 08:57:32PM -0500, Shawn Pearce wrote:
> Try "git --bare log".  Or "git --git-dir=/path/to log".
>
> Actually most commands work on a bare repository.
> Very few don't: the ones that require a working directory.
> E.g. status/revert/cherry-pick/commit/am/merge/pull.  (You can
> pull from a bare repository, but you cannot run pull *in* a bare
> repository.)

Ah, right.  Thanks, I missed the --bare option.  It should probably be
mentioned in the git-clone man page, instead of only in the top-level
git manpage.

> > confused, but maybe we could fix that.  What if we were to change "git
> > clone --bare" to create the .git -> . symlink, and then add a check to
> > commands that require a working directory to see if ".git" is a symlink
> > to ., and if so, give an error message, "operation not supported on bare
> > repository"?
> 
> No.  Better would be to make git's repository setup logic
> automatically detect if "." is a Git repository, and if so let the
> commands that work without a working directory run.

That makes sense, although the hueristic for determining whether or
not "." is a Git repository might be a little interesting.  Say, if
there is no containing directory which has a .git directory, and the
directories "objects", "info", and "refs" are present?

						- Ted

^ permalink raw reply

* Re: SIGSEGV in merge recursive
From: Luben Tuikov @ 2006-12-31  2:13 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Junio C Hamano, git
In-Reply-To: <Pine.LNX.4.63.0612301204280.19693@wbgn013.biozentrum.uni-wuerzburg.de>

--- Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> Besides, Alexandre hit an interesting bug, which is not at all easy to 
> reproduce (except with three 70k files which I don't want to include in 
> the test set). Since Alexandre provided _examples_ where I can _reproduce_ 
> the problem, I will be working on that bug, and not yours.

After compiling and installing "master" I was able to do the pull/merge.
The only conflict was in .gitignore, and it was that same conflict that
was causing the segfault.  Normally (99% of the time) I pull
"next" into branchA (git-upstream), then I pull branchA into branchB (git-lt-work).
Both have various fixes, but I normally install and use branchB (git-lt-work).

It appears that that fix was in "next" which I hadn't already merged, and running
with an old "next" didn't allow me to the the "merge" since the fix for it was
in the "next" I was trying to merge...

Thus I needed to install "master" as is ("next" as is, would've worked just
as well), which contains the fix for the merge, so that I can merge the
branch which contains the fix ("next"/branchA).

I tested this and tried the exact same merge, with the newly merged and installed
branchB/git-lt-work, and it did work.

So it's all good now.  Thanks!
   Luben

^ permalink raw reply

* Re: SIGSEGV in merge recursive
From: Junio C Hamano @ 2006-12-31  2:30 UTC (permalink / raw)
  To: ltuikov; +Cc: Johannes Schindelin, git
In-Reply-To: <10871.51671.qm@web31801.mail.mud.yahoo.com>

Luben Tuikov <ltuikov@yahoo.com> writes:

> So it appears that the fix is in "master", but not in "next".
>
> Junio, can you confirm this?

It was merged into 'next' with 1e48a691, about 2 hours after I
merged the fix 5d6b151f to 'master'.  Sometimes when I have a
fix that is urgent enough, I push out only 'master' before
merging and retesting it in 'next', so it is plausible that your
fetch grabbed such a state.

^ permalink raw reply

* Re: [PATCH 1/2] Teach Git how to parse standard power of 2 suffixes.
From: Junio C Hamano @ 2006-12-31  2:33 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: git
In-Reply-To: <20061231020218.GA5366@spearce.org>

"Shawn O. Pearce" <spearce@spearce.org> writes:

> Sometimes its necessary to supply a value as a power of two in a
> configuration parameter.  In this case the user may want to use
> the standard suffixes such as K, KB, KiB, etc. to indicate that
> the numerical value should be multiplied by a constant base before
> being used.

If you are really going to do this, I think we would need
something similar to --bool to repo-config command.

Also can we fix the definition of core.packedGitWindowSize to be
independent of the page size on a particular platform?

^ permalink raw reply

* Re: [PATCH 1/2] Teach Git how to parse standard power of 2 suffixes.
From: Shawn Pearce @ 2006-12-31  2:38 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vodpkhjzq.fsf@assigned-by-dhcp.cox.net>

Junio C Hamano <junkio@cox.net> wrote:
> "Shawn O. Pearce" <spearce@spearce.org> writes:
> 
> > Sometimes its necessary to supply a value as a power of two in a
> > configuration parameter.  In this case the user may want to use
> > the standard suffixes such as K, KB, KiB, etc. to indicate that
> > the numerical value should be multiplied by a constant base before
> > being used.
> 
> If you are really going to do this, I think we would need
> something similar to --bool to repo-config command.

That shouldn't be difficult.
 
> Also can we fix the definition of core.packedGitWindowSize to be
> independent of the page size on a particular platform?

What do you mean?  mmap() only works in page units, and because of
the way the code is built the minimum size we can allow is 2 pages.

Asking mmap() to map less than a full page in the last page of
a given window is silly, as that is just going to waste virtual
address space or cause pain for the OS, depending on how that
gets implemented.

-- 
Shawn.

^ permalink raw reply

* [PATCH 1/3] Remove unnecessary argc parameter from run_command_v.
From: Shawn O. Pearce @ 2006-12-31  2:55 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

The argc parameter is never used by the run_command_v family of
functions.  Instead they require that the passed argv[] be NULL
terminated so they can rely on the operating system's execvp
function to correctly pass the arguments to the new process.

Making the caller pass the argc is just confusing, as the caller
could be mislead into believing that the argc might take precendece
over the argv, or that the argv does not need to be NULL terminated.
So goodbye argc.  Don't come back.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 builtin-push.c |    2 +-
 receive-pack.c |    4 ++--
 run-command.c  |    8 ++++----
 run-command.h  |    4 ++--
 4 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/builtin-push.c b/builtin-push.c
index b7412e8..7a3d2bb 100644
--- a/builtin-push.c
+++ b/builtin-push.c
@@ -275,7 +275,7 @@ static int do_push(const char *repo)
 		argv[dest_argc] = NULL;
 		if (verbose)
 			fprintf(stderr, "Pushing to %s\n", dest);
-		err = run_command_v(argc, argv);
+		err = run_command_v(argv);
 		if (!err)
 			continue;
 		switch (err) {
diff --git a/receive-pack.c b/receive-pack.c
index eea7e48..af05a61 100644
--- a/receive-pack.c
+++ b/receive-pack.c
@@ -187,7 +187,7 @@ static void run_update_post_hook(struct command *cmd)
 		argc++;
 	}
 	argv[argc] = NULL;
-	run_command_v_opt(argc, argv, RUN_COMMAND_NO_STDIO);
+	run_command_v_opt(argv, RUN_COMMAND_NO_STDIO);
 }
 
 /*
@@ -283,7 +283,7 @@ static const char *unpack(void)
 		unpacker[0] = "unpack-objects";
 		unpacker[1] = hdr_arg;
 		unpacker[2] = NULL;
-		code = run_command_v_opt(1, unpacker, RUN_GIT_CMD);
+		code = run_command_v_opt(unpacker, RUN_GIT_CMD);
 		switch (code) {
 		case 0:
 			return NULL;
diff --git a/run-command.c b/run-command.c
index 492ad3e..d0330c3 100644
--- a/run-command.c
+++ b/run-command.c
@@ -2,7 +2,7 @@
 #include "run-command.h"
 #include "exec_cmd.h"
 
-int run_command_v_opt(int argc, const char **argv, int flags)
+int run_command_v_opt(const char **argv, int flags)
 {
 	pid_t pid = fork();
 
@@ -46,9 +46,9 @@ int run_command_v_opt(int argc, const char **argv, int flags)
 	}
 }
 
-int run_command_v(int argc, const char **argv)
+int run_command_v(const char **argv)
 {
-	return run_command_v_opt(argc, argv, 0);
+	return run_command_v_opt(argv, 0);
 }
 
 int run_command(const char *cmd, ...)
@@ -69,5 +69,5 @@ int run_command(const char *cmd, ...)
 	va_end(param);
 	if (MAX_RUN_COMMAND_ARGS <= argc)
 		return error("too many args to run %s", cmd);
-	return run_command_v_opt(argc, argv, 0);
+	return run_command_v_opt(argv, 0);
 }
diff --git a/run-command.h b/run-command.h
index 70b477a..82a0861 100644
--- a/run-command.h
+++ b/run-command.h
@@ -13,8 +13,8 @@ enum {
 
 #define RUN_COMMAND_NO_STDIO 1
 #define RUN_GIT_CMD	     2	/*If this is to be git sub-command */
-int run_command_v_opt(int argc, const char **argv, int opt);
-int run_command_v(int argc, const char **argv);
+int run_command_v_opt(const char **argv, int opt);
+int run_command_v(const char **argv);
 int run_command(const char *cmd, ...);
 
 #endif
-- 
1.5.0.rc0.g6bb1

^ permalink raw reply related

* [PATCH 2/3] Redirect update hook stdout to stderr.
From: Shawn O. Pearce @ 2006-12-31  2:55 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <a0aecffe21074288c911c396f92901bfb558d591.1167533707.git.spearce@spearce.org>

If an update hook outputs to stdout then that output will be sent
back over the wire to the push client as though it were part of
the git protocol.  This tends to cause protocol errors on the
client end of the connection, as the hook output is not expected
in that context.  Most hook developers work around this by making
sure their hook outputs everything to stderr.

But hooks shouldn't need to perform such special behavior.  Instead
we can just dup stderr to stdout prior to invoking the update hook.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 receive-pack.c |    3 ++-
 run-command.c  |   32 ++++++++++++++++++++++++++------
 run-command.h  |    2 ++
 3 files changed, 30 insertions(+), 7 deletions(-)

diff --git a/receive-pack.c b/receive-pack.c
index af05a61..64289e9 100644
--- a/receive-pack.c
+++ b/receive-pack.c
@@ -73,7 +73,8 @@ static int run_update_hook(const char *refname,
 
 	if (access(update_hook, X_OK) < 0)
 		return 0;
-	code = run_command(update_hook, refname, old_hex, new_hex, NULL);
+	code = run_command_opt(RUN_COMMAND_STDOUT_TO_STDERR,
+		update_hook, refname, old_hex, new_hex, NULL);
 	switch (code) {
 	case 0:
 		return 0;
diff --git a/run-command.c b/run-command.c
index d0330c3..7e4ca43 100644
--- a/run-command.c
+++ b/run-command.c
@@ -14,7 +14,8 @@ int run_command_v_opt(const char **argv, int flags)
 			dup2(fd, 0);
 			dup2(fd, 1);
 			close(fd);
-		}
+		} else if (flags & RUN_COMMAND_STDOUT_TO_STDERR)
+			dup2(2, 1);
 		if (flags & RUN_GIT_CMD) {
 			execv_git_cmd(argv);
 		} else {
@@ -51,14 +52,12 @@ int run_command_v(const char **argv)
 	return run_command_v_opt(argv, 0);
 }
 
-int run_command(const char *cmd, ...)
+static int run_command_va_opt(int opt, const char *cmd, va_list param)
 {
 	int argc;
 	const char *argv[MAX_RUN_COMMAND_ARGS];
 	const char *arg;
-	va_list param;
 
-	va_start(param, cmd);
 	argv[0] = (char*) cmd;
 	argc = 1;
 	while (argc < MAX_RUN_COMMAND_ARGS) {
@@ -66,8 +65,29 @@ int run_command(const char *cmd, ...)
 		if (!arg)
 			break;
 	}
-	va_end(param);
 	if (MAX_RUN_COMMAND_ARGS <= argc)
 		return error("too many args to run %s", cmd);
-	return run_command_v_opt(argv, 0);
+	return run_command_v_opt(argv, opt);
+}
+
+int run_command_opt(int opt, const char *cmd, ...)
+{
+	va_list params;
+	int r;
+
+	va_start(params, cmd);
+	r = run_command_va_opt(opt, cmd, params);
+	va_end(params);
+	return r;
+}
+
+int run_command(const char *cmd, ...)
+{
+	va_list params;
+	int r;
+
+	va_start(params, cmd);
+	r = run_command_va_opt(0, cmd, params);
+	va_end(params);
+	return r;
 }
diff --git a/run-command.h b/run-command.h
index 82a0861..8156eac 100644
--- a/run-command.h
+++ b/run-command.h
@@ -13,8 +13,10 @@ enum {
 
 #define RUN_COMMAND_NO_STDIO 1
 #define RUN_GIT_CMD	     2	/*If this is to be git sub-command */
+#define RUN_COMMAND_STDOUT_TO_STDERR 4
 int run_command_v_opt(const char **argv, int opt);
 int run_command_v(const char **argv);
+int run_command_opt(int opt, const char *cmd, ...);
 int run_command(const char *cmd, ...);
 
 #endif
-- 
1.5.0.rc0.g6bb1

^ permalink raw reply related


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