Git development
 help / color / mirror / Atom feed
* Re: cogito remote branch
From: Jakub Narebski @ 2007-11-10 11:52 UTC (permalink / raw)
  To: git
In-Reply-To: <Pine.LNX.4.64.0711101217130.4330@castor.milkiway.cos>

Michael Dressel wrote:
> On Fri, 9 Nov 2007, Jon Loeliger wrote: 
>> On Fri, 2007-11-09 at 10:10, MichaelTiloDressel@t-online.de wrote:
>>
>>>  There are just some features
>>> which simplify things for me in cogito. E.g. in cogito in the simplest
>>> way you don't need to be aware of the index. While with git
>>> you have to remember to add the changes to the index explicitly
>>> to get them committed. 
>> 
>> "git commit -a ..." might be useful for you.
>> 
>> Other lingering cogito-isms you think are lacking in git?
> 
> Thanks for the hint. I have to use git for a while to understand what may 
> still be lacking (at least for me, if at all). Off the top of my head one 
> other difference is that if I do a cg-push the remote (or origin) head is 
> updated automatically, I think.

If you mean that tracking branches are updated on push, this is what
git also does from some time (perhaps not in released version, so please
wait or run 'master').

-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git

^ permalink raw reply

* [PATCH] Simplify strchrnul() compat code
From: René Scharfe @ 2007-11-10 11:55 UTC (permalink / raw)
  To: Andreas Ericsson, Junio C Hamano
  Cc: Pierre Habouzit, Git Mailing List, Johannes Schindelin,
	Jakub Narebski
In-Reply-To: <473434ED.50002@op5.se>

From: Andreas Ericsson <ae@op5.se>

strchrnul() was introduced in glibc in April 1999 and included in
glibc-2.1. Checking for that version means the majority of all git
users would get to use the optimized version in glibc. Of the
remaining few some might get to use a slightly slower version
than necessary but probably not slower than what we have today.

Rediffed-against-next-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
---
I agree that we can do without providing a way to use a native
version outside glibc until we actually encounter such a thing.

 Makefile           |   13 -------------
 compat/strchrnul.c |    8 --------
 git-compat-util.h  |    9 +++++++--
 3 files changed, 7 insertions(+), 23 deletions(-)

diff --git a/Makefile b/Makefile
index 4f1d7ca..4a5d2b9 100644
--- a/Makefile
+++ b/Makefile
@@ -30,8 +30,6 @@ all::
 #
 # Define NO_MEMMEM if you don't have memmem.
 #
-# Define NO_STRCHRNUL if you don't have strchrnul.
-#
 # Define NO_STRLCPY if you don't have strlcpy.
 #
 # Define NO_STRTOUMAX if you don't have strtoumax in the C library.
@@ -409,7 +407,6 @@ ifeq ($(uname_S),Darwin)
 	OLD_ICONV = UnfortunatelyYes
 	NO_STRLCPY = YesPlease
 	NO_MEMMEM = YesPlease
-	NO_STRCHRNUL = YesPlease
 endif
 ifeq ($(uname_S),SunOS)
 	NEEDS_SOCKET = YesPlease
@@ -417,7 +414,6 @@ ifeq ($(uname_S),SunOS)
 	SHELL_PATH = /bin/bash
 	NO_STRCASESTR = YesPlease
 	NO_MEMMEM = YesPlease
-	NO_STRCHRNUL = YesPlease
 	NO_HSTRERROR = YesPlease
 	ifeq ($(uname_R),5.8)
 		NEEDS_LIBICONV = YesPlease
@@ -443,7 +439,6 @@ ifeq ($(uname_O),Cygwin)
 	NO_D_INO_IN_DIRENT = YesPlease
 	NO_STRCASESTR = YesPlease
 	NO_MEMMEM = YesPlease
-	NO_STRCHRNUL = YesPlease
 	NO_SYMLINK_HEAD = YesPlease
 	NEEDS_LIBICONV = YesPlease
 	NO_FAST_WORKING_DIRECTORY = UnfortunatelyYes
@@ -458,14 +453,12 @@ endif
 ifeq ($(uname_S),FreeBSD)
 	NEEDS_LIBICONV = YesPlease
 	NO_MEMMEM = YesPlease
-	NO_STRCHRNUL = YesPlease
 	BASIC_CFLAGS += -I/usr/local/include
 	BASIC_LDFLAGS += -L/usr/local/lib
 endif
 ifeq ($(uname_S),OpenBSD)
 	NO_STRCASESTR = YesPlease
 	NO_MEMMEM = YesPlease
-	NO_STRCHRNUL = YesPlease
 	NEEDS_LIBICONV = YesPlease
 	BASIC_CFLAGS += -I/usr/local/include
 	BASIC_LDFLAGS += -L/usr/local/lib
@@ -481,7 +474,6 @@ endif
 ifeq ($(uname_S),AIX)
 	NO_STRCASESTR=YesPlease
 	NO_MEMMEM = YesPlease
-	NO_STRCHRNUL = YesPlease
 	NO_STRLCPY = YesPlease
 	NEEDS_LIBICONV=YesPlease
 endif
@@ -494,7 +486,6 @@ ifeq ($(uname_S),IRIX64)
 	NO_SETENV=YesPlease
 	NO_STRCASESTR=YesPlease
 	NO_MEMMEM = YesPlease
-	NO_STRCHRNUL = YesPlease
 	NO_STRLCPY = YesPlease
 	NO_SOCKADDR_STORAGE=YesPlease
 	SHELL_PATH=/usr/gnu/bin/bash
@@ -705,10 +696,6 @@ ifdef NO_MEMMEM
 	COMPAT_CFLAGS += -DNO_MEMMEM
 	COMPAT_OBJS += compat/memmem.o
 endif
-ifdef NO_STRCHRNUL
-	COMPAT_CFLAGS += -DNO_STRCHRNUL
-	COMPAT_OBJS += compat/strchrnul.o
-endif
 
 ifdef THREADED_DELTA_SEARCH
 	BASIC_CFLAGS += -DTHREADED_DELTA_SEARCH
diff --git a/compat/strchrnul.c b/compat/strchrnul.c
deleted file mode 100644
index 51839fe..0000000
--- a/compat/strchrnul.c
+++ /dev/null
@@ -1,8 +0,0 @@
-#include "../git-compat-util.h"
-
-char *gitstrchrnul(const char *s, int c)
-{
-	while (*s && *s != c)
-		s++;
-	return (char *)s;
-}
diff --git a/git-compat-util.h b/git-compat-util.h
index e72654b..11e6df6 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -183,9 +183,14 @@ void *gitmemmem(const void *haystack, size_t haystacklen,
                 const void *needle, size_t needlelen);
 #endif
 
-#ifdef NO_STRCHRNUL
+#if !defined(__GLIBC__) && !__GLIBC_PREREQ(2, 1)
 #define strchrnul gitstrchrnul
-char *gitstrchrnul(const char *s, int c);
+static inline char *gitstrchrnul(const char *s, int c)
+{
+        while (*s && *s != c)
+                s++;
+        return (char *)s;
+}
 #endif
 
 extern void release_pack_memory(size_t, int);

^ permalink raw reply related

* Re: [PATCH REPLACEMENT for 2/2] git status: show relative paths when run in a subdirectory
From: Michel Marti @ 2007-11-10 12:08 UTC (permalink / raw)
  To: git
In-Reply-To: <Pine.LNX.4.64.0711091529570.4362@racer.site>

Untracked files in the current dir don't include the relative path 
to the project-root, but changed/updated files do:

# Changes to be committed:
#   (use "git reset HEAD <file>..." to unstage)
#
#       new file: ../subdir/hello
#
# Untracked files:
#   (use "git add <file>..." to include in what will be committed)
#
#       world

With the patch below (on top of your changes), the output becomes

