* Re: [PATCH] rev-list --bisect: Fix best == NULL case.
From: Junio C Hamano @ 2007-09-20 7:08 UTC (permalink / raw)
To: Christian Couder; +Cc: git
In-Reply-To: <7vmyvhltpy.fsf@gitster.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> The patch makes sense. In fact you can crash any released
> version like this:
>
> $ git rev-list --bisect v1.5.2..v1.5.0
>
> I'll need to apply a moral equivalent to 'maint'.
I spoke too fast. This is not broken in any released version.
Only with the patch from somebody that broke it with commit
ce0cbad7727457854d631a6314d9aee7c837cb65 (rev-list --bisect:
Move finding bisection into do_find_bisection.), the "empty set"
case would segfault ;-).
^ permalink raw reply
* Re: [PATCH 1/5] strbuf API additions and enhancements.
From: Kalle Olavi Niemitalo @ 2007-09-20 7:20 UTC (permalink / raw)
To: git
In-Reply-To: <46F21097.5030901@eudaptics.com>
Johannes Sixt <j.sixt@eudaptics.com> writes:
> Edgar Toernig schrieb:
>> The second vsnprintf won't work as the first one consumed all args
>> from va_list ap. You need to va_copy the ap.
>
> Your analysis is not correct. The second vsnprintf receives the same
> argument pointer as the first, and, hence, consumes the same set of
> arguments.
C99 7.9.16.2p2 has a footnote: "As the functions vfprintf,
vfscanf, vprintf, vscanf, vsnprintf, vsprintf, and vsscanf invoke
the va_arg macro, the value of arg after the return is
indeterminate."
Normative text in 7.15p3 confirms this: "The object ap may be
passed as an argument to another function; if that function
invokes the va_arg macro with parameter ap, the value of ap in
the calling function is indeterminate and shall be passed to the
va_end macro prior to any further reference to ap."
Therefore va_copy is needed here, at least in principle.
^ permalink raw reply
* Re: [PATCH 2/7] nfv?asprintf are broken without va_copy, workaround them.
From: Pierre Habouzit @ 2007-09-20 8:27 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vwsumkll8.fsf@gitster.siamese.dyndns.org>
[-- Attachment #1: Type: text/plain, Size: 2072 bytes --]
On Thu, Sep 20, 2007 at 04:27:31AM +0000, Junio C Hamano wrote:
> Pierre Habouzit <madcoder@debian.org> writes:
>
> > * drop nfasprintf.
> > * move nfvasprintf into imap-send.c back, and let it work on a 8k buffer,
> > and die() in case of overflow. It should be enough for imap commands, if
> > someone cares about imap-send, he's welcomed to fix it properly.
> > * replace nfvasprintf use in merge-recursive with a copy of the strbuf_addf
> > logic, it's one place, we'll live with it.
> > To ease the change, output_buffer string list is replaced with a strbuf ;)
>
> While I'd agree with all of the above,
>
> > * rework trace.c API's so that only one of the trace functions takes a
> > vararg. It's used to format strerror()s and git command names, it should
> > never be more than a few octets long, let it work on a 8k static buffer
> > with vsnprintf or die loudly.
>
> and I'd agree with this in principle, there is a minor nit with
> the implementation and use in trace.c. E.g.
>
> > diff --git a/exec_cmd.c b/exec_cmd.c
> > index 9b74ed2..c0f954e 100644
> > --- a/exec_cmd.c
> > +++ b/exec_cmd.c
> > @@ -97,7 +97,8 @@ int execv_git_cmd(const char **argv)
> > tmp = argv[0];
> > argv[0] = git_command;
> >
> > - trace_argv_printf(argv, -1, "trace: exec:");
> > + trace_printf("trace: exec:");
> > + trace_argv(argv, -1);
>
> This used to be a single call into trace.c which would format a
> single string to write(2) out. Now these two messages go
> through separate write(2) and can be broken up. I think the
> atomicity of the log/trace message was the primary reason the
> original had such a strange calling convention.
Okay, given that the formats (as you can see) are always very short,
and that it will always fit in a big enough static buffer, I'll
reinstate this and use a vsnprintf twice then.
--
·O· Pierre Habouzit
··O madcoder@debian.org
OOO http://www.madism.org
[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* [SUPERSEDES PATCH 2/7] nfv?asprintf are broken without va_copy, workaround them.
From: Pierre Habouzit @ 2007-09-20 8:43 UTC (permalink / raw)
To: Junio C Hamano, git
In-Reply-To: <20070920082701.GA2053@artemis.corp>
* drop nfasprintf.
* move nfvasprintf into imap-send.c back, and let it work on a 8k buffer,
and die() in case of overflow. It should be enough for imap commands, if
someone cares about imap-send, he's welcomed to fix it properly.
* replace nfvasprintf use in merge-recursive with a copy of the strbuf_addf
logic, it's one place, we'll live with it.
To ease the change, output_buffer string list is replaced with a strbuf ;)
* rework trace.c to call vsnprintf itself. It's used to format strerror()s
and git command names, it should never be more than a few octets long, let
it work on a 8k static buffer with vsnprintf or die loudly.
Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---
This reinstates the trace_argv_printf API. The implementation is
stupid, but is rewritten in a latter commit. I didn't wanted to bother
optimizing it.
cache.h | 2 -
imap-send.c | 13 ++++++++
merge-recursive.c | 74 ++++++++++++++++++++-----------------------
trace.c | 90 ++++++++++++++++-------------------------------------
4 files changed, 74 insertions(+), 105 deletions(-)
diff --git a/cache.h b/cache.h
index c57ccd6..b127c43 100644
--- a/cache.h
+++ b/cache.h
@@ -587,8 +587,6 @@ extern void *alloc_object_node(void);
extern void alloc_report(void);
/* trace.c */
-extern int nfasprintf(char **str, const char *fmt, ...);
-extern int nfvasprintf(char **str, const char *fmt, va_list va);
extern void trace_printf(const char *format, ...);
extern void trace_argv_printf(const char **argv, int count, const char *format, ...);
diff --git a/imap-send.c b/imap-send.c
index 905d097..e95cdde 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -105,6 +105,19 @@ static void free_generic_messages( message_t * );
static int nfsnprintf( char *buf, int blen, const char *fmt, ... );
+static int nfvasprintf(char **strp, const char *fmt, va_list ap)
+{
+ int len;
+ char tmp[8192];
+
+ len = vsnprintf(tmp, sizeof(tmp), fmt, ap);
+ if (len < 0)
+ die("Fatal: Out of memory\n");
+ if (len >= sizeof(tmp))
+ die("imap command overflow !\n");
+ *strp = xmemdupz(tmp, len);
+ return len;
+}
static void arc4_init( void );
static unsigned char arc4_getbyte( void );
diff --git a/merge-recursive.c b/merge-recursive.c
index 14b56c2..4e27549 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -85,63 +85,57 @@ struct stage_data
unsigned processed:1;
};
-struct output_buffer
-{
- struct output_buffer *next;
- char *str;
-};
-
static struct path_list current_file_set = {NULL, 0, 0, 1};
static struct path_list current_directory_set = {NULL, 0, 0, 1};
static int call_depth = 0;
static int verbosity = 2;
static int buffer_output = 1;
-static struct output_buffer *output_list, *output_end;
+static struct strbuf obuf = STRBUF_INIT;
-static int show (int v)
+static int show(int v)
{
return (!call_depth && verbosity >= v) || verbosity >= 5;
}
-static void output(int v, const char *fmt, ...)
+static void flush_output(void)
{
- va_list args;
- va_start(args, fmt);
- if (buffer_output && show(v)) {
- struct output_buffer *b = xmalloc(sizeof(*b));
- nfvasprintf(&b->str, fmt, args);
- b->next = NULL;
- if (output_end)
- output_end->next = b;
- else
- output_list = b;
- output_end = b;
- } else if (show(v)) {
- int i;
- for (i = call_depth; i--;)
- fputs(" ", stdout);
- vfprintf(stdout, fmt, args);
- fputc('\n', stdout);
+ if (obuf.len) {
+ fputs(obuf.buf, stdout);
+ strbuf_reset(&obuf);
}
- va_end(args);
}
-static void flush_output(void)
+static void output(int v, const char *fmt, ...)
{
- struct output_buffer *b, *n;
- for (b = output_list; b; b = n) {
- int i;
- for (i = call_depth; i--;)
- fputs(" ", stdout);
- fputs(b->str, stdout);
- fputc('\n', stdout);
- n = b->next;
- free(b->str);
- free(b);
+ if (show(v)) {
+ int len;
+ va_list ap;
+
+ strbuf_grow(&obuf, call_depth);
+ memset(obuf.buf + obuf.len, ' ', call_depth);
+ strbuf_setlen(&obuf, obuf.len + call_depth);
+
+ va_start(ap, fmt);
+ len = vsnprintf(obuf.buf, strbuf_avail(&obuf) + 1, fmt, ap);
+ va_end(ap);
+
+ if (len < 0)
+ len = 0;
+ if (len > strbuf_avail(&obuf)) {
+ strbuf_grow(&obuf, len);
+ va_start(ap, fmt);
+ len = vsnprintf(obuf.buf, strbuf_avail(&obuf) + 1, fmt, ap);
+ va_end(ap);
+ if (len > strbuf_avail(&obuf)) {
+ die("this should not happen, your snprintf is broken");
+ }
+ }
+
+ strbuf_setlen(&obuf, obuf.len + len);
+ if (!buffer_output)
+ flush_output();
}
- output_list = NULL;
- output_end = NULL;
}
static void output_commit_title(struct commit *commit)
diff --git a/trace.c b/trace.c
index 7961a27..91548a5 100644
--- a/trace.c
+++ b/trace.c
@@ -25,33 +25,6 @@
#include "cache.h"
#include "quote.h"
-/* Stolen from "imap-send.c". */
-int nfvasprintf(char **strp, const char *fmt, va_list ap)
-{
- int len;
- char tmp[1024];
-
- if ((len = vsnprintf(tmp, sizeof(tmp), fmt, ap)) < 0 ||
- !(*strp = xmalloc(len + 1)))
- die("Fatal: Out of memory\n");
- if (len >= (int)sizeof(tmp))
- vsprintf(*strp, fmt, ap);
- else
- memcpy(*strp, tmp, len + 1);
- return len;
-}
-
-int nfasprintf(char **str, const char *fmt, ...)
-{
- int rc;
- va_list args;
-
- va_start(args, fmt);
- rc = nfvasprintf(str, fmt, args);
- va_end(args);
- return rc;
-}
-
/* Get a trace file descriptor from GIT_TRACE env variable. */
static int get_trace_fd(int *need_close)
{
@@ -89,63 +62,54 @@ static int get_trace_fd(int *need_close)
static const char err_msg[] = "Could not trace into fd given by "
"GIT_TRACE environment variable";
-void trace_printf(const char *format, ...)
+void trace_printf(const char *fmt, ...)
{
- char *trace_str;
- va_list rest;
- int need_close = 0;
- int fd = get_trace_fd(&need_close);
+ char buf[8192];
+ va_list ap;
+ int fd, len, need_close = 0;
+ fd = get_trace_fd(&need_close);
if (!fd)
return;
- va_start(rest, format);
- nfvasprintf(&trace_str, format, rest);
- va_end(rest);
-
- write_or_whine_pipe(fd, trace_str, strlen(trace_str), err_msg);
-
- free(trace_str);
+ va_start(ap, fmt);
+ len = vsnprintf(buf, sizeof(buf), fmt, ap);
+ va_end(ap);
+ if (len >= sizeof(buf))
+ die("unreasonnable trace length");
+ write_or_whine_pipe(fd, buf, len, err_msg);
if (need_close)
close(fd);
}
-void trace_argv_printf(const char **argv, int count, const char *format, ...)
+void trace_argv_printf(const char **argv, int count, const char *fmt, ...)
{
- char *argv_str, *format_str, *trace_str;
- size_t argv_len, format_len, trace_len;
- va_list rest;
- int need_close = 0;
- int fd = get_trace_fd(&need_close);
+ char buf[8192];
+ va_list ap;
+ char *argv_str;
+ size_t argv_len;
+ int fd, len, need_close = 0;
+ fd = get_trace_fd(&need_close);
if (!fd)
return;
+ va_start(ap, fmt);
+ len = vsnprintf(buf, sizeof(buf), fmt, ap);
+ va_end(ap);
+ if (len >= sizeof(buf))
+ die("unreasonnable trace length");
+
/* Get the argv string. */
argv_str = sq_quote_argv(argv, count);
argv_len = strlen(argv_str);
- /* Get the formated string. */
- va_start(rest, format);
- nfvasprintf(&format_str, format, rest);
- va_end(rest);
-
- /* Allocate buffer for trace string. */
- format_len = strlen(format_str);
- trace_len = argv_len + format_len + 1; /* + 1 for \n */
- trace_str = xmalloc(trace_len + 1);
-
- /* Copy everything into the trace string. */
- strncpy(trace_str, format_str, format_len);
- strncpy(trace_str + format_len, argv_str, argv_len);
- strcpy(trace_str + trace_len - 1, "\n");
-
- write_or_whine_pipe(fd, trace_str, trace_len, err_msg);
+ write_or_whine_pipe(fd, buf, len, err_msg);
+ write_or_whine_pipe(fd, argv_str, argv_len, err_msg);
+ write_or_whine_pipe(fd, "\n", 1, err_msg);
free(argv_str);
- free(format_str);
- free(trace_str);
if (need_close)
close(fd);
--
1.5.3.2.1036.gca8d2
^ permalink raw reply related
* [SUPERSEDES PATCH 4/7] sq_quote_argv and add_to_string rework with strbuf's.
From: Pierre Habouzit @ 2007-09-20 8:44 UTC (permalink / raw)
To: Junio C Hamano, git
In-Reply-To: <20070920082701.GA2053@artemis.corp>
[-- Attachment #1: Type: text/plain, Size: 7902 bytes --]
* sq_quote_buf is made public, and works on a strbuf.
* sq_quote_argv also works on a strbuf.
* make sq_quote_argv take a "maxlen" argument to check the buffer won't grow
too big.
Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---
connect.c | 21 ++++++--------
git.c | 16 +++-------
quote.c | 94 +++++++++++++++---------------------------------------------
quote.h | 9 ++----
trace.c | 25 +++++++---------
5 files changed, 52 insertions(+), 113 deletions(-)
diff --git a/connect.c b/connect.c
index 1653a0e..06d279e 100644
--- a/connect.c
+++ b/connect.c
@@ -577,16 +577,13 @@ pid_t git_connect(int fd[2], char *url, const char *prog, int flags)
if (pid < 0)
die("unable to fork");
if (!pid) {
- char command[MAX_CMD_LEN];
- char *posn = command;
- int size = MAX_CMD_LEN;
- int of = 0;
+ struct strbuf cmd;
- of |= add_to_string(&posn, &size, prog, 0);
- of |= add_to_string(&posn, &size, " ", 0);
- of |= add_to_string(&posn, &size, path, 1);
-
- if (of)
+ 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);
@@ -606,10 +603,10 @@ pid_t git_connect(int fd[2], char *url, const char *prog, int flags)
ssh_basename++;
if (!port)
- execlp(ssh, ssh_basename, host, command, NULL);
+ execlp(ssh, ssh_basename, host, cmd.buf, NULL);
else
execlp(ssh, ssh_basename, "-p", port, host,
- command, NULL);
+ cmd.buf, NULL);
}
else {
unsetenv(ALTERNATE_DB_ENVIRONMENT);
@@ -618,7 +615,7 @@ pid_t git_connect(int fd[2], char *url, const char *prog, int flags)
unsetenv(GIT_WORK_TREE_ENVIRONMENT);
unsetenv(GRAFT_ENVIRONMENT);
unsetenv(INDEX_ENVIRONMENT);
- execlp("sh", "sh", "-c", command, NULL);
+ execlp("sh", "sh", "-c", cmd.buf, NULL);
}
die("exec failed");
}
diff --git a/git.c b/git.c
index 6773c12..d7c6bca 100644
--- a/git.c
+++ b/git.c
@@ -187,19 +187,13 @@ static int handle_alias(int *argcp, const char ***argv)
if (alias_string) {
if (alias_string[0] == '!') {
if (*argcp > 1) {
- int i, sz = PATH_MAX;
- char *s = xmalloc(sz), *new_alias = s;
+ struct strbuf buf;
- add_to_string(&s, &sz, alias_string, 0);
+ strbuf_init(&buf, PATH_MAX);
+ strbuf_addstr(&buf, alias_string);
+ sq_quote_argv(&buf, (*argv) + 1, *argcp - 1, PATH_MAX);
free(alias_string);
- alias_string = new_alias;
- for (i = 1; i < *argcp &&
- !add_to_string(&s, &sz, " ", 0) &&
- !add_to_string(&s, &sz, (*argv)[i], 1)
- ; i++)
- ; /* do nothing */
- if (!sz)
- die("Too many or long arguments");
+ alias_string = buf.buf;
}
trace_printf("trace: alias to shell cmd: %s => %s\n",
alias_command, alias_string + 1);
diff --git a/quote.c b/quote.c
index d88bf75..edd1a09 100644
--- a/quote.c
+++ b/quote.c
@@ -12,37 +12,31 @@
* a'b ==> a'\''b ==> 'a'\''b'
* a!b ==> a'\!'b ==> 'a'\!'b'
*/
-#undef EMIT
-#define EMIT(x) do { if (++len < n) *bp++ = (x); } while(0)
-
static inline int need_bs_quote(char c)
{
return (c == '\'' || c == '!');
}
-static size_t sq_quote_buf(char *dst, size_t n, const char *src)
+void sq_quote_buf(struct strbuf *dst, const char *src)
{
- char c;
- char *bp = dst;
- size_t len = 0;
-
- EMIT('\'');
- while ((c = *src++)) {
- if (need_bs_quote(c)) {
- EMIT('\'');
- EMIT('\\');
- EMIT(c);
- EMIT('\'');
- } else {
- EMIT(c);
+ char *to_free = NULL;
+
+ if (dst->buf == src)
+ to_free = strbuf_detach(dst);
+
+ strbuf_addch(dst, '\'');
+ while (*src) {
+ size_t len = strcspn(src, "'\\");
+ strbuf_add(dst, src, len);
+ src += len;
+ while (need_bs_quote(*src)) {
+ strbuf_addstr(dst, "'\\");
+ strbuf_addch(dst, *src++);
+ strbuf_addch(dst, '\'');
}
}
- EMIT('\'');
-
- if ( n )
- *bp = 0;
-
- return len;
+ strbuf_addch(dst, '\'');
+ free(to_free);
}
void sq_quote_print(FILE *stream, const char *src)
@@ -62,11 +56,10 @@ void sq_quote_print(FILE *stream, const char *src)
fputc('\'', stream);
}
-char *sq_quote_argv(const char** argv, int count)
+void sq_quote_argv(struct strbuf *dst, const char** argv, int count,
+ size_t maxlen)
{
- char *buf, *to;
int i;
- size_t len = 0;
/* Count argv if needed. */
if (count < 0) {
@@ -74,53 +67,14 @@ char *sq_quote_argv(const char** argv, int count)
; /* just counting */
}
- /* Special case: no argv. */
- if (!count)
- return xcalloc(1,1);
-
- /* Get destination buffer length. */
- for (i = 0; i < count; i++)
- len += sq_quote_buf(NULL, 0, argv[i]) + 1;
-
- /* Alloc destination buffer. */
- to = buf = xmalloc(len + 1);
-
/* Copy into destination buffer. */
+ strbuf_grow(dst, 32 * count);
for (i = 0; i < count; ++i) {
- *to++ = ' ';
- to += sq_quote_buf(to, len, argv[i]);
+ strbuf_addch(dst, ' ');
+ sq_quote_buf(dst, argv[i]);
+ if (maxlen && dst->len > maxlen)
+ die("Too many or long arguments");
}
-
- return buf;
-}
-
-/*
- * Append a string to a string buffer, with or without shell quoting.
- * Return true if the buffer overflowed.
- */
-int add_to_string(char **ptrp, int *sizep, const char *str, int quote)
-{
- char *p = *ptrp;
- int size = *sizep;
- int oc;
- int err = 0;
-
- if (quote)
- oc = sq_quote_buf(p, size, str);
- else {
- oc = strlen(str);
- memcpy(p, str, (size <= oc) ? size - 1 : oc);
- }
-
- if (size <= oc) {
- err = 1;
- oc = size - 1;
- }
-
- *ptrp += oc;
- **ptrp = '\0';
- *sizep -= oc;
- return err;
}
char *sq_dequote(char *arg)
diff --git a/quote.h b/quote.h
index 8a59cc5..78e8d3e 100644
--- a/quote.h
+++ b/quote.h
@@ -29,13 +29,10 @@
*/
extern void sq_quote_print(FILE *stream, const char *src);
-extern char *sq_quote_argv(const char** argv, int count);
-/*
- * Append a string to a string buffer, with or without shell quoting.
- * Return true if the buffer overflowed.
- */
-extern int add_to_string(char **ptrp, int *sizep, const char *str, int quote);
+extern void sq_quote_buf(struct strbuf *, const char *src);
+extern void sq_quote_argv(struct strbuf *, const char **argv, int count,
+ size_t maxlen);
/* This unwraps what sq_quote() produces in place, but returns
* NULL if the input does not look like what sq_quote would have
diff --git a/trace.c b/trace.c
index 91548a5..efc656b 100644
--- a/trace.c
+++ b/trace.c
@@ -85,32 +85,29 @@ void trace_printf(const char *fmt, ...)
void trace_argv_printf(const char **argv, int count, const char *fmt, ...)
{
- char buf[8192];
va_list ap;
- char *argv_str;
- size_t argv_len;
+ struct strbuf trace;
int fd, len, need_close = 0;
fd = get_trace_fd(&need_close);
if (!fd)
return;
+ strbuf_init(&trace, 0);
+ strbuf_grow(&trace, 8192);
+
va_start(ap, fmt);
- len = vsnprintf(buf, sizeof(buf), fmt, ap);
+ len = vsnprintf(trace.buf, strbuf_avail(&trace) + 1, fmt, ap);
va_end(ap);
- if (len >= sizeof(buf))
+ if (len >= strbuf_avail(&trace))
die("unreasonnable trace length");
+ strbuf_setlen(&trace, len);
- /* Get the argv string. */
- argv_str = sq_quote_argv(argv, count);
- argv_len = strlen(argv_str);
-
- write_or_whine_pipe(fd, buf, len, err_msg);
- write_or_whine_pipe(fd, argv_str, argv_len, err_msg);
- write_or_whine_pipe(fd, "\n", 1, err_msg);
-
- free(argv_str);
+ sq_quote_argv(&trace, argv, count, 0);
+ strbuf_addch(&trace, '\n');
+ write_or_whine_pipe(fd, trace.buf, trace.len, err_msg);
+ strbuf_release(&trace);
if (need_close)
close(fd);
}
--
1.5.3.2.1036.gca8d2
[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply related
* Re: [PATCH] Drop UTF-8 characters in manual pages
From: Lukas Sandström @ 2007-09-20 9:08 UTC (permalink / raw)
To: Git Mailing List, Junio C Hamano; +Cc: mh
In-Reply-To: <7vd4wgwczs.fsf@gitster.siamese.dyndns.org>
Junio C Hamano wrote:
> Mike Hommey <mh@glandium.org> writes:
>> so
>> UTF-8 characters are just displayed as their sequence of bytes, which is
>> not very appealing.
>
> Perhaps not. I cannot decide what to do with
> Documentation/git-pack-redundant.txt, though.
>
These days when we have delta reusing and repacking is quite cheap,
I'd say just drop pack-redundant and its man page completely.
Of course, some pople might still be using it, but I haven't
felt the need for it in over a year.
/Lukas Sandström
^ permalink raw reply
* Re: [PATCH] Add git-rev-list --invert-match
From: Johannes Schindelin @ 2007-09-20 10:32 UTC (permalink / raw)
To: Bart Trojanowski; +Cc: git
In-Reply-To: <20070919202615.GK3076@jukie.net>
Hi,
On Wed, 19 Sep 2007, Bart Trojanowski wrote:
> git log --invert-match --grep="uninteresting"
IMHO this is only half a solution. Some of us want to say "git log
--grep=this --but-not-(in-the-whole-message) --grep=that".
Your code would be obsoleted by such a method (since it cannot be
extended), and further would also benefit git-grep, not only git-log.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH] User Manual: add a chapter for submodules
From: Johannes Schindelin @ 2007-09-20 10:34 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Miklos Vajna, git, J. Bruce Fields
In-Reply-To: <7v1wcum0ox.fsf@gitster.siamese.dyndns.org>
Hi,
On Wed, 19 Sep 2007, Junio C Hamano wrote:
> Miklos Vajna <vmiklos@frugalware.org> writes:
>
> > Signed-off-by: Michael Smith <msmith@cbnco.com>
> > Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>
> > ---
> >
> > Sorry, I sent the original patch again. So here is the updated second version.
>
> Looks Ok to me, although I didn't verify the examples by
> actually running them myself this time (last round I did).
So maybe we should do the same as with the tutorial: stick the examples
into a test script?
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH 2/5] Refactor struct transport_ops inlined into struct transport
From: Johannes Schindelin @ 2007-09-20 10:59 UTC (permalink / raw)
To: Daniel Barkalow; +Cc: Shawn O. Pearce, Junio C Hamano, git
In-Reply-To: <Pine.LNX.4.64.0709192226150.21941@iabervon.org>
Hi,
On Wed, 19 Sep 2007, Daniel Barkalow wrote:
> On Wed, 19 Sep 2007, Johannes Schindelin wrote:
>
> > On Wed, 19 Sep 2007, Shawn O. Pearce wrote:
> >
> > > diff --git a/transport.c b/transport.c
> > > index cc76e3f..d8458dc 100644
> > > --- a/transport.c
> > > +++ b/transport.c
> > > @@ -44,8 +44,6 @@ static int disconnect_walker(struct transport *transport)
> > > return 0;
> > > }
> > >
> > > -static const struct transport_ops rsync_transport;
> > > -
> > > static int curl_transport_push(struct transport *transport, int refspec_nr, const char **refspec, int flags) {
> > > const char **argv;
> > > int argc;
> > > @@ -431,18 +406,31 @@ struct transport *transport_get(struct remote *remote, const char *url)
> > > ret->url = url;
> > >
> > > if (!prefixcmp(url, "rsync://")) {
> > > - ret->ops = &rsync_transport;
> > > + /* not supported; don't populate any ops */
> > > +
> >
> > That is sneaky. What are the reasons to remove rsync support? I know it
> > is deprecated, but I'd still like to have it, especially for initial
> > clones on small-RAMed machines.
>
> It never got implemented in a way called from C. This is just removing the
> pointer to the empty struct where support would go. If anybody knows
> enough about interfacing with rsync to write the necessary functions, it
> can be restored.
Okay, will do once I find the time.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH 0/3] the return of the strbuf
From: Johannes Schindelin @ 2007-09-20 11:49 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: Pierre Habouzit, git
In-Reply-To: <20070918035721.GL3099@spearce.org>
Hi,
On Mon, 17 Sep 2007, Shawn O. Pearce wrote:
> $ git log --pretty=format:%an --since=6.months.ago -- fast-import.c \
> | sort | uniq -c | sort -nr
> 14 Shawn O. Pearce
> 3 Pierre Habouzit
> 3 Junio C Hamano
> 2 Simon Hausmann
> 2 Alex Riesen
> 1 Theodore Ts'o
> 1 Sven Verdoolaege
> 1 Sami Farin
> 1 Nicolas Pitre
> 1 Luiz Fernando N. Capitulino
> 1 Dana L. How
FWIW I'd do
git shortlog -n --since=6.months.ago HEAD -- fast-import.c|grep "^[A-Z]"
instead...
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH] Add git-rev-list --invert-match
From: Bart Trojanowski @ 2007-09-20 12:18 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vbqbym15x.fsf@gitster.siamese.dyndns.org>
* Junio C Hamano <gitster@pobox.com> [070920 00:05]:
> > +--invert-match::
> > +
> > + Show those parts of history that do not match any of the regular
> > + expression patterns.
> > +
>
> Tabs and spaces?
Noted. Will fix after I address Johannes' request.
> I am afraid people may not understand what "any of the regular
> expression patterns" means. How about being a bit more
> explicit, like this?
>
> When filtering the commits based on log message (`--grep`)
> or people (`--author` and `--committer`), show commits
> that do not match the criteria.
Yes, that sounds better.
Thanks,
-Bart
--
WebSig: http://www.jukie.net/~bart/sig/
^ permalink raw reply
* Re: [PATCH] Add git-rev-list --invert-match
From: Bart Trojanowski @ 2007-09-20 12:38 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0709201132381.28395@racer.site>
* Johannes Schindelin <Johannes.Schindelin@gmx.de> [070920 06:34]:
> On Wed, 19 Sep 2007, Bart Trojanowski wrote:
> > git log --invert-match --grep="uninteresting"
>
> IMHO this is only half a solution. Some of us want to say "git log
> --grep=this --but-not-(in-the-whole-message) --grep=that".
I have noticed that unique negation flags are getting scarce... we
already have --reverse, --inverse, and --not mean something elsewhere.
--but-not maybe be good.
I also agree that git-grep could use this extension.
Anyways, I can see four solutions for adding "show me this but not that"
functionality to git-rev-list:
1) adding a --but-not flag, as you suggested. It separates positive
matches that precede it with negative matches that follow.
Example:
git log --grep=this --but-not --grep=that --committer="${MY_EMAIL}"
2) Adding --not-grep, --not-author, --not-committer which add negative
matches. Maybe even --grep!=PATTERN, --author!=PATTERN, ...
Example:
git log --grep=this --not-grep=that --committer!="${MY_EMAIL}"
3) Extending the PATTERN we accept in --grep, --author, and --committer,
such that a prefix in the pattern tells us how to use the match:
--grep=!PATTERN
Example:
git log --grep=this --grep='!that' --committer="!${MY_EMAIL}"
4) (going on a limb here) Can this kind of match be done with perl
regular expressions? Maybe we could use --perl-regexp
Example:
I've got nothing :)
Personally I am currently in favour of 2. It seems intuitive, and
unlike option 3, the '!' cannot clash with the pattern. Although 3 has
the bonus of allowing other flags like extended regexp, perl regexp,
case insensitive, negation, etc.
Comments?
-Bart
--
WebSig: http://www.jukie.net/~bart/sig/
^ permalink raw reply
* [PATCH 1/2] git-merge: add option --no-ff
From: Lars Hjemli @ 2007-09-20 12:53 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <1190292789-16203-1-git-send-email-hjemli@gmail.com>
This option forces all merges to create a "true" merge commit, i.e. a commit
with multiple parents.
A fast-forward merge will often be considered The Right Thing when working
in a distributed environment, but it doesn't work well when the branches to
be merged originated in subversion and the merge commit will be pushed back
with 'git svn dcommit'. In this case, a fast-forward merge will make git-svn
mistakenly think that the current branch is tracking the merged upstream
branch. But if the merge is created with --no-ff, 'git svn dcommit' will work
as expected (and the history in subversion and git will be equal).
There are also a use-case for --no-ff in 'native git' when integrating topic
branches: if the merge of a topic-branch results in a fast-forward operation,
there will be no way to detect that the topic-branch ever existed. This might
be good or bad dependent on project policy, but both options (ff/no-ff) are
technically sane.
Signed-off-by: Lars Hjemli <hjemli@gmail.com>
---
Documentation/git-svn.txt | 13 +++++++++++
Documentation/merge-options.txt | 5 ++++
git-merge.sh | 13 +++++++++-
t/t6029-merge-integration.sh | 46 +++++++++++++++++++++++++++++++++++++++
4 files changed, 75 insertions(+), 2 deletions(-)
create mode 100755 t/t6029-merge-integration.sh
diff --git a/Documentation/git-svn.txt b/Documentation/git-svn.txt
index e157c6a..619023d 100644
--- a/Documentation/git-svn.txt
+++ b/Documentation/git-svn.txt
@@ -475,6 +475,19 @@ use 'git-svn rebase' to update your work branch instead of 'git pull' or
when committing into SVN, which can lead to merge commits reversing
previous commits in SVN.
+If you use 'git-svn dcommit' to commit your local work to the upstream
+subversion branch, merge commits are usually handled correctly, i.e.
+git-svn will only follow the first parent of each merge commit and create
+a single subversion revision for each of them. An exception is when two
+subversion branches has been merged locally and the merge ended up as a
+fast-forward operation. This will make git-svn believe that there are no
+local changes to dcommit. To work around this issue, one can redo the
+merge using the --no-ff option:
+
+ $ git reset --hard HEAD@{1} ## undo the fast-forward merge
+ $ git merge --no-ff <branch>
+
+
DESIGN PHILOSOPHY
-----------------
Merge tracking in Subversion is lacking and doing branched development
diff --git a/Documentation/merge-options.txt b/Documentation/merge-options.txt
index d64c259..b34b888 100644
--- a/Documentation/merge-options.txt
+++ b/Documentation/merge-options.txt
@@ -25,3 +25,8 @@
If there is no `-s` option, a built-in list of strategies
is used instead (`git-merge-recursive` when merging a single
head, `git-merge-octopus` otherwise).
+
+--no-ff::
+ Force the creation of a merge commit even when the merge would
+ have resolved as a fast-forward operation. See gitlink:git-svn[1]
+ for a use-case for this option.
diff --git a/git-merge.sh b/git-merge.sh
index 3a01db0..70ca5ff 100755
--- a/git-merge.sh
+++ b/git-merge.sh
@@ -3,7 +3,7 @@
# Copyright (c) 2005 Junio C Hamano
#
-USAGE='[-n] [--summary] [--no-commit] [--squash] [-s <strategy>] [-m=<merge-message>] <commit>+'
+USAGE='[-n] [--summary] [--no-commit] [--no-ff] [--squash] [-s <strategy>] [-m=<merge-message>] <commit>+'
SUBDIRECTORY_OK=Yes
. git-sh-setup
@@ -165,6 +165,9 @@ do
merge_msg="$1"
have_message=t
;;
+ --no-ff)
+ allow_fast_forward=f
+ ;;
-*) usage ;;
*) break ;;
esac
@@ -444,7 +447,13 @@ done
# auto resolved the merge cleanly.
if test '' != "$result_tree"
then
- parents=$(git show-branch --independent "$head" "$@" | sed -e 's/^/-p /')
+ if test "$allow_fast_forward" = "f"
+ then
+ parents=$(git rev-parse "$head" "$@")
+ else
+ parents=$(git show-branch --independent "$head" "$@")
+ fi
+ parents=$(echo "$parents" | sed -e 's/^/-p /')
result_commit=$(printf '%s\n' "$merge_msg" | git commit-tree $result_tree $parents) || exit
finish "$result_commit" "Merge made by $wt_strategy."
dropsave
diff --git a/t/t6029-merge-integration.sh b/t/t6029-merge-integration.sh
new file mode 100755
index 0000000..6ba7dd9
--- /dev/null
+++ b/t/t6029-merge-integration.sh
@@ -0,0 +1,46 @@
+#!/bin/sh
+
+test_description='merge --no-ff'
+
+. ./test-lib.sh
+
+test_expect_success setup '
+ >file &&
+ git add file &&
+ test_tick &&
+ git commit -m initial &&
+ git tag c0 &&
+ c0=$(git rev-parse c0)
+
+ echo second >file &&
+ git add file &&
+ test_tick &&
+ git commit -m second &&
+ git tag c1 &&
+ c1=$(git rev-parse c1)
+ git branch test
+'
+
+test_expect_success 'merge c1' '
+
+ git reset --hard c0 &&
+ test_tick &&
+ git merge c1 &&
+ test $c0 = $(git rev-parse HEAD^) &&
+ test $c1 = $(git rev-parse HEAD)
+
+'
+
+test_expect_success 'merge --no-ff c1' '
+
+ git reset --hard c0 &&
+ test_tick &&
+ git merge --no-ff c1 &&
+ test $c0 = $(git rev-parse HEAD^1) &&
+ test $c1 = $(git rev-parse HEAD^2)
+
+'
+
+test_debug 'gitk &'
+
+test_done
--
1.5.3.2.82.g75c8d
^ permalink raw reply related
* [PATCH 0/2] add support for per branch merge policy
From: Lars Hjemli @ 2007-09-20 12:53 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
These two patches extends git-merge with the options --ff and --no-ff,
and uses the configuration setting branch.<name>.integrationonly to
select the default policy per branch (it obviously defaults to --ff), as
mentioned in http://article.gmane.org/gmane.comp.version-control.git/41980
This can also be used as a clean solution to the problem related to
fast-forward merges between git-svn tracking branches, especially if
git-svn could be taught to set integrationonly=true on all relevant
branches whenever this setting is undefined.
Diffstat:
Documentation/config.txt | 7 +++
Documentation/git-svn.txt | 17 ++++++
Documentation/merge-options.txt | 13 +++++
git-merge.sh | 30 ++++++++++-
t/t6029-merge-integration.sh | 108 +++++++++++++++++++++++++++++++++++++++
5 files changed, 172 insertions(+), 3 deletions(-)
^ permalink raw reply
* [PATCH 2/2] add support for branch.<name>.integrationonly
From: Lars Hjemli @ 2007-09-20 12:53 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <1190292789-16203-2-git-send-email-hjemli@gmail.com>
This setting can be used to set --no-ff as default merge policy for
selected branches. There is also a new new merge-option, --ff, which can
be used to override the default.
Signed-off-by: Lars Hjemli <hjemli@gmail.com>
---
Documentation/config.txt | 7 ++++
Documentation/git-svn.txt | 4 ++
Documentation/merge-options.txt | 14 +++++++--
git-merge.sh | 19 ++++++++++-
t/t6029-merge-integration.sh | 64 ++++++++++++++++++++++++++++++++++++++-
5 files changed, 102 insertions(+), 6 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 015910f..fa7c3c1 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -337,6 +337,13 @@ branch.<name>.merge::
branch.<name>.merge to the desired branch, and use the special setting
`.` (a period) for branch.<name>.remote.
+branch.<name>.integrationonly::
+ When merging into branch <name>, this setting specifies whether
+ fast-forward merges are disallowed. The setting can be overridden
+ by the --ff and --no-ff options of gitlink:git-merge[1]. See
+ gitlink:git-svn[1] for information about this setting when
+ working with subversion branches. Defaults to false.
+
clean.requireForce::
A boolean to make git-clean do nothing unless given -f or -n. Defaults
to false.
diff --git a/Documentation/git-svn.txt b/Documentation/git-svn.txt
index 619023d..55828a3 100644
--- a/Documentation/git-svn.txt
+++ b/Documentation/git-svn.txt
@@ -487,6 +487,10 @@ merge using the --no-ff option:
$ git reset --hard HEAD@{1} ## undo the fast-forward merge
$ git merge --no-ff <branch>
+Alternatively, one can configure the appropriate local branches to use
+--no-ff as default by setting branch.<name>.integrationonly=true in
+$GIT_DIR/config.
+
DESIGN PHILOSOPHY
-----------------
diff --git a/Documentation/merge-options.txt b/Documentation/merge-options.txt
index b34b888..f83bba9 100644
--- a/Documentation/merge-options.txt
+++ b/Documentation/merge-options.txt
@@ -26,7 +26,15 @@
is used instead (`git-merge-recursive` when merging a single
head, `git-merge-octopus` otherwise).
+--ff::
+ Explicitly allow fast-forward merges. This can be used to
+ override the merge policy of the current branch, as specified
+ by the branch.<branch>.integrationonly configuration setting (see
+ gitlink:git-config[1] for further details).
+
--no-ff::
- Force the creation of a merge commit even when the merge would
- have resolved as a fast-forward operation. See gitlink:git-svn[1]
- for a use-case for this option.
+ Explicitly disallow fast-forward merges. This can be used to
+ override the merge policy of the current branch, as specified
+ by the branch.<branch>.integrationonly configuration setting (see
+ gitlink:git-config[1] for further details, and gitlink:git-svn[1]
+ for a use-case for this option).
diff --git a/git-merge.sh b/git-merge.sh
index 70ca5ff..4775767 100755
--- a/git-merge.sh
+++ b/git-merge.sh
@@ -3,7 +3,7 @@
# Copyright (c) 2005 Junio C Hamano
#
-USAGE='[-n] [--summary] [--no-commit] [--no-ff] [--squash] [-s <strategy>] [-m=<merge-message>] <commit>+'
+USAGE='[-n] [--summary] [--no-commit] [--ff] [--no-ff] [--squash] [-s <strategy>] [-m=<merge-message>] <commit>+'
SUBDIRECTORY_OK=Yes
. git-sh-setup
@@ -23,7 +23,7 @@ no_fast_forward_strategies='subtree ours'
no_trivial_strategies='recursive recur subtree ours'
use_strategies=
-allow_fast_forward=t
+allow_fast_forward=
allow_trivial_merge=t
dropsave() {
@@ -165,6 +165,9 @@ do
merge_msg="$1"
have_message=t
;;
+ --ff)
+ allow_fast_forward=t
+ ;;
--no-ff)
allow_fast_forward=f
;;
@@ -245,6 +248,18 @@ do
done
set x $remoteheads ; shift
+if test -z "$allow_fast_forward"
+then
+ branch=$(git-symbolic-ref HEAD | sed -e 's|^refs/heads/||')
+ integrate=$(git config --bool "branch.$branch.integrationonly")
+ if test -n "$branch" -a "$integrate" = "true"
+ then
+ allow_fast_forward=f
+ else
+ allow_fast_forward=t
+ fi
+fi
+
case "$use_strategies" in
'')
case "$#" in
diff --git a/t/t6029-merge-integration.sh b/t/t6029-merge-integration.sh
index 6ba7dd9..c5ffa34 100755
--- a/t/t6029-merge-integration.sh
+++ b/t/t6029-merge-integration.sh
@@ -31,6 +31,8 @@ test_expect_success 'merge c1' '
'
+test_debug 'gitk'
+
test_expect_success 'merge --no-ff c1' '
git reset --hard c0 &&
@@ -41,6 +43,66 @@ test_expect_success 'merge --no-ff c1' '
'
-test_debug 'gitk &'
+test_debug 'gitk'
+
+test_expect_success 'setup integrationonly' '
+
+ git config branch.master.integrationonly true
+
+'
+
+test_expect_success 'merge c1' '
+
+ git reset --hard c0 &&
+ test_tick &&
+ git merge c1 &&
+ test $c0 = $(git rev-parse HEAD^1) &&
+ test $c1 = $(git rev-parse HEAD^2)
+
+'
+
+test_debug 'gitk'
+
+test_expect_success 'merge c1 --ff' '
+
+ git reset --hard c0 &&
+ test_tick &&
+ git merge --ff c1 &&
+ test $c0 = $(git rev-parse HEAD^) &&
+ test $c1 = $(git rev-parse HEAD)
+
+'
+
+test_debug 'gitk'
+
+test_expect_success 'revert integrationonly' '
+
+ git config branch.master.integrationonly false
+
+'
+
+test_expect_success 'merge c1' '
+
+ git reset --hard c0 &&
+ test_tick &&
+ git merge c1 &&
+ test $c0 = $(git rev-parse HEAD^) &&
+ test $c1 = $(git rev-parse HEAD)
+
+'
+
+test_debug 'gitk'
+
+test_expect_success 'merge c1 --no-ff' '
+
+ git reset --hard c0 &&
+ test_tick &&
+ git merge --no-ff c1 &&
+ test $c0 = $(git rev-parse HEAD^1) &&
+ test $c1 = $(git rev-parse HEAD^2)
+
+'
+
+test_debug 'gitk'
test_done
--
1.5.3.2.82.g75c8d
^ permalink raw reply related
* Re: [PATCH 0/2] add support for per branch merge policy
From: Lars Hjemli @ 2007-09-20 13:07 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <1190292789-16203-1-git-send-email-hjemli@gmail.com>
On 9/20/07, Lars Hjemli <hjemli@gmail.com> wrote:
> branch.<name>.integrationonly
Btw: we might want to call this 'branch.<name>.mergepolicy' with the
possible values 'synchronize' and 'integrate'.
--
larsh
^ permalink raw reply
* Re: [PATCH] Add git-rev-list --invert-match
From: Johannes Schindelin @ 2007-09-20 13:12 UTC (permalink / raw)
To: Bart Trojanowski; +Cc: git
In-Reply-To: <20070920123849.GD12076@jukie.net>
Hi,
On Thu, 20 Sep 2007, Bart Trojanowski wrote:
> * Johannes Schindelin <Johannes.Schindelin@gmx.de> [070920 06:34]:
> > On Wed, 19 Sep 2007, Bart Trojanowski wrote:
> > > git log --invert-match --grep="uninteresting"
> >
> > IMHO this is only half a solution. Some of us want to say "git log
> > --grep=this --but-not-(in-the-whole-message) --grep=that".
>
> I have noticed that unique negation flags are getting scarce... we
> already have --reverse, --inverse, and --not mean something elsewhere.
> --but-not maybe be good.
>
> I also agree that git-grep could use this extension.
>
> Anyways, I can see four solutions for adding "show me this but not that"
> functionality to git-rev-list:
>
> 1) adding a --but-not flag, as you suggested. It separates positive
> matches that precede it with negative matches that follow.
>
> Example:
> git log --grep=this --but-not --grep=that --committer="${MY_EMAIL}"
That is basically the approach taken by
http://article.gmane.org/gmane.comp.version-control.git/51874
(Yes, since you refused to search in the mailing list archives, I did it
for you... this time!)
> 2) Adding --not-grep, --not-author, --not-committer which add negative
> matches. Maybe even --grep!=PATTERN, --author!=PATTERN, ...
>
> Example:
> git log --grep=this --not-grep=that --committer!="${MY_EMAIL}"
That sounds sensible.
> 3) Extending the PATTERN we accept in --grep, --author, and --committer,
> such that a prefix in the pattern tells us how to use the match:
> --grep=!PATTERN
>
> Example:
> git log --grep=this --grep='!that' --committer="!${MY_EMAIL}"
Now you can no longer avoid defining clean semantics: what does that mean?
Does it mean that there is _one_ line that does not have "that" in it, or
is it the complete message?
Further, it probably makes sense to have the option to say _both_: "Find
me a commit that contains Bart in one line, but not Simpson, and that
does not contain the word "Sverdoolaege" at all."
> 4) (going on a limb here) Can this kind of match be done with perl
> regular expressions? Maybe we could use --perl-regexp
>
> Example:
> I've got nothing :)
This time you'll have to find the thread yourself. Hint: search for pcre.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH 2/2] add support for branch.<name>.integrationonly
From: Johannes Schindelin @ 2007-09-20 13:14 UTC (permalink / raw)
To: Lars Hjemli; +Cc: Junio C Hamano, git
In-Reply-To: <1190292789-16203-3-git-send-email-hjemli@gmail.com>
Hi,
On Thu, 20 Sep 2007, Lars Hjemli wrote:
> +branch.<name>.integrationonly::
Why not have something like branch.<name>.mergeOptions instead?
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH 0/2] add support for per branch merge policy
From: Johannes Schindelin @ 2007-09-20 13:24 UTC (permalink / raw)
To: Lars Hjemli; +Cc: Junio C Hamano, git
In-Reply-To: <8c5c35580709200607s428e898ay32fd4cdad30f096e@mail.gmail.com>
Hi,
On Thu, 20 Sep 2007, Lars Hjemli wrote:
> On 9/20/07, Lars Hjemli <hjemli@gmail.com> wrote:
> > branch.<name>.integrationonly
>
> Btw: we might want to call this 'branch.<name>.mergepolicy' with the
> possible values 'synchronize' and 'integrate'.
IMHO this is way too limiting.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH 2/2] add support for branch.<name>.integrationonly
From: Matthieu Moy @ 2007-09-20 13:36 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Lars Hjemli, Junio C Hamano, git
In-Reply-To: <Pine.LNX.4.64.0709201413440.28395@racer.site>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> On Thu, 20 Sep 2007, Lars Hjemli wrote:
>
>> +branch.<name>.integrationonly::
>
> Why not have something like branch.<name>.mergeOptions instead?
Why make it plural? I agree that it's better to have a multiple values
option than having just "true/false", which makes it more extensible,
but I don't understand the kind of things you could put here that
wouldn't be better located in a separate configuration option.
--
Matthieu
^ permalink raw reply
* Re: [PATCH 0/2] add support for per branch merge policy
From: Lars Hjemli @ 2007-09-20 13:38 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Junio C Hamano, git
In-Reply-To: <Pine.LNX.4.64.0709201424270.28395@racer.site>
[...joining two posts in one...]
On 9/20/07, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> > On 9/20/07, Lars Hjemli <hjemli@gmail.com> wrote:
> > > branch.<name>.integrationonly
> >
> > Btw: we might want to call this 'branch.<name>.mergepolicy' with the
> > possible values 'synchronize' and 'integrate'.
>
> IMHO this is way too limiting.
> Why not have something like branch.<name>.mergeOptions instead?
Yeah, that would be more powerful, I'll give it a try.
--
larsh
^ permalink raw reply
* Re: [RFC] Convert builin-mailinfo.c to use The Better String Library.
From: Steven Burns @ 2007-09-20 14:06 UTC (permalink / raw)
To: git
In-Reply-To: <20070907061554.GB30161@thunk.org>
> a = b + "/share/" + c + serial_num;
>
> where you can have absolutely no idea how many memory allocations are
> done, due to type coercions, overloaded operators
You are assuming (incorrectly) everybody will use dumb string classes like
that.
It is very possible to create a string class that instead of allocating all
those strings simply concatenates tiny temporary objects and performs one
single operation in the end. Not to mention those temporaries are optimized
away by any decent compiler and you end up with code that runs at the same
speed as your C code.
I've done it, many other programmers have. As a reference, I'd like to
mention Matthew Wilson's chapter on efficient string concatenation in his
book "Imperfect C++". He uses expression templates (that's the technique I
just described) and gets impressive results.
With that said, your point is valid. 90% of C++ programmers will use string
classes that are very inefficient for concatenation, starting with
std::string which I hate for that reason (and many other reasons, e.g. you
have to
resort to Boost for mundane things like trimming)
Steven Burns
"Theodore Tso" <tytso@mit.edu> wrote in message
news:20070907061554.GB30161@thunk.org...
> On Thu, Sep 06, 2007 at 08:09:23PM -0700, Dmitry Kakurin wrote:
>> > Total BS. The string/memory management is not at all relevant. Look at
>> > the
>> > code (I bet you didn't). This isn't the important, or complex part.
>>
>> Not only have I looked at the code, I've also debugged it quite a bit.
>> Granted most of my problems had to do with handling paths on Windows
>> (i.e. string manipulations).
>
> I consider string manipulation to be one of the places where C++ is a
> total disaster. It's way to easy for idiots to do something like this:
>
> a = b + "/share/" + c + serial_num;
>
> where you can have absolutely no idea how many memory allocations are
> done, due to type coercions, overloaded operators (good God, you can
> overload the comma operator in C++!!!), and then when something like
> that ends up in an inner loop, the result is a disaster from a
> performance point of view, and it's not even obvious *why*!
>
>> My goal is to *use* Git. When something does not work *for me* I want
>> to be able to fix it (and contribute the fix) in *shortest time
>> possible* and with *minimal efforts*. As for me it's a diversion from
>> my main activities.
>
> Yes, and if you contribute something the shortest time possible, and
> it ends up being crap, who gets to rewrite it and fix it? I've seen
> too many C++ programs which get this kind of crap added, and it's not
> noticed right away (because C++ is really good at hiding such
> performance killers so they are not visible), and then later on, it's
> even harder to find the performance problems and fix them.
>
>> Now, I realize that I'm a very infrequent contributor to Git, but I
>> want my opinion to be heard.
>
> And if git were written in C++, it's precisely the infrequent
> contributors (who are in a hurry, who only care about the quick hack
> to get them going, and not about the long-term maintainability and
> performance of the package) that are be in the position to do the
> most damage...
>
> - Ted
^ permalink raw reply
* Re: [PATCH 0/3] the return of the strbuf
From: Shawn O. Pearce @ 2007-09-20 14:19 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Pierre Habouzit, git
In-Reply-To: <Pine.LNX.4.64.0709201248400.28395@racer.site>
Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> On Mon, 17 Sep 2007, Shawn O. Pearce wrote:
>
> > $ git log --pretty=format:%an --since=6.months.ago -- fast-import.c \
> > | sort | uniq -c | sort -nr
...
> FWIW I'd do
>
> git shortlog -n --since=6.months.ago HEAD -- fast-import.c|grep "^[A-Z]"
>
> instead...
Yea, Junio pointed out how stupid I was being on #git. I don't
know why I didn't think of using shortlog here as this is one
of the things it was built for. Whatever. I forgot my git-fu
on Monday. :)
--
Shawn.
^ permalink raw reply
* Re: [RFC] Convert builin-mailinfo.c to use The Better String Library.
From: Andreas Ericsson @ 2007-09-20 14:56 UTC (permalink / raw)
To: Steven Burns; +Cc: git
In-Reply-To: <fctuo4$t93$1@sea.gmane.org>
Steven Burns wrote:
>> a = b + "/share/" + c + serial_num;
>>
>> where you can have absolutely no idea how many memory allocations are
>> done, due to type coercions, overloaded operators
>
> You are assuming (incorrectly) everybody will use dumb string classes like
> that.
>
Not really. He said "It's way to easy for idiots to do something like this:"
just prior to the line you quoted. I wholeheartedly agree, but in no way
does anyone assume that everybody will use dumb string classes.
I'm sure it's perfectly possible to write properly functioning programs in
C++. I know I use a few of them myself. That doesn't change the fact that
it's an idiot-friendly language to write code in that's extremely annoying
for competent programmers to fix up later.
--
Andreas Ericsson andreas.ericsson@op5.se
OP5 AB www.op5.se
Tel: +46 8-230225 Fax: +46 8-230231
^ permalink raw reply
* Re: [PATCH] [git-p4] Detect exec bit in more cases.
From: David Brown @ 2007-09-20 15:16 UTC (permalink / raw)
To: Simon Hausmann; +Cc: git
In-Reply-To: <200709192103.53526.simon@lst.de>
On Wed, Sep 19, 2007 at 09:03:50PM +0200, Simon Hausmann wrote:
>On Wednesday 19 September 2007 20:15:03 David Brown wrote:
>> git-p4 was missing the execute bit setting if the file had other attribute
>> bits set.
>> ---
>I'm fine with this, so unless you find a better way:
>
>Acked-By: Simon Hausmann <simon@lst.de>
I sent out an improved version of this patch yesterday
<1190232768445-git-send-email-git@davidb.org> that I'd like to get
approved. I guess I'm not quite sure what happens at this point with a
patch.
Thanks,
David
^ 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