* Re: [AGGREGATED PATCH] Fix in-place editing functions in convert.c
From: Sam Ravnborg @ 2007-10-05 16:21 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Pierre Habouzit, Junio C Hamano, git, Bernt Hansen
In-Reply-To: <alpine.LFD.0.999.0710050819540.23684@woody.linux-foundation.org>
On Fri, Oct 05, 2007 at 08:26:44AM -0700, Linus Torvalds wrote:
>
>
> On Fri, 5 Oct 2007, Pierre Habouzit wrote:
> >
> > - strbuf_grow(buf, len);
> > + /* only grow if not in place */
> > + if (strbuf_avail(buf) + buf->len < len)
> > + strbuf_grow(buf, len - buf->len);
>
> Umm. This is really ugly.
>
> The whole point of strbuf's was that you shouldn't be doing your own
> allocation decisions etc. So why do it?
>
> Wouldn't it be much better to have a strbuf_make_room() interface that
> just guarantees that there is enough room fo "len"?
>
> Otherwise, code like the above would seem to make the whole point of a
> safer string interface rather pointless. The above code only makes sense
> if you know how the strbuf's are internally done, so it should not exists
> except as internal strbuf code. No?
Took a short look at strbuf.h after seeing the above code.
And I was suprised to see that all strbuf users were exposed to
the strbuf structure.
Following patch would at least make sure noone fiddle with strbuf internals.
Cut'n'paste - only for the example of it.
It simply moves strbuf declaration to the .c file where it rightfully belongs.
git did not build with this change....
Sam
diff --git a/strbuf.c b/strbuf.c
index e33d06b..0d2d578 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -1,6 +1,14 @@
#include "cache.h"
#include "strbuf.h"
+struct strbuf {
+ int alloc;
+ int len;
+ int eof;
+ char *buf;
+};
+
+
void strbuf_init(struct strbuf *sb) {
sb->buf = NULL;
sb->eof = sb->alloc = sb->len = 0;
diff --git a/strbuf.h b/strbuf.h
index 74cc012..c057be3 100644
--- a/strbuf.h
+++ b/strbuf.h
@@ -1,11 +1,6 @@
#ifndef STRBUF_H
#define STRBUF_H
-struct strbuf {
- int alloc;
- int len;
- int eof;
- char *buf;
-};
+struct strbuf;
extern void strbuf_init(struct strbuf *);
extern void read_line(struct strbuf *, FILE *, int);
^ permalink raw reply related
* [PATCH 3/2] Document the fact that git-svn now runs git-gc
From: Steven Grimm @ 2007-10-05 16:15 UTC (permalink / raw)
To: git
In-Reply-To: <47066255.6080500@midwinter.com>
Signed-off-by: Steven Grimm <koreth@midwinter.com>
---
Documentation/git-svn.txt | 10 +++++++++-
1 files changed, 9 insertions(+), 1 deletions(-)
diff --git a/Documentation/git-svn.txt b/Documentation/git-svn.txt
index e157c6a..26f0f39 100644
--- a/Documentation/git-svn.txt
+++ b/Documentation/git-svn.txt
@@ -125,7 +125,15 @@ and have no uncommitted changes.
alternative to HEAD.
This is advantageous over 'set-tree' (below) because it produces
cleaner, more linear history.
-+
+
+When the commit is finished, gitlink:git-gc[1] is run with the
+`--prune` and `--auto` options to clean up the git object database,
+including removing old unreachable objects (some of which are
+created by the process of committing to SVN.) Set the `gc.auto`
+config option to 0 if you don't want your repository to be cleaned,
+e.g., because you are intentionally keeping unreachable objects in
+your repository.
+
--no-rebase;;
After committing, do not rebase or reset.
--
--
1.5.3.4.203.gcc61a
^ permalink raw reply related
* Re: [PATCH 2/2] Run garbage collection with loose object pruning after svn dcommit
From: Steven Grimm @ 2007-10-05 16:12 UTC (permalink / raw)
To: Peter Baumann; +Cc: git
In-Reply-To: <20071005082110.GA4797@xp.machine.xx>
Peter Baumann wrote:
> I don't like the automatic prune. What if someone has other objects in
> there which shouldn't be pruned? Making git svn dcommit doing the prune
> would be at least suprising, because how is one supposed to know that
> doing a commit into svn will prune all your precious objects?
>
"git commit" already does garbage collection, so we've already set a
precedent for a commit operation also doing some cleanup at the end.
However, you're correct that this cleanup behavior (and the way to turn
it off) should be documented so that there's some way to know about it.
Doc patch forthcoming.
> Sure, I can unterstand from where you are coming from, but I'd prefere
> if this could be specified on a case by case basis, e.g. from the
> cmdline or as a config option.
>
This code (by virtue of only doing the prune if the "too many loose
objects" test succeeds) will obey the existing gc.auto config option. So
it's already possible to turn off as is. I'll note that in the doc patch.
-Steve
^ permalink raw reply
* RE: [ALTERNATE PATCH] Add a simple option parser.
From: Medve Emilian-EMMEDVE1 @ 2007-10-05 16:10 UTC (permalink / raw)
To: Pierre Habouzit; +Cc: Mike Hommey, Kristian Høgsberg, git, Junio C Hamano
In-Reply-To: <20071005155647.GC20305@artemis.corp>
Hi Pierre,
> -----Original Message-----
> From: Pierre Habouzit [mailto:madcoder@debian.org]
> Sent: Friday, October 05, 2007 10:57 AM
> To: Medve Emilian-EMMEDVE1
> Cc: Mike Hommey; Kristian Høgsberg; git@vger.kernel.org;
> Junio C Hamano
> Subject: Re: [ALTERNATE PATCH] Add a simple option parser.
>
> On ven, oct 05, 2007 at 03:45:36 +0000, Medve Emilian-EMMEDVE1 wrote:
> > You probably already considered and rejected the GNU argp parser. I
> > used it before and I'd like to know reasons I should stay away from
> > it.
>
> Because it's GNU and that it's a heavy dependency to begin with.
So it's more of a political decision then a technical one?
> Moreover, getopt_long doesn't deal with argument types (like
> integers).
AFAIK, getopt_long in not argp.
Cheers,
Emil.
^ permalink raw reply
* Re: [ALTERNATE PATCH] Add a simple option parser.
From: Pierre Habouzit @ 2007-10-05 15:56 UTC (permalink / raw)
To: Medve Emilian-EMMEDVE1
Cc: Mike Hommey, Kristian Høgsberg, git, Junio C Hamano
In-Reply-To: <598D5675D34BE349929AF5EDE9B03E2701624FD6@az33exm24.fsl.freescale.net>
[-- Attachment #1: Type: text/plain, Size: 542 bytes --]
On ven, oct 05, 2007 at 03:45:36 +0000, Medve Emilian-EMMEDVE1 wrote:
> You probably already considered and rejected the GNU argp parser. I
> used it before and I'd like to know reasons I should stay away from
> it.
Because it's GNU and that it's a heavy dependency to begin with.
Moreover, getopt_long doesn't deal with argument types (like integers).
--
·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: Question about "git commit -a"
From: Kristian Høgsberg @ 2007-10-05 15:56 UTC (permalink / raw)
To: Andreas Ericsson
Cc: Paolo Ciarrocchi, Johannes Schindelin, Nguyen Thai Ngoc Duy,
Wincent Colaiuta, Git Mailing List
In-Reply-To: <4705FB52.3030208@op5.se>
> I just don't do 'git commit -a' for the same reason I don't do
> 'git commit -m', really. It tends to be habit-forming, and bisect
> has saved my arse enough times for me to *want* my changes to be
> small and isolated. Debugging a 5-line patch is so much more pleasant
> than debugging a 30k-lines one that spans over several different files.
I understand why people like staging and commit without -a, seeing how
it's faster and all, but I have a serious problem with this practice
that I haven't seen brought up on the list. How do you know what you
commit actually works or even compiles? The reason that I almost
exclusively use -a with commit is that I want to know that what I just
compiled and tested is what I will be committing. I don't want to just
commit half the files in my working copy, I want to make sure that the
exact state of my project that I just compiled and tested is what gets
into version controlled history.
git commit -a isn't sloppy to me - eye balling some subset of your
working copy and committing that under the assumption that you don't
make mistakes and don't need to compile what you commit... that is
sloppy.
Kristian
^ permalink raw reply
* Re: [ALTERNATE PATCH] Add a simple option parser.
From: Pierre Habouzit @ 2007-10-05 15:54 UTC (permalink / raw)
To: Kristian Høgsberg; +Cc: git, Junio C Hamano
In-Reply-To: <1191598424.7117.10.camel@hinata.boston.redhat.com>
[-- Attachment #1: Type: text/plain, Size: 1890 bytes --]
On Fri, Oct 05, 2007 at 03:33:44PM +0000, Kristian Høgsberg wrote:
> On Fri, 2007-10-05 at 16:25 +0200, Pierre Habouzit wrote:
> > The option parser takes argc, argv, an array of struct option
> > and a usage string. Each of the struct option elements in the array
> > describes a valid option, its type and a pointer to the location where the
> > value is written. The entry point is parse_options(), which scans through
> > the given argv, and matches each option there against the list of valid
> > options. During the scan, argv is rewritten to only contain the
> > non-option command line arguments and the number of these is returned.
> >
> > Aggregation of single switches is allowed:
> > -rC0 is the same as -r -C 0 (supposing that -C wants an arg).
> >
> > Boolean switches automatically support the option with the same name,
> > prefixed with 'no-' to disable the switch:
> > --no-color / --color only need to have an entry for "color".
> >
> > Long options are supported either with '=' or without:
> > --some-option=foo is the same as --some-option foo
>
> That looks great, works for me. One comment, though: it looks like
> you're not sure whether to call these things "options" or "switches".
> We should choose one and stick with it.
I use the word "switch" when it's a short_option, and "option" when
it's a long one. But maybe the distinction doesn't make sense, and it's
a non-native speaker glitch. I don't care that much btw.
> > oh and I don't grok what OPTION_LAST is for, so I left it apart, but
> > it seems unused ?
>
> Oh, kill that. I used that as the option array terminator before we
> switched to ARRAY_SIZE().
Okay :)
--
·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: [AGGREGATED PATCH] Fix in-place editing functions in convert.c
From: Pierre Habouzit @ 2007-10-05 15:50 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Junio C Hamano, git, Bernt Hansen
In-Reply-To: <alpine.LFD.0.999.0710050819540.23684@woody.linux-foundation.org>
[-- Attachment #1: Type: text/plain, Size: 3385 bytes --]
On Fri, Oct 05, 2007 at 03:26:44PM +0000, Linus Torvalds wrote:
>
>
> On Fri, 5 Oct 2007, Pierre Habouzit wrote:
> >
> > - strbuf_grow(buf, len);
> > + /* only grow if not in place */
> > + if (strbuf_avail(buf) + buf->len < len)
> > + strbuf_grow(buf, len - buf->len);
>
> Umm. This is really ugly.
I agree.
> The whole point of strbuf's was that you shouldn't be doing your own
> allocation decisions etc. So why do it?
the point here is that it's in a "filter" that is called like this:
some_filter(buf->buf, buf->len, buf);
src len dst
You can call the filter with src/len being data from anywere,
including the current content of the destination buffer.
Then there is two cases, either the filter is known to be done in
place, either we can't know or we know it wont.
In the latter case, we have a bit of code like that:
char *to_free = NULL;
if (buf->buf == src)
to_free = strbuf_detach(&buf);
.. hack ..
free(to_free);
In the former case, then there is a small glitch, being that if we are
doing in place editing, we should not touch buffer at all (or it would
invalidate "src"). If we are not in the in-place editing code though,
then we have to make the resulting buffer be big enough...
> Wouldn't it be much better to have a strbuf_make_room() interface that
> just guarantees that there is enough room fo "len"?
Right, that would do the same btw ;)
> Otherwise, code like the above would seem to make the whole point of a
> safer string interface rather pointless. The above code only makes sense
> if you know how the strbuf's are internally done, so it should not exists
> except as internal strbuf code. No?
Well, the above code is used in filters to spare reallocations. So if
we want to "blackbox" such a think, strbuf_make_room isn't the proper
API. We should rather use
void *strbuf_begin_filter(struct strbuf *sb, const char *src, size_t reslen);
strbuf_end_filter(void *);
`strbuf_begin_filter` would decide upon the hint `reslen` argument if
we know if we can work in place or not (has a meaning iff src points
into the strbuf buffer). If not, it could stash the strbuf buffer in the
returned void * to be freed at the end of the filter. It seems like a
better alternative than a strbuf_make_room.
Of course, strbuf_begin_filter() would really be simple and basically
be:
char *tmp;
if (src points into sb->buf && reslen > sb->alloc - 1) {
// in place editing is OK
return NULL;
}
tmp = strbuf_release(&sb);
strbuf_grow(&sb, len);
return tmp;
and strbuf_end_filter would just be "free" :)
We could even make "reslen" be a ssize_t so that -1 would mean "I've
absolutely no idea how much space I'll need (or just in place editing is
not supported). This way, both hacks I described in this mail could be
hidden in the strbuf module, and be properly documented _and_ safe _and_
efficient.
What do you think ?
[Though if we do that, I still think it's more important to fix the
bug in master, and have a new patch implementing this approach]
--
·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: [ALTERNATE PATCH] Add a simple option parser.
From: Kristian Høgsberg @ 2007-10-05 15:33 UTC (permalink / raw)
To: Pierre Habouzit; +Cc: git, Junio C Hamano
In-Reply-To: <20071005142507.GL19879@artemis.corp>
On Fri, 2007-10-05 at 16:25 +0200, Pierre Habouzit wrote:
> The option parser takes argc, argv, an array of struct option
> and a usage string. Each of the struct option elements in the array
> describes a valid option, its type and a pointer to the location where the
> value is written. The entry point is parse_options(), which scans through
> the given argv, and matches each option there against the list of valid
> options. During the scan, argv is rewritten to only contain the
> non-option command line arguments and the number of these is returned.
>
> Aggregation of single switches is allowed:
> -rC0 is the same as -r -C 0 (supposing that -C wants an arg).
>
> Boolean switches automatically support the option with the same name,
> prefixed with 'no-' to disable the switch:
> --no-color / --color only need to have an entry for "color".
>
> Long options are supported either with '=' or without:
> --some-option=foo is the same as --some-option foo
That looks great, works for me. One comment, though: it looks like
you're not sure whether to call these things "options" or "switches".
We should choose one and stick with it.
Acked-by: Kristian Høgsberg <krh@redhat.com>
> Signed-off-by: Pierre Habouzit <madcoder@debian.org>
> ---
>
> I'm sorry about the "From" I don't intend to "steal" the patch in any
> sense, it's just an alternate proposal.
No worries, I'm glad to see this move forward.
> oh and I don't grok what OPTION_LAST is for, so I left it apart, but
> it seems unused ?
Oh, kill that. I used that as the option array terminator before we
switched to ARRAY_SIZE().
>
> Makefile | 2 +-
> parse-options.c | 154 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
> parse-options.h | 29 ++++++++++
> 3 files changed, 184 insertions(+), 1 deletions(-)
> create mode 100644 parse-options.c
> create mode 100644 parse-options.h
>
> diff --git a/Makefile b/Makefile
> index 62bdac6..d90e959 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -310,7 +310,7 @@ LIB_OBJS = \
> alloc.o merge-file.o path-list.o help.o unpack-trees.o $(DIFF_OBJS) \
> color.o wt-status.o archive-zip.o archive-tar.o shallow.o utf8.o \
> convert.o attr.o decorate.o progress.o mailmap.o symlinks.o remote.o \
> - transport.o bundle.o
> + transport.o bundle.o parse-options.o
>
> BUILTIN_OBJS = \
> builtin-add.o \
> diff --git a/parse-options.c b/parse-options.c
> new file mode 100644
> index 0000000..eb3ff40
> --- /dev/null
> +++ b/parse-options.c
> @@ -0,0 +1,154 @@
> +#include "git-compat-util.h"
> +#include "parse-options.h"
> +
> +struct optparse_t {
> + const char **argv;
> + int argc;
> + const char *opt;
> +};
> +
> +static inline const char *skippfx(const char *str, const char *prefix)
> +{
> + size_t len = strlen(prefix);
> + return strncmp(str, prefix, len) ? NULL : str + len;
> +}
> +
> +static int opterror(struct option *opt, const char *reason, int shorterr)
> +{
> + if (shorterr) {
> + return error("switch `%c' %s", opt->short_name, reason);
> + } else {
> + return error("option `%s' %s", opt->long_name, reason);
> + }
> +}
option/switch?
> +static int get_value(struct optparse_t *p, struct option *opt,
> + int boolean, int shorterr)
> +{
> + switch (opt->type) {
> + const char *s;
> + int v;
> +
> + case OPTION_BOOLEAN:
> + *(int *)opt->value = boolean;
> + return 0;
> +
> + case OPTION_STRING:
> + if (p->opt && *p->opt) {
> + *(const char **)opt->value = p->opt;
> + p->opt = NULL;
> + } else {
> + if (p->argc < 1)
> + return opterror(opt, "requires a value", shorterr);
> + *(const char **)opt->value = *++p->argv;
> + p->argc--;
> + }
> + return 0;
> +
> + case OPTION_INTEGER:
> + if (p->opt && *p->opt) {
> + v = strtol(p->opt, (char **)&s, 10);
> + p->opt = NULL;
> + } else {
> + if (p->argc < 1)
> + return opterror(opt, "requires a value", shorterr);
> + v = strtol(*++p->argv, (char **)&s, 10);
> + p->argc--;
> + }
> + if (*s)
> + return opterror(opt, "expects a numerical value", shorterr);
> + *(int *)opt->value = v;
> + return 0;
> + }
> +
> + abort();
> +}
> +
> +static int parse_short_opt(struct optparse_t *p, struct option *options, int count)
> +{
> + int i;
> +
> + for (i = 0; i < count; i++) {
> + if (options[i].short_name == *p->opt) {
> + p->opt++;
> + return get_value(p, options + i, 1, 1);
> + }
> + }
> + return error("unknown switch `%c'", *p->opt);
> +}
> +
> +static int parse_long_opt(struct optparse_t *p, const char *arg,
> + struct option *options, int count)
> +{
> + int boolean = 1;
> + int i;
> +
> + for (i = 0; i < count; i++) {
> + const char *rest;
> +
> + if (!options[i].long_name)
> + continue;
> +
> + rest = skippfx(arg, options[i].long_name);
> + if (!rest && options[i].type == OPTION_BOOLEAN) {
> + if (!rest && skippfx(arg, "no-")) {
> + rest = skippfx(arg + 3, options[i].long_name);
> + boolean = 0;
> + }
> + if (rest && *rest == '=')
> + return opterror(options + i, "takes no value", 0);
> + }
> + if (!rest || (*rest && *rest != '='))
> + continue;
> + if (*rest) {
> + p->opt = rest;
> + }
> + return get_value(p, options + i, boolean, 0);
> + }
> + return error("unknown option `%s'", arg);
> +}
> +
> +int parse_options(int argc, const char **argv,
> + struct option *options, int count,
> + const char *usage_string)
> +{
> + struct optparse_t optp = { argv + 1, argc - 1, NULL };
> + int j = 0;
> +
> + while (optp.argc) {
> + const char *arg = optp.argv[0];
> +
> + if (*arg != '-' || !arg[1]) {
> + argv[j++] = *optp.argv++;
> + optp.argc--;
> + continue;
> + }
> +
> + if (arg[1] != '-') {
> + optp.opt = arg + 1;
> + while (*optp.opt) {
> + if (parse_short_opt(&optp, options, count) < 0) {
> + usage(usage_string);
> + return -1;
> + }
> + }
> + optp.argc--;
> + optp.argv++;
> + continue;
> + }
> +
> + if (!arg[2]) /* "--" */
> + break;
> +
> + if (parse_long_opt(&optp, arg + 2, options, count)) {
> + usage(usage_string);
> + return -1;
> + }
> + optp.argc--;
> + optp.argv++;
> + }
> +
> + memmove(argv + j, optp.argv, optp.argc * sizeof(argv));
> + argv[j + optp.argc] = NULL;
> + return j + optp.argc;
> +}
> diff --git a/parse-options.h b/parse-options.h
> new file mode 100644
> index 0000000..e4749d0
> --- /dev/null
> +++ b/parse-options.h
> @@ -0,0 +1,29 @@
> +#ifndef PARSE_OPTIONS_H
> +#define PARSE_OPTIONS_H
> +
> +enum option_type {
> + OPTION_BOOLEAN,
> + OPTION_STRING,
> + OPTION_INTEGER,
> +#if 0
> + OPTION_LAST,
> +#endif
> +};
> +
> +struct option {
> + enum option_type type;
> + const char *long_name;
> + char short_name;
> + void *value;
> +};
> +
> +/* parse_options() will filter out the processed options and leave the
> + * non-option argments in argv[]. The return value is the number of
> + * arguments left in argv[].
> + */
> +
> +extern int parse_options(int argc, const char **argv,
> + struct option *options, int count,
> + const char *usage_string);
> +
> +#endif
^ permalink raw reply
* RE: [ALTERNATE PATCH] Add a simple option parser.
From: Medve Emilian-EMMEDVE1 @ 2007-10-05 15:45 UTC (permalink / raw)
To: Pierre Habouzit, Mike Hommey; +Cc: Kristian Høgsberg, git, Junio C Hamano
In-Reply-To: <20071005144540.GM19879@artemis.corp>
Hi,
You probably already considered and rejected the GNU argp parser. I used it before and I'd like to know reasons I should stay away from it.
Cheers,
Emil.
> -----Original Message-----
> From: git-owner@vger.kernel.org
> [mailto:git-owner@vger.kernel.org] On Behalf Of Pierre Habouzit
> Sent: Friday, October 05, 2007 9:46 AM
> To: Mike Hommey
> Cc: Kristian Høgsberg; git@vger.kernel.org; Junio C Hamano
> Subject: Re: [ALTERNATE PATCH] Add a simple option parser.
>
> On Fri, Oct 05, 2007 at 02:30:14PM +0000, Mike Hommey wrote:
> > On Fri, Oct 05, 2007 at 04:25:07PM +0200, Pierre Habouzit
> <madcoder@debian.org> wrote:
> > > The option parser takes argc, argv, an array of struct option
> > > and a usage string. Each of the struct option elements
> in the array
> > > describes a valid option, its type and a pointer to the
> location where the
> > > value is written. The entry point is parse_options(),
> which scans through
> > > the given argv, and matches each option there against the
> list of valid
> > > options. During the scan, argv is rewritten to only contain the
> > > non-option command line arguments and the number of these
> is returned.
> > >
> > > Aggregation of single switches is allowed:
> > > -rC0 is the same as -r -C 0 (supposing that -C wants an arg).
> >
> > I like options aggregation, but I'm not sure aggregating
> option arguments
> > is a good idea... I can't even think of an application that does it.
>
> You mean like `grep -A1` or `diff -u3` or `ls -w10` ?
>
> getopt does that by default as well, so you may not have aware of it,
> but it's how things work in your system already.
>
> btw `ls -rw10` works, though `ls -w10r` drops the 'r'
> silently. FWIW I
> don't, in that case, the alternate patch I propose complains
> about "10r"
> not being a valid integer, and that's because unlike getopt, the patch
> krh proposed knows what an integer is ;)
> --
> ·O· Pierre Habouzit
> ··O madcoder@debian.org
> OOO
> http://www.madism.org
^ permalink raw reply
* Re: [AGGREGATED PATCH] Fix in-place editing functions in convert.c
From: Linus Torvalds @ 2007-10-05 15:26 UTC (permalink / raw)
To: Pierre Habouzit; +Cc: Junio C Hamano, git, Bernt Hansen
In-Reply-To: <20071005085522.32EFF1E16E@madism.org>
On Fri, 5 Oct 2007, Pierre Habouzit wrote:
>
> - strbuf_grow(buf, len);
> + /* only grow if not in place */
> + if (strbuf_avail(buf) + buf->len < len)
> + strbuf_grow(buf, len - buf->len);
Umm. This is really ugly.
The whole point of strbuf's was that you shouldn't be doing your own
allocation decisions etc. So why do it?
Wouldn't it be much better to have a strbuf_make_room() interface that
just guarantees that there is enough room fo "len"?
Otherwise, code like the above would seem to make the whole point of a
safer string interface rather pointless. The above code only makes sense
if you know how the strbuf's are internally done, so it should not exists
except as internal strbuf code. No?
Linus
^ permalink raw reply
* Re: [ALTERNATE PATCH] Add a simple option parser.
From: David Kastrup @ 2007-10-05 14:59 UTC (permalink / raw)
To: git
In-Reply-To: <20071005143014.GA18176@glandium.org>
Mike Hommey <mh@glandium.org> writes:
> On Fri, Oct 05, 2007 at 04:25:07PM +0200, Pierre Habouzit <madcoder@debian.org> wrote:
>> The option parser takes argc, argv, an array of struct option
>> and a usage string. Each of the struct option elements in the array
>> describes a valid option, its type and a pointer to the location where the
>> value is written. The entry point is parse_options(), which scans through
>> the given argv, and matches each option there against the list of valid
>> options. During the scan, argv is rewritten to only contain the
>> non-option command line arguments and the number of these is returned.
>>
>> Aggregation of single switches is allowed:
>> -rC0 is the same as -r -C 0 (supposing that -C wants an arg).
>
> I like options aggregation, but I'm not sure aggregating option arguments
> is a good idea... I can't even think of an application that does it.
I think most allow this for the last option in a row. Tar is somewhat
more perverse with its non-option command string:
tar xfzbv filename.tgz 40
uses filename.tgz as the option argument for "f" and 40 for "b".
Note that while tar accepts options instead of the initial command
string,
tar -xfzbv filename.tgz 40
will _not_ work, while
tar -xffilename.tgz -z -b40 -v
presumably would (have no time to test this right now).
--
David Kastrup
^ permalink raw reply
* Re: Many gits are offline this week
From: Frank Lichtenheld @ 2007-10-05 15:13 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: git
In-Reply-To: <20071005010448.GQ2137@spearce.org>
On Thu, Oct 04, 2007 at 09:04:48PM -0400, Shawn O. Pearce wrote:
> With three gits offline for at least the next few days perhaps
> someone would be willing to step up and collect patches so that Junio
> has a reasonable place to pick up from when he can get back online?
While I have neither the time nor the abilities to do this I
will at least accumulate the various cvsserver patches floating
around the last few days so that I can submit them to Junio in one
batch.
Gruesse,
--
Frank Lichtenheld <frank@lichtenheld.de>
www: http://www.djpig.de/
^ permalink raw reply
* Re: [PATCH] git-shell and git-cvsserver
From: Frank Lichtenheld @ 2007-10-05 15:07 UTC (permalink / raw)
To: Jan Wielemaker; +Cc: Git Mailing List
In-Reply-To: <200710051453.47622.wielemak@science.uva.nl>
On Fri, Oct 05, 2007 at 02:53:47PM +0200, Jan Wielemaker wrote:
> +#define EXEC_PATH "/usr/local/bin"
> +
> +static int do_cvs_cmd(const char *me, char *arg)
> +{
> + const char *my_argv[4];
> + const char *oldpath;
> +
> + if ( !arg )
> + die("no argument");
> + if ( strcmp(arg, "server") )
> + die("only allows git-cvsserver server: %s", arg);
> +
> + my_argv[0] = "cvsserver";
> + my_argv[1] = "server";
> + my_argv[2] = NULL;
> +
> + if ( (oldpath=getenv("PATH")) ) {
> + char *newpath = malloc(strlen(oldpath)+strlen(EXEC_PATH)+5+1+1);
> +
> + sprintf(newpath, "PATH=%s:%s", EXEC_PATH, oldpath);
> + putenv(newpath);
> + } else {
> + char *newpath = malloc(strlen(EXEC_PATH)+5+1);
> +
> + sprintf(newpath, "PATH=%s", EXEC_PATH);
> + putenv(newpath);
> + }
> +
> + return execv_git_cmd(my_argv);
> +}
This seems to be mostly a duplication of prepend_to_path from git.c
Gruesse,
--
Frank Lichtenheld <frank@lichtenheld.de>
www: http://www.djpig.de/
^ permalink raw reply
* Re: [ALTERNATE PATCH] Add a simple option parser.
From: Pierre Habouzit @ 2007-10-05 14:45 UTC (permalink / raw)
To: Mike Hommey; +Cc: Kristian Høgsberg, git, Junio C Hamano
In-Reply-To: <20071005143014.GA18176@glandium.org>
[-- Attachment #1: Type: text/plain, Size: 1575 bytes --]
On Fri, Oct 05, 2007 at 02:30:14PM +0000, Mike Hommey wrote:
> On Fri, Oct 05, 2007 at 04:25:07PM +0200, Pierre Habouzit <madcoder@debian.org> wrote:
> > The option parser takes argc, argv, an array of struct option
> > and a usage string. Each of the struct option elements in the array
> > describes a valid option, its type and a pointer to the location where the
> > value is written. The entry point is parse_options(), which scans through
> > the given argv, and matches each option there against the list of valid
> > options. During the scan, argv is rewritten to only contain the
> > non-option command line arguments and the number of these is returned.
> >
> > Aggregation of single switches is allowed:
> > -rC0 is the same as -r -C 0 (supposing that -C wants an arg).
>
> I like options aggregation, but I'm not sure aggregating option arguments
> is a good idea... I can't even think of an application that does it.
You mean like `grep -A1` or `diff -u3` or `ls -w10` ?
getopt does that by default as well, so you may not have aware of it,
but it's how things work in your system already.
btw `ls -rw10` works, though `ls -w10r` drops the 'r' silently. FWIW I
don't, in that case, the alternate patch I propose complains about "10r"
not being a valid integer, and that's because unlike getopt, the patch
krh proposed knows what an integer is ;)
--
·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: Many gits are offline this week
From: Dmitry Potapov @ 2007-10-05 14:41 UTC (permalink / raw)
To: Randal L. Schwartz; +Cc: git
In-Reply-To: <86tzp54sez.fsf@blue.stonehenge.com>
Hi Randal,
> I've had the slides reviewed by Smarter People Than Me on #git already, so
> hopefully most of it is accurate. :) They're temporarily at
>
> http://www.stonehenge.com/pic/Git-2.0.3-to-be.pdf
I believe I have found one mistake in your slides. Slide 18 reads:
"git-commit -a" is like "git-add .; git-commit"
But it is incorrect, because "git-commit -a" does not add new files, so
it works like "git-add -u .; git-commit".
Dmitry
^ permalink raw reply
* Re: [ALTERNATE PATCH] Add a simple option parser.
From: Mike Hommey @ 2007-10-05 14:30 UTC (permalink / raw)
To: Pierre Habouzit, Kristian Høgsberg, git, Junio C Hamano
In-Reply-To: <20071005142507.GL19879@artemis.corp>
On Fri, Oct 05, 2007 at 04:25:07PM +0200, Pierre Habouzit <madcoder@debian.org> wrote:
> The option parser takes argc, argv, an array of struct option
> and a usage string. Each of the struct option elements in the array
> describes a valid option, its type and a pointer to the location where the
> value is written. The entry point is parse_options(), which scans through
> the given argv, and matches each option there against the list of valid
> options. During the scan, argv is rewritten to only contain the
> non-option command line arguments and the number of these is returned.
>
> Aggregation of single switches is allowed:
> -rC0 is the same as -r -C 0 (supposing that -C wants an arg).
I like options aggregation, but I'm not sure aggregating option arguments
is a good idea... I can't even think of an application that does it.
Mike
^ permalink raw reply
* Re: [PATCH] git-shell and git-cvsserver
From: Miklos Vajna @ 2007-10-05 14:31 UTC (permalink / raw)
To: Jan Wielemaker; +Cc: Git Mailing List
In-Reply-To: <200710051453.47622.wielemak@science.uva.nl>
[-- Attachment #1: Type: text/plain, Size: 187 bytes --]
On Fri, Oct 05, 2007 at 02:53:47PM +0200, Jan Wielemaker <wielemak@science.uva.nl> wrote:
> +#define EXEC_PATH "/usr/local/bin"
why don't you use $(prefix) from the Makefile?
- VMiklos
[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* Re: Correction for post-receive-email
From: Andy Parkins @ 2007-10-05 14:29 UTC (permalink / raw)
To: git; +Cc: Bill Lear, Eric Mertens
In-Reply-To: <18182.12163.311826.242309@lisa.zopyra.com>
On Friday 2007 October 05, Bill Lear wrote:
> I have a few changes I would like to see in this script, ones that I think
> would make it generally more useful. I don't have a clean patch, though,
> so should I just submit suggestions to you directly, Andy?
Me; or the list. Or both.
I'm happy to try to accommodate any suggestions. I'm happy if it is useful to
anyone other than just me :-)
--- SNIP THIS BIT IF YOU'RE NOT INTERESTED IN ITS BUGS ---
The big fault in it as it stands is that it doesn't try to reorder the refs
being updated to the most logical form. For example:
O --- * --- A (ref2)
\
B (ref1)
Let's say that both ref1 and ref2 were originally at O and this push has moved
them to these new locations. The email hook gets sent this information like
this:
refs/heads/ref1 O B
refs/heads/ref2 O A
The hook iterates through this list, for each ref update it shows only the
commits introduced by the change that aren't already included in an existing
ref. This is the problem, ref2 introduced "*" and "A" and ref1
introduced "B", ideally then the two emails would show
ref2 updated from O to A
new revs: *, A
ref1 updated from O to B
new revs: B
But because ref1 is alphabetically before ref1, what you get is:
ref1 updated from O to B
new revs: *, A, B
ref2 updated from O to A
new revs: <none>
I can't say I know what the answer is; nor even what the correct output should
be. If anyone has opinions on this, I'll be glad to hear them.
Andy
--
Dr Andy Parkins, M Eng (hons), MIET
andyparkins@gmail.com
^ permalink raw reply
* [ALTERNATE PATCH] Add a simple option parser.
From: Pierre Habouzit @ 2007-10-05 14:25 UTC (permalink / raw)
To: Kristian Høgsberg, git; +Cc: Junio C Hamano
In-Reply-To: <20071005142140.GK19879@artemis.corp>
[-- Attachment #1: Type: text/plain, Size: 6456 bytes --]
The option parser takes argc, argv, an array of struct option
and a usage string. Each of the struct option elements in the array
describes a valid option, its type and a pointer to the location where the
value is written. The entry point is parse_options(), which scans through
the given argv, and matches each option there against the list of valid
options. During the scan, argv is rewritten to only contain the
non-option command line arguments and the number of these is returned.
Aggregation of single switches is allowed:
-rC0 is the same as -r -C 0 (supposing that -C wants an arg).
Boolean switches automatically support the option with the same name,
prefixed with 'no-' to disable the switch:
--no-color / --color only need to have an entry for "color".
Long options are supported either with '=' or without:
--some-option=foo is the same as --some-option foo
Signed-off-by: Kristian Høgsberg <krh@redhat.com>
Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---
I'm sorry about the "From" I don't intend to "steal" the patch in any
sense, it's just an alternate proposal.
oh and I don't grok what OPTION_LAST is for, so I left it apart, but
it seems unused ?
Makefile | 2 +-
parse-options.c | 154 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
parse-options.h | 29 ++++++++++
3 files changed, 184 insertions(+), 1 deletions(-)
create mode 100644 parse-options.c
create mode 100644 parse-options.h
diff --git a/Makefile b/Makefile
index 62bdac6..d90e959 100644
--- a/Makefile
+++ b/Makefile
@@ -310,7 +310,7 @@ LIB_OBJS = \
alloc.o merge-file.o path-list.o help.o unpack-trees.o $(DIFF_OBJS) \
color.o wt-status.o archive-zip.o archive-tar.o shallow.o utf8.o \
convert.o attr.o decorate.o progress.o mailmap.o symlinks.o remote.o \
- transport.o bundle.o
+ transport.o bundle.o parse-options.o
BUILTIN_OBJS = \
builtin-add.o \
diff --git a/parse-options.c b/parse-options.c
new file mode 100644
index 0000000..eb3ff40
--- /dev/null
+++ b/parse-options.c
@@ -0,0 +1,154 @@
+#include "git-compat-util.h"
+#include "parse-options.h"
+
+struct optparse_t {
+ const char **argv;
+ int argc;
+ const char *opt;
+};
+
+static inline const char *skippfx(const char *str, const char *prefix)
+{
+ size_t len = strlen(prefix);
+ return strncmp(str, prefix, len) ? NULL : str + len;
+}
+
+static int opterror(struct option *opt, const char *reason, int shorterr)
+{
+ if (shorterr) {
+ return error("switch `%c' %s", opt->short_name, reason);
+ } else {
+ return error("option `%s' %s", opt->long_name, reason);
+ }
+}
+
+static int get_value(struct optparse_t *p, struct option *opt,
+ int boolean, int shorterr)
+{
+ switch (opt->type) {
+ const char *s;
+ int v;
+
+ case OPTION_BOOLEAN:
+ *(int *)opt->value = boolean;
+ return 0;
+
+ case OPTION_STRING:
+ if (p->opt && *p->opt) {
+ *(const char **)opt->value = p->opt;
+ p->opt = NULL;
+ } else {
+ if (p->argc < 1)
+ return opterror(opt, "requires a value", shorterr);
+ *(const char **)opt->value = *++p->argv;
+ p->argc--;
+ }
+ return 0;
+
+ case OPTION_INTEGER:
+ if (p->opt && *p->opt) {
+ v = strtol(p->opt, (char **)&s, 10);
+ p->opt = NULL;
+ } else {
+ if (p->argc < 1)
+ return opterror(opt, "requires a value", shorterr);
+ v = strtol(*++p->argv, (char **)&s, 10);
+ p->argc--;
+ }
+ if (*s)
+ return opterror(opt, "expects a numerical value", shorterr);
+ *(int *)opt->value = v;
+ return 0;
+ }
+
+ abort();
+}
+
+static int parse_short_opt(struct optparse_t *p, struct option *options, int count)
+{
+ int i;
+
+ for (i = 0; i < count; i++) {
+ if (options[i].short_name == *p->opt) {
+ p->opt++;
+ return get_value(p, options + i, 1, 1);
+ }
+ }
+ return error("unknown switch `%c'", *p->opt);
+}
+
+static int parse_long_opt(struct optparse_t *p, const char *arg,
+ struct option *options, int count)
+{
+ int boolean = 1;
+ int i;
+
+ for (i = 0; i < count; i++) {
+ const char *rest;
+
+ if (!options[i].long_name)
+ continue;
+
+ rest = skippfx(arg, options[i].long_name);
+ if (!rest && options[i].type == OPTION_BOOLEAN) {
+ if (!rest && skippfx(arg, "no-")) {
+ rest = skippfx(arg + 3, options[i].long_name);
+ boolean = 0;
+ }
+ if (rest && *rest == '=')
+ return opterror(options + i, "takes no value", 0);
+ }
+ if (!rest || (*rest && *rest != '='))
+ continue;
+ if (*rest) {
+ p->opt = rest;
+ }
+ return get_value(p, options + i, boolean, 0);
+ }
+ return error("unknown option `%s'", arg);
+}
+
+int parse_options(int argc, const char **argv,
+ struct option *options, int count,
+ const char *usage_string)
+{
+ struct optparse_t optp = { argv + 1, argc - 1, NULL };
+ int j = 0;
+
+ while (optp.argc) {
+ const char *arg = optp.argv[0];
+
+ if (*arg != '-' || !arg[1]) {
+ argv[j++] = *optp.argv++;
+ optp.argc--;
+ continue;
+ }
+
+ if (arg[1] != '-') {
+ optp.opt = arg + 1;
+ while (*optp.opt) {
+ if (parse_short_opt(&optp, options, count) < 0) {
+ usage(usage_string);
+ return -1;
+ }
+ }
+ optp.argc--;
+ optp.argv++;
+ continue;
+ }
+
+ if (!arg[2]) /* "--" */
+ break;
+
+ if (parse_long_opt(&optp, arg + 2, options, count)) {
+ usage(usage_string);
+ return -1;
+ }
+ optp.argc--;
+ optp.argv++;
+ }
+
+ memmove(argv + j, optp.argv, optp.argc * sizeof(argv));
+ argv[j + optp.argc] = NULL;
+ return j + optp.argc;
+}
diff --git a/parse-options.h b/parse-options.h
new file mode 100644
index 0000000..e4749d0
--- /dev/null
+++ b/parse-options.h
@@ -0,0 +1,29 @@
+#ifndef PARSE_OPTIONS_H
+#define PARSE_OPTIONS_H
+
+enum option_type {
+ OPTION_BOOLEAN,
+ OPTION_STRING,
+ OPTION_INTEGER,
+#if 0
+ OPTION_LAST,
+#endif
+};
+
+struct option {
+ enum option_type type;
+ const char *long_name;
+ char short_name;
+ void *value;
+};
+
+/* parse_options() will filter out the processed options and leave the
+ * non-option argments in argv[]. The return value is the number of
+ * arguments left in argv[].
+ */
+
+extern int parse_options(int argc, const char **argv,
+ struct option *options, int count,
+ const char *usage_string);
+
+#endif
--
1.5.3.4.1156.ga72f9d-dirty
[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply related
* Re: [PATCH] Add a simple option parser.
From: Pierre Habouzit @ 2007-10-05 14:21 UTC (permalink / raw)
To: Kristian Høgsberg; +Cc: git, gitster
In-Reply-To: <1191447902-27326-1-git-send-email-krh@redhat.com>
[-- Attachment #1: Type: text/plain, Size: 1004 bytes --]
> +/* Parse the given options against the list of known options. The
> + * order of the option structs matters, in that ambiguous
> + * abbreviations (eg, --in could be short for --include or
> + * --interactive) are matched by the first option that share the
> + * prefix.
Do we really want that ?
I do believe that it's a very bad idea, as it silently breaks. Most of
the command line switches people need to use have a short form, or their
shell will complete it properly.
A very interesting feature though, would be to finally be able to
parse aggregated switches (`git rm -rf` anyone ?).
I also believe that it's a pity that parse_options isn't able to
generate the usage by itself. But we can add that later.
I've though an alternate proposal, based on your work, for the first
patch.
--
·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: Problems using StGit and -rt kernel patchset
From: Catalin Marinas @ 2007-10-05 13:45 UTC (permalink / raw)
To: Clark Williams; +Cc: git
In-Reply-To: <4703A4EE.3000002@gmail.com>
Clark,
What version of StGIT are you using? You might use a too new GIT with an
older StGIT or maybe there are just some bugs in StGIT.
On Wed, 2007-10-03 at 09:19 -0500, Clark Williams wrote:
> I've been working on the -rt patch series for the kernel and would like to to use
> StGit to manage the patches. Unfortunately I've had limited success, so I thought I'd
> ask the git/stgit community if what I'm doing is wrong.
>
> I clone Linus's tree to a common directory, then clone it locally to work:
>
> $ git clone -s -l /home/src/linux-2.6.git scratch.git
> $ cd scratch.git
> $ stg init
> $ stg branch --create rt-2.6.23-rc8-rt1 v2.6.23-rc8
> $ stg import --series --ignore --replace ../sources/patch-queue-2.6.23-rc8-rt1/series
> <fix the things quilt lets through and stg barfs on, like malformed email addresses>
If git-quiltimport behaves better with malformed patches, use it and run
'stg uncommit -n 368' afterwards (the 'uncommit' takes some other useful
options as well, see --help).
> <watch 368 patches be applied and committed>
> <work work work>
Do you modify any of the -rt patches or you create new ones?
> <get a new patch queue>
> $ (cd /home/src/linux-2.6.git && git pull)
> $ stg pull
> $ stg branch --create rt-2.6.23-rc8-rt1 v2.6.23-rc9
> $ stg import --series --ignore --replace ../sources/patch-queue-2.6.23-rc9-rt1/series
> Checking for changes in the working directory ... done
> stg import: env git-commit-tree 520b9d0db6a1142271a68b2b38cca002be40f6cb -p
> da0a81e98c06aa0d1e05b9012c2b2facb1807e12 failed (fatal:
> da0a81e98c06aa0d1e05b9012c2b2facb1807e12 is not a valid 'commit' object)
I'm not sure why the first import worked. It seems that StGIT uses the
tag id (da0a81e9) rather than the corresponding commit id (3146b39c). I
remember having this problem in the past when creating branches and I
fixed StGIT to always get the corresponding commit id. Using
'v2.6.23-rc9^{commit}' as the 'branch' argument rather than just the tag
should fix the problem.
> At this point I'm clueless as to:
>
> 1. What I've done wrong
Probably nothing (just hidden features of StGIT :-))
> 2. How to recover/debug this
You can recreate the branch with the commit rather than tag id. With a
sufficiently new StGIT, you could use 'stg rebase <id>' on the branch. I
assume that no patch was pushed because import failed (though the first
imported patch might be in an undefined state and can be removed).
Catalin
^ permalink raw reply
* Re: Many gits are offline this week
From: Randal L. Schwartz @ 2007-10-05 13:12 UTC (permalink / raw)
To: Paolo Ciarrocchi; +Cc: Shawn O. Pearce, git
In-Reply-To: <4d8e3fd30710050214j135260cn842ee7396a3d63c7@mail.gmail.com>
>>>>> "Paolo" == Paolo Ciarrocchi <paolo.ciarrocchi@gmail.com> writes:
Paolo> is there any material (slides, docs) you can share before the talks?
I've had the slides reviewed by Smarter People Than Me on #git already, so
hopefully most of it is accurate. :) They're temporarily at
http://www.stonehenge.com/pic/Git-2.0.3-to-be.pdf
I still hope to have a few hours to go in and add a few sadly missing
graphics, particularly on the rebase vs merge section.
--
Randal L. Schwartz - Stonehenge Consulting Services, Inc. - +1 503 777 0095
<merlyn@stonehenge.com> <URL:http://www.stonehenge.com/merlyn/>
Perl/Unix/security consulting, Technical writing, Comedy, etc. etc.
See PerlTraining.Stonehenge.com for onsite and open-enrollment Perl training!
^ permalink raw reply
* Re: [AGGREGATED PATCH] Fix in-place editing functions in convert.c
From: Bernt Hansen @ 2007-10-05 13:07 UTC (permalink / raw)
To: Pierre Habouzit; +Cc: Junio C Hamano, git
In-Reply-To: <20071005085522.32EFF1E16E@madism.org>
This fixes it for me. Thanks!!
Bernt
^ permalink raw reply
* [PATCH] git-shell and git-cvsserver
From: Jan Wielemaker @ 2007-10-05 12:53 UTC (permalink / raw)
To: Git Mailing List
Hi,
I know, I shouldn't be using git-cvsserver :-( Anyway, I patched
git-shell to start git-cvsserver if it is started interactively and the
one and only line given to it is "cvs server".
The patch to shell.c is below. The trick with the EXEC_PATH is needed
because git-cvsserver doesn't appear to be working if you do not include
the git bindir in $PATH. I think that should be fixed in git-cvsserver
and otherwise we should at least make the value come from the prefix
make variable. With this patch I was able to use both Unix and Windows
cvs clients using git-shell as login shell.
Note that you must provide ~/.gitconfig with user and email in the
restricted environment.
Enjoy --- Jan
--- shell.c.org 2007-10-05 13:08:47.000000000 +0200
+++ shell.c 2007-10-05 14:24:11.000000000 +0200
@@ -18,27 +18,80 @@
return execv_git_cmd(my_argv);
}
+#define EXEC_PATH "/usr/local/bin"
+
+static int do_cvs_cmd(const char *me, char *arg)
+{
+ const char *my_argv[4];
+ const char *oldpath;
+
+ if ( !arg )
+ die("no argument");
+ if ( strcmp(arg, "server") )
+ die("only allows git-cvsserver server: %s", arg);
+
+ my_argv[0] = "cvsserver";
+ my_argv[1] = "server";
+ my_argv[2] = NULL;
+
+ if ( (oldpath=getenv("PATH")) ) {
+ char *newpath = malloc(strlen(oldpath)+strlen(EXEC_PATH)+5+1+1);
+
+ sprintf(newpath, "PATH=%s:%s", EXEC_PATH, oldpath);
+ putenv(newpath);
+ } else {
+ char *newpath = malloc(strlen(EXEC_PATH)+5+1);
+
+ sprintf(newpath, "PATH=%s", EXEC_PATH);
+ putenv(newpath);
+ }
+
+ return execv_git_cmd(my_argv);
+}
+
+
static struct commands {
const char *name;
int (*exec)(const char *me, char *arg);
} cmd_list[] = {
{ "git-receive-pack", do_generic_cmd },
{ "git-upload-pack", do_generic_cmd },
+ { "cvs", do_cvs_cmd },
{ NULL },
};
int main(int argc, char **argv)
{
char *prog;
+ char buf[256];
struct commands *cmd;
/* We want to see "-c cmd args", and nothing else */
- if (argc != 3 || strcmp(argv[1], "-c"))
- die("What do you think I am? A shell?");
+ if (argc == 1) {
+ if (fgets(buf, sizeof(buf)-1, stdin)) {
+ char *end;
+
+ if ( (end=strchr(buf, '\n')) )
+ { while(end>buf && end[-1] <= ' ')
+ end--;
+ *end = '\0';
+ } else {
+ die("Bad command");
+ }
+
+ prog = buf;
+ } else {
+ die("No command");
+ }
+ } else {
+ if (argc != 3 || strcmp(argv[1], "-c"))
+ die("What do you think I am? A shell?");
+
+ prog = argv[2];
+ argv += 2;
+ argc -= 2;
+ }
- prog = argv[2];
- argv += 2;
- argc -= 2;
for (cmd = cmd_list ; cmd->name ; cmd++) {
int len = strlen(cmd->name);
char *arg;
^ 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