# Changes to be committed:
#   (use "git reset HEAD <file>..." to unstage)
#
#       new file: hello
#
# Untracked files:
#   (use "git add <file>..." to include in what will be committed)
#
#       world

Cheers,

- Michel

diff --git a/wt-status.c b/wt-status.c
index 0d25362..2cdc8ce 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -133,8 +133,8 @@ static void wt_status_print_filepair(struct wt_status *s,
 
        strbuf_init(&onebuf, 0);
        strbuf_init(&twobuf, 0);
-       one = quote_path(p->one->path, -1, &onebuf, s->prefix);
-       two = quote_path(p->two->path, -1, &twobuf, s->prefix);
+       one = quote_path(p->one->path, strlen(p->one->path), &onebuf, s->prefix);
+       two = quote_path(p->two->path, strlen(p->two->path), &twobuf, s->prefix);
 
        color_fprintf(s->fp, color(WT_STATUS_HEADER), "#\t");
        switch (p->status) {
@@ -233,7 +233,8 @@ static void wt_status_print_initial(struct wt_status *s)
        for (i = 0; i < active_nr; i++) {
                color_fprintf(s->fp, color(WT_STATUS_HEADER), "#\t");
                color_fprintf_ln(s->fp, color(WT_STATUS_UPDATED), "new file: %s",
-                               quote_path(active_cache[i]->name, -1,
+                               quote_path(active_cache[i]->name,
+                                       strlen(active_cache[i]->name),
                                           &buf, s->prefix));
        }
        if (active_nr)

^ permalink raw reply related

* Re: [PATCH] Make builtin-tag.c use parse_options.
From: Carlos Rica @ 2007-11-10 12:25 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Pierre Habouzit
In-Reply-To: <7vabpmpr9y.fsf@gitster.siamese.dyndns.org>

2007/11/10, Junio C Hamano <gitster@pobox.com>:
> Carlos Rica <jasampler@gmail.com> writes:
>
> > Also, this removes those tests ensuring that repeated
> > -m options don't allocate memory more than once, because now
> > this is done after parsing options, using the last one
> > when more are given. The same for -F.
>
> The reason for this change is...?  Is this because it is
> cumbersome to detect and refuse multiple -m options using the
> parseopt API?  If so, the API may be what needs to be fixed.
> Taking the last one and discarding earlier ones feels to me an
> arbitrary choice.
>
> While I freely admit that I do not particularly find the "One -m
> introduces one new line, concatenated to form the final
> paragraph" handling of multiple -m options done by git-commit
> nice nor useful, I suspect that it would make more sense to make
> git-tag and git-commit handle multiple -m option consistently,
> if you are going to change the existing semantics.  Since some
> people really seem to like multiple -m handling of git-commit,
> the avenue of the least resistance for better consistency would
> be to accept and concatenate (with LF in between) multiple -m
> options.
>
> With multiple -F, I think erroring out would be the sensible
> thing to do, but some people might prefer concatenation.  I do
> not care either way as long as commit and tag behave
> consistently.

A solution not needing memory allocation into the option parser
could be setting a callback running over the repeated option
arguments, passing them to the function one per each call.
Then, the user will be able to decide if he wants the arguments
concatenated or only need one of them and prefers erroring out.

Is this already possible with the current parser or the callback
mode only calls using the last option?

^ permalink raw reply

* Re: `git-send-email' doesn't specify `Content-Type'
From: Björn Steinbrink @ 2007-11-10 12:25 UTC (permalink / raw)
  To: Brian Swetland; +Cc: Johannes Schindelin, Ludovic Courtès, git
In-Reply-To: <20071110101420.GA21353@bulgaria>

On 2007.11.10 02:14:20 -0800, Brian Swetland wrote:
> [Johannes Schindelin <Johannes.Schindelin@gmx.de>]
> > Hi,
> > 
> > On Sat, 10 Nov 2007, Ludovic Court?s wrote:
> > 
> > > Apparently, `git-send-email' doesn't specify the email's `Content-Type',
> > > notably its charset, while it should really add something like:
> > > 
> > >   Content-Type: text/plain; charset=UTF-8
> > > 
> > > Or did I miss an option or something?
> > 
> > Apparently.  There was a thread some days ago, about that very issue.  
> > Please find and read it.
> 
> The thread I found says that git-send-email should do the right thing if
> there are non-ascii characters, but this does not seem to be the case
> for me.
> 
> The example I have involves a coworker's name which needs non-ascii
> characters.  They are properly escaped in the From: line generated by
> git-format-patch.  git-send-email puts the generated From: line at the
> top of the body of the email, unescapes it (to utf-8), and proceeds to
> send the email with no Content-Type specified.

You mean that it converts the header field to utf-8? It doesn't do that
here (neither master nor 1.5.3.5) and IIRC that would be invalid anyway,
because Content-Type applies to exactly that, content, not headers. Your
sample has no non-ASCII characters (or at least I didn't see any), so
git-send-email doesn't add a header to specify a charset.

Björn

> This behaviour is observed in 1.5.3.5.  A sample output from
> git-format-patch follows, which demonstrates the problem:
> 
> 
> >From 3440baaed3b21138f6fc8b80e03769e3903f9c11 Mon Sep 17 00:00:00 2001
> From: =?utf-8?q?Arve=20Hj=C3=B8nnev=C3=A5g?= <arve@android.com>
> Date: Wed, 7 Nov 2007 22:51:44 -0800
> Subject: [PATCH] hrtimer: Add timer back to pending list if it was reactivated and has already expired again.
> 
> This avoids problems with timer hardware that does not respond to timers set in the past.
> 
> Signed-off-by: Brian Swetland <swetland@android.com>
> ---
>  kernel/hrtimer.c |   10 ++++++++--
>  1 files changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c
> index 22a2514..7c60769 100644
> --- a/kernel/hrtimer.c
> +++ b/kernel/hrtimer.c
> @@ -1149,8 +1149,14 @@ static void run_hrtimer_softirq(struct softirq_action *h)
>  			 * If the timer was rearmed on another CPU, reprogram
>  			 * the event device.
>  			 */
> -			if (timer->base->first == &timer->node)
> -				hrtimer_reprogram(timer, timer->base);
> +			if (timer->base->first == &timer->node) {
> +				if(hrtimer_reprogram(timer, timer->base)) {
> +					__remove_hrtimer(timer, timer->base,
> +							 HRTIMER_STATE_PENDING, 0);
> +					list_add_tail(&timer->cb_entry,
> +						      &cpu_base->cb_pending);
> +				}
> +			}
>  		}
>  	}
>  	spin_unlock_irq(&cpu_base->lock);

^ permalink raw reply

* Re: `git-send-email' doesn't specify `Content-Type'
From: Brian Swetland @ 2007-11-10 12:35 UTC (permalink / raw)
  To: Björn Steinbrink; +Cc: Johannes Schindelin, Ludovic Courtès, git
In-Reply-To: <20071110122528.GA4977@atjola.homenet>

[-- Attachment #1: Type: text/plain, Size: 1367 bytes --]

[Björn Steinbrink <B.Steinbrink@gmx.de>]
> On 2007.11.10 02:14:20 -0800, Brian Swetland wrote:
> > 
> > The example I have involves a coworker's name which needs non-ascii
> > characters.  They are properly escaped in the From: line generated by
> > git-format-patch.  git-send-email puts the generated From: line at the
> > top of the body of the email, unescapes it (to utf-8), and proceeds to
> > send the email with no Content-Type specified.
> 
> You mean that it converts the header field to utf-8? It doesn't do that
> here (neither master nor 1.5.3.5) and IIRC that would be invalid anyway,
> because Content-Type applies to exactly that, content, not headers. Your
> sample has no non-ASCII characters (or at least I didn't see any), so
> git-send-email doesn't add a header to specify a charset.

The first line of the patch is a From: field with Arve's name, in
an (rfc822?) encoded format):
From: =?utf-8?q?Arve=20Hj=C3=B8nnev=C3=A5g?= <arve@android.com>

The mail generated by git-send-email makes this From: line the first
line of the *body* of the generated email.  This line in the body
is no longer escaped, the utf8 characters are visible, but the header
of the message does not have a Content-Type indicating a non-ascii
encoding.

Attached are the result of git-format-patch and the actual email
received from git-send-email (mbox format).

Brian

[-- Attachment #2: 0001-hrtimer-Add-timer-back-to-pending-list-if-it-was-re.patch --]
[-- Type: text/x-diff, Size: 1224 bytes --]

>From 3440baaed3b21138f6fc8b80e03769e3903f9c11 Mon Sep 17 00:00:00 2001
From: =?utf-8?q?Arve=20Hj=C3=B8nnev=C3=A5g?= <arve@android.com>
Date: Wed, 7 Nov 2007 22:51:44 -0800
Subject: [PATCH] hrtimer: Add timer back to pending list if it was reactivated and has already expired again.

This avoids problems with timer hardware that does not respond to timers set in the past.

Signed-off-by: Brian Swetland <swetland@android.com>
---
 kernel/hrtimer.c |   10 ++++++++--
 1 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c
index 22a2514..7c60769 100644
--- a/kernel/hrtimer.c
+++ b/kernel/hrtimer.c
@@ -1149,8 +1149,14 @@ static void run_hrtimer_softirq(struct softirq_action *h)
 			 * If the timer was rearmed on another CPU, reprogram
 			 * the event device.
 			 */
-			if (timer->base->first == &timer->node)
-				hrtimer_reprogram(timer, timer->base);
+			if (timer->base->first == &timer->node) {
+				if(hrtimer_reprogram(timer, timer->base)) {
+					__remove_hrtimer(timer, timer->base,
+							 HRTIMER_STATE_PENDING, 0);
+					list_add_tail(&timer->cb_entry,
+						      &cpu_base->cb_pending);
+				}
+			}
 		}
 	}
 	spin_unlock_irq(&cpu_base->lock);
-- 
1.5.3.5


[-- Attachment #3: mbox --]
[-- Type: text/plain, Size: 2456 bytes --]

>From swetland@google.com Sat Nov 10 02:04:08 2007
Return-Path: <swetland@google.com>
X-Original-To: swetland@frotz.net
Delivered-To: swetland@frotz.net
Received: from smtp-out.google.com (smtp-out.google.com [216.239.45.13])
	by mumble.frotz.net (Postfix) with ESMTP id 1BC002500D
	for <swetland@frotz.net>; Sat, 10 Nov 2007 02:04:08 -0800 (PST)
Received: from zps35.corp.google.com (zps35.corp.google.com [172.25.146.35])
	by smtp-out.google.com with ESMTP id lAAA5hUj030761;
	Sat, 10 Nov 2007 02:05:43 -0800
DomainKey-Signature: a=rsa-sha1; s=beta; d=google.com; c=nofws; q=dns;
	h=received:from:to:cc:subject:date:message-id:x-mailer;
	b=g2B628wRsJJahlIpNw3mgNDqOQKNMcUCPOurvqj+3fO6qLH+vpBS0ZwN1lLv6BnC7
	w4QLOotDo7t+nI2KgZDVQ==
Received: from bulgaria (bulgaria.corp.google.com [172.18.102.38])
	by zps35.corp.google.com with ESMTP id lAAA5e22002202;
	Sat, 10 Nov 2007 02:05:42 -0800
Received: by bulgaria (Postfix, from userid 1000)
	id 613018F45E; Sat, 10 Nov 2007 02:05:25 -0800 (PST)
From: swetland@google.com
To: swetland@frotz.net
Cc: =?utf-8?q?Arve=20Hj=C3=B8nnev=C3=A5g?= <arve@android.com>
Subject: [PATCH] hrtimer: Add timer back to pending list if it was reactivated and has already expired again.
Date: Sat, 10 Nov 2007 02:05:25 -0800
Message-Id: <1194689125-21319-1-git-send-email-swetland@google.com>
X-Mailer: git-send-email 1.5.3.5
X-SpamProbe: GOOD 0.0000099 b892c7c5c469d044f28ab48846487cf5
X-SpamCheck: OKAY
Status: RO
Content-Length: 983
Lines: 33

From: Arve Hjønnevåg <arve@android.com>

This avoids problems with timer hardware that does not respond to timers set in the past.

Signed-off-by: Brian Swetland <swetland@android.com>
---
 kernel/hrtimer.c |   10 ++++++++--
 1 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c
index 22a2514..7c60769 100644
--- a/kernel/hrtimer.c
+++ b/kernel/hrtimer.c
@@ -1149,8 +1149,14 @@ static void run_hrtimer_softirq(struct softirq_action *h)
 			 * If the timer was rearmed on another CPU, reprogram
 			 * the event device.
 			 */
-			if (timer->base->first == &timer->node)
-				hrtimer_reprogram(timer, timer->base);
+			if (timer->base->first == &timer->node) {
+				if(hrtimer_reprogram(timer, timer->base)) {
+					__remove_hrtimer(timer, timer->base,
+							 HRTIMER_STATE_PENDING, 0);
+					list_add_tail(&timer->cb_entry,
+						      &cpu_base->cb_pending);
+				}
+			}
 		}
 	}
 	spin_unlock_irq(&cpu_base->lock);
-- 
1.5.3.5



^ permalink raw reply related

* Re: cogito remote branch
From: Michael Dressel @ 2007-11-10 12:35 UTC (permalink / raw)
  To: jnareb; +Cc: git

>Michael Dressel wrote:
>> On Fri, 9 Nov 2007, Jon Loeliger wrote: 
>>> On Fri, 2007-11-09 at 10:10, MichaelTiloDressel@t-online.de wrote:
>>>
>>>>  There are just some features
>>>> which simplify things for me in cogito. E.g. in cogito in the 
simplest
>>>> way you don't need to be aware of the index. While with git
>>>> you have to remember to add the changes to the index explicitly
>>>> to get them committed. 
>>> 
>>> "git commit -a ..." might be useful for you.
>>> 
>>> Other lingering cogito-isms you think are lacking in git?
>> 
>> Thanks for the hint. I have to use git for a while to understand what 
may 
>> still be lacking (at least for me, if at all). Off the top of my head 
one 
>> other difference is that if I do a cg-push the remote (or origin) head 
is 
>> updated automatically, I think.

Jakub Narebski wrote:
>If you mean that tracking branches are updated on push, this is what
>git also does from some time (perhaps not in released version, so please
>wait or run 'master').

Ok nice. Another thing is that git-push will push all the tracking 
branches in refs/remotes/origin. 

I was wondering if it was possible to have some cogito like wrapper 
scripts (lets say kg-...) for git again which would use the git commands 
with some arguments in order to resemble the cogito behavior 
even more closely. But it dawned on me that this would most likely end up 
in a rather large amount of script code again.

Cheers,
Michael

^ permalink raw reply

* Re: `git-send-email' doesn't specify `Content-Type'
From: Björn Steinbrink @ 2007-11-10 12:51 UTC (permalink / raw)
  To: Brian Swetland; +Cc: Johannes Schindelin, Ludovic Courtès, git
In-Reply-To: <20071110123505.GA24445@bulgaria>

On 2007.11.10 04:35:05 -0800, Brian Swetland wrote:
> [Björn Steinbrink <B.Steinbrink@gmx.de>]
> > On 2007.11.10 02:14:20 -0800, Brian Swetland wrote:
> > > 
> > > The example I have involves a coworker's name which needs non-ascii
> > > characters.  They are properly escaped in the From: line generated by
> > > git-format-patch.  git-send-email puts the generated From: line at the
> > > top of the body of the email, unescapes it (to utf-8), and proceeds to
> > > send the email with no Content-Type specified.
> > 
> > You mean that it converts the header field to utf-8? It doesn't do that
> > here (neither master nor 1.5.3.5) and IIRC that would be invalid anyway,
> > because Content-Type applies to exactly that, content, not headers. Your
> > sample has no non-ASCII characters (or at least I didn't see any), so
> > git-send-email doesn't add a header to specify a charset.
> 
> The first line of the patch is a From: field with Arve's name, in
> an (rfc822?) encoded format):
> From: =?utf-8?q?Arve=20Hj=C3=B8nnev=C3=A5g?= <arve@android.com>
> 
> The mail generated by git-send-email makes this From: line the first
> line of the *body* of the generated email.  This line in the body
> is no longer escaped, the utf8 characters are visible, but the header
> of the message does not have a Content-Type indicating a non-ascii
> encoding.

Ah! Commit author differs from mail sender, didn't think of that. That's
probably the same problem as with the -s option, ie. that git-send-email
only looks at the existing text and not add anything it adds itself when
checking the encoding. Sorry for the noise.

Björn

^ permalink raw reply

* Re: gitweb, updating 'last changed' column on the project page
From: Jon Smirl @ 2007-11-10 13:05 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: git
In-Reply-To: <200711101034.47861.jnareb@gmail.com>

On 11/10/07, Jakub Narebski <jnareb@gmail.com> wrote:
> On Saturday, 10 November 2007, Jon Smirl wrote:
> > On 11/10/07, Jon Smirl <jonsmirl@gmail.com> wrote:
> >> On 11/9/07, Jakub Narebski <jnareb@gmail.com> wrote:
> >>> Jon Smirl wrote:
> >>>
> >>>> At http://git.digispeaker.com/ the 'last change' column is not getting updated.
> >>>>
> >>>> mpc5200b.git
> >>>>       DigiSpeaker for Freescale MPC5200B.
> >>>>       Jon Smirl
> >>>>       5 weeks ago
> >>>>       summary | shortlog | log | tree
> >>>>
> >>>> It still says 5 weeks ago, but if I click on the project last change is today.
> >>>>
> >>>> What controls this? I tried running update-server-info
> >>>
> >>> What does
> >>>
> >>>   git for-each-ref --format="%(refname):%09%(committer)" --sort=-committerdate
> >>>       refs/heads
> >>
> >> [daedalus]$ git for-each-ref --format="%(refname):%09%(committer)" \
> >> --sort=-committerdate refs/heads
> >> refs/heads/m24: Jon Smirl <jonsmirl@gmail.com> 1191362799 -0400
> >> refs/heads/m25: Jon Smirl <jonsmirl@gmail.com> 1191472422 -0400
> >> refs/heads/m26: Jon Smirl <jonsmirl@gmail.com> 1194382038 -0500
> >> refs/heads/m28: Jon Smirl <jonsmirl@gmail.com> 1194385071 -0500
> >> refs/heads/m29: Jon Smirl <jonsmirl@gmail.com> 1194585780 -0500
> >
> > It appears to be using the first head instead of the most recent date.
> It appears to not _sort_ the output by committerdate, as it should with
> '--sort=-committerdate'.

It is sorted by committerdate, the sort is ascending. Did you expect
it to be descending, pick off the last entry instead of the first?

>
> 1442:[gitweb/web!git]$ git for-each-ref --format="%(refname):%09%(committer)" \
>   --sort=-committerdate refs/heads
> refs/heads/gitweb/web:  Jakub Narebski <jnareb@gmail.com> 1194616779 +0100
> refs/heads/man: Junio C Hamano <junio@hera.kernel.org> 1194602628 +0000
> refs/heads/html:        Junio C Hamano <junio@hera.kernel.org> 1194602626 +0000
> refs/heads/origin:      Junio C Hamano <gitster@pobox.com> 1194602274 -0800
> [...]
> refs/heads/gitweb-snapshot+navbar:      Sven Verdoolaege <skimo@kotnet.org> 1134765981 +0100
>
> 1443:[gitweb/web!git]$ git --version
> git version 1.5.3.5
>
>
> Note that git-for-each-ref with those options returns most recent head
> first, sorting output by date of commit (date of adding to repository)
>
> >>>
> >>> return? Does adding --count select proper branch, with proper update
> >>> date?
> >>
> >> Is it looking for master, and just picking the first branch instead?
>
> Gitweb should not (and I think does not) have 'master' hardcoded
> anywhere. It might use HEAD in some cases you don't want it to...
>
> >>>
> >>> Which gitweb version is this?
> >>
> >> <!-- git web interface version 1.5.3.5.605.g79fa-dirty, (C) 2005-2006,
> >> Kay Sievers <kay.sievers@vrfy.org>, Christian Gierke -->
> >> <!-- git core binaries version 1.5.3.5.605.g79fa-dirty -->
>
> Older version of gitweb used HEAD branch for'last changed' info on
> the projects list page. That is why I asked about gitweb version.
>
> But this is not the case of your problem:
> 1. Your gitweb is new enough to use git-for-each-ref. It use
>    git for-each-ref --format="%(committer)" --sort=-committerdate
>                     --count=1 refs/heads
> 2. Looking at 'heads' view (or 'heads' part of summary view) one can see
>    that m29 is current branch (HEAD), and it is most recent.
>
> Strange...
> --
> Jakub Narebski
> Poland
>


-- 
Jon Smirl
jonsmirl@gmail.com

^ permalink raw reply

* Re: [PATCH] Make builtin-tag.c use parse_options.
From: Pierre Habouzit @ 2007-11-10 13:13 UTC (permalink / raw)
  To: Carlos Rica; +Cc: Junio C Hamano, git
In-Reply-To: <1b46aba20711100425o2f351ac5o81537adc6f09dc80@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 2320 bytes --]

