* Re: Git-submodule questions
From: Linus Torvalds @ 2007-10-19 21:22 UTC (permalink / raw)
To: Evan Carroll; +Cc: git
In-Reply-To: <428b865e0710191354v59f558bbv4536d60902977ac@mail.gmail.com>
On Fri, 19 Oct 2007, Evan Carroll wrote:
>
> I've got a set of 5 git repositories on one computer. I'm looking for
> a way to more cleanly distribute them, and keep them in sync when i
> distribute them. I'm trying to discern whether the scantly documented
> git-submodule can accomplish this...
Sounds like subprojects are indeed a good way for you.
> All of my git modules are in one place, /srv/DM: /srv/DM/a ..
> /srv/DM/e to add them all into one super repository for easier
> distributing I have to move them all to /srv/DM/git-old, then git init
> in /srv/DM, then I git submodule add all of the modules in
> /srv/DM/git-old. The question then comes down to why do I need to have
> my git-submodules in /srv/DM, and /srv/DM/git-old.
You shouldn't need to do anything even half-way complicated like that.
In fact, it should be enough to do
cd /srv/DM
git init
git add .
git commit
and you're now literally all done!
Linus
^ permalink raw reply
* Re: [RFC/PATCH] git-fetch: mega-terse fetch output
From: Theodore Tso @ 2007-10-19 21:17 UTC (permalink / raw)
To: Nicolas Pitre
Cc: Karl Hasselström, Santi Béjar, Shawn O. Pearce,
David Symonds, Jeff King, git
In-Reply-To: <alpine.LFD.0.9999.0710191058570.19446@xanadu.home>
On Fri, Oct 19, 2007 at 11:03:00AM -0400, Nicolas Pitre wrote:
> Well, the important thing is that the _content_ is moving from the
> remote repository to the local one. That's how the arrow should be
> interpreted conceptually. The fact that technically we end up assigning
> the local ref with the remote value is a technical issue.
If the _content_ is moving from the remote repository to the local
one, I would think the arrow should be pointing from the remote
repoistory to the local one, i.e.:
* 895be02..2fe5433 next <- spearce/next
But right now we are proposing:
* 895be02..2fe5433 next -> spearce/next
I would think the former makes more sense is the content is going
*from* spearce/next into the local next branch.
This isn't a huge deal, but these tiny things make a large amount of
difference in usability for the novice who just getting started with
git....
- Ted
^ permalink raw reply
* Re: [PATCH] git-cherry-pick: improve description of -x.
From: Frank Lichtenheld @ 2007-10-19 21:11 UTC (permalink / raw)
To: Ralf Wildenhues, git
In-Reply-To: <20071019174134.GD9906@ins.uni-bonn.de>
On Fri, Oct 19, 2007 at 07:41:34PM +0200, Ralf Wildenhues wrote:
> Further, I am surprised that -x seems to be nonfunctional when the
> cherry pick introduces a conflict. Example:
[...]
> The prototype commit message now does not contain the
> | (cherry picked from commit ...).
>
> Is that by design (because there were conflicts) or an omission?
> In case of the former, maybe the description of -x should mention this.
git commit currently doesn't know that you commit a cherry-pick. The -c
only says to use the commit message of the original commit. So this is
currently by design.
> diff --git a/Documentation/git-cherry-pick.txt b/Documentation/git-cherry-pick.txt
> index 47b1e8c..c7d83ce 100644
> --- a/Documentation/git-cherry-pick.txt
> +++ b/Documentation/git-cherry-pick.txt
> @@ -27,10 +27,10 @@ OPTIONS
> message prior committing.
>
> -x::
> - Cause the command to append which commit was
> - cherry-picked after the original commit message when
> - making a commit. Do not use this option if you are
> - cherry-picking from your private branch because the
> + When recording the commit, the original commit message will
> + be appended with a note that indicates which commit this
> + change was cherry-picked from. Do not use this option if
> + you are cherry-picking from your private branch because the
Is "will be appended with" correct English? Sounds odd to me (but I'm no
native speaker, so it might be perfectly fine).
Gruesse,
--
Frank Lichtenheld <frank@lichtenheld.de>
www: http://www.djpig.de/
^ permalink raw reply
* Re: Git User's Survey 2007 unfinished summary continued
From: Federico Mena Quintero @ 2007-10-19 20:57 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Jakub Narebski, git
In-Reply-To: <Pine.LNX.4.64.0710130130380.25221@racer.site>
On Sat, 2007-10-13 at 01:46 +0100, Johannes Schindelin wrote:
> Jakub, thank you very much for doing this. It is a very tedious work, and
> I deem it invaluable.
Seconded! This survey is very valuable, Jakub!
> > 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. Why do people expect others to work for them for free? Hard?
The "barriers to entry" / "data model" comment came from me :)
"Find out why people find git hard to learn and eliminate those barriers
to entry" is what we do with usability tests e.g. in GNOME. You ask
people to use your software to accomplish well-defined tasks ("send a
mail to foo@bar.com", "using the word processor, copy this fancy printed
layout"). Then, you see how they *expect* your software to work, you
see in which places it doesn't behave like that, and you fix it. This
produces very good results. For Git in particular this could be things
like, "Import this project from SVN, fix a bug, commit the patch", or
"You are a maintainer, merge in these two branches from two
contributors".
"Make git more task-oriented rather than data-model-oriented" is about
making the tool adapt to what you usually want to do, instead of making
*you* adapt to the way the tool wants to work. Many commands in Git
have documentation like
"option --foo updates the refs without modifying the index. Requires
a clean working tree"
This is gibberish for people who are not very familiar with Git's
internals. "Git for computer scientists" provides a *very nice*
explanation of the DAG and refs and tags, but unfortunately it doesn't
explain the index, why you would want to know about it, etc.
Git introduces a lot of terminlogy: refs, index, working tree, remotes,
origin, HEAD (which is not the same as CVS HEAD!), detached head,
rebasing, porcelains, etc. Even the basic documentation is hard to read
when you don't know all the terms yet.
It's nice that Git lets you manipulate the repository in all kinds of
ways, but presenting porcelains at the same level as plumbing makes
things hard for users to learn. I was just in our Beijing office,
teaching people about development tools and Git in particular.
Federico: "get Git from this location"
Beijing hacker: tap tap tap, "okay, it's installed now"
Federico "Git commands all start with 'git'"
Beijing hacker: git<Tab>
Bash: Display all 150 possibilities?
Beijing hacker: "oh, shit..."
It's hard to know where to begin :) Do I need "git-cherry-pick" or
"git-cherry"? Why is the "apply a patch" command called "git-am"? Why
is it different from "git-apply"? From "git-applypatch"? Etc.
Federico
^ permalink raw reply
* Re: Announcement of Git wikibook
From: Steffen Prohaska @ 2007-10-19 20:58 UTC (permalink / raw)
To: Evan Carroll; +Cc: git
In-Reply-To: <428b865e0710191321ndd08564yec6366cb10705af6@mail.gmail.com>
On Oct 19, 2007, at 10:21 PM, Evan Carroll wrote:
> I've create a git wikibook if anyone wants to help expand it.
> http://en.wikibooks.org/wiki/Source_Control_Management_With_Git
I'm just curious. What is the advantage of a wikibook?
We already have a manual
http://www.kernel.org/pub/software/scm/git/docs/user-manual.html
including a todo list
http://www.kernel.org/pub/software/scm/git/docs/user-manual.html#todo
So, why don't you send patches improving the manual, but instead
started a wiki book from scratch?
Steffen
^ permalink raw reply
* Git-submodule questions
From: Evan Carroll @ 2007-10-19 20:54 UTC (permalink / raw)
To: git
I've got a set of 5 git repositories on one computer. I'm looking for
a way to more cleanly distribute them, and keep them in sync when i
distribute them. I'm trying to discern whether the scantly documented
git-submodule can accomplish this...
All of my git modules are in one place, /srv/DM: /srv/DM/a ..
/srv/DM/e to add them all into one super repository for easier
distributing I have to move them all to /srv/DM/git-old, then git init
in /srv/DM, then I git submodule add all of the modules in
/srv/DM/git-old. The question then comes down to why do I need to have
my git-submodules in /srv/DM, and /srv/DM/git-old.
Can I make a superproject out of /srv/DM and just some how add all of
that directories contents as submodules. When I try to add them as
like that I just get submodule 'a' already exists (because it does as
a non-submodule but as a git-repos.)
Picture a svn repository, now chop of the root, that's how my git
looks, and I'm trying to amend that. In svn all of your companies
projects can be tracked by one svn repository, not so in git. I'm
wondering if submodule is or isn't the tool to deliver this
functionality.
--
Evan Carroll
System Lord of the Internets
me@evancarroll.com
832-445-8877
^ permalink raw reply
* Announcement of Git wikibook
From: Evan Carroll @ 2007-10-19 20:21 UTC (permalink / raw)
To: git
I've create a git wikibook if anyone wants to help expand it.
http://en.wikibooks.org/wiki/Source_Control_Management_With_Git
--
Evan Carroll
System Lord of the Internets
me@evancarroll.com
832-445-8877
^ permalink raw reply
* Re: git on afs
From: Daniel Barkalow @ 2007-10-19 20:19 UTC (permalink / raw)
To: Todd T. Fries; +Cc: Shawn O. Pearce, git, Brandon Casey
In-Reply-To: <200710190742.08174.todd@fries.net>
On Fri, 19 Oct 2007, Todd T. Fries wrote:
> You're the second one to point out that I should check the errno of link() on
> afs.
>
> It should not matter, but I'm using arla's afs client on OpenBSD; the errno
> is 17 (EEXIST), the very errno that bypasses the coda hack's rename():
According to a quick web search, OpenAFS gives EXDEV (unless there are
other errors that apply). Getting EEXIST when the new path doesn't exist
sounds like a bug in arla to me, although it could also be an AFS server
issue, AFAICT.
-Daniel
*This .sig left intentionally blank*
^ permalink raw reply
* [PATCH 11/14] upload-pack: Run rev-list in an asynchronous function.
From: Johannes Sixt @ 2007-10-19 19:48 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: git, Johannes Sixt
In-Reply-To: <1192823286-9654-11-git-send-email-johannes.sixt@telecom.at>
This gets rid of an explicit fork().
Since upload-pack has to coordinate two processes (rev-list and
pack-objects), we cannot use the normal finish_async(), but have to monitor
the process explicitly. Hence, there are no changes at this front.
Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at>
---
upload-pack.c | 46 ++++++++++++++++++----------------------------
1 files changed, 18 insertions(+), 28 deletions(-)
diff --git a/upload-pack.c b/upload-pack.c
index ccdc306..6799468 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -97,11 +97,12 @@ 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)
+static int do_rev_list(int fd, void *create_full_pack)
{
int i;
struct rev_info revs;
+ pack_pipe = fdopen(fd, "w");
if (create_full_pack)
use_thin_pack = 0; /* no point doing it */
init_revisions(&revs, NULL);
@@ -131,14 +132,12 @@ static void do_rev_list(int create_full_pack)
prepare_revision_walk(&revs);
mark_edges_uninteresting(revs.commits, &revs, show_edge);
traverse_commit_list(&revs, show_commit, show_object);
+ return 0;
}
static void create_pack_file(void)
{
- /* Pipe from rev-list to pack-objects
- */
- int lp_pipe[2];
- pid_t pid_rev_list;
+ struct async 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];
@@ -148,22 +147,12 @@ static void create_pack_file(void)
const char *argv[10];
int arg = 0;
- if (pipe(lp_pipe) < 0)
- die("git-upload-pack: unable to create pipe");
- pid_rev_list = fork();
- if (pid_rev_list < 0)
+ rev_list.proc = do_rev_list;
+ /* .data is just a boolean: any non-NULL value will do */
+ rev_list.data = create_full_pack ? &rev_list : NULL;
+ if (start_async(&rev_list))
die("git-upload-pack: unable to fork git-rev-list");
- if (!pid_rev_list) {
- close(lp_pipe[0]);
- pack_pipe = fdopen(lp_pipe[1], "w");
- do_rev_list(create_full_pack);
- exit(0);
- }
-
- /* writable pipe end must not be inherited by pack_objects */
- close(lp_pipe[1]);
-
argv[arg++] = "pack-objects";
argv[arg++] = "--stdout";
if (!no_progress)
@@ -173,14 +162,15 @@ static void create_pack_file(void)
argv[arg++] = NULL;
memset(&pack_objects, 0, sizeof(pack_objects));
- pack_objects.in = lp_pipe[0]; /* start_command closes it */
+ pack_objects.in = rev_list.out; /* 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);
+ kill(rev_list.pid, SIGKILL);
die("git-upload-pack: unable to fork git-pack-objects");
}
@@ -280,11 +270,11 @@ static void create_pack_file(void)
}
/* See if the children are still there */
- if (pid_rev_list || pack_objects.pid) {
+ if (rev_list.pid || pack_objects.pid) {
pid = waitpid(-1, &status, WNOHANG);
if (!pid)
continue;
- who = ((pid == pid_rev_list) ? "git-rev-list" :
+ who = ((pid == rev_list.pid) ? "git-rev-list" :
(pid == pack_objects.pid) ? "git-pack-objects" :
NULL);
if (!who) {
@@ -302,11 +292,11 @@ static void create_pack_file(void)
who);
goto fail;
}
- if (pid == pid_rev_list)
- pid_rev_list = 0;
+ if (pid == rev_list.pid)
+ rev_list.pid = 0;
if (pid == pack_objects.pid)
pack_objects.pid = 0;
- if (pid_rev_list || pack_objects.pid)
+ if (rev_list.pid || pack_objects.pid)
continue;
}
@@ -329,8 +319,8 @@ static void create_pack_file(void)
fail:
if (pack_objects.pid)
kill(pack_objects.pid, SIGKILL);
- if (pid_rev_list)
- kill(pid_rev_list, SIGKILL);
+ if (rev_list.pid)
+ kill(rev_list.pid, SIGKILL);
send_client_data(3, abort_msg, sizeof(abort_msg));
die("git-upload-pack: %s", abort_msg);
}
--
1.5.3.4.315.g2ce38
^ permalink raw reply related
* [PATCH 14/14] Use the asyncronous function infrastructure to run the content filter.
From: Johannes Sixt @ 2007-10-19 19:48 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: git, Johannes Sixt
In-Reply-To: <1192823286-9654-14-git-send-email-johannes.sixt@telecom.at>
Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at>
---
convert.c | 61 ++++++++++++++++++++++++++++---------------------------------
1 files changed, 28 insertions(+), 33 deletions(-)
diff --git a/convert.c b/convert.c
index 8dc9965..4df7559 100644
--- a/convert.c
+++ b/convert.c
@@ -192,15 +192,21 @@ static int crlf_to_worktree(const char *path, const char *src, size_t len,
return 1;
}
-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;
@@ -208,17 +214,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 %d", params->cmd, -status);
return (write_err || status);
}
@@ -231,47 +237,36 @@ static int apply_filter(const char *path, const char *src, size_t len,
*
* (child --> cmd) --> us
*/
- int pipe_feed[2];
- int status, ret = 1;
- struct child_process child_process;
+ int ret = 1;
struct strbuf nbuf;
+ struct async async;
+ struct filter_params params;
if (!cmd)
return 0;
- memset(&child_process, 0, sizeof(child_process));
-
- if (pipe(pipe_feed) < 0) {
- error("cannot create pipe to run external filter %s", cmd);
- return 0;
- }
+ memset(&async, 0, sizeof(async));
+ async.proc = filter_buffer;
+ async.data = ¶ms;
+ params.src = src;
+ params.size = len;
+ 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 0;
- }
- if (!child_process.pid) {
- close(pipe_feed[0]);
- exit(filter_buffer(pipe_feed[1], src, len, cmd));
- }
- close(pipe_feed[1]);
+ if (start_async(&async))
+ return 0; /* error was already reported */
strbuf_init(&nbuf, 0);
- if (strbuf_read(&nbuf, pipe_feed[0], len) < 0) {
+ if (strbuf_read(&nbuf, async.out, len) < 0) {
error("read from external filter %s failed", cmd);
ret = 0;
}
- if (close(pipe_feed[0])) {
+ if (close(async.out)) {
error("read from external filter %s failed", cmd);
ret = 0;
}
- 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);
ret = 0;
}
--
1.5.3.4.315.g2ce38
^ 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-19 19:48 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: git, Johannes Sixt
In-Reply-To: <1192823286-9654-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 d83c2fc..8dc9965 100644
--- a/convert.c
+++ b/convert.c
@@ -192,7 +192,7 @@ static int crlf_to_worktree(const char *path, const char *src, size_t len,
return 1;
}
-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)
{
/*
@@ -205,6 +205,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);
@@ -254,10 +255,8 @@ static int apply_filter(const char *path, const char *src, size_t len,
return 0;
}
if (!child_process.pid) {
- dup2(pipe_feed[1], 1);
close(pipe_feed[0]);
- close(pipe_feed[1]);
- exit(filter_buffer(path, src, len, cmd));
+ exit(filter_buffer(pipe_feed[1], src, len, cmd));
}
close(pipe_feed[1]);
--
1.5.3.4.315.g2ce38
^ permalink raw reply related
* [PATCH 12/14] t0021-conversion.sh: Test that the clean filter really cleans content.
From: Johannes Sixt @ 2007-10-19 19:48 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: git, Johannes Sixt
In-Reply-To: <1192823286-9654-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.4.315.g2ce38
^ permalink raw reply related
* [PATCH 10/14] upload-pack: Move the revision walker into a separate function.
From: Johannes Sixt @ 2007-10-19 19:48 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: git, Johannes Sixt
In-Reply-To: <1192823286-9654-10-git-send-email-johannes.sixt@telecom.at>
This allows us later to use start_async() with this function, and at
the same time is a nice cleanup that makes a long function
(create_pack_file()) shorter.
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 5c0c0cc..ccdc306 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.4.315.g2ce38
^ 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-19 19:47 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: git, Johannes Sixt
In-Reply-To: <1192823286-9654-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..5c0c0cc 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) {
+ /* writable pipe end must not be 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.4.315.g2ce38
^ permalink raw reply related
* [PATCH 08/14] Add infrastructure to run a function asynchronously.
From: Johannes Sixt @ 2007-10-19 19:48 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: git, Johannes Sixt
In-Reply-To: <1192823286-9654-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 | 52 ++++++++++++++++++++++++++++++++++++++++++++--------
run-command.h | 22 ++++++++++++++++++++++
2 files changed, 66 insertions(+), 8 deletions(-)
diff --git a/run-command.c b/run-command.c
index d00c03b..db02f75 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,35 @@ 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_pair(pipe_out);
+ 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..94e1e9d 100644
--- a/run-command.h
+++ b/run-command.h
@@ -43,4 +43,26 @@ 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 caller reads.
+ *
+ * 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 here and closes it */
+ pid_t pid;
+};
+
+int start_async(struct async *async);
+int finish_async(struct async *async);
+
#endif
--
1.5.3.4.315.g2ce38
^ permalink raw reply related
* [PATCH 09/14] Use the asyncronous function infrastructure in builtin-fetch-pack.c.
From: Johannes Sixt @ 2007-10-19 19:48 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: git, Johannes Sixt
In-Reply-To: <1192823286-9654-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.4.315.g2ce38
^ 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-19 19:47 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: git, Johannes Sixt
In-Reply-To: <1192823286-9654-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.4.315.g2ce38
^ permalink raw reply related
* [PATCH 05/14] Use start_comand() in builtin-fetch-pack.c instead of explicit fork/exec.
From: Johannes Sixt @ 2007-10-19 19:47 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: git, Johannes Sixt
In-Reply-To: <1192823286-9654-5-git-send-email-johannes.sixt@telecom.at>
Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at>
---
builtin-fetch-pack.c | 56 ++++++++++++++-----------------------------------
1 files changed, 16 insertions(+), 40 deletions(-)
diff --git a/builtin-fetch-pack.c b/builtin-fetch-pack.c
index f56592f..871b704 100644
--- a/builtin-fetch-pack.c
+++ b/builtin-fetch-pack.c
@@ -7,6 +7,7 @@
#include "pack.h"
#include "sideband.h"
#include "fetch-pack.h"
+#include "run-command.h"
static int transfer_unpack_limit = -1;
static int fetch_unpack_limit = -1;
@@ -491,18 +492,19 @@ static pid_t setup_sideband(int fd[2], int xd[2])
static int get_pack(int xd[2], char **pack_lockfile)
{
- int status;
- pid_t pid, side_pid;
+ pid_t side_pid;
int fd[2];
const char *argv[20];
char keep_arg[256];
char hdr_arg[256];
const char **av;
int do_keep = args.keep_pack;
- int keep_pipe[2];
+ struct child_process cmd;
side_pid = setup_sideband(fd, xd);
+ memset(&cmd, 0, sizeof(cmd));
+ cmd.argv = argv;
av = argv;
*hdr_arg = 0;
if (!args.keep_pack && unpack_limit) {
@@ -519,8 +521,8 @@ static int get_pack(int xd[2], char **pack_lockfile)
}
if (do_keep) {
- if (pack_lockfile && pipe(keep_pipe))
- die("fetch-pack: pipe setup failure: %s", strerror(errno));
+ if (pack_lockfile)
+ cmd.out = -1;
*av++ = "index-pack";
*av++ = "--stdin";
if (!args.quiet && !args.no_progress)
@@ -544,43 +546,17 @@ static int get_pack(int xd[2], char **pack_lockfile)
*av++ = hdr_arg;
*av++ = NULL;
- pid = fork();
- if (pid < 0)
+ cmd.in = fd[0];
+ cmd.git_cmd = 1;
+ if (start_command(&cmd))
die("fetch-pack: unable to fork off %s", argv[0]);
- if (!pid) {
- dup2(fd[0], 0);
- if (do_keep && pack_lockfile) {
- dup2(keep_pipe[1], 1);
- close(keep_pipe[0]);
- close(keep_pipe[1]);
- }
- close(fd[0]);
- close(fd[1]);
- execv_git_cmd(argv);
- die("%s exec failed", argv[0]);
- }
- close(fd[0]);
close(fd[1]);
- if (do_keep && pack_lockfile) {
- close(keep_pipe[1]);
- *pack_lockfile = index_pack_lockfile(keep_pipe[0]);
- close(keep_pipe[0]);
- }
- while (waitpid(pid, &status, 0) < 0) {
- if (errno != EINTR)
- die("waiting for %s: %s", argv[0], strerror(errno));
- }
- if (WIFEXITED(status)) {
- int code = WEXITSTATUS(status);
- if (code)
- die("%s died with error code %d", argv[0], code);
- return 0;
- }
- if (WIFSIGNALED(status)) {
- int sig = WTERMSIG(status);
- die("%s died of signal %d", argv[0], sig);
- }
- die("%s died of unnatural causes %d", argv[0], status);
+ if (do_keep && pack_lockfile)
+ *pack_lockfile = index_pack_lockfile(cmd.out);
+
+ if (finish_command(&cmd))
+ die("%s failed", argv[0]);
+ return 0;
}
static struct ref *do_fetch_pack(int fd[2],
--
1.5.3.4.315.g2ce38
^ permalink raw reply related
* [PATCH 01/14] Change git_connect() to return a struct child_process instead of a pid_t.
From: Johannes Sixt @ 2007-10-19 19:47 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: git, Johannes Sixt
In-Reply-To: <1192823286-9654-1-git-send-email-johannes.sixt@telecom.at>
This prepares the API of git_connect() and finish_connect() to operate on
a struct child_process. Currently, we just use that object as a placeholder
for the pid that we used to return. A follow-up patch will change the
implementation of git_connect() and finish_connect() to make full use
of the object.
Old code had early-return-on-error checks at the calling sites of
git_connect(), but since git_connect() dies on errors anyway, these checks
were removed.
Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at>
---
builtin-archive.c | 8 +++-----
builtin-fetch-pack.c | 8 +++-----
cache.h | 4 ++--
connect.c | 31 +++++++++++++++++--------------
peek-remote.c | 8 +++-----
send-pack.c | 8 +++-----
transport.c | 9 ++-------
7 files changed, 33 insertions(+), 43 deletions(-)
diff --git a/builtin-archive.c b/builtin-archive.c
index 04385de..76f8d3d 100644
--- a/builtin-archive.c
+++ b/builtin-archive.c
@@ -30,7 +30,7 @@ static int run_remote_archiver(const char *remote, int argc,
{
char *url, buf[LARGE_PACKET_MAX];
int fd[2], i, len, rv;
- pid_t pid;
+ struct child_process *conn;
const char *exec = "git-upload-archive";
int exec_at = 0;
@@ -46,9 +46,7 @@ static int run_remote_archiver(const char *remote, int argc,
}
url = xstrdup(remote);
- pid = git_connect(fd, url, exec, 0);
- if (pid < 0)
- return pid;
+ conn = git_connect(fd, url, exec, 0);
for (i = 1; i < argc; i++) {
if (i == exec_at)
@@ -76,7 +74,7 @@ static int run_remote_archiver(const char *remote, int argc,
rv = recv_sideband("archive", fd[0], 1, 2);
close(fd[0]);
close(fd[1]);
- rv |= finish_connect(pid);
+ rv |= finish_connect(conn);
return !!rv;
}
diff --git a/builtin-fetch-pack.c b/builtin-fetch-pack.c
index 8f25d50..f56592f 100644
--- a/builtin-fetch-pack.c
+++ b/builtin-fetch-pack.c
@@ -762,7 +762,7 @@ struct ref *fetch_pack(struct fetch_pack_args *my_args,
{
int i, ret;
int fd[2];
- pid_t pid;
+ struct child_process *conn;
struct ref *ref;
struct stat st;
@@ -773,16 +773,14 @@ struct ref *fetch_pack(struct fetch_pack_args *my_args,
st.st_mtime = 0;
}
- pid = git_connect(fd, (char *)dest, uploadpack,
+ conn = git_connect(fd, (char *)dest, uploadpack,
args.verbose ? CONNECT_VERBOSE : 0);
- if (pid < 0)
- return NULL;
if (heads && nr_heads)
nr_heads = remove_duplicates(nr_heads, heads);
ref = do_fetch_pack(fd, nr_heads, heads, pack_lockfile);
close(fd[0]);
close(fd[1]);
- ret = finish_connect(pid);
+ ret = finish_connect(conn);
if (!ret && nr_heads) {
/* If the heads to pull were given, we should have
diff --git a/cache.h b/cache.h
index 27485d3..bfffa05 100644
--- a/cache.h
+++ b/cache.h
@@ -503,8 +503,8 @@ struct ref {
#define REF_TAGS (1u << 2)
#define CONNECT_VERBOSE (1u << 0)
-extern pid_t git_connect(int fd[2], char *url, const char *prog, int flags);
-extern int finish_connect(pid_t pid);
+extern struct child_process *git_connect(int fd[2], char *url, const char *prog, int flags);
+extern int finish_connect(struct child_process *conn);
extern int path_match(const char *path, int nr, char **match);
extern int get_ack(int fd, unsigned char *result_sha1);
extern struct ref **get_remote_heads(int in, struct ref **list, int nr_match, char **match, unsigned int flags);
diff --git a/connect.c b/connect.c
index 3d5c4ab..e66e171 100644
--- a/connect.c
+++ b/connect.c
@@ -468,21 +468,22 @@ char *get_port(char *host)
}
/*
- * This returns 0 if the transport protocol does not need fork(2),
- * or a process id if it does. Once done, finish the connection
+ * This returns NULL if the transport protocol does not need fork(2), or a
+ * struct child_process object if it does. Once done, finish the connection
* with finish_connect() with the value returned from this function
- * (it is safe to call finish_connect() with 0 to support the former
+ * (it is safe to call finish_connect() with NULL to support the former
* case).
*
- * Does not return a negative value on error; it just dies.
+ * If it returns, the connect is successful; it just dies on errors.
*/
-pid_t git_connect(int fd[2], char *url, const char *prog, int flags)
+struct child_process *git_connect(int fd[2], char *url,
+ const char *prog, int flags)
{
char *host, *path = url;
char *end;
int c;
int pipefd[2][2];
- pid_t pid;
+ struct child_process *conn;
enum protocol protocol = PROTO_LOCAL;
int free_path = 0;
char *port = NULL;
@@ -568,15 +569,16 @@ pid_t git_connect(int fd[2], char *url, const char *prog, int flags)
free(target_host);
if (free_path)
free(path);
- return 0;
+ return NULL;
}
if (pipe(pipefd[0]) < 0 || pipe(pipefd[1]) < 0)
die("unable to create pipe pair for communication");
- pid = fork();
- if (pid < 0)
+ conn = xcalloc(1, sizeof(*conn));
+ conn->pid = fork();
+ if (conn->pid < 0)
die("unable to fork");
- if (!pid) {
+ if (!conn->pid) {
struct strbuf cmd;
strbuf_init(&cmd, MAX_CMD_LEN);
@@ -625,17 +627,18 @@ pid_t git_connect(int fd[2], char *url, const char *prog, int flags)
close(pipefd[1][0]);
if (free_path)
free(path);
- return pid;
+ return conn;
}
-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;
}
+ free(conn);
return 0;
}
diff --git a/peek-remote.c b/peek-remote.c
index ceb7871..8d20f7c 100644
--- a/peek-remote.c
+++ b/peek-remote.c
@@ -25,7 +25,7 @@ int main(int argc, char **argv)
int i, ret;
char *dest = NULL;
int fd[2];
- pid_t pid;
+ struct child_process *conn;
int nongit = 0;
unsigned flags = 0;
@@ -64,12 +64,10 @@ int main(int argc, char **argv)
if (!dest || i != argc - 1)
usage(peek_remote_usage);
- pid = git_connect(fd, dest, uploadpack, 0);
- if (pid < 0)
- return 1;
+ conn = git_connect(fd, dest, uploadpack, 0);
ret = peek_remote(fd, flags);
close(fd[0]);
close(fd[1]);
- ret |= finish_connect(pid);
+ ret |= finish_connect(conn);
return !!ret;
}
diff --git a/send-pack.c b/send-pack.c
index 7b77624..147f44b 100644
--- a/send-pack.c
+++ b/send-pack.c
@@ -365,7 +365,7 @@ int main(int argc, char **argv)
char *dest = NULL;
char **heads = NULL;
int fd[2], ret;
- pid_t pid;
+ struct child_process *conn;
char *remote_name = NULL;
struct remote *remote = NULL;
@@ -433,12 +433,10 @@ int main(int argc, char **argv)
}
}
- pid = git_connect(fd, dest, receivepack, verbose ? CONNECT_VERBOSE : 0);
- if (pid < 0)
- return 1;
+ conn = git_connect(fd, dest, receivepack, verbose ? CONNECT_VERBOSE : 0);
ret = send_pack(fd[0], fd[1], remote, nr_heads, heads);
close(fd[0]);
close(fd[1]);
- ret |= finish_connect(pid);
+ ret |= finish_connect(conn);
return !!ret;
}
diff --git a/transport.c b/transport.c
index 400af71..5132d28 100644
--- a/transport.c
+++ b/transport.c
@@ -601,18 +601,13 @@ static struct ref *get_refs_via_connect(const struct transport *transport)
struct git_transport_data *data = transport->data;
struct ref *refs;
int fd[2];
- pid_t pid;
char *dest = xstrdup(transport->url);
-
- pid = git_connect(fd, dest, data->uploadpack, 0);
-
- if (pid < 0)
- die("Failed to connect to \"%s\"", transport->url);
+ struct child_process *conn = git_connect(fd, dest, data->uploadpack, 0);
get_remote_heads(fd[0], &refs, 0, NULL, 0);
packet_flush(fd[1]);
- finish_connect(pid);
+ finish_connect(conn);
free(dest);
--
1.5.3.4.315.g2ce38
^ permalink raw reply related
* [PATCH 04/14] Use run_command() to spawn external diff programs instead of fork/exec.
From: Johannes Sixt @ 2007-10-19 19:47 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: git, Johannes Sixt
In-Reply-To: <1192823286-9654-4-git-send-email-johannes.sixt@telecom.at>
Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at>
---
diff.c | 38 +++-----------------------------------
1 files changed, 3 insertions(+), 35 deletions(-)
diff --git a/diff.c b/diff.c
index 6648e01..30b7544 100644
--- a/diff.c
+++ b/diff.c
@@ -9,6 +9,7 @@
#include "xdiff-interface.h"
#include "color.h"
#include "attr.h"
+#include "run-command.h"
#ifdef NO_FAST_WORKING_DIRECTORY
#define FAST_WORKING_DIRECTORY 0
@@ -1748,40 +1749,6 @@ static void remove_tempfile_on_signal(int signo)
raise(signo);
}
-static int spawn_prog(const char *pgm, const char **arg)
-{
- pid_t pid;
- int status;
-
- fflush(NULL);
- pid = fork();
- if (pid < 0)
- die("unable to fork");
- if (!pid) {
- execvp(pgm, (char *const*) arg);
- exit(255);
- }
-
- while (waitpid(pid, &status, 0) < 0) {
- if (errno == EINTR)
- continue;
- return -1;
- }
-
- /* Earlier we did not check the exit status because
- * diff exits non-zero if files are different, and
- * we are not interested in knowing that. It was a
- * mistake which made it harder to quit a diff-*
- * session that uses the git-apply-patch-script as
- * the GIT_EXTERNAL_DIFF. A custom GIT_EXTERNAL_DIFF
- * should also exit non-zero only when it wants to
- * abort the entire diff-* session.
- */
- if (WIFEXITED(status) && !WEXITSTATUS(status))
- return 0;
- return -1;
-}
-
/* An external diff command takes:
*
* diff-cmd name infile1 infile1-sha1 infile1-mode \
@@ -1834,7 +1801,8 @@ static void run_external_diff(const char *pgm,
*arg++ = name;
}
*arg = NULL;
- retval = spawn_prog(pgm, spawn_arg);
+ fflush(NULL);
+ retval = run_command_v_opt(spawn_arg, 0);
remove_tempfile();
if (retval) {
fprintf(stderr, "external diff died, stopping at %s.\n", name);
--
1.5.3.4.315.g2ce38
^ permalink raw reply related
* [PATCH 02/14] Use start_command() in git_connect() instead of explicit fork/exec.
From: Johannes Sixt @ 2007-10-19 19:47 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: git, Johannes Sixt
In-Reply-To: <1192823286-9654-2-git-send-email-johannes.sixt@telecom.at>
The child process handling is delegated to start_command() and
finish_command().
Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at>
---
connect.c | 103 ++++++++++++++++++++++++++++--------------------------------
1 files changed, 48 insertions(+), 55 deletions(-)
diff --git a/connect.c b/connect.c
index e66e171..d19c7b1 100644
--- a/connect.c
+++ b/connect.c
@@ -482,11 +482,12 @@ struct child_process *git_connect(int fd[2], char *url,
char *host, *path = url;
char *end;
int c;
- int pipefd[2][2];
struct child_process *conn;
enum protocol protocol = PROTO_LOCAL;
int free_path = 0;
char *port = NULL;
+ const char **arg;
+ struct strbuf cmd;
/* Without this we cannot rely on waitpid() to tell
* what happened to our children.
@@ -572,59 +573,52 @@ struct child_process *git_connect(int fd[2], char *url,
return NULL;
}
- if (pipe(pipefd[0]) < 0 || pipe(pipefd[1]) < 0)
- die("unable to create pipe pair for communication");
conn = xcalloc(1, sizeof(*conn));
- conn->pid = fork();
- if (conn->pid < 0)
- die("unable to fork");
- if (!conn->pid) {
- struct strbuf cmd;
-
- strbuf_init(&cmd, MAX_CMD_LEN);
- strbuf_addstr(&cmd, prog);
- strbuf_addch(&cmd, ' ');
- sq_quote_buf(&cmd, path);
- if (cmd.len >= MAX_CMD_LEN)
- die("command line too long");
-
- dup2(pipefd[1][0], 0);
- dup2(pipefd[0][1], 1);
- close(pipefd[0][0]);
- close(pipefd[0][1]);
- close(pipefd[1][0]);
- close(pipefd[1][1]);
- if (protocol == PROTO_SSH) {
- const char *ssh, *ssh_basename;
- ssh = getenv("GIT_SSH");
- if (!ssh) ssh = "ssh";
- ssh_basename = strrchr(ssh, '/');
- if (!ssh_basename)
- ssh_basename = ssh;
- else
- ssh_basename++;
- if (!port)
- execlp(ssh, ssh_basename, host, cmd.buf, NULL);
- else
- execlp(ssh, ssh_basename, "-p", port, host,
- cmd.buf, NULL);
+ strbuf_init(&cmd, MAX_CMD_LEN);
+ strbuf_addstr(&cmd, prog);
+ strbuf_addch(&cmd, ' ');
+ sq_quote_buf(&cmd, path);
+ if (cmd.len >= MAX_CMD_LEN)
+ die("command line too long");
+
+ conn->in = conn->out = -1;
+ conn->argv = arg = xcalloc(6, sizeof(*arg));
+ if (protocol == PROTO_SSH) {
+ const char *ssh = getenv("GIT_SSH");
+ if (!ssh) ssh = "ssh";
+
+ *arg++ = ssh;
+ if (port) {
+ *arg++ = "-p";
+ *arg++ = port;
}
- else {
- unsetenv(ALTERNATE_DB_ENVIRONMENT);
- unsetenv(DB_ENVIRONMENT);
- unsetenv(GIT_DIR_ENVIRONMENT);
- unsetenv(GIT_WORK_TREE_ENVIRONMENT);
- unsetenv(GRAFT_ENVIRONMENT);
- unsetenv(INDEX_ENVIRONMENT);
- execlp("sh", "sh", "-c", cmd.buf, NULL);
- }
- die("exec failed");
+ *arg++ = host;
+ }
+ else {
+ /* remove these from the environment */
+ const char *env[] = {
+ ALTERNATE_DB_ENVIRONMENT,
+ DB_ENVIRONMENT,
+ GIT_DIR_ENVIRONMENT,
+ GIT_WORK_TREE_ENVIRONMENT,
+ GRAFT_ENVIRONMENT,
+ INDEX_ENVIRONMENT,
+ NULL
+ };
+ conn->env = env;
+ *arg++ = "sh";
+ *arg++ = "-c";
}
- fd[0] = pipefd[0][0];
- fd[1] = pipefd[1][1];
- close(pipefd[0][1]);
- close(pipefd[1][0]);
+ *arg++ = cmd.buf;
+ *arg = NULL;
+
+ if (start_command(conn))
+ die("unable to fork");
+
+ fd[0] = conn->out; /* read from child's stdout */
+ fd[1] = conn->in; /* write to child's stdin */
+ strbuf_release(&cmd);
if (free_path)
free(path);
return conn;
@@ -632,13 +626,12 @@ struct child_process *git_connect(int fd[2], char *url,
int finish_connect(struct child_process *conn)
{
+ int code;
if (conn == NULL)
return 0;
- while (waitpid(conn->pid, NULL, 0) < 0) {
- if (errno != EINTR)
- return -1;
- }
+ code = finish_command(conn);
+ free(conn->argv);
free(conn);
- return 0;
+ return code;
}
--
1.5.3.4.315.g2ce38
^ permalink raw reply related
* [PATCH 0/14 resend] fork/exec removal series
From: Johannes Sixt @ 2007-10-19 19:47 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: git, Johannes Sixt
Here it is again, a series of patches that removes a number fork/exec pairs.
The series goes on top of the builtin-fetch pack topic, specifically,
cfa5b2b7f (Avoid scary errors...) and obsoletes q/jsi/no-fork of Lars'
patch queue.
The difference compared to the last time I sent are in:
[02/14] Use start_command() in git_connect() instead of explicit fork/exec.
[13/14] Avoid a dup2(2) in apply_filter() - start_command() can do it for us.
[14/14] Use the asyncronous function infrastructure to run the content filter.
which had conflicts with the strbuf series.
builtin-archive.c | 8 +-
builtin-fetch-pack.c | 101 +++++++++----------------
cache.h | 4 +-
connect.c | 128 +++++++++++++++----------------
convert.c | 88 ++++++++--------------
diff.c | 38 +---------
peek-remote.c | 8 +-
run-command.c | 78 +++++++++++++++++---
run-command.h | 23 ++++++
send-pack.c | 8 +-
t/t0021-conversion.sh | 7 ++-
transport.c | 9 +--
upload-pack.c | 199 ++++++++++++++++++++++---------------------------
13 files changed, 332 insertions(+), 367 deletions(-)
-- Hannes
^ permalink raw reply
* [PATCH 03/14] Use start_command() to run content filters instead of explicit fork/exec.
From: Johannes Sixt @ 2007-10-19 19:47 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: git, Johannes Sixt
In-Reply-To: <1192823286-9654-3-git-send-email-johannes.sixt@telecom.at>
The previous code already used finish_command() to wait for the process
to terminate, but did not use start_command() to run it.
Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at>
---
convert.c | 30 +++++++-----------------------
1 files changed, 7 insertions(+), 23 deletions(-)
diff --git a/convert.c b/convert.c
index aa95834..d83c2fc 100644
--- a/convert.c
+++ b/convert.c
@@ -199,34 +199,18 @@ static int filter_buffer(const char *path, const char *src,
* Spawn cmd and feed the buffer contents through its stdin.
*/
struct child_process child_process;
- int pipe_feed[2];
int write_err, status;
+ const char *argv[] = { "sh", "-c", cmd, NULL };
memset(&child_process, 0, sizeof(child_process));
+ child_process.argv = argv;
+ child_process.in = -1;
- if (pipe(pipe_feed) < 0) {
- error("cannot create pipe to run external filter %s", cmd);
- return 1;
- }
-
- 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 1;
- }
- if (!child_process.pid) {
- dup2(pipe_feed[0], 0);
- close(pipe_feed[0]);
- close(pipe_feed[1]);
- execlp("sh", "sh", "-c", cmd, NULL);
- return 1;
- }
- close(pipe_feed[0]);
+ if (start_command(&child_process))
+ return error("cannot fork to run external filter %s", cmd);
- write_err = (write_in_full(pipe_feed[1], src, size) < 0);
- if (close(pipe_feed[1]))
+ write_err = (write_in_full(child_process.in, src, size) < 0);
+ if (close(child_process.in))
write_err = 1;
if (write_err)
error("cannot feed the input to external filter %s", cmd);
--
1.5.3.4.315.g2ce38
^ permalink raw reply related
* Re: gitk patch collection pull request
From: Linus Torvalds @ 2007-10-19 19:31 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: Paul Mackerras, git
In-Reply-To: <20071019052823.GI14735@spearce.org>
On Fri, 19 Oct 2007, Shawn O. Pearce wrote:
>
> The following changes since commit 719c2b9d926bf2be4879015e3620d27d32f007b6:
> Paul Mackerras (1):
> gitk: Fix bug causing undefined variable error when cherry-picking
The biggest problem I have with gitk is that it is almost totally useless
for when you have a file limit.
I do "gitk --merge" (which has an implicit file limit of the list
of unmerged files) or just "gitk ORIG_HEAD.. Makefile" and the *history*
of gitk looks fine.
But the diffs are crap and useless, because they contain all the stuff I
did *not* want, and hides all the relevant information.
So then I might use the nice gitk history window to see the commits, but I
will fall back on "git log -p --merge" and "git log -p ORIG_HEAD.. Makefile"
for the real work.
I had that happen unusually many times lately, since we had a fair number
of (mostly trivial) conflicts during this merge window. And it's sad. Gitk
is so good in so many other ways, and then it is so *totally* useless when
it comes to something as fundamental as just looking at the history of a
few files.
Linus
^ permalink raw reply
* Re: git on afs
From: Linus Torvalds @ 2007-10-19 19:06 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: Todd T. Fries, git
In-Reply-To: <20071019060624.GK14735@spearce.org>
On Fri, 19 Oct 2007, Shawn O. Pearce wrote:
>
> What about this instead? It avoids the double lstat() of Todd's
> original patch but seems like it would fix the issue here. Or did
> I misunderstand the problem?
No. As far as I can tell without testing it, this patch will make it show
ignored regular files if they have DT_UNKNOWN in the directory entry (or
the filesystem doesn't support it). Not good.
Linus
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox