Git development
 help / color / mirror / Atom feed
* Re: [PATCH] init-db: support --import to add all files and commit right after init
From: Miles Bader @ 2009-03-31  9:09 UTC (permalink / raw)
  To: Jeff King; +Cc: Nguyễn Thái Ngọc Duy, git
In-Reply-To: <20090325041934.GA15524@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:
> I seem to recall that we were phasing out "--long-option <arg>" at some
> point, and that all long-options should use "--long-option=". But maybe
> I am mis-remembering.

Surely it should support both where possible, since both are standard
syntaxes for passing args to "--"-style long options (so users expect
both to work).

-Miles

-- 
Occam's razor split hairs so well, I bought the whole argument!

^ permalink raw reply

* Re: [PATCH 07/10] rev-list: call new "filter_skip" function
From: Johannes Schindelin @ 2009-03-31  9:23 UTC (permalink / raw)
  To: Christian Couder; +Cc: Junio C Hamano, git, John Tapsell
In-Reply-To: <200903310845.23674.chriscool@tuxfamily.org>

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1398 bytes --]

Hi,

On Tue, 31 Mar 2009, Christian Couder wrote:

> Le lundi 30 mars 2009, Johannes Schindelin a écrit :
>
> > On Mon, 30 Mar 2009, Christian Couder wrote:
> > > Le jeudi 26 mars 2009, Junio C Hamano a écrit :
> > > >
> > > > I've learned to suspect without reading a qsort() callback that 
> > > > does not derefence its arguments.  Is this doing the right thing?
> > >
> > > I think so.
> >
> > I suspect something much worse: you are trying to build a list of 
> > sha1s of commits that need to be skipped, later to look up every 
> > commit via binary search.
> >
> > But it has been proven a lot of times that using a hash set is 
> > superior to that approach, and even better: we already have the 
> > framework in place in the form of struct decorate.
> 
> I had a look, and "struct decorate" can be used to store objects, but I 
> want to store only sha1s.

No, you want to _look up_ sha1s.  And struct decorate is not about storing 
objects, but to attach things to objects.  From decorate.h:

	struct object_decoration {
	        const struct object *base;
	        void *decoration;
	};

So, if you just do

	struct decoration all_the_sha1s;

	...

		add_decoration(&all_the_sha1s, &commit->object, sha1);

you can look up the sha1 much more efficiently than with binary searches 
like this:

		unsigned char *sha1 = lookup_decoration(&all_the_sha1s,
			&commit->object);

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH] init-db: support --import to add all files and commit right after init
From: Jeff King @ 2009-03-31  9:27 UTC (permalink / raw)
  To: Miles Bader; +Cc: Nguyễn Thái Ngọc Duy, git
In-Reply-To: <buomyb2cb3c.fsf@dhlpc061.dev.necel.com>

On Tue, Mar 31, 2009 at 06:09:43PM +0900, Miles Bader wrote:

> Jeff King <peff@peff.net> writes:
> > I seem to recall that we were phasing out "--long-option <arg>" at some
> > point, and that all long-options should use "--long-option=". But maybe
> > I am mis-remembering.
> 
> Surely it should support both where possible, since both are standard
> syntaxes for passing args to "--"-style long options (so users expect
> both to work).

Yes.  See later in the thread where I correctly remember the issue.

-Peff

^ permalink raw reply

* Re: [PATCH 1/2] send-email: refactor and ensure prompting doesn't  loop forever
From: Matthieu Moy @ 2009-03-31  9:32 UTC (permalink / raw)
  To: Jay Soffian; +Cc: git, Junio C Hamano
In-Reply-To: <76718490903300904k7e8c1054m99edb92d8d582387@mail.gmail.com>

Jay Soffian <jaysoffian@gmail.com> writes:

> On Mon, Mar 30, 2009 at 11:45 AM, Jay Soffian <jaysoffian@gmail.com> wrote:
>> Okay, well, I figured out how to work the polish. Term::ReadLine is
>> attempting to use /dev/tty for input/output, which is closed. And
>> because send-email enables warnings, its attempt to do so emits the
>> messages you are seeing. Can you confirm that this patch squelches the
>> warnings?
>
> Ugh, not that. This:
>
> diff --git a/git-send-email.perl b/git-send-email.perl
> index f0405c8..81240ef 100755
> --- a/git-send-email.perl
> +++ b/git-send-email.perl
> @@ -612,6 +612,9 @@ sub ask {
>         my $default = $arg{default};
>         my $resp;
>         my $i = 0;
> +       return defined $default ? $default : undef
> +               unless defined $term->IN and defined fileno($term->IN) and
> +                      defined $term->OUT and defined fileno($term->OUT);
>         while ($i++ < 10) {
>                 $resp = $term->readline($prompt);
>                 if (!defined $resp) { # EOF

Surprising. I did the test with 3 commits to send (titled "hello",
"hello-again" and "hello-oncemore"). the log says this
  
Send this email reply required at /home/moy/local/usr/libexec/git-core//git-send-email line 866.
OK. Log says:
Sendmail: /usr/sbin/sendmail -i Matthieu.Moy@imag.fr
From: Matthieu.Moy@imag.fr
To: Matthieu.Moy@imag.fr
Subject: [PATCH 1/3] hello
Date: Tue, 31 Mar 2009 10:47:59 +0200
Message-Id: <1238489281-5518-1-git-send-email-Matthieu.Moy@imag.fr>
X-Mailer: git-send-email 1.6.2.1.413.gf2ad.dirty

Result: OK
(mbox) Adding cc: moy <moy@973704de-e02c-4a59-87bb-087b52aa604b> from line 'From: moy <moy@973704de-e02c-4a59-87bb-087b52aa604b>'

From: Matthieu.Moy@imag.fr
To: Matthieu.Moy@imag.fr
Subject: [PATCH 2/3] hello-again
Date: Tue, 31 Mar 2009 10:48:00 +0200
Message-Id: <1238489281-5518-2-git-send-email-Matthieu.Moy@imag.fr>
X-Mailer: git-send-email 1.6.2.1.413.gf2ad.dirty
In-Reply-To: <1238489281-5518-1-git-send-email-Matthieu.Moy@imag.fr>
References: <1238489281-5518-1-git-send-email-Matthieu.Moy@imag.fr>

(line 866 is this: die "Send this email reply required" unless defined $_;)

And I received only patch 1/3.

Actually, this seems to be a totally separate issue. What happens is
that for the first email, the script executes :

		if ($needs_confirm eq "inform") {
			$confirm_unconfigured = 0; # squelch this message for the rest of this run
			$ask_default = "y"; # assume yes on EOF since user hasn't explicitly asked for confirmation
			... (show message, ...)
		}
		$_ = ask("Send this email? ([y]es|[n]o|[q]uit|[a]ll): ",
		         valid_re => qr/^(?:yes|y|no|n|quit|q|all|a)/i,
		         default => $ask_default);

and the second time, since $confirm_unconfigured = 0 has been set, the
message is not shown again, _and_ $ask_default not set (since
$needs_confirm is set according to $confirm_unconfigured). I think the
solution is to use a variable different from $confirm_unconfigured to
say whether the message has already been displayed, along the lines
of:

diff --git a/git-send-email.perl b/git-send-email.perl
index 5916c86..9e36c35 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -357,6 +357,7 @@ if ($confirm_unconfigured) {
 };
 die "Unknown --confirm setting: '$confirm'\n"
        unless $confirm =~ /^(?:auto|cc|compose|always|never)/;
+my $confirm_msg_shown = undef;
 
 # Debugging, print out the suppressions.
 if (0) {
@@ -844,17 +849,21 @@ X-Mailer: git-send-email $gitversion
                print "\n$header\n";
                my $ask_default;
                if ($needs_confirm eq "inform") {
-                       $confirm_unconfigured = 0; # squelch this message for the rest of this run
                        $ask_default = "y"; # assume yes on EOF since user hasn't explicitly asked for confirmation
-                       print "    The Cc list above has been expanded by additional\n";
-                       print "    addresses found in the patch commit message. By default\n";
-                       print "    send-email prompts before sending whenever this occurs.\n";
-                       print "    This behavior is controlled by the sendemail.confirm\n";
-                       print "    configuration setting.\n";
-                       print "\n";
-                       print "    For additional information, run 'git send-email --help'.\n";
-                       print "    To retain the current behavior, but squelch this message,\n";
-                       print "    run 'git config --global sendemail.confirm auto'.\n\n";
+                       if (!defined $confirm_msg_shown) {
+                               $confirm_msg_shown = 0; # squelch this message for the rest of this run
+                               print "    The Cc list above has been expanded by additional\n";
+                               print "    addresses found in the patch commit message. By default\n";
+                               print "    send-email prompts before sending whenever this occurs.\n";
+                               print "    This behavior is controlled by the sendemail.confirm\n";
+                               print "    configuration setting.\n";
+                               print "\n";
+                               print "    For additional information, run 'git send-email --help'.\n";
+                               print "    To retain the current behavior, but squelch this message,\n";
+                               print "    run 'git config --global sendemail.confirm auto'.\n\n";
+                       }
                }
                $_ = ask("Send this email? ([y]es|[n]o|[q]uit|[a]ll): ",
                         valid_re => qr/^(?:yes|y|no|n|quit|q|all|a)/i,


(worksforme at least)

-- 
Matthieu

^ permalink raw reply related

* Re: [PATCH] git-svn: fix ls-tree usage with dash-prefixed paths
From: Björn Steinbrink @ 2009-03-31  9:41 UTC (permalink / raw)
  To: Eric Wong; +Cc: Junio C Hamano, git, Anton Gyllenberg
In-Reply-To: <20090331073147.GB3307@atjola.homenet>

On 2009.03.31 09:31:47 +0200, Björn Steinbrink wrote:
> On 2009.03.31 09:11:00 +0200, Björn Steinbrink wrote:
> > And interestingly, the problem doesn't seem to be in
> > show_tree/show_recursive, but in match_tree_entry.
> > 
> > With "git ls-tree HEAD gitweb/git-favicon.png g" we descend into gitweb/
> > and at some point we get:
> > 
> > match = "g"
> > base = "gitweb/"
> > 
> > And we have:
> > if (baselen >= matchlen) {
> > 	if (strncmp(base, match, matchlen))
> > 		continue;
> > 	/* The base is a subdirectory of a path which was specified */
> > 	return 1;
> > }
> > 
> > So we return 1 there. The code doesn't do what the comment says, so I
> > guess we can be pretty sure that the behaviour is not intended.
> 
> Yup, it's in match_tree_entry, you get the same thing with git show.
> With git.git, you can try with:
> 
> git show 4fa535a -- Documentation/git-merge.txt D
> 
> I'll try to get a patch done, if noone beats me to it.

Ah, crap, "git show" actually uses a different function,
tree_entry_interesting, which happens to have the same problem, but
needs a slightly different fix.

Björn

^ permalink raw reply

* [PATCH 0/4] Header Includes Cleanup Part 2
From: Nathaniel P Dawson @ 2009-03-31  9:45 UTC (permalink / raw)
  To: git

Here is part 2 of this project; I expect another 2 parts before
completion.

Regards,
Nathaniel P Dawson

^ permalink raw reply

* [PATCH 1/4] Header Includes Cleanup Part 2
From: Nathaniel P Dawson @ 2009-03-31  9:45 UTC (permalink / raw)
  To: git; +Cc: Nathaniel P Dawson
In-Reply-To: <1238492721-92484-1-git-send-email-nathaniel.dawson@gmail.com>

Deleted header includes of cache.h, strbuf.h, and remote.h where
http.h is already included since it includes them.

Also deleted a couple includes I missed during part 1.

Signed-off-by: Nathaniel P Dawson <nathaniel.dawson@gmail.com>
---
 builtin-rev-parse.c                       |    4 +---
 contrib/convert-objects/convert-objects.c |    1 -
 http-push.c                               |    4 +---
 http-walker.c                             |    3 +--
 http.h                                    |    1 -
 5 files changed, 3 insertions(+), 10 deletions(-)

diff --git a/builtin-rev-parse.c b/builtin-rev-parse.c
index 81d5a6f..dc0a9ab 100644
--- a/builtin-rev-parse.c
+++ b/builtin-rev-parse.c
@@ -3,11 +3,9 @@
  *
  * Copyright (C) Linus Torvalds, 2005
  */
-#include "cache.h"
-#include "commit.h"
+#include "builtin.h"
 #include "refs.h"
 #include "quote.h"
-#include "builtin.h"
 #include "parse-options.h"
 
 #define DO_REVS		1
diff --git a/contrib/convert-objects/convert-objects.c b/contrib/convert-objects/convert-objects.c
index 90e7900..99a7cfd 100644
--- a/contrib/convert-objects/convert-objects.c
+++ b/contrib/convert-objects/convert-objects.c
@@ -1,7 +1,6 @@
 #include "cache.h"
 #include "blob.h"
 #include "commit.h"
-#include "tree.h"
 
 struct entry {
 	unsigned char old_sha1[20];
diff --git a/http-push.c b/http-push.c
index 6ce5a1d..97b2e68 100644
--- a/http-push.c
+++ b/http-push.c
@@ -1,14 +1,12 @@
-#include "cache.h"
+#include "http.h"
 #include "commit.h"
 #include "pack.h"
 #include "tag.h"
 #include "blob.h"
-#include "http.h"
 #include "refs.h"
 #include "diff.h"
 #include "revision.h"
 #include "exec_cmd.h"
-#include "remote.h"
 #include "list-objects.h"
 #include "sigchain.h"
 
diff --git a/http-walker.c b/http-walker.c
index 0dbad3c..6ecd754 100644
--- a/http-walker.c
+++ b/http-walker.c
@@ -1,8 +1,7 @@
-#include "cache.h"
+#include "http.h"
 #include "commit.h"
 #include "pack.h"
 #include "walker.h"
-#include "http.h"
 
 #define PREV_BUF_SIZE 4096
 #define RANGE_HEADER_SIZE 30
diff --git a/http.h b/http.h
index 905b462..e424356 100644
--- a/http.h
+++ b/http.h
@@ -6,7 +6,6 @@
 #include <curl/curl.h>
 #include <curl/easy.h>
 
-#include "strbuf.h"
 #include "remote.h"
 
 /*
-- 
1.6.1.3

^ permalink raw reply related

* [PATCH 2/4] Header Includes Cleanup Part 2
From: Nathaniel P Dawson @ 2009-03-31  9:45 UTC (permalink / raw)
  To: git; +Cc: Nathaniel P Dawson
In-Reply-To: <1238492721-92484-2-git-send-email-nathaniel.dawson@gmail.com>

Deleted includes of revision.h where log-tree.h was included since
log-tree.h includes it.

Signed-off-by: Nathaniel P Dawson <nathaniel.dawson@gmail.com>
---
 builtin-commit.c      |    3 +--
 builtin-diff.c        |    1 -
 builtin-fast-export.c |    1 -
 builtin-log.c         |    1 -
 builtin-merge.c       |    3 +--
 builtin-rev-list.c    |    3 +--
 diff-no-index.c       |    1 -
 pretty.c              |    3 +--
 revision.c            |    3 +--
 9 files changed, 5 insertions(+), 14 deletions(-)

diff --git a/builtin-commit.c b/builtin-commit.c
index aba5660..0e21313 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -11,11 +11,10 @@
 #include "dir.h"
 #include "diff.h"
 #include "diffcore.h"
-#include "revision.h"
+#include "log-tree.h"
 #include "wt-status.h"
 #include "run-command.h"
 #include "refs.h"
-#include "log-tree.h"
 #include "utf8.h"
 #include "parse-options.h"
 #include "string-list.h"
diff --git a/builtin-diff.c b/builtin-diff.c
index 252c519..50239a9 100644
--- a/builtin-diff.c
+++ b/builtin-diff.c
@@ -9,7 +9,6 @@
 #include "tag.h"
 #include "diff.h"
 #include "diffcore.h"
-#include "revision.h"
 #include "log-tree.h"
 
 struct blobinfo {
diff --git a/builtin-fast-export.c b/builtin-fast-export.c
index b62e48d..e1aa036 100644
--- a/builtin-fast-export.c
+++ b/builtin-fast-export.c
@@ -9,7 +9,6 @@
 #include "diff.h"
 #include "diffcore.h"
 #include "log-tree.h"
-#include "revision.h"
 #include "decorate.h"
 #include "string-list.h"
 #include "utf8.h"
diff --git a/builtin-log.c b/builtin-log.c
index 5105c91..db7c344 100644
--- a/builtin-log.c
+++ b/builtin-log.c
@@ -7,7 +7,6 @@
 #include "builtin.h"
 #include "color.h"
 #include "diff.h"
-#include "revision.h"
 #include "log-tree.h"
 #include "tag.h"
 #include "reflog-walk.h"
diff --git a/builtin-merge.c b/builtin-merge.c
index cb4c645..fb6e61a 100644
--- a/builtin-merge.c
+++ b/builtin-merge.c
@@ -12,12 +12,11 @@
 #include "diff.h"
 #include "refs.h"
 #include "diffcore.h"
-#include "revision.h"
+#include "log-tree.h"
 #include "unpack-trees.h"
 #include "cache-tree.h"
 #include "dir.h"
 #include "utf8.h"
-#include "log-tree.h"
 #include "color.h"
 #include "rerere.h"
 #include "help.h"
diff --git a/builtin-rev-list.c b/builtin-rev-list.c
index cedfe9a..faac425 100644
--- a/builtin-rev-list.c
+++ b/builtin-rev-list.c
@@ -4,9 +4,8 @@
 #include "tag.h"
 #include "tree.h"
 #include "diff.h"
-#include "revision.h"
-#include "list-objects.h"
 #include "log-tree.h"
+#include "list-objects.h"
 #include "graph.h"
 
 /* bits #0-15 in revision.h */
diff --git a/diff-no-index.c b/diff-no-index.c
index cb68b8d..fa8b669 100644
--- a/diff-no-index.c
+++ b/diff-no-index.c
@@ -10,7 +10,6 @@
 #include "tag.h"
 #include "diff.h"
 #include "diffcore.h"
-#include "revision.h"
 #include "log-tree.h"
 #include "string-list.h"
 
diff --git a/pretty.c b/pretty.c
index efa7024..0465de1 100644
--- a/pretty.c
+++ b/pretty.c
@@ -2,10 +2,9 @@
 #include "commit.h"
 #include "utf8.h"
 #include "diff.h"
-#include "revision.h"
+#include "log-tree.h"
 #include "string-list.h"
 #include "mailmap.h"
-#include "log-tree.h"
 #include "color.h"
 
 static char *user_format;
diff --git a/revision.c b/revision.c
index 827108d..18425d6 100644
--- a/revision.c
+++ b/revision.c
@@ -4,12 +4,11 @@
 #include "commit.h"
 #include "diff.h"
 #include "refs.h"
-#include "revision.h"
+#include "log-tree.h"
 #include "graph.h"
 #include "grep.h"
 #include "reflog-walk.h"
 #include "patch-ids.h"
-#include "log-tree.h"
 
 volatile show_early_output_fn_t show_early_output;
 
-- 
1.6.1.3

^ permalink raw reply related

* [PATCH 3/4] Header Includes Cleanup Part 2
From: Nathaniel P Dawson @ 2009-03-31  9:45 UTC (permalink / raw)
  To: git; +Cc: Nathaniel P Dawson
In-Reply-To: <1238492721-92484-3-git-send-email-nathaniel.dawson@gmail.com>

Deleted includes of string-list.h where merge-recursive.h was included
since it includes string-list.h.

Signed-off-by: Nathaniel P Dawson <nathaniel.dawson@gmail.com>
---
 merge-recursive.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/merge-recursive.c b/merge-recursive.c
index 370151e..40d636f 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -10,11 +10,10 @@
 #include "diffcore.h"
 #include "tag.h"
 #include "unpack-trees.h"
-#include "string-list.h"
+#include "merge-recursive.h"
 #include "xdiff-interface.h"
 #include "ll-merge.h"
 #include "attr.h"
-#include "merge-recursive.h"
 #include "dir.h"
 
 static struct tree *shift_tree_object(struct tree *one, struct tree *two)
-- 
1.6.1.3

^ permalink raw reply related

* [PATCH 4/4] Header Includes Cleanup Part 2
From: Nathaniel P Dawson @ 2009-03-31  9:45 UTC (permalink / raw)
  To: git; +Cc: Nathaniel P Dawson
In-Reply-To: <1238492721-92484-4-git-send-email-nathaniel.dawson@gmail.com>

Deleted includes of cache.h where reflog-walk.h was included since
reflog-walk.h includes it.

Signed-off-by: Nathaniel P Dawson <nathaniel.dawson@gmail.com>
---
 log-tree.c    |    3 +--
 reflog-walk.c |    3 +--
 revision.c    |    3 +--
 3 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/log-tree.c b/log-tree.c
index 9565c18..0433e61 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -1,10 +1,9 @@
-#include "cache.h"
+#include "reflog-walk.h"
 #include "diff.h"
 #include "commit.h"
 #include "tag.h"
 #include "graph.h"
 #include "log-tree.h"
-#include "reflog-walk.h"
 #include "refs.h"
 #include "string-list.h"
 
diff --git a/reflog-walk.c b/reflog-walk.c
index fd065f4..9198500 100644
--- a/reflog-walk.c
+++ b/reflog-walk.c
@@ -1,10 +1,9 @@
-#include "cache.h"
+#include "reflog-walk.h"
 #include "commit.h"
 #include "refs.h"
 #include "diff.h"
 #include "revision.h"
 #include "string-list.h"
-#include "reflog-walk.h"
 
 struct complete_reflogs {
 	char *ref;
diff --git a/revision.c b/revision.c
index 18425d6..b9d2aed 100644
--- a/revision.c
+++ b/revision.c
@@ -1,4 +1,4 @@
-#include "cache.h"
+#include "reflog-walk.h"
 #include "tag.h"
 #include "blob.h"
 #include "commit.h"
@@ -7,7 +7,6 @@
 #include "log-tree.h"
 #include "graph.h"
 #include "grep.h"
-#include "reflog-walk.h"
 #include "patch-ids.h"
 
 volatile show_early_output_fn_t show_early_output;
-- 
1.6.1.3

^ permalink raw reply related

* Re: Detached HEAD warning (again)
From: Johannes Schindelin @ 2009-03-31 10:04 UTC (permalink / raw)
  To: Pieter de Bie; +Cc: Git Mailing List
In-Reply-To: <9099EAF5-6B43-4F15-A905-9E21B45B7AE9@ai.rug.nl>

Hi,

On Mon, 30 Mar 2009, Pieter de Bie wrote:

> I strongly remember there being a discussion about this a few weeks ago, 
> but I can't find it. Basically, someone wanted to introduce a warning 
> every time someone commits on a detached HEAD. This was shot down 
> because there already is a big warning when you detach your HEAD (with 
> which I agree).
> 
> However, someone here: http://news.ycombinator.com/item?id=538619 
> pointed to an example here: http://book.git-scm.com/5_submodules.html , 
> which works with submodules:
> 
> 	$ git submodule update --init
> 	# sub/ is created
> 	$ (cd sub && touch a && git add a && git commit -am "Add new file")
> 	[detached HEAD 8641889] Add new file
> 	 0 files changed, 0 insertions(+), 0 deletions(-)
> 	 create mode 100644 a
> 
> 	$ git submodule update
> 	$ ls sub/a
> 	ls: sub/a: No such file or directory

	$ cd sub
	$ git checkout HEAD@{1}

Ciao,
Dscho

^ permalink raw reply

* Re: [RFC/PATCH] graph API: Added logic for colored edges.
From: Johannes Schindelin @ 2009-03-31 10:13 UTC (permalink / raw)
  To: Allan Caffee; +Cc: git, Jeff King, Nanako Shiraishi
In-Reply-To: <20090330141322.GA6221@linux.vnet>

Hi,

On Mon, 30 Mar 2009, Allan Caffee wrote:

> +static void strbuf_write_column(struct strbuf *sb, const struct column *c,
> +		const char *s)
> +{
> +	/*
> +	 * TODO: I get the creeping suspicion that this isn't the
> +	 * right flag to be checking since --no-color doesn't turn
> +	 * this off.
> +	 */

Heh, of course I forgot to remove this with my to-be-squashed-in patch...

> +	if (diff_use_color_default)
> +		strbuf_addstr(sb, c->color);
> +	strbuf_addstr(sb, s);
> +	if (diff_use_color_default)
> +		strbuf_addstr(sb, GIT_COLOR_RESET);
> +}

How about this function instead?

static void strbuf_add_column(struct strbuf *sb,
	const struct column *column, const char *fmt, ...)
{
        va_list ap;

        va_start(ap, fmt);
	if (column->color)
		strbuf_addstr(sb, column->color);
        strbuf_vaddf(sb, fmt, ap);
	if (column->color)
		strbuf_addstr(sb, GIT_COLOR_RESET);
        va_end(ap);
}

Hmm?

Ciao,
Dscho

^ permalink raw reply

* Re: [RFC/PATCH] graph API: Added logic for colored edges.
From: Johannes Schindelin @ 2009-03-31 10:21 UTC (permalink / raw)
  To: Allan Caffee; +Cc: git, Jeff King, Nanako Shiraishi
In-Reply-To: <20090330141322.GA6221@linux.vnet>

Hi,

On Mon, 30 Mar 2009, Allan Caffee wrote:

> @@ -445,6 +492,12 @@ static void graph_update_columns(struct git_graph *graph)
>  			for (parent = first_interesting_parent(graph);
>  			     parent;
>  			     parent = next_interesting_parent(graph, parent)) {
> +				/*
> +				 * If this is a merge increment the current
> +				 * color.
> +				 */
> +				if (graph->num_parents > 1)
> +					get_next_column_color(graph);
>  				graph_insert_into_new_columns(graph,
>  							      parent->item,
>  							      &mapping_idx);

Hmm.  I would have expected the color to be an argument to 
graph_insert_into_new_columns()...

Oh, and please forget about my stupid babbling about using struct 
decoration for colors: the column already knows commit and color, so if 
you need the color in a functino in addition to the commit, you should 
pass either the column struct instead, or the commit and the color as 
individual parameters.

This concludes my review ;-)

Thanks,
Dscho

^ permalink raw reply

* Re: [RFC/PATCH] graph API: Added logic for colored edges.
From: Johannes Sixt @ 2009-03-31 10:26 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Allan Caffee, git, Jeff King, Nanako Shiraishi
In-Reply-To: <alpine.DEB.1.00.0903311210000.10279@pacific.mpi-cbg.de>

Johannes Schindelin schrieb:
> How about this function instead?
> 
> static void strbuf_add_column(struct strbuf *sb,
> 	const struct column *column, const char *fmt, ...)
> {
>         va_list ap;
> 
>         va_start(ap, fmt);
> 	if (column->color)
> 		strbuf_addstr(sb, column->color);
>         strbuf_vaddf(sb, fmt, ap);
> 	if (column->color)
> 		strbuf_addstr(sb, GIT_COLOR_RESET);
>         va_end(ap);
> }
> 
> Hmm?

Except the strbuf_vaddf() is only in your private repository ;)

-- Hannes

^ permalink raw reply

* Re: [PATCH 2/2] send-email: add tests for refactored prompting
From: Björn Steinbrink @ 2009-03-31 10:33 UTC (permalink / raw)
  To: Jay Soffian; +Cc: git, Matthieu Moy, Junio C Hamano
In-Reply-To: <1238290751-57461-2-git-send-email-jaysoffian@gmail.com>

On 2009.03.28 21:39:11 -0400, Jay Soffian wrote:
> +test_expect_success 'confirm detects EOF (auto causes failure)' '
> +	CONFIRM=$(git config --get sendemail.confirm) &&
> +	git config sendemail.confirm auto &&
> +	GIT_SEND_EMAIL_NOTTY=1 \
> +		test_must_fail git send-email \
> +			--from="Example <nobody@example.com>" \
> +			--to=nobody@example.com \
> +			--smtp-server="$(pwd)/fake.sendmail" \
> +			$patches < /dev/null
> +	ret="$?"
> +	git config sendemail.confirm ${CONFIRM:-never}
> +	test $ret = "0"
> +'
> +
> +test_expect_success 'confirm doesnt loop forever' '
> +	CONFIRM=$(git config --get sendemail.confirm) &&
> +	git config sendemail.confirm auto &&
> +	yes "bogus" | GIT_SEND_EMAIL_NOTTY=1 \
> +		test_must_fail git send-email \
> +			--from="Example <nobody@example.com>" \
> +			--to=nobody@example.com \
> +			--smtp-server="$(pwd)/fake.sendmail" \
> +			$patches
> +	ret="$?"
> +	git config sendemail.confirm ${CONFIRM:-never}
> +	test $ret = "0"

These two cause interactive prompts for me.

Björn

^ permalink raw reply

* Re: [PATCH] AIX 5.2 - bug with 1.6.2.1
From: Jeff King @ 2009-03-31 10:37 UTC (permalink / raw)
  To: Pierre Poissinger; +Cc: git, gitster
In-Reply-To: <3930158b0903301647o790f7381l37ba61089713ce80@mail.gmail.com>

[I'm cc'ing the git-list; please send bug reports there]

On Tue, Mar 31, 2009 at 01:47:47AM +0200, Pierre Poissinger wrote:

> Looks like the patch posted here
> http://osdir.com/ml/git/2009-01/msg01128.html
> (around d8e96fd86d415554a9c2e09ffb929a9e22fdad25 I think - too late to
> look for it - love google that gave me this guy as first hit...)
> 
> breaks git on AIX 5.2 with gcc 2.9 :-(

Wow, I didn't know people were still using the 2.9 branch of gcc.

> Nothing new to pack.
> trace: built-in: git 'prune-packed'
> The end...
> fatal: unable to run 'git-repack'
> error: waitpid 155256: code 128 (Error 0)
> error: failed to run repack

Hmm. Can you confirm the status being passed back by run_command? The
following patch should do it:

diff --git a/git.c b/git.c
index c2b181e..a99c31b 100644
--- a/git.c
+++ b/git.c
@@ -413,6 +413,7 @@ static void execv_dashed_external(const char **argv)
 	 * OK to return. Otherwise, we just pass along the status code.
 	 */
 	status = run_command_v_opt(argv, 0);
+	fprintf(stderr, "run_command produced status %d\n", status);
 	if (status != -ERR_RUN_COMMAND_EXEC) {
 		if (IS_RUN_COMMAND_ERR(status))
 			die("unable to run '%s'", argv[0]);

> The bottom line: Looks like my box really don't like the run-command
> error define as:
> #define IS_RUN_COMMAND_ERR(x) ((x) <= -ERR_RUN_COMMAND_FORK)
> 
> it will trigger even with x==0 :-)

That seems very wrong. I wonder if it is a problem with the signedness
of enums in that version of gcc. Can you run the following program and
report on its output?

-- >8 --
#include <stdio.h>
enum { FOO = 10000 };
int main()
{
  printf("-FOO: %d\n", -FOO);
  printf("0 <= -FOO: %d\n", 0 <= -FOO);
  printf("-10000 <= -FOO: %d\n", -10000 <= -FOO);
  return 0;
}
-- 8< --

> So, following patch simply change this define to
> #define IS_RUN_COMMAND_ERR(x) ((x) > ERR_RUN_COMMAND_FORK)

That's not right; x is going to be a large negative value. You could
try:

  #define IS_RUN_COMMAND_ERR(x) ((-x) > ERR_RUN_COMMAND_FORK)

The other option is to rework run_command to just return positive values
(which should be fine as long as they remain out of the range of normal
exit codes).

-Peff

^ permalink raw reply related

* bsd group semantics
From: Jeff King @ 2009-03-31 11:26 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Alex Riesen, git

The recently added test t1301.17 is failing for me on FreeBSD; the
created directories don't have the GID bit set.

I traced this back to the fact that FORCE_DIR_SET_GID is a no-op on
FreeBSD due to the DIR_HAS_BSD_GROUP_SEMANTICS flag being set. I am not
sure I understand the original reasoning for this flag at all; GID seems
to work just fine on my FreeBSD 6.1 test system.

The original thread is here:

  http://article.gmane.org/gmane.comp.version-control.git/75999

It looks like Alex was having the problem on FreeBSD 4. I'm not sure if
this flag can go away with the new set_shared_perm, or if I need to just
enable on FreeBSD <= 4.

-Peff

^ permalink raw reply

* Re: [PATCH] Add warning about known issues to documentation of cvsimport
From: Jeff King @ 2009-03-31 11:28 UTC (permalink / raw)
  To: Heiko Voigt; +Cc: Junio C Hamano, git
In-Reply-To: <20090330223646.GC68118@macbook.lan>

On Tue, Mar 31, 2009 at 12:36:46AM +0200, Heiko Voigt wrote:

> > Note the extra blank line between each heading and its list, and the
> > lack of a blank line between the end of the first list and the heading
> > of the second. Your source is very readable, so it really is just
> > asciidoc being silly, but I wonder if there is a way to work around
> > that.
> 
> My xmlto is not working at the moment. I will check that.

I looked into it a little more; it happens all over the place, so it is
a problem somewhere in the documentation toolchain. So don't worry about
it for this particular patch.

-Peff

^ permalink raw reply

* [StGit PATCH] Convert "pop" to the lib infrastructure
From: Catalin Marinas @ 2009-03-31 11:30 UTC (permalink / raw)
  To: git, Karl Hasselström

Signed-off-by: Catalin Marinas <catalin.marinas@gmail.com>
---
 stgit/commands/pop.py |   67 ++++++++++++++++++++-----------------------------
 t/t3101-reset-hard.sh |    4 +--
 t/t3103-undo-hard.sh  |    4 +--
 3 files changed, 32 insertions(+), 43 deletions(-)

diff --git a/stgit/commands/pop.py b/stgit/commands/pop.py
index 2c78ac2..eace090 100644
--- a/stgit/commands/pop.py
+++ b/stgit/commands/pop.py
@@ -16,11 +16,10 @@ along with this program; if not, write to the Free Software
 Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
 """
 
-import sys, os
+from stgit.commands import common
+from stgit.lib import transaction
+from stgit import argparse
 from stgit.argparse import opt
-from stgit.commands.common import *
-from stgit.utils import *
-from stgit import argparse, stack, git
 
 help = 'Pop one or more patches from the stack'
 kind = 'stack'
@@ -40,50 +39,40 @@ options = [
     opt('-a', '--all', action = 'store_true',
         short = 'Pop all the applied patches'),
     opt('-n', '--number', type = 'int',
-        short = 'Pop the specified number of patches'),
-    opt('-k', '--keep', action = 'store_true',
-        short = 'Keep the local changes')]
+        short = 'Pop the specified number of patches')
+    ] + argparse.keep_option()
 
-directory = DirectoryGotoToplevel(log = True)
+directory = common.DirectoryHasRepositoryLib()
 
 def func(parser, options, args):
-    """Pop the topmost patch from the stack
-    """
-    check_conflicts()
-    check_head_top_equal(crt_series)
+    """Pop the given patches or the topmost one from the stack."""
+    stack = directory.repository.current_stack
+    iw = stack.repository.default_iw
+    clean_iw = (not options.keep and iw) or None
+    trans = transaction.StackTransaction(stack, 'pop',
+                                         check_clean_iw = clean_iw)
 
-    if not options.keep:
-        check_local_changes()
-
-    applied = crt_series.get_applied()
-    if not applied:
-        raise CmdException, 'No patches applied'
+    if not trans.applied:
+        raise common.CmdException('No patches applied')
 
     if options.all:
-        patches = applied
+        patches = trans.applied
     elif options.number:
         # reverse it twice to also work with negative or bigger than
         # the length numbers
-        patches = applied[::-1][:options.number][::-1]
-    elif len(args) == 0:
-        patches = [applied[-1]]
+        patches = trans.applied[::-1][:options.number][::-1]
+    elif not args:
+        patches = [trans.applied[-1]]
     else:
-        patches = parse_patches(args, applied, ordered = True)
+        patches = common.parse_patches(args, trans.applied, ordered = True)
 
     if not patches:
-        raise CmdException, 'No patches to pop'
-
-    # pop to the most distant popped patch
-    topop = applied[applied.index(patches[0]):]
-    # push those not in the popped range
-    topush = [p for p in topop if p not in patches]
-
-    if options.keep and topush:
-        raise CmdException, 'Cannot pop arbitrary patches with --keep'
-
-    topop.reverse()
-    pop_patches(crt_series, topop, options.keep)
-    if topush:
-        push_patches(crt_series, topush)
-
-    print_crt_patch(crt_series)
+        raise common.CmdException('No patches to pop')
+
+    applied = [p for p in trans.applied if not p in set(patches)]
+    unapplied = patches + trans.unapplied
+    try:
+        trans.reorder_patches(applied, unapplied, iw = iw)
+    except transaction.TransactionException:
+        pass
+    return trans.run(iw)
diff --git a/t/t3101-reset-hard.sh b/t/t3101-reset-hard.sh
index 2807ba3..bd97b3a 100755
--- a/t/t3101-reset-hard.sh
+++ b/t/t3101-reset-hard.sh
@@ -28,7 +28,7 @@ cat > expected.txt <<EOF
 C a
 EOF
 test_expect_success 'Pop middle patch, creating a conflict' '
-    conflict_old stg pop p2 &&
+    conflict stg pop p2 &&
     stg status a > actual.txt &&
     test_cmp expected.txt actual.txt &&
     test "$(echo $(stg series))" = "+ p1 > p3 - p2"
@@ -47,7 +47,7 @@ test_expect_success 'Try to reset with --hard' '
     stg reset --hard master.stgit^~1 &&
     stg status a > actual.txt &&
     test_cmp expected.txt actual.txt &&
-    test "$(echo $(stg series))" = "> p1 - p3 - p2"
+    test "$(echo $(stg series))" = "> p1 - p2 - p3"
 '
 
 test_done
diff --git a/t/t3103-undo-hard.sh b/t/t3103-undo-hard.sh
index 599aa43..ce71668 100755
--- a/t/t3103-undo-hard.sh
+++ b/t/t3103-undo-hard.sh
@@ -28,7 +28,7 @@ cat > expected.txt <<EOF
 C a
 EOF
 test_expect_success 'Pop middle patch, creating a conflict' '
-    conflict_old stg pop p2 &&
+    conflict stg pop p2 &&
     stg status a > actual.txt &&
     test_cmp expected.txt actual.txt &&
     test "$(echo $(stg series))" = "+ p1 > p3 - p2"
@@ -47,7 +47,7 @@ test_expect_success 'Try to undo with --hard' '
     stg undo --hard &&
     stg status a > actual.txt &&
     test_cmp expected.txt actual.txt &&
-    test "$(echo $(stg series))" = "> p1 - p3 - p2"
+    test "$(echo $(stg series))" = "> p1 - p2 - p3"
 '
 
 test_done

^ permalink raw reply related

* [StGit PATCH] Convert "push" to the lib infrastructure
From: Catalin Marinas @ 2009-03-31 11:30 UTC (permalink / raw)
  To: git, Karl Hasselström

Signed-off-by: Catalin Marinas <catalin.marinas@gmail.com>
---
 stgit/commands/push.py   |   68 ++++++++++++++++++++++++----------------------
 t/t0002-status.sh        |    2 +
 t/t1200-push-modified.sh |    2 +
 t/t1202-push-undo.sh     |    4 +--
 t/t1203-push-conflict.sh |    2 +
 t/t1205-push-subdir.sh   |    2 +
 t/t2500-clean.sh         |    2 +
 7 files changed, 43 insertions(+), 39 deletions(-)

diff --git a/stgit/commands/push.py b/stgit/commands/push.py
index 818e02d..0d25a65 100644
--- a/stgit/commands/push.py
+++ b/stgit/commands/push.py
@@ -16,12 +16,10 @@ along with this program; if not, write to the Free Software
 Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
 """
 
-import sys, os
+from stgit.commands import common
+from stgit.lib import transaction
+from stgit import argparse
 from stgit.argparse import opt
-from stgit.commands.common import *
-from stgit.utils import *
-from stgit.out import *
-from stgit import argparse, stack, git
 
 help = 'Push one or more patches onto the stack'
 kind = 'stack'
@@ -45,39 +43,45 @@ options = [
     opt('-n', '--number', type = 'int',
         short = 'Push the specified number of patches'),
     opt('--reverse', action = 'store_true',
-        short = 'Push the patches in reverse order'),
-    opt('-m', '--merged', action = 'store_true',
-        short = 'Check for patches merged upstream')]
+        short = 'Push the patches in reverse order')
+    ] + argparse.keep_option() + argparse.merged_option()
 
-directory = DirectoryGotoToplevel(log = True)
+directory = common.DirectoryHasRepositoryLib()
 
 def func(parser, options, args):
-    """Pushes the given patch or all onto the series
-    """
-
-    check_local_changes()
-    check_conflicts()
-    check_head_top_equal(crt_series)
-
-    unapplied = crt_series.get_unapplied()
-    if not unapplied:
-        raise CmdException, 'No more patches to push'
-
-    if options.number:
-        patches = unapplied[:options.number]
-    elif options.all:
-        patches = unapplied
-    elif len(args) == 0:
-        patches = [unapplied[0]]
+    """Pushes the given patches or the first unapplied onto the stack."""
+    stack = directory.repository.current_stack
+    iw = stack.repository.default_iw
+    clean_iw = (not options.keep and iw) or None
+    trans = transaction.StackTransaction(stack, 'pop',
+                                         check_clean_iw = clean_iw)
+
+    if not trans.unapplied:
+        raise common.CmdException('No patches to push')
+
+    if options.all:
+        patches = list(trans.unapplied)
+    elif options.number:
+        patches = trans.unapplied[:options.number]
+    elif not args:
+        patches = [trans.unapplied[0]]
     else:
-        patches = parse_patches(args, unapplied)
+        patches = common.parse_patches(args, trans.unapplied)
 
-    if patches == []:
-        raise CmdException, 'No patches to push'
+    if not patches:
+        raise common.CmdException('No patches to push')
 
     if options.reverse:
         patches.reverse()
 
-    push_patches(crt_series, patches, options.merged)
-
-    print_crt_patch(crt_series)
+    try:
+        if options.merged:
+            merged = set(trans.check_merged(patches))
+        else:
+            merged = set()
+        for pn in patches:
+            trans.push_patch(pn, iw, allow_interactive = True,
+                             already_merged = pn in merged)
+    except transaction.TransactionHalted:
+        pass
+    return trans.run(iw)
diff --git a/t/t0002-status.sh b/t/t0002-status.sh
index d95a83b..ffcad77 100755
--- a/t/t0002-status.sh
+++ b/t/t0002-status.sh
@@ -111,7 +111,7 @@ A fie
 C foo/bar
 EOF
 test_expect_success 'Status after conflicting push' '
-    conflict_old stg push &&
+    conflict stg push &&
     stg status > output.txt &&
     test_cmp expected.txt output.txt
 '
diff --git a/t/t1200-push-modified.sh b/t/t1200-push-modified.sh
index a591124..2077492 100755
--- a/t/t1200-push-modified.sh
+++ b/t/t1200-push-modified.sh
@@ -49,7 +49,7 @@ test_expect_success \
 
 test_expect_success \
     'Attempt to push the first of those patches without --merged' \
-    "(cd bar && conflict_old stg push
+    "(cd bar && conflict stg push
      )
 "
 
diff --git a/t/t1202-push-undo.sh b/t/t1202-push-undo.sh
index 79439de..14a3d6d 100755
--- a/t/t1202-push-undo.sh
+++ b/t/t1202-push-undo.sh
@@ -43,7 +43,7 @@ test_expect_success \
 test_expect_success \
 	'Push the second patch with conflict' \
 	'
-	conflict_old stg push bar
+	conflict stg push bar
 	'
 
 test_expect_success \
@@ -55,7 +55,7 @@ test_expect_success \
 test_expect_success \
 	'Check the push after undo fails as well' \
 	'
-	conflict_old stg push bar
+	conflict stg push bar
 	'
 
 test_expect_success \
diff --git a/t/t1203-push-conflict.sh b/t/t1203-push-conflict.sh
index 96fee15..8027e6c 100755
--- a/t/t1203-push-conflict.sh
+++ b/t/t1203-push-conflict.sh
@@ -38,7 +38,7 @@ test_expect_success \
 test_expect_success \
 	'Push the first patch with conflict' \
 	'
-	conflict_old stg push foo
+	conflict stg push foo
 	'
 
 test_expect_success \
diff --git a/t/t1205-push-subdir.sh b/t/t1205-push-subdir.sh
index f852762..2d5918b 100755
--- a/t/t1205-push-subdir.sh
+++ b/t/t1205-push-subdir.sh
@@ -47,7 +47,7 @@ test_expect_success 'Conflicting push from subdir' '
     [ "$(echo $(cat x.txt))" = "x0" ] &&
     [ "$(echo $(cat foo/y.txt))" = "y0" ] &&
     cd foo &&
-    conflict_old stg push p2 &&
+    conflict stg push p2 &&
     cd .. &&
     [ "$(echo $(stg status --conflict))" = "foo/y.txt x.txt" ]
 '
diff --git a/t/t2500-clean.sh b/t/t2500-clean.sh
index 99fd29f..7f821f5 100755
--- a/t/t2500-clean.sh
+++ b/t/t2500-clean.sh
@@ -32,7 +32,7 @@ test_expect_success 'Create a conflict' '
     stg new p2 -m p2
     echo quux > foo.txt &&
     stg refresh &&
-    conflict_old stg push
+    conflict stg push
 '
 
 test_expect_success 'Make sure conflicting patches are preserved' '

^ permalink raw reply related

* Re: [RFC/PATCH] graph API: Added logic for colored edges.
From: Johannes Schindelin @ 2009-03-31 12:09 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: Allan Caffee, git, Jeff King, Nanako Shiraishi
In-Reply-To: <49D1EFF2.303@viscovery.net>

Hi,

On Tue, 31 Mar 2009, Johannes Sixt wrote:

> Johannes Schindelin schrieb:
> > How about this function instead?
> > 
> > static void strbuf_add_column(struct strbuf *sb,
> > 	const struct column *column, const char *fmt, ...)
> > {
> >         va_list ap;
> > 
> >         va_start(ap, fmt);
> > 	if (column->color)
> > 		strbuf_addstr(sb, column->color);
> >         strbuf_vaddf(sb, fmt, ap);
> > 	if (column->color)
> > 		strbuf_addstr(sb, GIT_COLOR_RESET);
> >         va_end(ap);
> > }
> > 
> > Hmm?
> 
> Except the strbuf_vaddf() is only in your private repository ;)

LOL & schenkelklopf!

You're absolutely correct... I'm sorry,
Dscho

^ permalink raw reply

* Re: [PATCH] AIX 5.2 - bug with 1.6.2.1
From: Pierre Poissinger @ 2009-03-31 12:20 UTC (permalink / raw)
  To: Jeff King; +Cc: git, gitster
In-Reply-To: <20090331103703.GA1589@coredump.intra.peff.net>

On Tue, Mar 31, 2009 at 12:37 PM, Jeff King <peff@peff.net> wrote:
> [I'm cc'ing the git-list; please send bug reports there]
[snip]
>> breaks git on AIX 5.2 with gcc 2.9 :-(
>
> Wow, I didn't know people were still using the 2.9 branch of gcc.

Yep, that or "C for AIX Compiler, Version 6" but this one is awol for git
[Hey, it feels cutting edge - I use Emacs 19 and digital C daily on
OpenVMS 7.1...]

> Hmm. Can you confirm the status being passed back by run_command?
>[snip]
0 - that's why I found out the funny enum stuff...

> That seems very wrong. I wonder if it is a problem with the signedness
> of enums in that version of gcc. Can you run the following program and
> report on its output?
Not only to you... just that's what I noticed...

> -- >8 --
-FOO: -10000
0 <= -FOO: 1
-10000 <= -FOO: 1

This seems to be a "Old GCC'ism" - XLC (version 6...cannot even
understand git code) does it right:
-FOO: -10000
0 <= -FOO: 0
-10000 <= -FOO: 1

>  #define IS_RUN_COMMAND_ERR(x) ((-x) > ERR_RUN_COMMAND_FORK)
oops... works for me with
#define IS_RUN_COMMAND_ERR(x) (-(x) > ERR_RUN_COMMAND_FORK)

> The other option is to rework run_command to just return positive values
> (which should be fine as long as they remain out of the range of normal
> exit codes).
Change define good enough for me and my oldies...

Regs,
Pierre,
-- 
>>> horsemen = ['war', 'pestilence', 'famine']
>>> horsemen.append('Powerbuilder')

^ permalink raw reply

* Re: [PATCH] AIX 5.2 - bug with 1.6.2.1
From: Jeff King @ 2009-03-31 12:29 UTC (permalink / raw)
  To: Pierre Poissinger; +Cc: git, gitster
In-Reply-To: <3930158b0903310520h1f421518ka67de5f7aad0690b@mail.gmail.com>

On Tue, Mar 31, 2009 at 02:20:17PM +0200, Pierre Poissinger wrote:

> >  #define IS_RUN_COMMAND_ERR(x) ((-x) > ERR_RUN_COMMAND_FORK)
> oops... works for me with
> #define IS_RUN_COMMAND_ERR(x) (-(x) > ERR_RUN_COMMAND_FORK)

Oops, it should actually be "-(x) >= ERR_RUN_COMMAND_FORK". But it
shouldn't make a difference in your test, since x was '0' in your case.

Junio, I think this patch should take care of it.

-- >8 --
Subject: [PATCH] fix portability problem with IS_RUN_COMMAND_ERR

Some old versions of gcc don't seem to like us negating an
enum constant. Let's work around it by negating the other
half of the comparison instead.

Reported by Pierre Poissinger on gcc 2.9.
---
 run-command.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/run-command.h b/run-command.h
index 15e870a..e345502 100644
--- a/run-command.h
+++ b/run-command.h
@@ -10,7 +10,7 @@ enum {
 	ERR_RUN_COMMAND_WAITPID_SIGNAL,
 	ERR_RUN_COMMAND_WAITPID_NOEXIT,
 };
-#define IS_RUN_COMMAND_ERR(x) ((x) <= -ERR_RUN_COMMAND_FORK)
+#define IS_RUN_COMMAND_ERR(x) (-(x) >= ERR_RUN_COMMAND_FORK)
 
 struct child_process {
 	const char **argv;
-- 
1.6.2.1.591.geb450

^ permalink raw reply related

* Re: [PATCH] AIX 5.2 - bug with 1.6.2.1
From: Pierre Poissinger @ 2009-03-31 12:48 UTC (permalink / raw)
  To: Jeff King; +Cc: git, gitster
In-Reply-To: <20090331122923.GA7173@coredump.intra.peff.net>

> Junio, I think this patch should take care of it.
Thanks !

Pierre,
-- 
>>> horsemen = ['war', 'pestilence', 'famine']
>>> horsemen.append('Powerbuilder')

^ permalink raw reply

* Re: [PATCHv2 4/4] Rename push.default to push.mode
From: Michael J Gruber @ 2009-03-31 12:55 UTC (permalink / raw)
  To: Jeff King; +Cc: Santi Béjar, git
In-Reply-To: <20090330180253.GA7220@coredump.intra.peff.net>

Jeff King venit, vidit, dixit 30.03.2009 20:02:
> On Mon, Mar 30, 2009 at 04:47:20PM +0200, Michael J Gruber wrote:
> 
>> Thanks for the explanation! I rechecked, and for sure it also happened
>> with patches from Tor Arne Vestbø and Nguyễn Thái Ngọc Duy.
>> Again I feel blessed by my umlautlessness... and challenged by gmane.
>> About time to ditch it for reading patches also, or at least to be aware
>> of it when saving patches.
> 
> The message gmane passes out has the content-transfer-encoding set
> properly. Your newsreader should properly un-QP it, or if it has an
> option to save it directly (encoding and all), then "git am" will un-QP
> it. If you were to "git apply" it directly, that would break, though
> (and it is user error). If your newsreader saves the QP text without the
> matching header, then that would also break (and your newsreader is
> broken).
> 
> Just curious how you are seeing the breakage (because I want to make
> sure there is no git bug).
> 
> -Peff

I haven't seen any breakage. I just noticed a different font being used
for Santi's posts (due to different character encoding, with a different
font preset in TB) and looked at the message source, where I saw =20 and
such all over the place (message display is fine). When saving from TB
the QP is undone. It just made me feel uneasy because of our recent
discussion regarding format-flowed and patch submission. I know ff \ne
qp, but still I thought git-{send-email,apply} would prefer straight ascii.

Michael

^ permalink raw reply


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