On Sat, Nov 10, 2007 at 12:25:44PM +0000, Carlos Rica wrote:
> 2007/11/10, Junio C Hamano <gitster@pobox.com>:
> > Carlos Rica <jasampler@gmail.com> writes:

> A solution not needing memory allocation into the option parser
> could be setting a callback running over the repeated option
> arguments, passing them to the function one per each call.
> Then, the user will be able to decide if he wants the arguments
> concatenated or only need one of them and prefers erroring out.
> 
> Is this already possible with the current parser or the callback
> mode only calls using the last option?

  Everything is possible, you just have to code it. With a callback
you have in the struct option two places to store "things". The void*
value pointer and the intptr_t defval. _Usually_ the void* is the
pointer to the data that will be _written_ and the defval the data that
will be put into the void* under some circumstances (e.g. when your
option is negated).

  For Your case I'd go with some kind of string list pointed into the
void * value, defval has no or little use. You don't really care about
allocating memory in the option parser, I mean, option parsing is done
once at the initialization phase. It's not evil. In pseudo-C here is how
I would write the callback:

int parse_opt_stringlist(const struct option *opt, const char *arg, int unset)
{
    string_list **l = opt->value;
    string_list_elem *e;

    if (unset) { /* negationg option clears the list */
	while (*l) {
	    string_list_elem_free(string_list_pop(l));
	}
	return 0;
    }

    e = string_list_elem_new();
    e->data = arg;
    string_list_push(l, e);
    return 0;
}

  And you're done, you can do what you want with that list from the caller.
