* Re: [PATCH 01/14] Change git_connect() to return a struct child_process instead of a pid_t.
From: Johannes Schindelin @ 2007-10-14 0:57 UTC (permalink / raw)
To: Johannes Sixt; +Cc: gitster, git
In-Reply-To: <1192305984-22594-2-git-send-email-johannes.sixt@telecom.at>
Hi,
On Sat, 13 Oct 2007, Johannes Sixt wrote:
> -int finish_connect(pid_t pid)
> +int finish_connect(struct child_process *conn)
> {
> - if (pid == 0)
> + if (conn == NULL)
> return 0;
>
> - while (waitpid(pid, NULL, 0) < 0) {
> + while (waitpid(conn->pid, NULL, 0) < 0) {
> if (errno != EINTR)
> return -1;
Just for completeness' sake: could you do a free(conn); before return -1;?
Thanks,
Dscho
^ permalink raw reply
* Re: Addition of "xmlto" to install documentation
From: Johannes Schindelin @ 2007-10-14 0:43 UTC (permalink / raw)
To: Markus Elfring; +Cc: git
In-Reply-To: <47112DAA.5080701@web.de>
Hi,
On Sat, 13 Oct 2007, Markus Elfring wrote:
> I have cloned the current Git release to my computer. I resolved all
> dependencies that were mentioned in the file "INSTALL". But when I've
> tried "make install install-doc" I got the message that "xmlto" was not
> found on my openSUSE 10.3 system. (I have installed it now.) Would you
> like to add this tool to the system requirements in the documentation?
Well, it is not strictly necessary to build git, and not even to install
it, if you have the "man" branch.
Ciao,
Dscho
^ permalink raw reply
* Re: Git User's Survey 2007 unfinished summary continued
From: Johannes Schindelin @ 2007-10-14 0:36 UTC (permalink / raw)
To: J. Bruce Fields; +Cc: Jakub Narebski, git
In-Reply-To: <20071013202713.GA2467@fieldses.org>
Hi,
On Sat, 13 Oct 2007, J. Bruce Fields wrote:
> On Sat, Oct 13, 2007 at 09:59:41PM +0200, David Kastrup wrote:
> > Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> >
> ...survey quote:
> > >> Figure out why people find git hard to learn and eliminate those
> > >> barriers to entry. Make git more task-oriented rather than
> > >> data-model-oriented the way it is now.
> > >
> > > Frankly, expectations like these make me want to bang somebody's
> > > head on the wall.
> >
> > And you wonder that people are unwilling to ask for things on the
> > list?
That is utter rubbish.
> Well, he does have a point that they could have been more specific.
>
> But, yes, "I wish we could get people to be more specific" might be the
> better way to put it.
Yes, there are politer ways to phrase it.
My main point is -- and always was -- that I'd like people to realise how
much it depends on _them_ if (and when) their wishes come true.
Ciao,
Dscho
^ permalink raw reply
* Re: Imports without Tariffs
From: Michael Witten @ 2007-10-13 23:04 UTC (permalink / raw)
To: git; +Cc: Jeff King
In-Reply-To: <20071013075723.GA27533@coredump.intra.peff.net>
On 13 Oct 2007, at 3:57:23 AM, Jeff King wrote:
> On Sat, Oct 13, 2007 at 12:30:09AM -0400, Michael Witten wrote:
>
>>> except that git-rebase is smart enough to realize that C == C'
>>> and skip
>>> it (so it's a "safe" way of moving forward).
>>
>> This is good to know! The documentation should mention this case!
>
> Yes, it probably should. Can you submit a patch describing the
> behavior
> where you think it ought to go?
I can make a patch, but at the moment I'm swamped and I don't want to
think
about doing that.
I'll get around to it eventually, I hope.
Do I just submit the patch to this list? How do I know it will be used?
>> Basically, the imported cvs history should be treated like
>> a remote that's being tracked. It seems like the solution
>> I proposed kind of does this and would work for other SCM
>> imports too.
>
> I think it's an interesting avenue to pursue, though I would worry a
> little about robustness. I like the fact that after rebasing, the
> commits have done a complete git->cvs->git loop and look identical
Frankly, I don't know how robust my idea is either, but it's simple
enough not to have many problems lurking in the shadows.
It would certainly be more useful than not.
> As an alternate idea, why not try to have the CVS commit contain all
> information necessary to create a particular git commit. IOW, describe
> all of the data that goes into the commit hash as textual comments in
> the CVS commit (committer name/time, author name/time). And then
> theoretically git-cvsimport can reconstruct the exact same commits
> again, and your git->cvs->git merge really _will_ be a fastforward.
I considered this too, but this exposes what we're doing. We don't
want the old farts to wonder what all these hash thingies are.
Michael Witten
^ permalink raw reply
* Re: [PATCH] Fixing path quoting issues
From: Andreas Ericsson @ 2007-10-13 22:36 UTC (permalink / raw)
To: Jonathan del Strother; +Cc: Johannes Sixt, git, Junio C Hamano
In-Reply-To: <92879AC5-2927-439B-8EB0-AC20AAEE412E@steelskies.com>
Jonathan del Strother wrote:
> On 11 Oct 2007, at 07:19, Johannes Sixt wrote:
>
>>> - git-commit -F msg -m amending ."
>>> + git-commit -F msg -m amending ."
>>
>> You fix whitespace...
>>
>>> test_expect_success \
>>> - "using message from other commit" \
>>> - "git-commit -C HEAD^ ."
>>> + "using message from other commit" \
>>> + "git-commit -C HEAD^ ."
>>
>> ... and you break it. More of these follow. Don't do that, it makes
>> patch review unnecessarily hard.
>
>
> I'm just preparing to release this patch... was that "don't break
> whitespace", or "don't try to fix whitespace in a patch that's has
> nothing to do with whitespacing-fixing" ?
>
Both, I think ;-)
> And while I'm here - tabs are preferred, are they? There seem to be a
> mixture of tabs & 4 space indentation.
1 hard tab / level of indent, but use spaces for continuation alignment.
--
Andreas Ericsson andreas.ericsson@op5.se
OP5 AB www.op5.se
Tel: +46 8-230225 Fax: +46 8-230231
^ permalink raw reply
* Re: Addition of "xmlto" to install documentation
From: Alex Riesen @ 2007-10-13 22:29 UTC (permalink / raw)
To: Markus Elfring; +Cc: git
In-Reply-To: <47112DAA.5080701@web.de>
Markus Elfring, Sat, Oct 13, 2007 22:42:18 +0200:
>
> I have cloned the current Git release to my computer. I resolved all
> dependencies that were mentioned in the file "INSTALL". But when I've
> tried "make install install-doc" I got the message that "xmlto" was not
> found on my openSUSE 10.3 system. (I have installed it now.)
> Would you like to add this tool to the system requirements in the
> documentation?
>
It is already there, in the same INSTALL file:
- To build and install documentation suite, you need to have
the asciidoc/xmlto toolchain. Because not many people are
inclined to install the tools, the default build target
("make all") does _not_ build them.
^ permalink raw reply
* Re: [PATCH] Color support added to git-add--interactive.
From: Jean-Luc Herren @ 2007-10-13 22:23 UTC (permalink / raw)
To: Dan Z, git
In-Reply-To: <cff973550710131450r3b54a328k8db97488f4b50e2a@mail.gmail.com>
Dan Z wrote:
> I think color.add is better, because git-add--interactive goes beyond
> coloring diffs. When this is complete, it should probably use
> color.diff.<slot> for the actual diff output, and color.add.<slot> for
> colored prompts/commands.
Or maybe rather color.interactive.<slot>, where <slot> could be
'prompt', 'header', etc. It's better to give it a name that
describes what it is for, instead of one that describes which tool
uses it. This way it could possibly be used for other potential
interactive tools in the future.
jlh
^ permalink raw reply
* Re: [PATCH] Add a simple option parser.
From: Alex Riesen @ 2007-10-13 22:14 UTC (permalink / raw)
To: Pierre Habouzit, git, Junio C Hamano
In-Reply-To: <20071013205404.GK7110@artemis.corp>
Pierre Habouzit, Sat, Oct 13, 2007 22:54:04 +0200:
> On Sat, Oct 13, 2007 at 07:16:55PM +0000, Alex Riesen wrote:
> > Pierre Habouzit, Sat, Oct 13, 2007 15:29:03 +0200:
> > BTW, if you just printed the usage message out (it is about usage of a
> > program, isn't it?) and called exit() everyone would be just as happy.
> > And you wouldn't have to include strbuf (it is the only use of it),
> > less code, too. It'd make simplier to stea^Wcopy your implementation,
> > which I like :)
>
> the reason is that usage() is a wrapper around a callback, and I
> suppose it's used by some GUI's or anything like that.
It is not. Not yet. What could they use a usage text for?
Besides, you could just export the callback (call_usage_callback or
something) from usage.c and call it.
> FWIW you can rework the .c like this:
on top of yours:
From: Alex Riesen <raa.lkml@gmail.com>
Date: Sun, 14 Oct 2007 00:10:51 +0200
Subject: [PATCH] Rework make_usage to print the usage message immediately
Signed-off-by: Alex Riesen <raa.lkml@gmail.com>
---
parse-options.c | 60 ++++++++++++++++++++++++------------------------------
1 files changed, 27 insertions(+), 33 deletions(-)
diff --git a/parse-options.c b/parse-options.c
index 07abb50..1e3940f 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -1,6 +1,5 @@
#include "git-compat-util.h"
#include "parse-options.h"
-#include "strbuf.h"
#define OPT_SHORT 1
#define OPT_UNSET 2
@@ -171,57 +170,52 @@ int parse_options(int argc, const char **argv,
void make_usage(const char * const usagestr[], struct option *opts, int cnt)
{
- struct strbuf sb;
-
- strbuf_init(&sb, 4096);
- do {
- strbuf_addstr(&sb, *usagestr++);
- strbuf_addch(&sb, '\n');
- } while (*usagestr);
+ fprintf(stderr, "usage: ");
+ while (*usagestr)
+ fprintf(stderr, "%s\n", *usagestr++);
if (cnt && opts->type != OPTION_GROUP)
- strbuf_addch(&sb, '\n');
+ fputc('\n', stderr);
for (; cnt-- > 0; opts++) {
size_t pos;
if (opts->type == OPTION_GROUP) {
- strbuf_addch(&sb, '\n');
+ fputc('\n', stderr);
if (*opts->help)
- strbuf_addf(&sb, "%s\n", opts->help);
+ fprintf(stderr, "%s\n", opts->help);
continue;
}
- pos = sb.len;
- strbuf_addstr(&sb, " ");
- if (opts->short_name) {
- strbuf_addf(&sb, "-%c", opts->short_name);
- }
- if (opts->long_name) {
- strbuf_addf(&sb, opts->short_name ? ", --%s" : "--%s",
- opts->long_name);
- }
+ pos = fprintf(stderr, " ");
+ if (opts->short_name)
+ pos += fprintf(stderr, "-%c", opts->short_name);
+ if (opts->long_name)
+ pos += fprintf(stderr,
+ opts->short_name ? ", --%s" : "--%s",
+ opts->long_name);
switch (opts->type) {
case OPTION_INTEGER:
- strbuf_addstr(&sb, " <n>");
+ fputs(" <n>", stderr);
+ pos += 4;
break;
case OPTION_STRING:
- if (opts->argh) {
- strbuf_addf(&sb, " <%s>", opts->argh);
- } else {
- strbuf_addstr(&sb, " ...");
+ if (opts->argh)
+ pos += fprintf(stderr, " <%s>", opts->argh);
+ else {
+ fputs(" ...", stderr);
+ pos += 4;
}
break;
default:
break;
}
- if (sb.len - pos <= USAGE_OPTS_WIDTH) {
- int pad = USAGE_OPTS_WIDTH - (sb.len - pos) + USAGE_GAP;
- strbuf_addf(&sb, "%*s%s\n", pad, "", opts->help);
- } else {
- strbuf_addf(&sb, "\n%*s%s\n", USAGE_OPTS_WIDTH + USAGE_GAP, "",
- opts->help);
- }
+ if (pos <= USAGE_OPTS_WIDTH) {
+ int pad = USAGE_OPTS_WIDTH - pos + USAGE_GAP;
+ fprintf(stderr, "%*s%s\n", pad, "", opts->help);
+ } else
+ fprintf(stderr, "\n%*s%s\n",
+ USAGE_OPTS_WIDTH + USAGE_GAP, "", opts->help);
}
- usage(sb.buf);
+ exit(129);
}
--
1.5.3.4.232.ga843
^ permalink raw reply related
* Re: [PATCH] Color support added to git-add--interactive.
From: Dan Z @ 2007-10-13 21:50 UTC (permalink / raw)
To: Wincent Colaiuta
Cc: Jeff King, Git Mailing List, Jonathan del Strother,
Johannes Schindelin, Frank Lichtenheld
In-Reply-To: <8DDFBF9A-2C68-404B-843C-BE63C52F0DAF@wincent.com>
On 10/13/07, Wincent Colaiuta <win@wincent.com> wrote:
> Or could you just piggy-back on the settings for color.diff.<slot>?
>
> And if a separate group for git-add is necessary, perhaps "add" would
> be enough, rather than "add-interactive".
>
> Wincent
>
I think color.add is better, because git-add--interactive goes beyond
coloring diffs. When this is complete, it should probably use
color.diff.<slot> for the actual diff output, and color.add.<slot> for
colored prompts/commands.
Dan
^ permalink raw reply
* Re: Git User's Survey 2007 unfinished summary continued
From: David Kastrup @ 2007-10-13 20:57 UTC (permalink / raw)
To: J. Bruce Fields; +Cc: Johannes Schindelin, Jakub Narebski, git
In-Reply-To: <20071013202713.GA2467@fieldses.org>
"J. Bruce Fields" <bfields@fieldses.org> writes:
> On Sat, Oct 13, 2007 at 09:59:41PM +0200, David Kastrup wrote:
>> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>>
> ...survey quote:
>> >> Figure out why people find git hard to learn and eliminate those
>> >> barriers to entry. Make git more task-oriented rather than
>> >> data-model-oriented the way it is now.
>> >
>> > Frankly, expectations like these make me want to bang somebody's
>> > head on the wall.
>>
>> And you wonder that people are unwilling to ask for things on the
>> list? When even mentioning something in a _survey_ makes core
>> developers want to bang their heads against a wall?
>
> Well, he does have a point that they could have been more specific.
He did not write "vague phrasings like this". He wrote
"_expectations_ like these make [him] want to bang somebody's head on
the wall".
> But, yes, "I wish we could get people to be more specific" might be
> the better way to put it.
It is something entirely different.
--
David Kastrup, Kriemhildstr. 15, 44793 Bochum
^ permalink raw reply
* Re: [PATCH] Add a simple option parser.
From: Pierre Habouzit @ 2007-10-13 20:54 UTC (permalink / raw)
To: Alex Riesen; +Cc: git, Junio C Hamano
In-Reply-To: <20071013191655.GA2875@steel.home>
[-- Attachment #1: Type: text/plain, Size: 2206 bytes --]
On Sat, Oct 13, 2007 at 07:16:55PM +0000, Alex Riesen wrote:
> Pierre Habouzit, Sat, Oct 13, 2007 15:29:03 +0200:
> > [...]
>
> "const struct option *opts"?
>
> Why not "const char *const *usagestr"? Especially if you change
> "usagestr" (the pointer itself) later. "[]" is sometimes a hint that
> the pointer itself should not be changed, being an array.
>
> And you want make opts const.
Ok.
> BTW, it does not "make" usage. It calls the usage() or prints a usage
> description. "make" implies it creates the "usage", which according to
> the prototype is later nowhere to be found.
Yes this has been spotted and fixed already.
>
> > +{
> > + struct strbuf sb;
> > +
> > + strbuf_init(&sb, 4096);
> > + do {
> > + strbuf_addstr(&sb, *usagestr++);
> > + strbuf_addch(&sb, '\n');
> > + } while (*usagestr);
>
> This will crash for empty usagestr, like "{ NULL }". Was it
> deliberately? (I'd make it deliberately, if I were you. I'd even used
> cnt of opts, to force people to document all options).
Yes this is intentional, there should be at least on string in the
usagestr array.
> > + strbuf_addf(&sb, "\n%*s%s\n", USAGE_OPTS_WIDTH + USAGE_GAP, "",
> > + opts->help);
> ....
> > + usage(sb.buf);
>
> BTW, if you just printed the usage message out (it is about usage of a
> program, isn't it?) and called exit() everyone would be just as happy.
> And you wouldn't have to include strbuf (it is the only use of it),
> less code, too. It'd make simplier to stea^Wcopy your implementation,
> which I like :)
the reason is that usage() is a wrapper around a callback, and I
suppose it's used by some GUI's or anything like that.
FWIW you can rework the .c like this:
pos = 0; /* and not pos = sb.len */
replace the strbuf_add* by the equivalents:
pos += printf("....");
and tada, you're done.
Note that in the most recent version, I also deal with a
OPTION_CALLBACK that passes the value to a callback.
Cheers,
--
·O· Pierre Habouzit
··O madcoder@debian.org
OOO http://www.madism.org
[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* Addition of "xmlto" to install documentation
From: Markus Elfring @ 2007-10-13 20:42 UTC (permalink / raw)
To: git
Hello,
I have cloned the current Git release to my computer. I resolved all
dependencies that were mentioned in the file "INSTALL". But when I've
tried "make install install-doc" I got the message that "xmlto" was not
found on my openSUSE 10.3 system. (I have installed it now.)
Would you like to add this tool to the system requirements in the
documentation?
Regards,
Markus
^ permalink raw reply
* Re: [PATCH] Color support added to git-add--interactive.
From: Wincent Colaiuta @ 2007-10-13 20:36 UTC (permalink / raw)
To: Dan Zwell
Cc: Jeff King, Git Mailing List, Jonathan del Strother,
Johannes Schindelin, Frank Lichtenheld
In-Reply-To: <47112491.8070309@gmail.com>
El 13/10/2007, a las 22:03, Dan Zwell escribió:
> The importance of the diff coloring pales in comparison to the
> prompt coloring. Diff coloring is useful, but prompt coloring is a
> basic usability concern (if people can't easily tell where a hunk
> begins, the tool becomes annoying). Perhaps we could split this
> into two patches, merging the first after a few small changes can
> be taken care of, while the second may need more discussion and
> testing. The coloring of the prompts is relatively low risk. It
> just needs to be modified to take color settings from .git/config.
> I was thinking that this might be the example that I would take
> settings from:
>
> [color]
> add-interactive = auto
> [color "add-interactive"]
> prompt = bold blue
> header = bold
> help = blue
Or could you just piggy-back on the settings for color.diff.<slot>?
And if a separate group for git-add is necessary, perhaps "add" would
be enough, rather than "add-interactive".
Wincent
^ permalink raw reply
* Re: Git User's Survey 2007 unfinished summary continued
From: J. Bruce Fields @ 2007-10-13 20:27 UTC (permalink / raw)
To: David Kastrup; +Cc: Johannes Schindelin, Jakub Narebski, git
In-Reply-To: <853awepyz6.fsf@lola.goethe.zz>
On Sat, Oct 13, 2007 at 09:59:41PM +0200, David Kastrup wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
...survey quote:
> >> Figure out why people find git hard to learn and eliminate those
> >> barriers to entry. Make git more task-oriented rather than
> >> data-model-oriented the way it is now.
> >
> > Frankly, expectations like these make me want to bang somebody's
> > head on the wall.
>
> And you wonder that people are unwilling to ask for things on the
> list? When even mentioning something in a _survey_ makes core
> developers want to bang their heads against a wall?
Well, he does have a point that they could have been more specific.
But, yes, "I wish we could get people to be more specific" might be the
better way to put it.
--b.
^ permalink raw reply
* Re: [PATCH] Port builtin-add.c to use the new option parser.
From: Pierre Habouzit @ 2007-10-13 20:27 UTC (permalink / raw)
To: Alex Riesen
Cc: Johannes Schindelin, git, Junio C Hamano, Kristian Høgsberg
In-Reply-To: <20071013192213.GB2875@steel.home>
[-- Attachment #1: Type: text/plain, Size: 1177 bytes --]
On Sat, Oct 13, 2007 at 07:22:13PM +0000, Alex Riesen wrote:
> Pierre Habouzit, Sat, Oct 13, 2007 17:03:06 +0200:
> > On Sat, Oct 13, 2007 at 02:47:20PM +0000, Johannes Schindelin wrote:
> > > Thinking about this more, I am reverting my stance on the ARRAY_SIZE()
> > > issue. I think if you introduce a "OPTION_NONE = 0" in the enum, then
> > > this single last comma should be enough.
> >
> > adding a trailing comma does not add a NULL after that, it's ignored,
> > you're confused.
>
> Yep
>
> > Note that I don't really like using ARRAY_SIZE either, I kept it that
> > way, but my taste would rather be to have an "empty" option, and
> > explicitely mark the end of the array.
>
> You can have both. Just stop at NULL-entry or when the 'size' elements
> passed, whatever happens first.
Well I dislike the "count" thing, and Dscho agreed that it somehow
sucked too. If you go see the current state of the ph/parseopt series
you'll see it's not here anymore.
--
·O· Pierre Habouzit
··O madcoder@debian.org
OOO http://www.madism.org
[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* Re: [PATCH] Color support added to git-add--interactive.
From: Tom Tobin @ 2007-10-13 20:26 UTC (permalink / raw)
To: Git Mailing List
In-Reply-To: <1192306873.6103.14.camel@athena>
On Sat, 2007-10-13 at 15:21 -0500, Tom Tobin wrote:
> Meh, I really need to start posting the stuff I've hacked into git.
> First the git-stash changes, now this. Sigh. ^_^
>
> I have a variant of git-add--interactive that properly adds coloration
> to diffs, taking the config file values already set for the color.diff
> key and colorizing the unadorned diffs internally (rather than expecting
> the output of git-diff/git-diff-files to be colorized).
>
> Give me a couple of hours (still setting up my Macbook after repaving it
> and installing Ubuntu) and I'll post what I've got for others to tear
> apart and point out where I screwed up. ;)
... and now Evolution is screwing up my From: address (it should be
"korpios@korpios.com"; probably since I'm routing everything through
Google Apps). Ah well, one more thing to fix first....
^ permalink raw reply
* Re: [PATCH] Color support added to git-add--interactive.
From: Tom Tobin @ 2007-10-13 20:21 UTC (permalink / raw)
To: Dan Zwell; +Cc: Git Mailing List
In-Reply-To: <471045DA.5050902@gmail.com>
On Fri, 2007-10-12 at 23:13 -0500, Dan Zwell wrote:
> Recently there was some talk of color for git-add--interactive, but the
> person who said he already had a patch didn't produce it.
Meh, I really need to start posting the stuff I've hacked into git.
First the git-stash changes, now this. Sigh. ^_^
I have a variant of git-add--interactive that properly adds coloration
to diffs, taking the config file values already set for the color.diff
key and colorizing the unadorned diffs internally (rather than expecting
the output of git-diff/git-diff-files to be colorized).
Give me a couple of hours (still setting up my Macbook after repaving it
and installing Ubuntu) and I'll post what I've got for others to tear
apart and point out where I screwed up. ;)
^ permalink raw reply
* [PATCH 14/14] Use the asyncronous function infrastructure to run the content filter.
From: Johannes Sixt @ 2007-10-13 20:06 UTC (permalink / raw)
To: gitster; +Cc: git, Johannes Sixt
In-Reply-To: <1192305984-22594-14-git-send-email-johannes.sixt@telecom.at>
Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at>
---
convert.c | 60 +++++++++++++++++++++++++++---------------------------------
1 files changed, 27 insertions(+), 33 deletions(-)
diff --git a/convert.c b/convert.c
index c870817..10161a0 100644
--- a/convert.c
+++ b/convert.c
@@ -201,15 +201,21 @@ static char *crlf_to_worktree(const char *path, const char *src, unsigned long *
return buffer;
}
-static int filter_buffer(int fd, const char *src,
- unsigned long size, const char *cmd)
+struct filter_params {
+ const char *src;
+ unsigned long size;
+ const char *cmd;
+};
+
+static int filter_buffer(int fd, void *data)
{
/*
* Spawn cmd and feed the buffer contents through its stdin.
*/
struct child_process child_process;
+ struct filter_params *params = (struct filter_params *)data;
int write_err, status;
- const char *argv[] = { "sh", "-c", cmd, NULL };
+ const char *argv[] = { "sh", "-c", params->cmd, NULL };
memset(&child_process, 0, sizeof(child_process));
child_process.argv = argv;
@@ -217,17 +223,17 @@ static int filter_buffer(int fd, const char *src,
child_process.out = fd;
if (start_command(&child_process))
- return error("cannot fork to run external filter %s", cmd);
+ return error("cannot fork to run external filter %s", params->cmd);
- write_err = (write_in_full(child_process.in, src, size) < 0);
+ write_err = (write_in_full(child_process.in, params->src, params->size) < 0);
if (close(child_process.in))
write_err = 1;
if (write_err)
- error("cannot feed the input to external filter %s", cmd);
+ error("cannot feed the input to external filter %s", params->cmd);
status = finish_command(&child_process);
if (status)
- error("external filter %s failed %d", cmd, -status);
+ error("external filter %s failed", params->cmd);
return (write_err || status);
}
@@ -241,42 +247,31 @@ static char *apply_filter(const char *path, const char *src,
* (child --> cmd) --> us
*/
const int SLOP = 4096;
- int pipe_feed[2];
- int status;
char *dst;
unsigned long dstsize, dstalloc;
- struct child_process child_process;
+ struct async async;
+ struct filter_params params;
if (!cmd)
return NULL;
- memset(&child_process, 0, sizeof(child_process));
-
- if (pipe(pipe_feed) < 0) {
- error("cannot create pipe to run external filter %s", cmd);
- return NULL;
- }
+ memset(&async, 0, sizeof(async));
+ async.proc = filter_buffer;
+ async.data = ¶ms;
+ params.src = src;
+ params.size = *sizep;
+ params.cmd = cmd;
fflush(NULL);
- child_process.pid = fork();
- if (child_process.pid < 0) {
- error("cannot fork to run external filter %s", cmd);
- close(pipe_feed[0]);
- close(pipe_feed[1]);
- return NULL;
- }
- if (!child_process.pid) {
- close(pipe_feed[0]);
- exit(filter_buffer(pipe_feed[1], src, *sizep, cmd));
- }
- close(pipe_feed[1]);
+ if (start_async(&async))
+ return 0; /* error was already reported */
dstalloc = *sizep;
dst = xmalloc(dstalloc);
dstsize = 0;
while (1) {
- ssize_t numread = xread(pipe_feed[0], dst + dstsize,
+ ssize_t numread = xread(async.out, dst + dstsize,
dstalloc - dstsize);
if (numread <= 0) {
@@ -293,15 +288,14 @@ static char *apply_filter(const char *path, const char *src,
dst = xrealloc(dst, dstalloc);
}
}
- if (close(pipe_feed[0])) {
+ if (close(async.out)) {
error("read from external filter %s failed", cmd);
free(dst);
dst = NULL;
}
- status = finish_command(&child_process);
- if (status) {
- error("external filter %s failed %d", cmd, -status);
+ if (finish_async(&async)) {
+ error("external filter %s failed", cmd);
free(dst);
dst = NULL;
}
--
1.5.3.3.1134.gee562
^ permalink raw reply related
* [PATCH 06/14] Have start_command() create a pipe to read the stderr of the child.
From: Johannes Sixt @ 2007-10-13 20:06 UTC (permalink / raw)
To: gitster; +Cc: git, Johannes Sixt
In-Reply-To: <1192305984-22594-6-git-send-email-johannes.sixt@telecom.at>
This adds another stanza that allocates a pipe that is connected to the
child's stderr and that the caller can read from. In order to request this
pipe, the caller sets cmd->err to -1.
The implementation is not exactly modeled after the stdout case: For stdout
the caller can supply an existing file descriptor, but this facility is
nowhere needed in the stderr case. Additionally, the caller is required to
close cmd->err.
Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at>
---
run-command.c | 26 ++++++++++++++++++++++++--
run-command.h | 1 +
2 files changed, 25 insertions(+), 2 deletions(-)
diff --git a/run-command.c b/run-command.c
index 7e779d3..d00c03b 100644
--- a/run-command.c
+++ b/run-command.c
@@ -17,8 +17,8 @@ static inline void dup_devnull(int to)
int start_command(struct child_process *cmd)
{
- int need_in, need_out;
- int fdin[2], fdout[2];
+ int need_in, need_out, need_err;
+ int fdin[2], fdout[2], fderr[2];
need_in = !cmd->no_stdin && cmd->in < 0;
if (need_in) {
@@ -41,12 +41,26 @@ int start_command(struct child_process *cmd)
cmd->close_out = 1;
}
+ need_err = cmd->err < 0;
+ if (need_err) {
+ if (pipe(fderr) < 0) {
+ if (need_in)
+ close_pair(fdin);
+ if (need_out)
+ close_pair(fdout);
+ return -ERR_RUN_COMMAND_PIPE;
+ }
+ cmd->err = fderr[0];
+ }
+
cmd->pid = fork();
if (cmd->pid < 0) {
if (need_in)
close_pair(fdin);
if (need_out)
close_pair(fdout);
+ if (need_err)
+ close_pair(fderr);
return -ERR_RUN_COMMAND_FORK;
}
@@ -73,6 +87,11 @@ int start_command(struct child_process *cmd)
close(cmd->out);
}
+ if (need_err) {
+ dup2(fderr[1], 2);
+ close_pair(fderr);
+ }
+
if (cmd->dir && chdir(cmd->dir))
die("exec %s: cd to %s failed (%s)", cmd->argv[0],
cmd->dir, strerror(errno));
@@ -102,6 +121,9 @@ int start_command(struct child_process *cmd)
else if (cmd->out > 1)
close(cmd->out);
+ if (need_err)
+ close(fderr[1]);
+
return 0;
}
diff --git a/run-command.h b/run-command.h
index 7958eb1..35b9fb6 100644
--- a/run-command.h
+++ b/run-command.h
@@ -16,6 +16,7 @@ struct child_process {
pid_t pid;
int in;
int out;
+ int err;
const char *dir;
const char *const *env;
unsigned close_in:1;
--
1.5.3.3.1134.gee562
^ permalink raw reply related
* [PATCH 08/14] Add infrastructure to run a function asynchronously.
From: Johannes Sixt @ 2007-10-13 20:06 UTC (permalink / raw)
To: gitster; +Cc: git, Johannes Sixt
In-Reply-To: <1192305984-22594-8-git-send-email-johannes.sixt@telecom.at>
This adds start_async() and finish_async(), which runs a function
asynchronously. Communication with the caller happens only via pipes.
For this reason, this implementation forks off a child process that runs
the function.
Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at>
---
run-command.c | 53 +++++++++++++++++++++++++++++++++++++++++++++--------
run-command.h | 23 +++++++++++++++++++++++
2 files changed, 68 insertions(+), 8 deletions(-)
diff --git a/run-command.c b/run-command.c
index d00c03b..111d584 100644
--- a/run-command.c
+++ b/run-command.c
@@ -127,16 +127,11 @@ int start_command(struct child_process *cmd)
return 0;
}
-int finish_command(struct child_process *cmd)
+static int wait_or_whine(pid_t pid)
{
- if (cmd->close_in)
- close(cmd->in);
- if (cmd->close_out)
- close(cmd->out);
-
for (;;) {
int status, code;
- pid_t waiting = waitpid(cmd->pid, &status, 0);
+ pid_t waiting = waitpid(pid, &status, 0);
if (waiting < 0) {
if (errno == EINTR)
@@ -144,7 +139,7 @@ int finish_command(struct child_process *cmd)
error("waitpid failed (%s)", strerror(errno));
return -ERR_RUN_COMMAND_WAITPID;
}
- if (waiting != cmd->pid)
+ if (waiting != pid)
return -ERR_RUN_COMMAND_WAITPID_WRONG_PID;
if (WIFSIGNALED(status))
return -ERR_RUN_COMMAND_WAITPID_SIGNAL;
@@ -158,6 +153,15 @@ int finish_command(struct child_process *cmd)
}
}
+int finish_command(struct child_process *cmd)
+{
+ if (cmd->close_in)
+ close(cmd->in);
+ if (cmd->close_out)
+ close(cmd->out);
+ return wait_or_whine(cmd->pid);
+}
+
int run_command(struct child_process *cmd)
{
int code = start_command(cmd);
@@ -200,3 +204,36 @@ int run_command_v_opt_cd_env(const char **argv, int opt, const char *dir, const
cmd.env = env;
return run_command(&cmd);
}
+
+int start_async(struct async *async)
+{
+ int pipe_out[2];
+
+ if (pipe(pipe_out) < 0) {
+ return error("cannot create pipe: %s", strerror(errno));
+ }
+
+ async->pid = fork();
+ if (async->pid < 0) {
+ error("fork (async) failed: %s", strerror(errno));
+ close(pipe_out[0]);
+ close(pipe_out[1]);
+ return -1;
+ }
+ if (!async->pid) {
+ close(pipe_out[0]);
+ exit(!!async->proc(pipe_out[1], async->data));
+ }
+ async->out = pipe_out[0];
+ close(pipe_out[1]);
+ return 0;
+}
+
+int finish_async(struct async *async)
+{
+ int ret = 0;
+
+ if (wait_or_whine(async->pid))
+ ret = error("waitpid (async) failed");
+ return ret;
+}
diff --git a/run-command.h b/run-command.h
index 35b9fb6..c5fd2fc 100644
--- a/run-command.h
+++ b/run-command.h
@@ -43,4 +43,27 @@ int run_command_v_opt_cd(const char **argv, int opt, const char *dir);
*/
int run_command_v_opt_cd_env(const char **argv, int opt, const char *dir, const char *const *env);
+/*
+ * The purpose of the following functions is to feed a pipe by running
+ * a function asynchronously and providing output that the call reads
+ * in a different pipe.
+ *
+ * It is expected that no synchronization and mutual exclusion between
+ * the caller and the feed function is necessary so that the function
+ * can run in a thread without interfering with the caller.
+ */
+struct async {
+ /*
+ * proc writes to fd and closes it;
+ * returns 0 on success, non-zero on failure
+ */
+ int (*proc)(int fd, void *data);
+ void *data;
+ int out; /* caller reads from this */
+ pid_t pid;
+};
+
+int start_async(struct async *async);
+int finish_async(struct async *async);
+
#endif
--
1.5.3.3.1134.gee562
^ permalink raw reply related
* [PATCH 07/14] upload-pack: Use start_command() to run pack-objects in create_pack_file().
From: Johannes Sixt @ 2007-10-13 20:06 UTC (permalink / raw)
To: gitster; +Cc: git, Johannes Sixt
In-Reply-To: <1192305984-22594-7-git-send-email-johannes.sixt@telecom.at>
This gets rid of an explicit fork/exec.
Since upload-pack has to coordinate two processes (rev-list and
pack-objects), we cannot use the normal finish_command(), but have to
monitor the processes explicitly. Hence, the waitpid() call remains.
Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at>
---
upload-pack.c | 105 ++++++++++++++++++++++++---------------------------------
1 files changed, 44 insertions(+), 61 deletions(-)
diff --git a/upload-pack.c b/upload-pack.c
index fe96ef1..ef2894a 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -9,6 +9,7 @@
#include "diff.h"
#include "revision.h"
#include "list-objects.h"
+#include "run-command.h"
static const char upload_pack_usage[] = "git-upload-pack [--strict] [--timeout=nn] <dir>";
@@ -98,16 +99,18 @@ static void show_edge(struct commit *commit)
static void create_pack_file(void)
{
- /* Pipes between rev-list to pack-objects, pack-objects to us
- * and pack-objects error stream for progress bar.
+ /* Pipe from rev-list to pack-objects
*/
- int lp_pipe[2], pu_pipe[2], pe_pipe[2];
- pid_t pid_rev_list, pid_pack_objects;
+ int lp_pipe[2];
+ pid_t pid_rev_list;
+ struct child_process pack_objects;
int create_full_pack = (nr_our_refs == want_obj.nr && !have_obj.nr);
char data[8193], progress[128];
char abort_msg[] = "aborting due to possible repository "
"corruption on the remote side.";
int buffered = -1;
+ const char *argv[10];
+ int arg = 0;
if (pipe(lp_pipe) < 0)
die("git-upload-pack: unable to create pipe");
@@ -154,52 +157,32 @@ static void create_pack_file(void)
exit(0);
}
- if (pipe(pu_pipe) < 0)
- die("git-upload-pack: unable to create pipe");
- if (pipe(pe_pipe) < 0)
- die("git-upload-pack: unable to create pipe");
- pid_pack_objects = fork();
- if (pid_pack_objects < 0) {
+ /* close this so that it is not inherited by pack_objects */
+ close(lp_pipe[1]);
+
+ argv[arg++] = "pack-objects";
+ argv[arg++] = "--stdout";
+ if (!no_progress)
+ argv[arg++] = "--progress";
+ if (use_ofs_delta)
+ argv[arg++] = "--delta-base-offset";
+ argv[arg++] = NULL;
+
+ memset(&pack_objects, 0, sizeof(pack_objects));
+ pack_objects.in = lp_pipe[0]; /* start_command closes it */
+ pack_objects.out = -1;
+ pack_objects.err = -1;
+ pack_objects.git_cmd = 1;
+ pack_objects.argv = argv;
+ if (start_command(&pack_objects)) {
/* daemon sets things up to ignore TERM */
kill(pid_rev_list, SIGKILL);
die("git-upload-pack: unable to fork git-pack-objects");
}
- if (!pid_pack_objects) {
- const char *argv[10];
- int i = 0;
-
- dup2(lp_pipe[0], 0);
- dup2(pu_pipe[1], 1);
- dup2(pe_pipe[1], 2);
-
- close(lp_pipe[0]);
- close(lp_pipe[1]);
- close(pu_pipe[0]);
- close(pu_pipe[1]);
- close(pe_pipe[0]);
- close(pe_pipe[1]);
-
- argv[i++] = "pack-objects";
- argv[i++] = "--stdout";
- if (!no_progress)
- argv[i++] = "--progress";
- if (use_ofs_delta)
- argv[i++] = "--delta-base-offset";
- argv[i++] = NULL;
-
- execv_git_cmd(argv);
- kill(pid_rev_list, SIGKILL);
- die("git-upload-pack: unable to exec git-pack-objects");
- }
-
- close(lp_pipe[0]);
- close(lp_pipe[1]);
- /* We read from pe_pipe[0] to capture stderr output for
- * progress bar, and pu_pipe[0] to capture the pack data.
+ /* We read from pack_objects.err to capture stderr output for
+ * progress bar, and pack_objects.out to capture the pack data.
*/
- close(pe_pipe[1]);
- close(pu_pipe[1]);
while (1) {
const char *who;
@@ -214,14 +197,14 @@ static void create_pack_file(void)
pollsize = 0;
pe = pu = -1;
- if (0 <= pu_pipe[0]) {
- pfd[pollsize].fd = pu_pipe[0];
+ if (0 <= pack_objects.out) {
+ pfd[pollsize].fd = pack_objects.out;
pfd[pollsize].events = POLLIN;
pu = pollsize;
pollsize++;
}
- if (0 <= pe_pipe[0]) {
- pfd[pollsize].fd = pe_pipe[0];
+ if (0 <= pack_objects.err) {
+ pfd[pollsize].fd = pack_objects.err;
pfd[pollsize].events = POLLIN;
pe = pollsize;
pollsize++;
@@ -254,13 +237,13 @@ static void create_pack_file(void)
*cp++ = buffered;
outsz++;
}
- sz = xread(pu_pipe[0], cp,
+ sz = xread(pack_objects.out, cp,
sizeof(data) - outsz);
if (0 < sz)
;
else if (sz == 0) {
- close(pu_pipe[0]);
- pu_pipe[0] = -1;
+ close(pack_objects.out);
+ pack_objects.out = -1;
}
else
goto fail;
@@ -279,13 +262,13 @@ static void create_pack_file(void)
/* Status ready; we ship that in the side-band
* or dump to the standard error.
*/
- sz = xread(pe_pipe[0], progress,
+ sz = xread(pack_objects.err, progress,
sizeof(progress));
if (0 < sz)
send_client_data(2, progress, sz);
else if (sz == 0) {
- close(pe_pipe[0]);
- pe_pipe[0] = -1;
+ close(pack_objects.err);
+ pack_objects.err = -1;
}
else
goto fail;
@@ -293,12 +276,12 @@ static void create_pack_file(void)
}
/* See if the children are still there */
- if (pid_rev_list || pid_pack_objects) {
+ if (pid_rev_list || pack_objects.pid) {
pid = waitpid(-1, &status, WNOHANG);
if (!pid)
continue;
who = ((pid == pid_rev_list) ? "git-rev-list" :
- (pid == pid_pack_objects) ? "git-pack-objects" :
+ (pid == pack_objects.pid) ? "git-pack-objects" :
NULL);
if (!who) {
if (pid < 0) {
@@ -317,9 +300,9 @@ static void create_pack_file(void)
}
if (pid == pid_rev_list)
pid_rev_list = 0;
- if (pid == pid_pack_objects)
- pid_pack_objects = 0;
- if (pid_rev_list || pid_pack_objects)
+ if (pid == pack_objects.pid)
+ pack_objects.pid = 0;
+ if (pid_rev_list || pack_objects.pid)
continue;
}
@@ -340,8 +323,8 @@ static void create_pack_file(void)
return;
}
fail:
- if (pid_pack_objects)
- kill(pid_pack_objects, SIGKILL);
+ if (pack_objects.pid)
+ kill(pack_objects.pid, SIGKILL);
if (pid_rev_list)
kill(pid_rev_list, SIGKILL);
send_client_data(3, abort_msg, sizeof(abort_msg));
--
1.5.3.3.1134.gee562
^ permalink raw reply related
* [PATCH 09/14] Use the asyncronous function infrastructure in builtin-fetch-pack.c.
From: Johannes Sixt @ 2007-10-13 20:06 UTC (permalink / raw)
To: gitster; +Cc: git, Johannes Sixt
In-Reply-To: <1192305984-22594-9-git-send-email-johannes.sixt@telecom.at>
We run the sideband demultiplexer in an asynchronous function.
Note that earlier there was a check in the child process that closed
xd[1] only if it was different from xd[0]; this test is no longer needed
because git_connect() always returns two different file descriptors
(see ec587fde0a76780931c7ac32474c8c000aa45134).
Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at>
---
builtin-fetch-pack.c | 39 ++++++++++++++++++---------------------
1 files changed, 18 insertions(+), 21 deletions(-)
diff --git a/builtin-fetch-pack.c b/builtin-fetch-pack.c
index 871b704..51d8a32 100644
--- a/builtin-fetch-pack.c
+++ b/builtin-fetch-pack.c
@@ -457,42 +457,37 @@ static int everything_local(struct ref **refs, int nr_match, char **match)
return retval;
}
-static pid_t setup_sideband(int fd[2], int xd[2])
+static int sideband_demux(int fd, void *data)
{
- pid_t side_pid;
+ int *xd = data;
+ close(xd[1]);
+ return recv_sideband("fetch-pack", xd[0], fd, 2);
+}
+
+static void setup_sideband(int fd[2], int xd[2], struct async *demux)
+{
if (!use_sideband) {
fd[0] = xd[0];
fd[1] = xd[1];
- return 0;
+ return;
}
/* xd[] is talking with upload-pack; subprocess reads from
* xd[0], spits out band#2 to stderr, and feeds us band#1
- * through our fd[0].
+ * through demux->out.
*/
- if (pipe(fd) < 0)
- die("fetch-pack: unable to set up pipe");
- side_pid = fork();
- if (side_pid < 0)
+ demux->proc = sideband_demux;
+ demux->data = xd;
+ if (start_async(demux))
die("fetch-pack: unable to fork off sideband demultiplexer");
- if (!side_pid) {
- /* subprocess */
- close(fd[0]);
- if (xd[0] != xd[1])
- close(xd[1]);
- if (recv_sideband("fetch-pack", xd[0], fd[1], 2))
- exit(1);
- exit(0);
- }
close(xd[0]);
- close(fd[1]);
+ fd[0] = demux->out;
fd[1] = xd[1];
- return side_pid;
}
static int get_pack(int xd[2], char **pack_lockfile)
{
- pid_t side_pid;
+ struct async demux;
int fd[2];
const char *argv[20];
char keep_arg[256];
@@ -501,7 +496,7 @@ static int get_pack(int xd[2], char **pack_lockfile)
int do_keep = args.keep_pack;
struct child_process cmd;
- side_pid = setup_sideband(fd, xd);
+ setup_sideband(fd, xd, &demux);
memset(&cmd, 0, sizeof(cmd));
cmd.argv = argv;
@@ -556,6 +551,8 @@ static int get_pack(int xd[2], char **pack_lockfile)
if (finish_command(&cmd))
die("%s failed", argv[0]);
+ if (use_sideband && finish_async(&demux))
+ die("error in sideband demultiplexer");
return 0;
}
--
1.5.3.3.1134.gee562
^ permalink raw reply related
* [PATCH 12/14] t0021-conversion.sh: Test that the clean filter really cleans content.
From: Johannes Sixt @ 2007-10-13 20:06 UTC (permalink / raw)
To: gitster; +Cc: git, Johannes Sixt
In-Reply-To: <1192305984-22594-12-git-send-email-johannes.sixt@telecom.at>
This test uses a rot13 filter, which is its own inverse. It tested only
that the content was the same as the original after both the 'clean' and
the 'smudge' filter were applied. This way it would not detect whether
any filter was run at all. Hence, here we add another test that checks
that the repository contained content that was processed by the 'clean'
filter.
Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at>
---
t/t0021-conversion.sh | 7 ++++++-
1 files changed, 6 insertions(+), 1 deletions(-)
diff --git a/t/t0021-conversion.sh b/t/t0021-conversion.sh
index a839f4e..cb86029 100755
--- a/t/t0021-conversion.sh
+++ b/t/t0021-conversion.sh
@@ -42,7 +42,12 @@ test_expect_success check '
git diff --raw --exit-code :test :test.i &&
id=$(git rev-parse --verify :test) &&
embedded=$(sed -ne "$script" test.i) &&
- test "z$id" = "z$embedded"
+ test "z$id" = "z$embedded" &&
+
+ git cat-file blob :test.t > test.r &&
+
+ ./rot13.sh < test.o > test.t &&
+ cmp test.r test.t
'
# If an expanded ident ever gets into the repository, we want to make sure that
--
1.5.3.3.1134.gee562
^ permalink raw reply related
* [PATCH 10/14] upload-pack: Move the revision walker into a separate function.
From: Johannes Sixt @ 2007-10-13 20:06 UTC (permalink / raw)
To: gitster; +Cc: git, Johannes Sixt
In-Reply-To: <1192305984-22594-10-git-send-email-johannes.sixt@telecom.at>
While this is mostly a cleanup and makes a long function shorter, it later
also allows us to use start_async() with this function.
Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at>
---
upload-pack.c | 70 ++++++++++++++++++++++++++++++--------------------------
1 files changed, 37 insertions(+), 33 deletions(-)
diff --git a/upload-pack.c b/upload-pack.c
index ef2894a..c5aa0ea 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -97,6 +97,42 @@ static void show_edge(struct commit *commit)
fprintf(pack_pipe, "-%s\n", sha1_to_hex(commit->object.sha1));
}
+static void do_rev_list(int create_full_pack)
+{
+ int i;
+ struct rev_info revs;
+
+ if (create_full_pack)
+ use_thin_pack = 0; /* no point doing it */
+ init_revisions(&revs, NULL);
+ revs.tag_objects = 1;
+ revs.tree_objects = 1;
+ revs.blob_objects = 1;
+ if (use_thin_pack)
+ revs.edge_hint = 1;
+
+ if (create_full_pack) {
+ const char *args[] = {"rev-list", "--all", NULL};
+ setup_revisions(2, args, &revs, NULL);
+ } else {
+ for (i = 0; i < want_obj.nr; i++) {
+ struct object *o = want_obj.objects[i].item;
+ /* why??? */
+ o->flags &= ~UNINTERESTING;
+ add_pending_object(&revs, o, NULL);
+ }
+ for (i = 0; i < have_obj.nr; i++) {
+ struct object *o = have_obj.objects[i].item;
+ o->flags |= UNINTERESTING;
+ add_pending_object(&revs, o, NULL);
+ }
+ setup_revisions(0, NULL, &revs, NULL);
+ }
+ prepare_revision_walk(&revs);
+ mark_edges_uninteresting(revs.commits, &revs, show_edge);
+ traverse_commit_list(&revs, show_commit, show_object);
+}
+
static void create_pack_file(void)
{
/* Pipe from rev-list to pack-objects
@@ -119,41 +155,9 @@ static void create_pack_file(void)
die("git-upload-pack: unable to fork git-rev-list");
if (!pid_rev_list) {
- int i;
- struct rev_info revs;
-
close(lp_pipe[0]);
pack_pipe = fdopen(lp_pipe[1], "w");
-
- if (create_full_pack)
- use_thin_pack = 0; /* no point doing it */
- init_revisions(&revs, NULL);
- revs.tag_objects = 1;
- revs.tree_objects = 1;
- revs.blob_objects = 1;
- if (use_thin_pack)
- revs.edge_hint = 1;
-
- if (create_full_pack) {
- const char *args[] = {"rev-list", "--all", NULL};
- setup_revisions(2, args, &revs, NULL);
- } else {
- for (i = 0; i < want_obj.nr; i++) {
- struct object *o = want_obj.objects[i].item;
- /* why??? */
- o->flags &= ~UNINTERESTING;
- add_pending_object(&revs, o, NULL);
- }
- for (i = 0; i < have_obj.nr; i++) {
- struct object *o = have_obj.objects[i].item;
- o->flags |= UNINTERESTING;
- add_pending_object(&revs, o, NULL);
- }
- setup_revisions(0, NULL, &revs, NULL);
- }
- prepare_revision_walk(&revs);
- mark_edges_uninteresting(revs.commits, &revs, show_edge);
- traverse_commit_list(&revs, show_commit, show_object);
+ do_rev_list(create_full_pack);
exit(0);
}
--
1.5.3.3.1134.gee562
^ permalink raw reply related
* [PATCH 13/14] Avoid a dup2(2) in apply_filter() - start_command() can do it for us.
From: Johannes Sixt @ 2007-10-13 20:06 UTC (permalink / raw)
To: gitster; +Cc: git, Johannes Sixt
In-Reply-To: <1192305984-22594-13-git-send-email-johannes.sixt@telecom.at>
When apply_filter() runs the external (clean or smudge) filter program, it
needs to pass the writable end of a pipe as its stdout. For this purpose,
it used to dup2(2) the file descriptor explicitly to stdout. Now we use
the facilities of start_command() to do it for us.
Furthermore, the path argument of a subordinate function, filter_buffer(),
was not used, so here we replace it to pass the fd instead.
Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at>
---
convert.c | 7 +++----
1 files changed, 3 insertions(+), 4 deletions(-)
diff --git a/convert.c b/convert.c
index 6d64994..c870817 100644
--- a/convert.c
+++ b/convert.c
@@ -201,7 +201,7 @@ static char *crlf_to_worktree(const char *path, const char *src, unsigned long *
return buffer;
}
-static int filter_buffer(const char *path, const char *src,
+static int filter_buffer(int fd, const char *src,
unsigned long size, const char *cmd)
{
/*
@@ -214,6 +214,7 @@ static int filter_buffer(const char *path, const char *src,
memset(&child_process, 0, sizeof(child_process));
child_process.argv = argv;
child_process.in = -1;
+ child_process.out = fd;
if (start_command(&child_process))
return error("cannot fork to run external filter %s", cmd);
@@ -265,10 +266,8 @@ static char *apply_filter(const char *path, const char *src,
return NULL;
}
if (!child_process.pid) {
- dup2(pipe_feed[1], 1);
close(pipe_feed[0]);
- close(pipe_feed[1]);
- exit(filter_buffer(path, src, *sizep, cmd));
+ exit(filter_buffer(pipe_feed[1], src, *sizep, cmd));
}
close(pipe_feed[1]);
--
1.5.3.3.1134.gee562
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox