* making "git stash" safer to use
From: Bruno Haible @ 2007-10-03 21:31 UTC (permalink / raw)
To: git; +Cc: Benoit SIGOURE, Eric Blake
In-Reply-To: <47023699.3080606@byu.net>
Hi,
Through a simple typo I lost modifications to 20 files:
> >>> $ git stash
> >>> $ git pull
> >>> $ git stash apply
> >>> $ git stash clean # typo!
> >>> $ git stash clear # fatal correction to typo!
It is just too easy to lose your modifications by using "git stash".
Eric Blake further says:
> While we're at it, I wish 'git stash clear' would take an optional
> argument that says which stash(es) to clear, rather than blindly clearing
> the entire stash.
It would help if git would store which of the stashes were applied since
they were created and which were not. A stash that was not yet applied must
be considered "precious", whereas a stash that was applied is redundant,
right?
According these lines, how about
1) changing "git stash clear" to remove only the redundant stashes,
(or alternatively: let it fail if there is at least one precious stash),
2) adding an option -f, so that "git stash -f clear" clears all stashes,
including the precious ones.
The rationale is that humans are bad at remembering the state of something.
Therefore instead of having a command that is commonly used in one state
and dangerous in the other state, better have two different commands - one
for the common case, and one for the dangerous one. Like "rm" and "rm -f".
Bruno
^ permalink raw reply
* Re: size_t vs "unsigned long"
From: Jan Wielemaker @ 2007-10-03 21:19 UTC (permalink / raw)
To: Pierre Habouzit; +Cc: Junio C Hamano, git
In-Reply-To: <20071003204801.GC28188@artemis.corp>
On Wednesday 03 October 2007 22:48, Pierre Habouzit wrote:
> On Wed, Oct 03, 2007 at 08:30:04PM +0000, Junio C Hamano wrote:
> > Traditionally, inside git, we have used the length of things
> > with "unsigned long" for pretty much anything, except where we
> > wanted the length exactly sized we used int32_t, uint64_t and
> > friends.
> >
> > A few places pass pointer to unsigned long as the second
> > parameter to strbuf_detach(), triggering type mismatch warnings.
> > An easy way out is to change strbuf_detach() to take a pointer
> > to ulong but I think it is going backwards. Most places that
> > use "unsigned long" can safely be converted (and made more
> > correct) to use size_t.
>
> Well, afaict, on every linux archs I know of, unsigned longs and
> size_t are the same. Though, I don't know if that holds for the msys
> port, and if that does not holds, then a s/unsigned long/size_t/ would
> help them. Else, for consistency sake, I believe the change is a good
> one.
Surely on the Microsoft 64-bit compilers size_t is 64-bits and long is
32-bits. Don't blame me, I'm just the messenger that learned the hard
way ...
--- Jan
^ permalink raw reply
* [ANNOUNCE] GIT 1.5.3.4
From: Junio C Hamano @ 2007-10-03 21:17 UTC (permalink / raw)
To: git; +Cc: linux-kernel
In-Reply-To: <7vve9thrhx.fsf@gitster.siamese.dyndns.org>
The latest maintenance release GIT 1.5.3.4 is available at the
usual places:
http://www.kernel.org/pub/software/scm/git/
git-1.5.3.4.tar.{gz,bz2} (tarball)
git-htmldocs-1.5.3.4.tar.{gz,bz2} (preformatted docs)
git-manpages-1.5.3.4.tar.{gz,bz2} (preformatted docs)
RPMS/$arch/git-*-1.5.3.4-1.$arch.rpm (RPM)
GIT v1.5.3.4 Release Notes
==========================
Fixes since v1.5.3.3
--------------------
* Change to "git-ls-files" in v1.5.3.3 that was introduced to support
partial commit of removal better had a segfaulting bug, which was
diagnosed and fixed by Keith and Carl.
* Performance improvements for rename detection has been backported
from the 'master' branch.
* "git-for-each-ref --format='%(numparent)'" was not working
correctly at all, and --format='%(parent)' was not working for
merge commits.
* Sample "post-receive-hook" incorrectly sent out push
notification e-mails marked as "From: " the committer of the
commit that happened to be at the tip of the branch that was
pushed, not from the person who pushed.
* "git-remote" did not exit non-zero status upon error.
* "git-add -i" did not respond very well to EOF from tty nor
bogus input.
* "git-rebase -i" squash subcommand incorrectly made the
author of later commit the author of resulting commit,
instead of taking from the first one in the squashed series.
* "git-stash apply --index" was not documented.
* autoconfiguration learned that "ar" command is found as "gas" on
some systems.
----------------------------------------------------------------
Changes since v1.5.3.3 are as follows:
Andy Parkins (1):
post-receive-hook: Remove the From field from the generated email header so that the pusher's name is used
Carl Worth (1):
Add test case for ls-files --with-tree
Federico Mena Quintero (4):
Say when --track is useful in the git-checkout docs.
Add documentation for --track and --no-track to the git-branch docs.
Note that git-branch will not automatically checkout the new branch
Make git-pull complain and give advice when there is nothing to merge with
Jari Aalto (1):
git-remote: exit with non-zero status after detecting errors.
Jean-Luc Herren (2):
git-add--interactive: Allow Ctrl-D to exit
git-add--interactive: Improve behavior on bogus input
Jeff King (1):
diffcore-rename: cache file deltas
Johan Herland (1):
Mention 'cpio' dependency in INSTALL
Johannes Schindelin (2):
rebase -i: squash should retain the authorship of the _first_ commit
Fix typo in config.txt
Junio C Hamano (5):
Whip post 1.5.3.3 maintenance series into shape.
git-commit: initialize TMP_INDEX just to be sure.
for-each-ref: fix %(numparent) and %(parent)
rename diff_free_filespec_data_large() to diff_free_filespec_blob()
GIT 1.5.3.4
Keith Packard (1):
Must not modify the_index.cache as it may be passed to realloc at some point.
Miklos Vajna (1):
git stash: document apply's --index switch
Robert Schiele (1):
the ar tool is called gar on some systems
Steffen Prohaska (1):
fixed link in documentation of diff-options
^ permalink raw reply
* Re: size_t vs "unsigned long"
From: Junio C Hamano @ 2007-10-03 21:05 UTC (permalink / raw)
To: Pierre Habouzit; +Cc: git
In-Reply-To: <20071003204801.GC28188@artemis.corp>
Pierre Habouzit <madcoder@debian.org> writes:
> Well, afaict, on every linux archs I know of, unsigned longs and
> size_t are the same. Though, I don't know if that holds for the msys
> port, and if that does not holds, then a s/unsigned long/size_t/ would
> help them. Else, for consistency sake, I believe the change is a good
> one.
FWIW, I am already getting bitten on a FC box with gcc 4.1.1
20060525 that warns about the wrong type being passed, as I
usually build things with -Werror; the issue is not just "they
are of the same underlying type".
^ permalink raw reply
* Re: size_t vs "unsigned long"
From: Pierre Habouzit @ 2007-10-03 20:48 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vabr0djqr.fsf@gitster.siamese.dyndns.org>
[-- Attachment #1: Type: text/plain, Size: 1092 bytes --]
On Wed, Oct 03, 2007 at 08:30:04PM +0000, Junio C Hamano wrote:
> Traditionally, inside git, we have used the length of things
> with "unsigned long" for pretty much anything, except where we
> wanted the length exactly sized we used int32_t, uint64_t and
> friends.
>
> A few places pass pointer to unsigned long as the second
> parameter to strbuf_detach(), triggering type mismatch warnings.
> An easy way out is to change strbuf_detach() to take a pointer
> to ulong but I think it is going backwards. Most places that
> use "unsigned long" can safely be converted (and made more
> correct) to use size_t.
Well, afaict, on every linux archs I know of, unsigned longs and
size_t are the same. Though, I don't know if that holds for the msys
port, and if that does not holds, then a s/unsigned long/size_t/ would
help them. Else, for consistency sake, I believe the change is a good
one.
--
·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] Support tags in uncommit - use git_id instead of rev_parse
From: Catalin Marinas @ 2007-10-03 20:35 UTC (permalink / raw)
To: Pavel Roskin; +Cc: git
In-Reply-To: <1191362591.26879.3.camel@dv>
On 02/10/2007, Pavel Roskin <proski@gnu.org> wrote:
> On Mon, 2007-10-01 at 23:00 +0100, Catalin Marinas wrote:
> > To allow tags, maybe just pass something like
> > "git.rev_parse(options.to + '^{commit}')" or just modify git.rev_parse
> > to do it (and git_id to avoid it).
>
> I prefer to work with software that understands what I mean and tells me
> that I cannot do it. It makes it easier to understand what is possible
> and how the command is working.
>
> Recognizing patch names in some commands but not others would be
> annoying and inconsistent. Dumbing downs interactive software on
> purpose is probably not worth the trouble.
Without this patch, the 'stg uncommit -t patch' fails with 'Unknown
revision: patch'. With the patch applied, it still fails but with
'Commit ... does not have exactly one parent'. I don't say that the
first one is good but I don't think the latter is clearer. The 'stg
uncommit --help' states that the '--to' option takes a commit argument
but if one passes a patch name the error message gets pretty
confusing.
--
Catalin
^ permalink raw reply
* size_t vs "unsigned long"
From: Junio C Hamano @ 2007-10-03 20:30 UTC (permalink / raw)
To: git; +Cc: Pierre Habouzit
Traditionally, inside git, we have used the length of things
with "unsigned long" for pretty much anything, except where we
wanted the length exactly sized we used int32_t, uint64_t and
friends.
A few places pass pointer to unsigned long as the second
parameter to strbuf_detach(), triggering type mismatch warnings.
An easy way out is to change strbuf_detach() to take a pointer
to ulong but I think it is going backwards. Most places that
use "unsigned long" can safely be converted (and made more
correct) to use size_t.
Any opinions?
^ permalink raw reply
* Re: [PATCH] Add test case for ls-files --with-head
From: Jeff King @ 2007-10-03 20:21 UTC (permalink / raw)
To: David Kastrup
Cc: Carl Worth, Johannes Schindelin, Johannes Sixt, Junio C Hamano,
Keith Packard, Git Mailing List
In-Reply-To: <85ejgcrx6r.fsf@lola.goethe.zz>
On Wed, Oct 03, 2007 at 06:15:56PM +0200, David Kastrup wrote:
> for i in {1,2,3,4,5}{0,1,2,3,4,5,6,7,8,9}
> do
> ...
> done
$ dash
$ for i in {1,2,3,4,5}{0,1,2,3,4,5,6,7,8,9}; do echo $i; done
{1,2,3,4,5}{0,1,2,3,4,5,6,7,8,9}
$
-Peff
^ permalink raw reply
* Re: [PATCH 1/4] Add a simple option parser for use by builtin-commit.c.
From: Jonas Fonseca @ 2007-10-03 20:11 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Kristian Høgsberg, gitster, git
In-Reply-To: <Pine.LNX.4.64.0710011909291.28395@racer.site>
Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote Mon, Oct 01, 2007:
> On Mon, 1 Oct 2007, Kristian H?gsberg wrote:
> > On Sun, 2007-09-30 at 15:11 +0200, Jonas Fonseca wrote:
> > > > +
> > > > +extern int parse_options(const char ***argv,
> > > > + struct option *options, int count,
> > > > + const char *usage_string);
> > >
> > > I think the interface could be improved a bit. For example, it doesn't
> > > need to count argument since the last entry in the options array is
> > > OPTION_LAST and thus the size can be detected that way.
> >
> > Hehe, yeah, that's how I did it first. I don't have a strong preference
> > for terminator elements vs. ARRAY_SIZE(), but Junio prefers the
> > ARRAY_SIZE() approach, I guess. At this point I'm just trying the get
> > the patches upstream...
>
> FWIW I like the ARRAY_SIZE() approach better, too, since it is less error
> prone.
OK, I must have missed that comment. Good point.
Thanks for the comments both of you. It's great to have something to
work from. However, I also fear it will also require that some extra
flags or information is added to the option information to make it more
generally usable. But I guess that is easier to discuss in the context
of a patch.
--
Jonas Fonseca
^ permalink raw reply
* [PATCH 5/5] Use start_comand() in builtin-fetch-pack.c instead of explicit fork/exec.
From: Johannes Sixt @ 2007-10-03 20:09 UTC (permalink / raw)
To: gitster; +Cc: git, Johannes Sixt
In-Reply-To: <1191442180-15905-5-git-send-email-johannes.sixt@telecom.at>
Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at>
---
fetch-pack.c | 35 ++++++++++-------------------------
1 files changed, 10 insertions(+), 25 deletions(-)
diff --git a/fetch-pack.c b/fetch-pack.c
index d06b5ec..80268e1 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -6,6 +6,7 @@
#include "exec_cmd.h"
#include "pack.h"
#include "sideband.h"
+#include "run-command.h"
static int keep_pack;
static int transfer_unpack_limit = -1;
@@ -502,9 +503,12 @@ static int get_pack(int xd[2])
char hdr_arg[256];
const char **av;
int do_keep = keep_pack;
+ struct child_process cmd;
side_pid = setup_sideband(fd, xd);
+ memset(&cmd, 0, sizeof(cmd));
+ cmd.argv = argv;
av = argv;
*hdr_arg = 0;
if (unpack_limit) {
@@ -544,33 +548,14 @@ static int get_pack(int xd[2])
*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);
- close(fd[0]);
- close(fd[1]);
- execv_git_cmd(argv);
- die("%s exec failed", argv[0]);
- }
- close(fd[0]);
close(fd[1]);
- 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 (finish_command(&cmd))
+ die("%s failed", argv[0]);
+ return 0;
}
static int fetch_pack(int fd[2], int nr_match, char **match)
--
1.5.3.3.1134.gee562
^ permalink raw reply related
* [PATCH 4/5] Use run_command() to spawn external diff programs instead of fork/exec.
From: Johannes Sixt @ 2007-10-03 20:09 UTC (permalink / raw)
To: gitster; +Cc: git, Johannes Sixt
In-Reply-To: <1191442180-15905-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.3.1134.gee562
^ permalink raw reply related
* [PATCH 1/5] Change git_connect() to return a struct child_process instead of a pid_t.
From: Johannes Sixt @ 2007-10-03 20:09 UTC (permalink / raw)
To: gitster; +Cc: git, Johannes Sixt
In-Reply-To: <1191442180-15905-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>
---
I don't replace the waitpid() in finish_connect() by
finish_command() because (1) that's not the purpose of this patch
and (2) there is no corresponding start_command(), yet.
-- Hannes
builtin-archive.c | 8 +++-----
cache.h | 4 ++--
connect.c | 31 +++++++++++++++++--------------
fetch-pack.c | 8 +++-----
peek-remote.c | 8 +++-----
send-pack.c | 8 +++-----
6 files changed, 31 insertions(+), 36 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/cache.h b/cache.h
index e0abcd6..548fb52 100644
--- a/cache.h
+++ b/cache.h
@@ -502,8 +502,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 06d279e..458a502 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/fetch-pack.c b/fetch-pack.c
index 9c81305..d06b5ec 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -668,7 +668,7 @@ int main(int argc, char **argv)
int i, ret, nr_heads;
char *dest = NULL, **heads;
int fd[2];
- pid_t pid;
+ struct child_process *conn;
struct stat st;
setup_git_directory();
@@ -733,15 +733,13 @@ int main(int argc, char **argv)
}
if (!dest)
usage(fetch_pack_usage);
- pid = git_connect(fd, dest, uploadpack, verbose ? CONNECT_VERBOSE : 0);
- if (pid < 0)
- return 1;
+ conn = git_connect(fd, dest, uploadpack, verbose ? CONNECT_VERBOSE : 0);
if (heads && nr_heads)
nr_heads = remove_duplicates(nr_heads, heads);
ret = fetch_pack(fd, nr_heads, heads);
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/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 f74e66a..f4c7bbf 100644
--- a/send-pack.c
+++ b/send-pack.c
@@ -362,7 +362,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;
@@ -426,12 +426,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;
}
--
1.5.3.3.1134.gee562
^ permalink raw reply related
* [PATCH 2/5] Use start_command() in git_connect() instead of explicit fork/exec.
From: Johannes Sixt @ 2007-10-03 20:09 UTC (permalink / raw)
To: gitster; +Cc: git, Johannes Sixt
In-Reply-To: <1191442180-15905-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 458a502..bb55d19 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,73 +573,65 @@ 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 */
if (free_path)
free(path);
+ strbuf_release(&cmd);
return conn;
}
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.3.1134.gee562
^ permalink raw reply related
* [PATCH 3/5] Use start_command() to run the filter instead of explicit fork/exec.
From: Johannes Sixt @ 2007-10-03 20:09 UTC (permalink / raw)
To: gitster; +Cc: git, Johannes Sixt
In-Reply-To: <1191442180-15905-3-git-send-email-johannes.sixt@telecom.at>
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 0d5e909..6a8c806 100644
--- a/convert.c
+++ b/convert.c
@@ -197,34 +197,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.3.1134.gee562
^ permalink raw reply related
* [PATCH 0/5, resend] fork/exec removal series
From: Johannes Sixt @ 2007-10-03 20:09 UTC (permalink / raw)
To: gitster; +Cc: git, Johannes Sixt
In-Reply-To: <200709302340.17644.johannes.sixt@telecom.at>
Here is a series of patches that removes a number fork/exec pairs.
They are replaced by delegating to start_command/finish_command/run_command.
I've addressed all issues that were raised. However, concerning whether
git_connect() shoud return NULL or not, I decided to stay with my earlier
approach to return NULL for non-forking connections. Consequently, I had to
remove all error checks because there is now no unique value that can
indicate failure. The motivation is that the alternate approach (to always
return non-NULL for any successful connection and NULL for failure) would
benefit *only* libification; but *if* that ever happens, a major audit
(and possibly) rewrite in the callers has to be done anyway because
currently we are juggling, leaking, and double-closing file descriptors
like mad - in error paths and normal paths.
Patch 2 depends on Patch 1; otherwise, there are no dependencies.
The series goes on top of 'master'. Note that 'next' has an additional
call of git_connect() in the new transport.c.
builtin-archive.c | 8 +--
cache.h | 4 +-
connect.c | 128 +++++++++++++++++++++++++---------------------------
convert.c | 30 +++----------
diff.c | 38 +--------------
fetch-pack.c | 43 +++++------------
peek-remote.c | 8 +--
send-pack.c | 8 +--
8 files changed, 96 insertions(+), 171 deletions(-)
-- Hannes
^ permalink raw reply
* git-cvsserver test failures (still)
From: Brian Gernhardt @ 2007-10-03 19:50 UTC (permalink / raw)
To: Git Mailing List
Periodically I am reminded that the git-cvsserver does not pass it's
tests (t9400-git-cvsserver.sh) on my machine, so I once again ask for
help. The failing tests are: (they say skip because this is from the
last "find other errors" run)
* skip 9: req_Root failure (strict-paths)
* skip 11: req_Root failure (w/o strict-paths)
* skip 13: req_Root failure (base-path)
No other tests fail, including other req_Root failure tests :
* ok 6: req_Root failure (relative pathname)
* ok 7: req_Root failure (conflicting roots)
* ok 15: req_Root failure (export-all w/o whitelist)
All three tests fail with the exact same error log:
E /Users/brian/dev/git/t/trash/gitcvs.git/ does not seem to be a
valid GIT repository
E
error 1 /Users/brian/dev/git/t/trash/gitcvs.git/ is not a valid
repository
E Invalid root /Users/brian/dev/git/t/trash/gitcvs.git
This appears to be from git-cvsserver.perl:148-9:
req_Root('root', $line) # reuse Root
or die "E Invalid root $line \n";
This fails the test suite because die() exits with code 255 (checked
with "perl -e 'die'; echo $?"), which is outside what
test_expect_failure accepts (see t/test-lib.sh:179).
My questions become:
1) Why hasn't this hit anyone else?
2) Is this where these tests are supposed to fail?
3) If it is, should the code be using print and exit 1 instead of die?
4) If not, should the test be altered to end with "|| false" or
similar so the test passes?
I'd happily submit a patch to fix this, but don't know what the
correct fix is.
~~ Brian Gernhardt
^ permalink raw reply
* Re: merging .gitignore
From: Junio C Hamano @ 2007-10-03 19:38 UTC (permalink / raw)
To: Johan Herland; +Cc: git, Johannes Schindelin, Andy Parkins, martin f krafft
In-Reply-To: <200710031506.23938.johan@herland.net>
Johan Herland <johan@herland.net> writes:
> On Wednesday 03 October 2007, Johannes Schindelin wrote:
>> On Wed, 3 Oct 2007, Johan Herland wrote:
>> > - Teach the .gitignore parser to ignore conflict markers (i.e. regard them
>> > as comments)
>>
>> You might be delighted to know that in practice, it works already (because
>> you usually do not have a file named "<<<<<< blablub" or "======" or
>> ">>>>>> blablub"...
>
> I suspected so... ;)
Yeah, and that is one of the reasons why we made gitignore and
gitattributes one-entry-per-line format.
^ permalink raw reply
* Re: [PATCH] Add test case for ls-files --with-head
From: Junio C Hamano @ 2007-10-03 19:29 UTC (permalink / raw)
To: Carl Worth
Cc: Johannes Schindelin, Johannes Sixt, Keith Packard,
Git Mailing List
In-Reply-To: <87myv0qj2u.wl%cworth@cworth.org>
Carl Worth <cworth@cworth.org> writes:
> On Wed, 3 Oct 2007 16:36:13 +0100 (BST), Johannes Schindelin wrote:
>> Or as
>>
>> i=1
>> while test $i -le 50
>> do
> ...
>> i=$(($i+1))
>> done
>
> /me steps aside to let the shell-script wizards finish the job
I've already pushed out a rewritten one. Thanks.
The bug makes 1.5.3.3 a dud, and 1.5.3.4 owes credits to Keith
and you for fixing it.
^ permalink raw reply
* Re: WIP: asciidoc replacement
From: Sam Ravnborg @ 2007-10-03 19:21 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Junio C Hamano, Wincent Colaiuta, git, msysgit
In-Reply-To: <Pine.LNX.4.64.0710031957170.28395@racer.site>
On Wed, Oct 03, 2007 at 07:57:48PM +0100, Johannes Schindelin wrote:
> Hi,
>
> On Wed, 3 Oct 2007, Sam Ravnborg wrote:
>
> > For a kernel integrated tool the dependencies shall be minimal which is
> > where asciidoc fails today.
>
> Is perl not too much already?
No - perl has not proved to be a problem.
But like git we have had a smaller share of docbook
incompatibilities.
We even require perl for a regular kernel build
for some arch's these days.
Sam
^ permalink raw reply
* Re: [PATCH] Add test case for ls-files --with-head
From: David Kastrup @ 2007-10-03 16:15 UTC (permalink / raw)
To: Carl Worth
Cc: Johannes Schindelin, Johannes Sixt, Junio C Hamano, Keith Packard,
Git Mailing List
In-Reply-To: <87myv0qj2u.wl%cworth@cworth.org>
Carl Worth <cworth@cworth.org> writes:
> On Wed, 3 Oct 2007 16:36:13 +0100 (BST), Johannes Schindelin wrote:
>> Or as
>>
>> i=1
>> while test $i -le 50
>> do
> ...
>> i=$(($i+1))
>> done
>
> /me steps aside to let the shell-script wizards finish the job
for i in {1,2,3,4,5}{0,1,2,3,4,5,6,7,8,9}
do
...
done
There is enough room for perversion in shell programming for everyone...
--
David Kastrup, Kriemhildstr. 15, 44793 Bochum
^ permalink raw reply
* Re: [PATCH] Add test case for ls-files --with-head
From: David Kastrup @ 2007-10-03 15:52 UTC (permalink / raw)
To: Johannes Schindelin
Cc: Johannes Sixt, Carl Worth, Junio C Hamano, Keith Packard,
Git Mailing List
In-Reply-To: <Pine.LNX.4.64.0710031634300.28395@racer.site>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> On Wed, 3 Oct 2007, Johannes Sixt wrote:
>>
>> seq is not universally available. Can we have that as
>>
>> for i in 0 1 2 3 4; do
>> for j in 0 1 2 3 4 5 6 7 8 9; do
>> > sub/file-$i$j
>> echo file-$i$j >> expected
>> done
>> done
>
> Or as
>
> i=1
> while test $i -le 50
> do
> num=$(printf %04d $i)
> > sub/file-$num
> echo file-$num >> expected
> i=$(($i+1))
> done
>
> This version should be as portable,
Huh? It uses the conceivably-not-builtin "test" (something which
_you_ picked as something to complain about in a patch of mine where
it was not used in an inner loop) on every iteration, it uses printf
and it uses $((...)) arithmetic expansion. Whereas the proposal by
Johannes works fine even on prehistoric shell versions. So the "as
portable" enough moniker is surely weird.
> with the benefit that it is easier to change for different start and
> end values.
Correct. But why would we want those here?
--
David Kastrup, Kriemhildstr. 15, 44793 Bochum
^ permalink raw reply
* Re: What's cooking in git.git (topics)
From: David Kastrup @ 2007-10-03 18:09 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Jeff King, Git Mailing List
In-Reply-To: <alpine.LFD.0.999.0710031007340.3579@woody.linux-foundation.org>
Linus Torvalds <torvalds@linux-foundation.org> writes:
> On Wed, 3 Oct 2007, Jeff King wrote:
>>
>> Try profiling the code, and you will see that the creation of the hashes
>> is totally dwarfed by the comparisons. So yes, you might be able to
>> speed up the creation code, but it's going to have a minimal impact on
>> the overall run time.
>
> Yeah. Oprofile is your friend.
Well, and if -Oprofile has no strong opinion, I'd let wc -l pitch in.
When we are not actually going to use the hash tables as hash tables,
why create them as such? If the first thing that actually looks at
the values of the hashes (except possibly for the optimization of not
storing the same hash twice in succession) is the sort, then there is
no code that can go out of whack when confronted with degenerate data.
Maybe it's not much of an optimization, but it certainly should be a
cleanup.
--
David Kastrup, Kriemhildstr. 15, 44793 Bochum
^ permalink raw reply
* Re: WIP: asciidoc replacement
From: Johannes Schindelin @ 2007-10-03 18:57 UTC (permalink / raw)
To: Sam Ravnborg; +Cc: Junio C Hamano, Wincent Colaiuta, git, msysgit
In-Reply-To: <20071003174659.GA13691@uranus.ravnborg.org>
Hi,
On Wed, 3 Oct 2007, Sam Ravnborg wrote:
> For a kernel integrated tool the dependencies shall be minimal which is
> where asciidoc fails today.
Is perl not too much already?
Ciao,
Dscho
^ permalink raw reply
* Re: git-cvsserver commit trouble (unexpected end of file in client)
From: Johannes Schindelin @ 2007-10-03 18:55 UTC (permalink / raw)
To: Jan Wielemaker; +Cc: git
In-Reply-To: <200710032042.14842.wielemak@science.uva.nl>
Hi,
On Wed, 3 Oct 2007, Jan Wielemaker wrote:
> On Wednesday 03 October 2007 18:11, Johannes Schindelin wrote:
> > Hi,
> >
> > On Wed, 3 Oct 2007, Jan Wielemaker wrote:
> > > 2007-10-03 12:25:16 : WARN - error 1 pserver cannot find the current
> > > HEAD of module
> >
> > AFAIR we do not allow committing via pserver protocol. Might that be
> > your problem?
>
> Thanks, but no. I'm using CVS over SSH. I've been looking around in
> git-cvsserver source a bit and it aborts quite quickly if you try a
> commit through pserver. I get a bit further, but it cannot find the HEAD
> revision for some reason and (from later message), if I try to checkout
> master instead of HEAD it finds the revision but I get a hash mismatch.
Okay, another stab: is your HEAD detached?
Ciao,
Dscho
^ permalink raw reply
* Re: git-cvsserver commit trouble (unexpected end of file in client)
From: Jan Wielemaker @ 2007-10-03 18:42 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0710031711070.28395@racer.site>
Dscho,
On Wednesday 03 October 2007 18:11, Johannes Schindelin wrote:
> Hi,
>
> On Wed, 3 Oct 2007, Jan Wielemaker wrote:
> > 2007-10-03 12:25:16 : WARN - error 1 pserver cannot find the current
> > HEAD of module
>
> AFAIR we do not allow committing via pserver protocol. Might that be your
> problem?
Thanks, but no. I'm using CVS over SSH. I've been looking around in
git-cvsserver source a bit and it aborts quite quickly if you try a
commit through pserver. I get a bit further, but it cannot find the HEAD
revision for some reason and (from later message), if I try to checkout
master instead of HEAD it finds the revision but I get a hash mismatch.
I've tried a bit debugging this, but in 15 years CVS experience I never
really needed to debug the protocol and my GIT experience is only 2
weeks old :-(
My hope is I'm doing something fundamentally wrong and git-cvsserver
just doesn't give a sensible error. I did setup the git repository using
two different routes, one adviced in the CVS conversion manual. GIT
operations work just fine, so does CVS checkout. I don't think you can
to that much wrong with cvs over ssh clients, especially if checkout
works just fine.
Does anyone out there has a working GIT <-> CVS+SHH setup? Based on
what version of GIT? Using what route to create the repository?
Thanks --- Jan
^ 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