There probably is such a structure in git, if not, it can probably be hacked
in a few lines.

  Remember, callbacks give you _full_ control on what you can do in the option
parser, and if you're not happy with Turing complete expressivity, there isn't
anything I can do for you :P Note that if you do write such a generic
callback, it belongs to parse-options.[hc].

-- 
·O·  Pierre Habouzit
··O                                                madcoder@debian.org
OOO                                                http://www.madism.org

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply

* Re: [PATCH] builtin-commit: fix --signoff
From: Pierre Habouzit @ 2007-11-10 13:16 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Johannes Schindelin, git, krh
In-Reply-To: <7vr6iyo4ff.fsf@gitster.siamese.dyndns.org>

[-- Attachment #1: Type: text/plain, Size: 907 bytes --]

On Sat, Nov 10, 2007 at 09:06:28AM +0000, Junio C Hamano wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> >  	if (signoff) {
> > ...
> > +		strbuf_init(&sob, 0);
> > +		strbuf_addstr(&sob, sign_off_header);
> > +		strbuf_addstr(&sob, git_committer_info(1));
> > +		p = strrchr(sob.buf, '>');
> > +		if (p)
> > +			strbuf_setlen(&sob, p + 1 - sob.buf);
> > +		strbuf_addch(&sob, '\n');
> > +
> > +		for (i = sb.len - 1; i > 0 && sb.buf[i - 1] != '\n'; i--)
> > +			; /* do nothing */
> > +		if (prefixcmp(sb.buf + i, sob.buf))
> > +			strbuf_addbuf(&sb, &sob);
> >  	}
> 
> At this point doesn't this leak sob.buf?

  It does, strbuf_addbuf copies `sob` but doesn't release resources.

-- 
·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: gitweb, updating 'last changed' column on the project page
From: Jakub Narebski @ 2007-11-10 13:27 UTC (permalink / raw)
  To: Jon Smirl; +Cc: git
In-Reply-To: <9e4733910711100505n78459612xdaa12eaa880773d8@mail.gmail.com>

Jon Smirl wrote:
> On 11/10/07, Jakub Narebski <jnareb@gmail.com> wrote:
>> On Saturday, 10 November 2007, Jon Smirl wrote:
>>> On 11/10/07, Jon Smirl <jonsmirl@gmail.com> wrote:
>>>
>>>> [daedalus]$ git for-each-ref --format="%(refname):%09%(committer)" \
>>>> --sort=-committerdate refs/heads
>>>> refs/heads/m24: Jon Smirl <jonsmirl@gmail.com> 1191362799 -0400
>>>> refs/heads/m25: Jon Smirl <jonsmirl@gmail.com> 1191472422 -0400
>>>> refs/heads/m26: Jon Smirl <jonsmirl@gmail.com> 1194382038 -0500
>>>> refs/heads/m28: Jon Smirl <jonsmirl@gmail.com> 1194385071 -0500
>>>> refs/heads/m29: Jon Smirl <jonsmirl@gmail.com> 1194585780 -0500
>>>
>>> It appears to be using the first head instead of the most recent date.
>>
>> It appears to not _sort_ the output by committerdate, as it should with
>> '--sort=-committerdate'.
> 
> It is sorted by committerdate, the sort is ascending. Did you expect
> it to be descending, pick off the last entry instead of the first?

Excerpts from git-for-each-ref(1):

  git-for-each-ref [--count=<count>]* (...) [--sort=<key>]* (...)

  <count>
          By  default  the  command  shows all refs that match <pattern>. This
          option makes it stop after showing that many refs.

   <key>  A field name to sort on. Prefix - to sort in descending order of the
          value.  When  unspecified,  refname is used. More than one sort keys
          can be given.

So I expect --sort=-committerdate to sort by date of committing, 
descending, and --count=1 pick first one, which means most recent.

It looks like "your" gitweb sorts ascending instead... strange...


How does git_get_last_activity subroutine in your gitweb.cgi looks like?
Does it have '--sort=-commiterdate'? If it has, then I think it is some
strange bug in git, if it doesn't it is strange modification of gitweb.

HTH
-- 
Jakub Narebski
Poland

^ permalink raw reply

* Re: [PATCH] builtin-commit: fix --signoff
From: Johannes Schindelin @ 2007-11-10 13:47 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, krh
In-Reply-To: <7vr6iyo4ff.fsf@gitster.siamese.dyndns.org>

Hi,

On Sat, 10 Nov 2007, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> >  	if (signoff) {
> > ...
> > +		strbuf_init(&sob, 0);
> > +		strbuf_addstr(&sob, sign_off_header);
> > +		strbuf_addstr(&sob, git_committer_info(1));
> > +		p = strrchr(sob.buf, '>');
> > +		if (p)
> > +			strbuf_setlen(&sob, p + 1 - sob.buf);
> > +		strbuf_addch(&sob, '\n');
> > +
> > +		for (i = sb.len - 1; i > 0 && sb.buf[i - 1] != '\n'; i--)
> > +			; /* do nothing */
> > +		if (prefixcmp(sb.buf + i, sob.buf))
> > +			strbuf_addbuf(&sb, &sob);
> >  	}
> 
> At this point doesn't this leak sob.buf?

Darn.  I had a "strbuf_release(&sob);" there, but at some stage removed it 
by mistake.

But there is more to be fixed.  Rather than let git_committer_info() add 
the timestamp, only to strip it away, is a waste.

Will redo the patch.

Ciao,
Dscho

^ permalink raw reply

* [PATCH v4] user-manual: Add section "Why bisecting merge commits can be harder ..."
From: Steffen Prohaska @ 2007-11-10 13:49 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Benoit Sigoure, Andreas Ericsson, Johannes Sixt, git,
	Steffen Prohaska
In-Reply-To: <B622E814-D7D1-4DC8-A724-666BA0A1220F@zib.de>

This commit adds a discussion of the challenge of bisecting
merge commits to the user manual.  The original author is
Junio C Hamano <gitster@pobox.com>, who posted the text to
the mailing list <http://marc.info/?l=git&m=119403257315527&w=2>.
His email was adapted for the manual.

The discussion is added to "Rewriting history and maintainig
patch series".  The text added requires good understanding of
merging and rebasing.  Therefore it should not be placed too
early in the manual.  Right after the section on "Problems with
rewriting history", the discussion of bisect gives another reason
for linearizing as much of the history as possible.

The text includes suggestions and fixes by
Ralf Wildenhues <Ralf.Wildenhues@gmx.de> and
Benoit Sigoure <tsuna@lrde.epita.fr>.

Signed-off-by: Steffen Prohaska <prohaska@zib.de>
---
 Documentation/user-manual.txt |   98 +++++++++++++++++++++++++++++++++++++++++
 1 files changed, 98 insertions(+), 0 deletions(-)

Since PATCH v3: I moved the section to chapter 5. It's not longer in a separate
section titled "Advance Topics".  I split the paragraph on merge C and
reordered some sentences.  I took part the paragraph on "experienced users"
from Junio's last email.

Junio, you can take ownership of the commit when applying -- if you like.  Most
of the text was written by you. Just mention me as the editor in the commit
message.

    Steffen

diff --git a/Documentation/user-manual.txt b/Documentation/user-manual.txt
index d99adc6..8ddc19f 100644
--- a/Documentation/user-manual.txt
+++ b/Documentation/user-manual.txt
@@ -2554,6 +2554,104 @@ branches into their own work.
 For true distributed development that supports proper merging,
 published branches should never be rewritten.
 
+[[bisect-merges]]
+Why bisecting merge commits can be harder than bisecting linear history
+-----------------------------------------------------------------------
+
+This section discusses how gitlink:git-bisect[1] plays
+with differently shaped histories.  The text is based upon
+an email by Junio C. Hamano to the git mailing list
+(link:http://marc.info/?l=git&m=119403257315527&w=2[link:http://marc.info/?l=git&m=119403257315527&w=2]).
+It was adapted for the user manual.
+
+Using gitlink:git-bisect[1] on a history with merges can be
+challenging.  Bisecting through merges is not a technical
+problem. The real problem is what to do when the culprit turns
+out to be a merge commit.  How to spot what really is wrong, and
+figure out how to fix it.  The problem is not for the tool but
+for the human, and it is real.
+
+Imagine this history:
+
+................................................
+      ---Z---o---X---...---o---A---C---D
+          \                       /
+           o---o---Y---...---o---B
+................................................
+
+Suppose that on the upper development line, the meaning of one
+of the functions that existed at Z was changed at commit X.  The
+commits from Z leading to A change both the function's
+implementation and all calling sites that existed at Z, as well
+as new calling sites they add, to be consistent.  There is no
+bug at A.
+
+Suppose that in the meantime the lower development line somebody
+added a new calling site for that function at commit Y.  The
+commits from Z leading to B all assume the old semantics of that
+function and the callers and the callee are consistent with each
+other.  There is no bug at B, either.
+
+Suppose further that the two development lines were merged at C
+and there was no textual conflict with this three way merge.
+The result merged cleanly.
+
+Now, during bisect you find that the merge C is broken.  You
+started to bisect, because you found D is bad and you know Z was
+good.  The breakage is understandable, as at C, the new calling
+site of the function added by the lower branch is not converted
+to the new semantics, while all the other calling sites that
+already existed at Z would have been converted by the merge.  The
+new calling site has semantic adjustment needed, but you do not
+know that yet.
+
+You need to find out what is the cause of the breakage by looking
+at the merge commit C and the history leading to it.  How would
+you do that?
+
+Both "git diff A C" and "git diff B C" would be an enormous patch.
+Each of them essentially shows the whole change on each branch
+since they diverged.  The developers may have well behaved to
+create good commits that follow the "commit small, commit often,
+commit well contained units" mantra, and each individual commit
+leading from Z to A and from Z to B may be easy to review and
+understand, but looking at these small and easily reviewable
+steps alone would not let you spot the breakage.  You need to
+have a global picture of what the upper branch did (and
+among many, one of them is to change the semantics of that
+particular function) and look first at the huge "diff A C"
+(which shows the change the lower branch introduces), and see if
+that huge change is consistent with what have been done between
+Z and A.
+
+On the other hand, if you did not merge at C but rebased the
+history between Z to B on top of A, you would have get this
+linear history:
+
+................................................................
+    ---Z---o---X--...---o---A---o---o---Y*--...---o---B*--D*
+................................................................
+
+Bisecting between Z and D* would hit a single culprit commit Y*
+instead.  This tends to be easier to understand why it is broken.
+
+For this reason, many experienced git users, even when they are
+working on an otherwise merge-heavy project, keep the histories
+linear by rebasing their work on top of public upstreams before
+publishing.
+
+But if you already made a merge C instead of rebasing, all
+is not lost.  In the illustrated case, you can easily rebase
+one parent branch on top of the other after the fact, just
+to understand the history and to make the history more
+easily to bisect.  Even though the published history should
+not be rewound without consent with others in the project,
+nobody gets hurt if you rebased to create alternate history
+privately.  After understanding the breakage and coming up
+with a fix on top of D*, you can discard that rebased
+history, and apply the same fix on top of D, as D* and D
+should have the identical trees.
+
 [[advanced-branch-management]]
 Advanced branch management
 ==========================
-- 
1.5.3.5.578.g886d

^ permalink raw reply related

* Re: [PATCH] Simplify strchrnul() compat code
From: Andreas Ericsson @ 2007-11-10 14:04 UTC (permalink / raw)
  To: René Scharfe
  Cc: Junio C Hamano, Pierre Habouzit, Git Mailing List,
	Johannes Schindelin, Jakub Narebski
In-Reply-To: <47359C44.6090903@lsrfire.ath.cx>

René Scharfe wrote:
>  
> -#ifdef NO_STRCHRNUL
> +#if !defined(__GLIBC__) && !__GLIBC_PREREQ(2, 1)

This will break things for users of glibc-2.1.1 (the first release still
available from ftp://sources.redhat.com/pub/glibc/old-releases that
includes the strchrnul() function), since __GLIBC_PREREQ() was invented
after strchrnul() was introduced.

Replacing __GLIBC__ with __GLIBC_PREREQ (as in the original patch) will
solve it nicely. Users of glibc-2.1.1 will be the odd minority where
strchrnul() is available in their libc but not used.

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

^ permalink raw reply

* Re: gitweb, updating 'last changed' column on the project page
From: Jon Smirl @ 2007-11-10 14:10 UTC (permalink / raw)
  To: Jakub Narebski, Junio C Hamano; +Cc: git
In-Reply-To: <200711101427.18215.jnareb@gmail.com>

On 11/10/07, Jakub Narebski <jnareb@gmail.com> wrote:
> > It is sorted by committerdate, the sort is ascending. Did you expect
> > it to be descending, pick off the last entry instead of the first?
>
> Excerpts from git-for-each-ref(1):
>
>   git-for-each-ref [--count=<count>]* (...) [--sort=<key>]* (...)
>
>   <count>
>           By  default  the  command  shows all refs that match <pattern>. This
>           option makes it stop after showing that many refs.
>
>    <key>  A field name to sort on. Prefix - to sort in descending order of the
>           value.  When  unspecified,  refname is used. More than one sort keys
>           can be given.
>
> So I expect --sort=-committerdate to sort by date of committing,
> descending, and --count=1 pick first one, which means most recent.

git has a bug, it is not implementing the - prefix. I am using git head.

jonsmirl@terra:~$ cd mpc5200b
jonsmirl@terra:~/mpc5200b$ git for-each-ref
--format="%(refname):%09%(committer)" --sort=-committerdate refs/heads
refs/heads/m24: Jon Smirl <jonsmirl@gmail.com> 1191362799 -0400
refs/heads/m25: Jon Smirl <jonsmirl@gmail.com> 1191472422 -0400
refs/heads/m26: Jon Smirl <jonsmirl@gmail.com> 1194382038 -0500
refs/heads/m28: Jon Smirl <jonsmirl@gmail.com> 1194385071 -0500
refs/heads/m29: Jon Smirl <jonsmirl@gmail.com> 1194674673 -0500
jonsmirl@terra:~/mpc5200b$ git for-each-ref
--format="%(refname):%09%(committer)" --sort=committerdate refs/heads
refs/heads/m24: Jon Smirl <jonsmirl@gmail.com> 1191362799 -0400
refs/heads/m25: Jon Smirl <jonsmirl@gmail.com> 1191472422 -0400
refs/heads/m26: Jon Smirl <jonsmirl@gmail.com> 1194382038 -0500
refs/heads/m28: Jon Smirl <jonsmirl@gmail.com> 1194385071 -0500
refs/heads/m29: Jon Smirl <jonsmirl@gmail.com> 1194674673 -0500
jonsmirl@terra:~/mpc5200b$ git --version
git version 1.5.3.5.1651.g30bf
jonsmirl@terra:~/mpc5200b$


>
> It looks like "your" gitweb sorts ascending instead... strange...
>
>
> How does git_get_last_activity subroutine in your gitweb.cgi looks like?
> Does it have '--sort=-commiterdate'? If it has, then I think it is some
> strange bug in git, if it doesn't it is strange modification of gitweb.
>
> HTH
> --
> Jakub Narebski
> Poland
>


-- 
Jon Smirl
jonsmirl@gmail.com

^ permalink raw reply

* Re: [PATCH REPLACEMENT for 2/2] git status: show relative paths when run in a subdirectory
From: Johannes Schindelin @ 2007-11-10 14:10 UTC (permalink / raw)
  To: Michel Marti; +Cc: git
In-Reply-To: <fh46vv$ooj$1@ger.gmane.org>

Hi,

please, please, please do not cull the Cc list.  I consider it rude to 
reply to _me_, but _address_ the mail to me, either the To: (preferred) or 
the Cc: (not so preferred).

On Sat, 10 Nov 2007, Michel Marti wrote:

> Untracked files in the current dir don't include the relative path 
> to the project-root, but changed/updated files do:
> 
> # Changes to be committed:
> #   (use "git reset HEAD <file>..." to unstage)
> #
> #       new file: ../subdir/hello
> #
> # Untracked files:
> #   (use "git add <file>..." to include in what will be committed)
> #
> #       world
> 
> With the patch below (on top of your changes), the output becomes
> 
> # Changes to be committed:
> #   (use "git reset HEAD <file>..." to unstage)
> #
> #       new file: hello
> #
> # Untracked files:
> #   (use "git add <file>..." to include in what will be committed)
> #
> #       world
> 
> Cheers,
> 
> - Michel
> 
> diff --git a/wt-status.c b/wt-status.c
> index 0d25362..2cdc8ce 100644
> --- a/wt-status.c
> +++ b/wt-status.c
> @@ -133,8 +133,8 @@ static void wt_status_print_filepair(struct wt_status *s,
>  
>         strbuf_init(&onebuf, 0);
>         strbuf_init(&twobuf, 0);
> -       one = quote_path(p->one->path, -1, &onebuf, s->prefix);
> -       two = quote_path(p->two->path, -1, &twobuf, s->prefix);
> +       one = quote_path(p->one->path, strlen(p->one->path), &onebuf, s->prefix);
> +       two = quote_path(p->two->path, strlen(p->two->path), &twobuf, s->prefix);
>  
>         color_fprintf(s->fp, color(WT_STATUS_HEADER), "#\t");
>         switch (p->status) {
> @@ -233,7 +233,8 @@ static void wt_status_print_initial(struct wt_status *s)
>         for (i = 0; i < active_nr; i++) {
>                 color_fprintf(s->fp, color(WT_STATUS_HEADER), "#\t");
>                 color_fprintf_ln(s->fp, color(WT_STATUS_UPDATED), "new file: %s",
> -                               quote_path(active_cache[i]->name, -1,
> +                               quote_path(active_cache[i]->name,
> +                                       strlen(active_cache[i]->name),
>                                            &buf, s->prefix));
>         }
>         if (active_nr)
> 

This patch is wrong.

If you want to go that way, move the strlen() call _into_ quote_path(), 
like I had it earlier.

But then we will have a double traversal of the strings again.  That's 
what I tried to avoid, but I missed one place:

In line 94, it says "... && off < len && ...".  This should read something 
like "((len < 0 && !in[off]) || off < len)" instead.  Or maybe even "(len 
< 0 || off < len)" and have an "} else if (in[off]) off++; else break;" in 
the loop block.

Besides, you completely ignored the nice examples how other people 
contribute their patches, with mail bodies that double as a commit 
message, a diffstat, and with a test case.

Hth,
Dscho

^ permalink raw reply

* Re: [PATCH 07/11] git-fetch: Limit automated tag following to only fetched objects
From: Andreas Ericsson @ 2007-11-10 14:10 UTC (permalink / raw)
  To: CJ van den Berg; +Cc: Shawn O. Pearce, Junio C Hamano, git
In-Reply-To: <20071109121228.GA4241@prefect.vdbonline.net>

CJ van den Berg wrote:
> On Fri, Nov 09, 2007 at 06:06:31AM -0500, Shawn O. Pearce wrote:
>> We now redefine the rule to be: "tags are fetched if they refer
>> to an object that was just transferred; that is an object that is
>> new to your repository".  This rule is quite simple to understand,
>> you only get a tag if you just got the object it refers to.
> 
> With this new rule a retrospectively pushed tag will never be fetched,
> right? With our local work flow tags are only ever pushed retrospectively
> because the tagged commit has to first pass regression tests. So this would
> be a major regression for us.
> 

Same for us. Deciding after something has been pushed that "ok, this version
works, let's make that one the release" is, I think, fairly common behaviour.

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

^ permalink raw reply

* [PATCH] test-lib.sh: move error line after error() declaration
From: Michele Ballabio @ 2007-11-10 14:17 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

This patch removes a spurious "command not found" error
and actually makes the "Test script did not set test_description."
string follow the command line option "--no-color".

Signed-off-by: Michele Ballabio <barra_cuda@katamail.com>
---

This is for the master branch.

 t/test-lib.sh |   15 ++++++++++-----
 1 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/t/test-lib.sh b/t/test-lib.sh
index 603a8cd..90b6844 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -66,9 +66,6 @@ esac
 	tput sgr0 >/dev/null 2>&1 &&
 	color=t
 
-test "${test_description}" != "" ||
-error "Test script did not set test_description."
-
 while test "$#" -ne 0
 do
 	case "$1" in
@@ -77,8 +74,7 @@ do
 	-i|--i|--im|--imm|--imme|--immed|--immedi|--immedia|--immediat|--immediate)
 		immediate=t; shift ;;
 	-h|--h|--he|--hel|--help)
-		echo "$test_description"
-		exit 0 ;;
+		help=t; shift ;;
 	-v|--v|--ve|--ver|--verb|--verbo|--verbos|--verbose)
 		verbose=t; shift ;;
 	-q|--q|--qu|--qui|--quie|--quiet)
@@ -124,6 +120,15 @@ say () {
 	say_color info "$*"
 }
 
+test "${test_description}" != "" ||
+error "Test script did not set test_description."
+
+if test "$help" = "t"
+then
+	echo "$test_description"
+	exit 0
+fi
+
 exec 5>&1
 if test "$verbose" = "t"
 then
-- 
1.5.3.5

^ permalink raw reply related

* [PATCH] builtin-commit: fix --signoff
From: Johannes Schindelin @ 2007-11-10 15:58 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, krh
In-Reply-To: <Pine.LNX.4.64.0711101346120.4362@racer.site>


The Signed-off-by: line contained a spurious timestamp.  The reason was
a call to git_committer_info(1), which automatically added the
timestamp.

Instead, fmt_ident() was taught to interpret an empty string for the
date (as opposed to NULL, which still triggers the default behavior)
as "do not bother with the timestamp", and builtin-commit.c uses it.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---

	On Sat, 10 Nov 2007, Johannes Schindelin wrote:

	> Will redo the patch.

	And here it is.

 builtin-commit.c  |   29 ++++++++++++++++++-----------
 ident.c           |   10 +++++++---
 t/t7500-commit.sh |   13 +++++++++++++
 3 files changed, 38 insertions(+), 14 deletions(-)

diff --git a/builtin-commit.c b/builtin-commit.c
index e8bc4c4..a4d69ad 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -181,21 +181,28 @@ static int prepare_log_message(const char *index_file, const char *prefix)
 		die("could not open %s\n", git_path(commit_editmsg));
 
 	stripspace(&sb, 0);
-	if (fwrite(sb.buf, 1, sb.len, fp) < sb.len)
-		die("could not write commit template: %s\n",
-		    strerror(errno));
 
 	if (signoff) {
-		const char *info, *bol;
-
-		info = git_committer_info(1);
-		strbuf_addch(&sb, '\0');
-		bol = strrchr(sb.buf + sb.len - 1, '\n');
-		if (!bol || prefixcmp(bol, sign_off_header))
-			fprintf(fp, "\n");
-		fprintf(fp, "%s%s\n", sign_off_header, git_committer_info(1));
+		struct strbuf sob;
+		int i;
+
+		strbuf_init(&sob, 0);
+		strbuf_addstr(&sob, sign_off_header);
+		strbuf_addstr(&sob, fmt_ident(getenv("GIT_COMMITTER_NAME"),
+                         getenv("GIT_COMMITTER_EMAIL"), "", 1));
+		strbuf_addch(&sob, '\n');
+
+		for (i = sb.len - 1; i > 0 && sb.buf[i - 1] != '\n'; i--)
+			; /* do nothing */
+		if (prefixcmp(sb.buf + i, sob.buf))
+			strbuf_addbuf(&sb, &sob);
+		strbuf_release(&sob);
 	}
 
+	if (fwrite(sb.buf, 1, sb.len, fp) < sb.len)
+		die("could not write commit template: %s\n",
+		    strerror(errno));
+
 	strbuf_release(&sb);
 
 	if (in_merge && !no_edit)
diff --git a/ident.c b/ident.c
index 9b2a852..5be7533 100644
--- a/ident.c
+++ b/ident.c
@@ -224,13 +224,17 @@ const char *fmt_ident(const char *name, const char *email,
 	}
 
 	strcpy(date, git_default_date);
-	if (date_str)
-		parse_date(date_str, date, sizeof(date));
+	if (date_str) {
+		if (*date_str)
+			parse_date(date_str, date, sizeof(date));
+		else
+			date[0] = '\0';
+	}
 
 	i = copy(buffer, sizeof(buffer), 0, name);
 	i = add_raw(buffer, sizeof(buffer), i, " <");
 	i = copy(buffer, sizeof(buffer), i, email);
-	i = add_raw(buffer, sizeof(buffer), i, "> ");
+	i = add_raw(buffer, sizeof(buffer), i, date[0] ? "> " : ">");
 	i = copy(buffer, sizeof(buffer), i, date);
 	if (i >= sizeof(buffer))
 		die("Impossibly long personal identifier");
diff --git a/t/t7500-commit.sh b/t/t7500-commit.sh
index abbf54b..13d5a0c 100755
--- a/t/t7500-commit.sh
+++ b/t/t7500-commit.sh
@@ -93,4 +93,17 @@ test_expect_success 'commit message from file should override template' '
 	commit_msg_is "standard input msg"
 '
 
+cat > expect << EOF
+zort
+Signed-off-by: C O Mitter <committer@example.com>
+EOF
+
+test_expect_success '--signoff' '
+	echo "yet another content *narf*" >> foo &&
+	echo "zort" |
+		GIT_EDITOR=../t7500/add-content git commit -s -F - foo &&
+	git cat-file commit HEAD | sed "1,/^$/d" > output &&
+	git diff expect output
+'
+
 test_done
-- 
1.5.3.5.1674.g6e7f7

^ permalink raw reply related

* [PATCH] for-each-ref: fix setup of option-parsing for --sort
From: Lars Hjemli @ 2007-11-10 16:07 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Jakub Narebski, Jon Smirl
In-Reply-To: <9e4733910711100610y478c62cend1d9af84e0ecc08b@mail.gmail.com>

The option value for --sort is already a pointer to a pointer to struct
ref_sort, so just use it.

Signed-off-by: Lars Hjemli <hjemli@gmail.com>
---
 builtin-for-each-ref.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/builtin-for-each-ref.c b/builtin-for-each-ref.c
index da8c794..e909e66 100644
--- a/builtin-for-each-ref.c
+++ b/builtin-for-each-ref.c
@@ -847,7 +847,7 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
 		OPT_GROUP(""),
 		OPT_INTEGER( 0 , "count", &maxcount, "show only <n> matched refs"),
 		OPT_STRING(  0 , "format", &format, "format", "format to use for the output"),
-		OPT_CALLBACK(0 , "sort", &sort_tail, "key",
+		OPT_CALLBACK(0 , "sort", sort_tail, "key",
 		            "field name to sort on", &opt_parse_sort),
 		OPT_END(),
 	};
-- 
1.5.3.5.623.g0a1d

^ permalink raw reply related

* Re: [PATCH 2/2] --pretty=format: on-demand format expansion
From: Johannes Schindelin @ 2007-11-10 16:07 UTC (permalink / raw)
  To: René Scharfe
  Cc: Jeff King, Junio C Hamano, Paul Mackerras, Git Mailing List,
	Pierre Habouzit
In-Reply-To: <47359221.7090707@lsrfire.ath.cx>

Hi,

On Sat, 10 Nov 2007, Ren? Scharfe wrote:

> [...] have cooked up a different one on top of a cleaned up version of 
> mine.  It plays the dirty trick of reading expansions of repeated 
> placeholders from the strbuf..

... which would not work (likely even segfault) if you work with the same 
private data on different strbufs.

But I guess it will not matter much in practice.

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH 2/2] --pretty=format: on-demand format expansion
From: René Scharfe @ 2007-11-10 16:24 UTC (permalink / raw)
  To: Johannes Schindelin
  Cc: Jeff King, Junio C Hamano, Paul Mackerras, Git Mailing List,
	Pierre Habouzit
In-Reply-To: <Pine.LNX.4.64.0711101605560.4362@racer.site>

Johannes Schindelin schrieb:
> Hi,
> 
> On Sat, 10 Nov 2007, René Scharfe wrote:
> 
>> [...] have cooked up a different one on top of a cleaned up version of 
>> mine.  It plays the dirty trick of reading expansions of repeated 
>> placeholders from the strbuf..
> 
> ... which would not work (likely even segfault) if you work with the same 
> private data on different strbufs.
> 
> But I guess it will not matter much in practice.

Only a single strbuf is used, and the function that copies the data
around, strbuf_adddup(), operates on a single strbuf, only.  Copying
data between two strbufs using strbuf_add() etc. would be safe.

What one should *not* do is this:

	strbuf_add(sb, sb->buf + offset, length);

This leads to problems when the buffer is realloc()ated by strbuf_add().

What other things can go wrong?  A segfault would definitely matter..

Thanks,
René

^ permalink raw reply

* Re: [PATCH] for-each-ref: fix setup of option-parsing for --sort
From: Johannes Schindelin @ 2007-11-10 16:25 UTC (permalink / raw)
  To: Lars Hjemli; +Cc: Junio C Hamano, git, Jakub Narebski, Jon Smirl
In-Reply-To: <1194710863-22868-1-git-send-email-hjemli@gmail.com>

Hi,

On Sat, 10 Nov 2007, Lars Hjemli wrote:

> The option value for --sort is already a pointer to a pointer to struct
> ref_sort, so just use it.

Could you add a test for that too, please?

Thanks,
Dscho

^ permalink raw reply

* [PATCH] for-each-ref: fix setup of option-parsing for --sort
From: Lars Hjemli @ 2007-11-10 16:47 UTC (permalink / raw)
  To: Johannes Schindelin, Junio C Hamano; +Cc: Jakub Narebski, Jon Smirl, git

The option value for --sort is already a pointer to a pointer to struct
ref_sort, so just use it.

Signed-off-by: Lars Hjemli <hjemli@gmail.com>
---

On Nov 10, 2007 5:25 PM, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> Could you add a test for that too, please?

Is this ok?


 builtin-for-each-ref.c  |    2 +-
 t/t6300-for-each-ref.sh |   22 ++++++++++++++++++++++
 2 files changed, 23 insertions(+), 1 deletions(-)

diff --git a/builtin-for-each-ref.c b/builtin-for-each-ref.c
index da8c794..e909e66 100644
--- a/builtin-for-each-ref.c
+++ b/builtin-for-each-ref.c
@@ -847,7 +847,7 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
 		OPT_GROUP(""),
 		OPT_INTEGER( 0 , "count", &maxcount, "show only <n> matched refs"),
 		OPT_STRING(  0 , "format", &format, "format", "format to use for the output"),
-		OPT_CALLBACK(0 , "sort", &sort_tail, "key",
+		OPT_CALLBACK(0 , "sort", sort_tail, "key",
 		            "field name to sort on", &opt_parse_sort),
 		OPT_END(),
 	};
diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index d0809eb..c722635 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -148,4 +148,26 @@ test_expect_success 'Check format "rfc2822" date fields output' '
 	git diff expected actual
 '
 
+cat >expected <<\EOF
+refs/heads/master
+refs/tags/testtag
+EOF
+
+test_expect_success 'Verify ascending sort' '
+	git-for-each-ref --format="%(refname)" --sort=refname >actual &&
+	git diff expected actual
+'
+
+
+cat >expected <<\EOF
+refs/tags/testtag
+refs/heads/master
+EOF
+
+test_expect_success 'Verify descending sort' '
+	git-for-each-ref --format="%(refname)" --sort=-refname >actual &&
+	git diff expected actual
+'
+
+
 test_done
-- 
1.5.3.5.623.g0a1d

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox